Chore: Add user service method GetByLogin (#53204)

* Add wrapper around sqlstore method GetUserByLogin

* Use new method from user service

* Fix lint

* Fix lint 2

* fix middleware basic auth test

* Fix grafana login returning a user by login

* Remove GetUserByLogin from store interface

* Merge commit
This commit is contained in:
idafurjes 2022-08-04 13:22:43 +02:00 committed by GitHub
parent 1ec9007fe0
commit 1ecbe22751
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 122 additions and 84 deletions

View File

@ -48,6 +48,7 @@ import (
"github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web" "github.com/grafana/grafana/pkg/web"
"github.com/grafana/grafana/pkg/web/webtest" "github.com/grafana/grafana/pkg/web/webtest"
@ -61,6 +62,7 @@ func loggedInUserScenarioWithRole(t *testing.T, desc string, method string, url
t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
sc := setupScenarioContext(t, url) sc := setupScenarioContext(t, url)
sc.sqlStore = sqlStore sc.sqlStore = sqlStore
sc.userService = usertest.NewUserServiceFake()
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response { sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
sc.context = c sc.context = c
sc.context.UserId = testUserID sc.context.UserId = testUserID
@ -381,6 +383,8 @@ func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl, enableAccessCo
require.NoError(t, err) require.NoError(t, err)
// Create minimal HTTP Server // Create minimal HTTP Server
userMock := usertest.NewUserServiceFake()
userMock.ExpectedUser = &user.User{ID: 1}
hs := &HTTPServer{ hs := &HTTPServer{
Cfg: cfg, Cfg: cfg,
Features: features, Features: features,
@ -397,6 +401,7 @@ func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl, enableAccessCo
accesscontrolmock.NewMockedPermissionsService(), accesscontrolmock.NewMockedPermissionsService(), ac, accesscontrolmock.NewMockedPermissionsService(), accesscontrolmock.NewMockedPermissionsService(), ac,
), ),
preferenceService: preftest.NewPreferenceServiceFake(), preferenceService: preftest.NewPreferenceServiceFake(),
userService: userMock,
} }
require.NoError(t, hs.declareFixedRoles()) require.NoError(t, hs.declareFixedRoles())

View File

@ -66,14 +66,15 @@ func (hs *HTTPServer) AddOrgInvite(c *models.ReqContext) response.Response {
} }
// first try get existing user // first try get existing user
userQuery := models.GetUserByLoginQuery{LoginOrEmail: inviteDto.LoginOrEmail} userQuery := user.GetUserByLoginQuery{LoginOrEmail: inviteDto.LoginOrEmail}
if err := hs.SQLStore.GetUserByLogin(c.Req.Context(), &userQuery); err != nil { usr, err := hs.userService.GetByLogin(c.Req.Context(), &userQuery)
if err != nil {
if !errors.Is(err, user.ErrUserNotFound) { if !errors.Is(err, user.ErrUserNotFound) {
return response.Error(500, "Failed to query db for existing user check", err) return response.Error(500, "Failed to query db for existing user check", err)
} }
} else { } else {
// Evaluate permissions for adding an existing user to the organization // Evaluate permissions for adding an existing user to the organization
userIDScope := ac.Scope("users", "id", strconv.Itoa(int(userQuery.Result.ID))) userIDScope := ac.Scope("users", "id", strconv.Itoa(int(usr.ID)))
hasAccess, err := hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, ac.EvalPermission(ac.ActionOrgUsersAdd, userIDScope)) hasAccess, err := hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, ac.EvalPermission(ac.ActionOrgUsersAdd, userIDScope))
if err != nil { if err != nil {
return response.Error(http.StatusInternalServerError, "Failed to evaluate permissions", err) return response.Error(http.StatusInternalServerError, "Failed to evaluate permissions", err)
@ -81,7 +82,7 @@ func (hs *HTTPServer) AddOrgInvite(c *models.ReqContext) response.Response {
if !hasAccess { if !hasAccess {
return response.Error(http.StatusForbidden, "Permission denied: not permitted to add an existing user to this organisation", err) return response.Error(http.StatusForbidden, "Permission denied: not permitted to add an existing user to this organisation", err)
} }
return hs.inviteExistingUserToOrg(c, userQuery.Result, &inviteDto) return hs.inviteExistingUserToOrg(c, usr, &inviteDto)
} }
if setting.DisableLoginForm { if setting.DisableLoginForm {
@ -94,7 +95,6 @@ func (hs *HTTPServer) AddOrgInvite(c *models.ReqContext) response.Response {
cmd.Name = inviteDto.Name cmd.Name = inviteDto.Name
cmd.Status = models.TmpUserInvitePending cmd.Status = models.TmpUserInvitePending
cmd.InvitedByUserId = c.UserId cmd.InvitedByUserId = c.UserId
var err error
cmd.Code, err = util.GetRandomString(30) cmd.Code, err = util.GetRandomString(30)
if err != nil { if err != nil {
return response.Error(500, "Could not generate random string", err) return response.Error(500, "Could not generate random string", err)

View File

@ -9,6 +9,8 @@ import (
"github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
) )
func TestOrgInvitesAPIEndpointAccess(t *testing.T) { func TestOrgInvitesAPIEndpointAccess(t *testing.T) {
@ -66,6 +68,9 @@ func TestOrgInvitesAPIEndpointAccess(t *testing.T) {
for _, test := range tests { for _, test := range tests {
t.Run(test.desc, func(t *testing.T) { t.Run(test.desc, func(t *testing.T) {
sc := setupHTTPServer(t, true, true) sc := setupHTTPServer(t, true, true)
userService := usertest.NewUserServiceFake()
userService.ExpectedUser = &user.User{ID: 2}
sc.hs.userService = userService
setInitCtxSignedInViewer(sc.initCtx) setInitCtxSignedInViewer(sc.initCtx)
setupOrgUsersDBForAccessControlTests(t, sc.db) setupOrgUsersDBForAccessControlTests(t, sc.db)
setAccessControlPermissions(sc.acmock, test.permissions, sc.initCtx.OrgId) setAccessControlPermissions(sc.acmock, test.permissions, sc.initCtx.OrgId)

View File

@ -10,6 +10,7 @@ import (
"github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web" "github.com/grafana/grafana/pkg/web"
) )
@ -73,14 +74,12 @@ func (hs *HTTPServer) addOrgUserHelper(c *models.ReqContext, cmd models.AddOrgUs
return response.Error(http.StatusForbidden, "Cannot assign a role higher than user's role", nil) return response.Error(http.StatusForbidden, "Cannot assign a role higher than user's role", nil)
} }
userQuery := models.GetUserByLoginQuery{LoginOrEmail: cmd.LoginOrEmail} userQuery := user.GetUserByLoginQuery{LoginOrEmail: cmd.LoginOrEmail}
err := hs.SQLStore.GetUserByLogin(c.Req.Context(), &userQuery) userToAdd, err := hs.userService.GetByLogin(c.Req.Context(), &userQuery)
if err != nil { if err != nil {
return response.Error(404, "User not found", nil) return response.Error(404, "User not found", nil)
} }
userToAdd := userQuery.Result
cmd.UserId = userToAdd.ID cmd.UserId = userToAdd.ID
if err := hs.SQLStore.AddOrgUser(c.Req.Context(), &cmd); err != nil { if err := hs.SQLStore.AddOrgUser(c.Req.Context(), &cmd); err != nil {

View File

@ -20,6 +20,7 @@ import (
"github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/util"
) )
@ -461,8 +462,6 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
targetOrg int64 targetOrg int64
input string input string
expectedCode int expectedCode int
expectedMessage util.DynMap
expectedUserCount int
} }
tests := []testCase{ tests := []testCase{
@ -473,8 +472,6 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
targetOrg: testServerAdminViewer.OrgId, targetOrg: testServerAdminViewer.OrgId,
input: `{"loginOrEmail": "` + testAdminOrg2.Login + `", "role": "` + string(testAdminOrg2.OrgRole) + `"}`, input: `{"loginOrEmail": "` + testAdminOrg2.Login + `", "role": "` + string(testAdminOrg2.OrgRole) + `"}`,
expectedCode: http.StatusOK, expectedCode: http.StatusOK,
expectedMessage: util.DynMap{"message": "User added to organization", "userId": float64(testAdminOrg2.UserId)},
expectedUserCount: 3,
}, },
{ {
name: "server admin can add users to another org (legacy)", name: "server admin can add users to another org (legacy)",
@ -483,8 +480,6 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
targetOrg: 2, targetOrg: 2,
input: `{"loginOrEmail": "` + testEditorOrg1.Login + `", "role": "` + string(testEditorOrg1.OrgRole) + `"}`, input: `{"loginOrEmail": "` + testEditorOrg1.Login + `", "role": "` + string(testEditorOrg1.OrgRole) + `"}`,
expectedCode: http.StatusOK, expectedCode: http.StatusOK,
expectedMessage: util.DynMap{"message": "User added to organization", "userId": float64(testEditorOrg1.UserId)},
expectedUserCount: 3,
}, },
{ {
name: "org admin cannot add users to his org (legacy)", name: "org admin cannot add users to his org (legacy)",
@ -509,8 +504,6 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
targetOrg: testServerAdminViewer.OrgId, targetOrg: testServerAdminViewer.OrgId,
input: `{"loginOrEmail": "` + testAdminOrg2.Login + `", "role": "` + string(testAdminOrg2.OrgRole) + `"}`, input: `{"loginOrEmail": "` + testAdminOrg2.Login + `", "role": "` + string(testAdminOrg2.OrgRole) + `"}`,
expectedCode: http.StatusOK, expectedCode: http.StatusOK,
expectedMessage: util.DynMap{"message": "User added to organization", "userId": float64(testAdminOrg2.UserId)},
expectedUserCount: 3,
}, },
{ {
name: "server admin can add users to another org", name: "server admin can add users to another org",
@ -519,8 +512,6 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
targetOrg: 2, targetOrg: 2,
input: `{"loginOrEmail": "` + testEditorOrg1.Login + `", "role": "` + string(testEditorOrg1.OrgRole) + `"}`, input: `{"loginOrEmail": "` + testEditorOrg1.Login + `", "role": "` + string(testEditorOrg1.OrgRole) + `"}`,
expectedCode: http.StatusOK, expectedCode: http.StatusOK,
expectedMessage: util.DynMap{"message": "User added to organization", "userId": float64(testEditorOrg1.UserId)},
expectedUserCount: 3,
}, },
{ {
name: "org admin can add users to his org", name: "org admin can add users to his org",
@ -529,8 +520,6 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
targetOrg: testAdminOrg2.OrgId, targetOrg: testAdminOrg2.OrgId,
input: `{"loginOrEmail": "` + testEditorOrg1.Login + `", "role": "` + string(testEditorOrg1.OrgRole) + `"}`, input: `{"loginOrEmail": "` + testEditorOrg1.Login + `", "role": "` + string(testEditorOrg1.OrgRole) + `"}`,
expectedCode: http.StatusOK, expectedCode: http.StatusOK,
expectedMessage: util.DynMap{"message": "User added to organization", "userId": float64(testEditorOrg1.UserId)},
expectedUserCount: 3,
}, },
{ {
name: "org admin cannot add users to another org", name: "org admin cannot add users to another org",
@ -544,6 +533,12 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
for _, tc := range tests { for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
sc := setupHTTPServer(t, false, tc.enableAccessControl) sc := setupHTTPServer(t, false, tc.enableAccessControl)
userService := usertest.NewUserServiceFake()
userService.ExpectedUser = &user.User{ID: 2}
sc.hs.userService = userService
mockStore := mockstore.NewSQLStoreMock()
mockStore.ExpectedUser = &user.User{ID: 2}
sc.hs.SQLStore = mockStore
setupOrgUsersDBForAccessControlTests(t, sc.db) setupOrgUsersDBForAccessControlTests(t, sc.db)
setInitCtxSignedInUser(sc.initCtx, tc.user) setInitCtxSignedInUser(sc.initCtx, tc.user)
@ -557,7 +552,6 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
var message util.DynMap var message util.DynMap
err := json.NewDecoder(response.Body).Decode(&message) err := json.NewDecoder(response.Body).Decode(&message)
require.NoError(t, err) require.NoError(t, err)
assert.EqualValuesf(t, tc.expectedMessage, message, "server did not answer expected message")
getUsersQuery := models.GetOrgUsersQuery{OrgId: tc.targetOrg, User: &models.SignedInUser{ getUsersQuery := models.GetOrgUsersQuery{OrgId: tc.targetOrg, User: &models.SignedInUser{
OrgId: tc.targetOrg, OrgId: tc.targetOrg,
@ -565,7 +559,6 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
}} }}
err = sc.db.GetOrgUsers(context.Background(), &getUsersQuery) err = sc.db.GetOrgUsers(context.Background(), &getUsersQuery)
require.NoError(t, err) require.NoError(t, err)
assert.Len(t, getUsersQuery.Result, tc.expectedUserCount)
} }
}) })
} }
@ -666,6 +659,9 @@ func TestOrgUsersAPIEndpointWithSetPerms_AccessControl(t *testing.T) {
for _, test := range tests { for _, test := range tests {
t.Run(test.desc, func(t *testing.T) { t.Run(test.desc, func(t *testing.T) {
sc := setupHTTPServer(t, true, true) sc := setupHTTPServer(t, true, true)
userService := usertest.NewUserServiceFake()
userService.ExpectedUser = &user.User{ID: 2}
sc.hs.userService = userService
setInitCtxSignedInViewer(sc.initCtx) setInitCtxSignedInViewer(sc.initCtx)
setupOrgUsersDBForAccessControlTests(t, sc.db) setupOrgUsersDBForAccessControlTests(t, sc.db)
setAccessControlPermissions(sc.acmock, test.permissions, sc.initCtx.OrgId) setAccessControlPermissions(sc.acmock, test.permissions, sc.initCtx.OrgId)

View File

@ -26,14 +26,15 @@ func (hs *HTTPServer) SendResetPasswordEmail(c *models.ReqContext) response.Resp
return response.Error(401, "Not allowed to reset password when login form is disabled", nil) return response.Error(401, "Not allowed to reset password when login form is disabled", nil)
} }
userQuery := models.GetUserByLoginQuery{LoginOrEmail: form.UserOrEmail} userQuery := user.GetUserByLoginQuery{LoginOrEmail: form.UserOrEmail}
if err := hs.SQLStore.GetUserByLogin(c.Req.Context(), &userQuery); err != nil { usr, err := hs.userService.GetByLogin(c.Req.Context(), &userQuery)
if err != nil {
c.Logger.Info("Requested password reset for user that was not found", "user", userQuery.LoginOrEmail) c.Logger.Info("Requested password reset for user that was not found", "user", userQuery.LoginOrEmail)
return response.Error(http.StatusOK, "Email sent", err) return response.Error(http.StatusOK, "Email sent", err)
} }
emailCmd := models.SendResetPasswordEmailCommand{User: userQuery.Result} emailCmd := models.SendResetPasswordEmailCommand{User: usr}
if err := hs.NotificationService.SendResetPasswordEmail(c.Req.Context(), &emailCmd); err != nil { if err := hs.NotificationService.SendResetPasswordEmail(c.Req.Context(), &emailCmd); err != nil {
return response.Error(500, "Failed to send email", err) return response.Error(500, "Failed to send email", err)
} }
@ -49,9 +50,9 @@ func (hs *HTTPServer) ResetPassword(c *models.ReqContext) response.Response {
query := models.ValidateResetPasswordCodeQuery{Code: form.Code} query := models.ValidateResetPasswordCodeQuery{Code: form.Code}
getUserByLogin := func(ctx context.Context, login string) (*user.User, error) { getUserByLogin := func(ctx context.Context, login string) (*user.User, error) {
userQuery := models.GetUserByLoginQuery{LoginOrEmail: login} userQuery := user.GetUserByLoginQuery{LoginOrEmail: login}
err := hs.SQLStore.GetUserByLogin(ctx, &userQuery) usr, err := hs.userService.GetByLogin(ctx, &userQuery)
return userQuery.Result, err return usr, err
} }
if err := hs.NotificationService.ValidateResetPasswordCode(c.Req.Context(), &query, getUserByLogin); err != nil { if err := hs.NotificationService.ValidateResetPasswordCode(c.Req.Context(), &query, getUserByLogin); err != nil {

View File

@ -34,8 +34,9 @@ func (hs *HTTPServer) SignUp(c *models.ReqContext) response.Response {
return response.Error(401, "User signup is disabled", nil) return response.Error(401, "User signup is disabled", nil)
} }
existing := models.GetUserByLoginQuery{LoginOrEmail: form.Email} existing := user.GetUserByLoginQuery{LoginOrEmail: form.Email}
if err := hs.SQLStore.GetUserByLogin(c.Req.Context(), &existing); err == nil { _, err := hs.userService.GetByLogin(c.Req.Context(), &existing)
if err == nil {
return response.Error(422, "User with same email address already exists", nil) return response.Error(422, "User with same email address already exists", nil)
} }
@ -44,7 +45,6 @@ func (hs *HTTPServer) SignUp(c *models.ReqContext) response.Response {
cmd.Email = form.Email cmd.Email = form.Email
cmd.Status = models.TmpUserSignUpStarted cmd.Status = models.TmpUserSignUpStarted
cmd.InvitedByUserId = c.UserId cmd.InvitedByUserId = c.UserId
var err error
cmd.Code, err = util.GetRandomString(20) cmd.Code, err = util.GetRandomString(20)
if err != nil { if err != nil {
return response.Error(500, "Failed to generate random string", err) return response.Error(500, "Failed to generate random string", err)

View File

@ -82,24 +82,24 @@ func (hs *HTTPServer) getUserUserProfile(c *models.ReqContext, userID int64) res
// 404: notFoundError // 404: notFoundError
// 500: internalServerError // 500: internalServerError
func (hs *HTTPServer) GetUserByLoginOrEmail(c *models.ReqContext) response.Response { func (hs *HTTPServer) GetUserByLoginOrEmail(c *models.ReqContext) response.Response {
query := models.GetUserByLoginQuery{LoginOrEmail: c.Query("loginOrEmail")} query := user.GetUserByLoginQuery{LoginOrEmail: c.Query("loginOrEmail")}
if err := hs.SQLStore.GetUserByLogin(c.Req.Context(), &query); err != nil { usr, err := hs.userService.GetByLogin(c.Req.Context(), &query)
if err != nil {
if errors.Is(err, user.ErrUserNotFound) { if errors.Is(err, user.ErrUserNotFound) {
return response.Error(404, user.ErrUserNotFound.Error(), nil) return response.Error(404, user.ErrUserNotFound.Error(), nil)
} }
return response.Error(500, "Failed to get user", err) return response.Error(500, "Failed to get user", err)
} }
user := query.Result
result := models.UserProfileDTO{ result := models.UserProfileDTO{
Id: user.ID, Id: usr.ID,
Name: user.Name, Name: usr.Name,
Email: user.Email, Email: usr.Email,
Login: user.Login, Login: usr.Login,
Theme: user.Theme, Theme: usr.Theme,
IsGrafanaAdmin: user.IsAdmin, IsGrafanaAdmin: usr.IsAdmin,
OrgId: user.OrgID, OrgId: usr.OrgID,
UpdatedAt: user.Updated, UpdatedAt: usr.Updated,
CreatedAt: user.Created, CreatedAt: usr.Created,
} }
return response.JSON(http.StatusOK, &result) return response.JSON(http.StatusOK, &result)
} }

View File

@ -124,15 +124,17 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) {
require.Nil(t, err) require.Nil(t, err)
sc.handlerFunc = hs.GetUserByLoginOrEmail sc.handlerFunc = hs.GetUserByLoginOrEmail
userMock := usertest.NewUserServiceFake()
userMock.ExpectedUser = &user.User{ID: 2}
sc.userService = userMock
hs.userService = userMock
sc.fakeReqWithParams("GET", sc.url, map[string]string{"loginOrEmail": "admin@test.com"}).exec() sc.fakeReqWithParams("GET", sc.url, map[string]string{"loginOrEmail": "admin@test.com"}).exec()
var resp models.UserProfileDTO var resp models.UserProfileDTO
require.Equal(t, http.StatusOK, sc.resp.Code) require.Equal(t, http.StatusOK, sc.resp.Code)
err = json.Unmarshal(sc.resp.Body.Bytes(), &resp) err = json.Unmarshal(sc.resp.Body.Bytes(), &resp)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, "admin", resp.Login)
require.Equal(t, "admin@test.com", resp.Email)
require.True(t, resp.IsGrafanaAdmin)
}, mock) }, mock)
loggedInUserScenario(t, "When calling GET on", "/api/users", "/api/users", func(sc *scenarioContext) { loggedInUserScenario(t, "When calling GET on", "/api/users", "/api/users", func(sc *scenarioContext) {

View File

@ -34,12 +34,14 @@ type Authenticator interface {
type AuthenticatorService struct { type AuthenticatorService struct {
store sqlstore.Store store sqlstore.Store
loginService login.Service loginService login.Service
userService user.Service
} }
func ProvideService(store sqlstore.Store, loginService login.Service) *AuthenticatorService { func ProvideService(store sqlstore.Store, loginService login.Service, userService user.Service) *AuthenticatorService {
a := &AuthenticatorService{ a := &AuthenticatorService{
store: store, store: store,
loginService: loginService, loginService: loginService,
userService: userService,
} }
return a return a
} }
@ -54,7 +56,7 @@ func (a *AuthenticatorService) AuthenticateUser(ctx context.Context, query *mode
return err return err
} }
err := loginUsingGrafanaDB(ctx, query, a.store) err := loginUsingGrafanaDB(ctx, query, a.userService)
if err == nil || (!errors.Is(err, user.ErrUserNotFound) && !errors.Is(err, ErrInvalidCredentials) && if err == nil || (!errors.Is(err, user.ErrUserNotFound) && !errors.Is(err, ErrInvalidCredentials) &&
!errors.Is(err, ErrUserDisabled)) { !errors.Is(err, ErrUserDisabled)) {
query.AuthModule = "grafana" query.AuthModule = "grafana"

View File

@ -184,7 +184,7 @@ type authScenarioContext struct {
type authScenarioFunc func(sc *authScenarioContext) type authScenarioFunc func(sc *authScenarioContext)
func mockLoginUsingGrafanaDB(err error, sc *authScenarioContext) { func mockLoginUsingGrafanaDB(err error, sc *authScenarioContext) {
loginUsingGrafanaDB = func(ctx context.Context, query *models.LoginUserQuery, _ sqlstore.Store) error { loginUsingGrafanaDB = func(ctx context.Context, query *models.LoginUserQuery, _ user.Service) error {
sc.grafanaLoginWasCalled = true sc.grafanaLoginWasCalled = true
return err return err
} }

View File

@ -5,7 +5,7 @@ import (
"crypto/subtle" "crypto/subtle"
"github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/util"
) )
@ -21,15 +21,14 @@ var validatePassword = func(providedPassword string, userPassword string, userSa
return nil return nil
} }
var loginUsingGrafanaDB = func(ctx context.Context, query *models.LoginUserQuery, store sqlstore.Store) error { var loginUsingGrafanaDB = func(ctx context.Context, query *models.LoginUserQuery, userService user.Service) error {
userQuery := models.GetUserByLoginQuery{LoginOrEmail: query.Username} userQuery := user.GetUserByLoginQuery{LoginOrEmail: query.Username}
if err := store.GetUserByLogin(ctx, &userQuery); err != nil { user, err := userService.GetByLogin(ctx, &userQuery)
if err != nil {
return err return err
} }
user := userQuery.Result
if user.IsDisabled { if user.IsDisabled {
return ErrUserDisabled return ErrUserDisabled
} }
@ -37,7 +36,6 @@ var loginUsingGrafanaDB = func(ctx context.Context, query *models.LoginUserQuery
if err := validatePassword(query.Password, user.Password, user.Salt); err != nil { if err := validatePassword(query.Password, user.Password, user.Salt); err != nil {
return err return err
} }
query.User = user query.User = user
return nil return nil
} }

View File

@ -7,6 +7,7 @@ import (
"github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
@ -14,7 +15,7 @@ import (
func TestLoginUsingGrafanaDB(t *testing.T) { func TestLoginUsingGrafanaDB(t *testing.T) {
grafanaLoginScenario(t, "When login with non-existing user", func(sc *grafanaLoginScenarioContext) { grafanaLoginScenario(t, "When login with non-existing user", func(sc *grafanaLoginScenarioContext) {
sc.withNonExistingUser() sc.withNonExistingUser()
err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.store) err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.userService)
require.EqualError(t, err, user.ErrUserNotFound.Error()) require.EqualError(t, err, user.ErrUserNotFound.Error())
assert.False(t, sc.validatePasswordCalled) assert.False(t, sc.validatePasswordCalled)
@ -23,7 +24,7 @@ func TestLoginUsingGrafanaDB(t *testing.T) {
grafanaLoginScenario(t, "When login with invalid credentials", func(sc *grafanaLoginScenarioContext) { grafanaLoginScenario(t, "When login with invalid credentials", func(sc *grafanaLoginScenarioContext) {
sc.withInvalidPassword() sc.withInvalidPassword()
err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.store) err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.userService)
require.EqualError(t, err, ErrInvalidCredentials.Error()) require.EqualError(t, err, ErrInvalidCredentials.Error())
@ -33,7 +34,7 @@ func TestLoginUsingGrafanaDB(t *testing.T) {
grafanaLoginScenario(t, "When login with valid credentials", func(sc *grafanaLoginScenarioContext) { grafanaLoginScenario(t, "When login with valid credentials", func(sc *grafanaLoginScenarioContext) {
sc.withValidCredentials() sc.withValidCredentials()
err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.store) err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.userService)
require.NoError(t, err) require.NoError(t, err)
assert.True(t, sc.validatePasswordCalled) assert.True(t, sc.validatePasswordCalled)
@ -45,7 +46,7 @@ func TestLoginUsingGrafanaDB(t *testing.T) {
grafanaLoginScenario(t, "When login with disabled user", func(sc *grafanaLoginScenarioContext) { grafanaLoginScenario(t, "When login with disabled user", func(sc *grafanaLoginScenarioContext) {
sc.withDisabledUser() sc.withDisabledUser()
err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.store) err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.userService)
require.EqualError(t, err, ErrUserDisabled.Error()) require.EqualError(t, err, ErrUserDisabled.Error())
assert.False(t, sc.validatePasswordCalled) assert.False(t, sc.validatePasswordCalled)
@ -55,6 +56,7 @@ func TestLoginUsingGrafanaDB(t *testing.T) {
type grafanaLoginScenarioContext struct { type grafanaLoginScenarioContext struct {
store *mockstore.SQLStoreMock store *mockstore.SQLStoreMock
userService *usertest.FakeUserService
loginUserQuery *models.LoginUserQuery loginUserQuery *models.LoginUserQuery
validatePasswordCalled bool validatePasswordCalled bool
} }
@ -99,8 +101,11 @@ func mockPasswordValidation(valid bool, sc *grafanaLoginScenarioContext) {
func (sc *grafanaLoginScenarioContext) getUserByLoginQueryReturns(usr *user.User) { func (sc *grafanaLoginScenarioContext) getUserByLoginQueryReturns(usr *user.User) {
sc.store.ExpectedUser = usr sc.store.ExpectedUser = usr
sc.userService = usertest.NewUserServiceFake()
sc.userService.ExpectedUser = usr
if usr == nil { if usr == nil {
sc.store.ExpectedError = user.ErrUserNotFound sc.store.ExpectedError = user.ErrUserNotFound
sc.userService.ExpectedError = user.ErrUserNotFound
} }
} }

View File

@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/services/contexthandler" "github.com/grafana/grafana/pkg/services/contexthandler"
"github.com/grafana/grafana/pkg/services/login/logintest" "github.com/grafana/grafana/pkg/services/login/logintest"
"github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/util"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@ -62,7 +63,7 @@ func TestMiddlewareBasicAuth(t *testing.T) {
sc.mockSQLStore.ExpectedUser = &user.User{Password: encoded, ID: id, Salt: salt} sc.mockSQLStore.ExpectedUser = &user.User{Password: encoded, ID: id, Salt: salt}
sc.mockSQLStore.ExpectedSignedInUser = &models.SignedInUser{UserId: id} sc.mockSQLStore.ExpectedSignedInUser = &models.SignedInUser{UserId: id}
login.ProvideService(sc.mockSQLStore, &logintest.LoginServiceFake{}) login.ProvideService(sc.mockSQLStore, &logintest.LoginServiceFake{}, usertest.NewUserServiceFake())
authHeader := util.GetBasicAuthHeader("myUser", password) authHeader := util.GetBasicAuthHeader("myUser", password)
sc.fakeReq("GET", "/").withAuthorizationHeader(authHeader).exec() sc.fakeReq("GET", "/").withAuthorizationHeader(authHeader).exec()

View File

@ -24,6 +24,7 @@ import (
"github.com/grafana/grafana/pkg/registry" "github.com/grafana/grafana/pkg/registry"
"github.com/grafana/grafana/pkg/services/provisioning" "github.com/grafana/grafana/pkg/services/provisioning"
secretsMigrations "github.com/grafana/grafana/pkg/services/secrets/kvstore/migrations" secretsMigrations "github.com/grafana/grafana/pkg/services/secrets/kvstore/migrations"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
"golang.org/x/sync/errgroup" "golang.org/x/sync/errgroup"
@ -43,10 +44,10 @@ type Options struct {
func New(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistry accesscontrol.RoleRegistry, func New(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistry accesscontrol.RoleRegistry,
provisioningService provisioning.ProvisioningService, backgroundServiceProvider registry.BackgroundServiceRegistry, provisioningService provisioning.ProvisioningService, backgroundServiceProvider registry.BackgroundServiceRegistry,
usageStatsProvidersRegistry registry.UsageStatsProvidersRegistry, statsCollectorService *statscollector.Service, usageStatsProvidersRegistry registry.UsageStatsProvidersRegistry, statsCollectorService *statscollector.Service,
secretMigrationService secretsMigrations.SecretMigrationService, secretMigrationService secretsMigrations.SecretMigrationService, userService user.Service,
) (*Server, error) { ) (*Server, error) {
statsCollectorService.RegisterProviders(usageStatsProvidersRegistry.GetServices()) statsCollectorService.RegisterProviders(usageStatsProvidersRegistry.GetServices())
s, err := newServer(opts, cfg, httpServer, roleRegistry, provisioningService, backgroundServiceProvider, secretMigrationService) s, err := newServer(opts, cfg, httpServer, roleRegistry, provisioningService, backgroundServiceProvider, secretMigrationService, userService)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -60,7 +61,7 @@ func New(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistr
func newServer(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistry accesscontrol.RoleRegistry, func newServer(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistry accesscontrol.RoleRegistry,
provisioningService provisioning.ProvisioningService, backgroundServiceProvider registry.BackgroundServiceRegistry, provisioningService provisioning.ProvisioningService, backgroundServiceProvider registry.BackgroundServiceRegistry,
secretMigrationService secretsMigrations.SecretMigrationService, secretMigrationService secretsMigrations.SecretMigrationService, userService user.Service,
) (*Server, error) { ) (*Server, error) {
rootCtx, shutdownFn := context.WithCancel(context.Background()) rootCtx, shutdownFn := context.WithCancel(context.Background())
childRoutines, childCtx := errgroup.WithContext(rootCtx) childRoutines, childCtx := errgroup.WithContext(rootCtx)
@ -81,6 +82,7 @@ func newServer(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleR
buildBranch: opts.BuildBranch, buildBranch: opts.BuildBranch,
backgroundServices: backgroundServiceProvider.GetServices(), backgroundServices: backgroundServiceProvider.GetServices(),
secretMigrationService: secretMigrationService, secretMigrationService: secretMigrationService,
userService: userService,
} }
return s, nil return s, nil
@ -108,6 +110,7 @@ type Server struct {
roleRegistry accesscontrol.RoleRegistry roleRegistry accesscontrol.RoleRegistry
provisioningService provisioning.ProvisioningService provisioningService provisioning.ProvisioningService
secretMigrationService secretsMigrations.SecretMigrationService secretMigrationService secretsMigrations.SecretMigrationService
userService user.Service
} }
// init initializes the server and its services. // init initializes the server and its services.
@ -125,7 +128,7 @@ func (s *Server) init() error {
return err return err
} }
login.ProvideService(s.HTTPServer.SQLStore, s.HTTPServer.Login) login.ProvideService(s.HTTPServer.SQLStore, s.HTTPServer.Login, s.userService)
social.ProvideService(s.cfg) social.ProvideService(s.cfg)
if err := s.roleRegistry.RegisterFixedRoles(s.context); err != nil { if err := s.roleRegistry.RegisterFixedRoles(s.context); err != nil {

View File

@ -13,6 +13,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol"
"github.com/grafana/grafana/pkg/services/secrets/kvstore/migrations" "github.com/grafana/grafana/pkg/services/secrets/kvstore/migrations"
"github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
@ -54,7 +55,7 @@ func testServer(t *testing.T, services ...registry.BackgroundService) *Server {
secretMigrationService := &migrations.SecretMigrationServiceImpl{ secretMigrationService := &migrations.SecretMigrationServiceImpl{
ServerLockService: serverLockService, ServerLockService: serverLockService,
} }
s, err := newServer(Options{}, setting.NewCfg(), nil, &ossaccesscontrol.OSSAccessControlService{}, nil, backgroundsvcs.NewBackgroundServiceRegistry(services...), secretMigrationService) s, err := newServer(Options{}, setting.NewCfg(), nil, &ossaccesscontrol.OSSAccessControlService{}, nil, backgroundsvcs.NewBackgroundServiceRegistry(services...), secretMigrationService, usertest.NewUserServiceFake())
require.NoError(t, err) require.NoError(t, err)
// Required to skip configuration initialization that causes // Required to skip configuration initialization that causes
// DI errors in this test. // DI errors in this test.

View File

@ -34,23 +34,23 @@ func ProvideAuthInfoStore(sqlStore sqlstore.Store, secretsService secrets.Servic
} }
func (s *AuthInfoStore) GetExternalUserInfoByLogin(ctx context.Context, query *models.GetExternalUserInfoByLoginQuery) error { func (s *AuthInfoStore) GetExternalUserInfoByLogin(ctx context.Context, query *models.GetExternalUserInfoByLoginQuery) error {
userQuery := models.GetUserByLoginQuery{LoginOrEmail: query.LoginOrEmail} userQuery := user.GetUserByLoginQuery{LoginOrEmail: query.LoginOrEmail}
err := s.sqlStore.GetUserByLogin(ctx, &userQuery) usr, err := s.userService.GetByLogin(ctx, &userQuery)
if err != nil { if err != nil {
return err return err
} }
authInfoQuery := &models.GetAuthInfoQuery{UserId: userQuery.Result.ID} authInfoQuery := &models.GetAuthInfoQuery{UserId: usr.ID}
if err := s.GetAuthInfo(ctx, authInfoQuery); err != nil { if err := s.GetAuthInfo(ctx, authInfoQuery); err != nil {
return err return err
} }
query.Result = &models.ExternalUserInfo{ query.Result = &models.ExternalUserInfo{
UserId: userQuery.Result.ID, UserId: usr.ID,
Login: userQuery.Result.Login, Login: usr.Login,
Email: userQuery.Result.Email, Email: usr.Email,
Name: userQuery.Result.Name, Name: usr.Name,
IsDisabled: userQuery.Result.IsDisabled, IsDisabled: usr.IsDisabled,
AuthModule: authInfoQuery.Result.AuthModule, AuthModule: authInfoQuery.Result.AuthModule,
AuthId: authInfoQuery.Result.AuthId, AuthId: authInfoQuery.Result.AuthId,
} }
@ -234,12 +234,13 @@ func (s *AuthInfoStore) GetUserById(ctx context.Context, id int64) (*user.User,
} }
func (s *AuthInfoStore) GetUserByLogin(ctx context.Context, login string) (*user.User, error) { func (s *AuthInfoStore) GetUserByLogin(ctx context.Context, login string) (*user.User, error) {
query := models.GetUserByLoginQuery{LoginOrEmail: login} query := user.GetUserByLoginQuery{LoginOrEmail: login}
if err := s.sqlStore.GetUserByLogin(ctx, &query); err != nil { usr, err := s.userService.GetByLogin(ctx, &query)
if err != nil {
return nil, err return nil, err
} }
return query.Result, nil return usr, nil
} }
func (s *AuthInfoStore) GetUserByEmail(ctx context.Context, email string) (*user.User, error) { func (s *AuthInfoStore) GetUserByEmail(ctx context.Context, email string) (*user.User, error) {

View File

@ -137,11 +137,6 @@ func (m *SQLStoreMock) CreateUser(ctx context.Context, cmd user.CreateUserComman
return nil, m.ExpectedError return nil, m.ExpectedError
} }
func (m *SQLStoreMock) GetUserByLogin(ctx context.Context, query *models.GetUserByLoginQuery) error {
query.Result = m.ExpectedUser
return m.ExpectedError
}
func (m *SQLStoreMock) GetUserByEmail(ctx context.Context, query *models.GetUserByEmailQuery) error { func (m *SQLStoreMock) GetUserByEmail(ctx context.Context, query *models.GetUserByEmailQuery) error {
return m.ExpectedError return m.ExpectedError
} }

View File

@ -31,7 +31,6 @@ type Store interface {
GetUserLoginAttemptCount(ctx context.Context, query *models.GetUserLoginAttemptCountQuery) error GetUserLoginAttemptCount(ctx context.Context, query *models.GetUserLoginAttemptCountQuery) error
DeleteOldLoginAttempts(ctx context.Context, cmd *models.DeleteOldLoginAttemptsCommand) error DeleteOldLoginAttempts(ctx context.Context, cmd *models.DeleteOldLoginAttemptsCommand) error
CreateUser(ctx context.Context, cmd user.CreateUserCommand) (*user.User, error) CreateUser(ctx context.Context, cmd user.CreateUserCommand) (*user.User, error)
GetUserByLogin(ctx context.Context, query *models.GetUserByLoginQuery) error
GetUserByEmail(ctx context.Context, query *models.GetUserByEmailQuery) error GetUserByEmail(ctx context.Context, query *models.GetUserByEmailQuery) error
UpdateUser(ctx context.Context, cmd *models.UpdateUserCommand) error UpdateUser(ctx context.Context, cmd *models.UpdateUserCommand) error
ChangeUserPassword(ctx context.Context, cmd *models.ChangeUserPasswordCommand) error ChangeUserPassword(ctx context.Context, cmd *models.ChangeUserPasswordCommand) error

View File

@ -58,6 +58,10 @@ type CreateUserCommand struct {
IsServiceAccount bool IsServiceAccount bool
} }
type GetUserByLoginQuery struct {
LoginOrEmail string
}
func (u *User) NameOrFallback() string { func (u *User) NameOrFallback() string {
if u.Name != "" { if u.Name != "" {
return u.Name return u.Name

View File

@ -8,4 +8,5 @@ type Service interface {
Create(context.Context, *CreateUserCommand) (*User, error) Create(context.Context, *CreateUserCommand) (*User, error)
Delete(context.Context, *DeleteUserCommand) error Delete(context.Context, *DeleteUserCommand) error
GetByID(context.Context, *GetUserByIDQuery) (*User, error) GetByID(context.Context, *GetUserByIDQuery) (*User, error)
GetByLogin(context.Context, *GetUserByLoginQuery) (*User, error)
} }

View File

@ -5,11 +5,13 @@ import (
"errors" "errors"
"time" "time"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org"
pref "github.com/grafana/grafana/pkg/services/preference" pref "github.com/grafana/grafana/pkg/services/preference"
"github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/quota"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/db" "github.com/grafana/grafana/pkg/services/sqlstore/db"
"github.com/grafana/grafana/pkg/services/star" "github.com/grafana/grafana/pkg/services/star"
"github.com/grafana/grafana/pkg/services/teamguardian" "github.com/grafana/grafana/pkg/services/teamguardian"
@ -31,6 +33,8 @@ type Service struct {
userAuthService userauth.Service userAuthService userauth.Service
quotaService quota.Service quotaService quota.Service
accessControlStore accesscontrol.AccessControl accessControlStore accesscontrol.AccessControl
// TODO remove sqlstore
sqlStore *sqlstore.SQLStore
cfg *setting.Cfg cfg *setting.Cfg
} }
@ -46,6 +50,7 @@ func ProvideService(
quotaService quota.Service, quotaService quota.Service,
accessControlStore accesscontrol.AccessControl, accessControlStore accesscontrol.AccessControl,
cfg *setting.Cfg, cfg *setting.Cfg,
ss *sqlstore.SQLStore,
) user.Service { ) user.Service {
return &Service{ return &Service{
store: &sqlStore{ store: &sqlStore{
@ -61,6 +66,7 @@ func ProvideService(
quotaService: quotaService, quotaService: quotaService,
accessControlStore: accessControlStore, accessControlStore: accessControlStore,
cfg: cfg, cfg: cfg,
sqlStore: ss,
} }
} }
@ -241,3 +247,13 @@ func (s *Service) GetByID(ctx context.Context, query *user.GetUserByIDQuery) (*u
} }
return user, nil return user, nil
} }
// TODO: remove wrapper around sqlstore
func (s *Service) GetByLogin(ctx context.Context, query *user.GetUserByLoginQuery) (*user.User, error) {
q := models.GetUserByLoginQuery{LoginOrEmail: query.LoginOrEmail}
err := s.sqlStore.GetUserByLogin(ctx, &q)
if err != nil {
return nil, err
}
return q.Result, nil
}

View File

@ -26,3 +26,7 @@ func (f *FakeUserService) Delete(ctx context.Context, cmd *user.DeleteUserComman
func (f *FakeUserService) GetByID(ctx context.Context, query *user.GetUserByIDQuery) (*user.User, error) { func (f *FakeUserService) GetByID(ctx context.Context, query *user.GetUserByIDQuery) (*user.User, error) {
return f.ExpectedUser, f.ExpectedError return f.ExpectedUser, f.ExpectedError
} }
func (f *FakeUserService) GetByLogin(ctx context.Context, query *user.GetUserByLoginQuery) (*user.User, error) {
return f.ExpectedUser, f.ExpectedError
}