From ef11e783f121c4dd64aceb72e768971ff597a65e Mon Sep 17 00:00:00 2001 From: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> Date: Wed, 9 Feb 2022 18:45:31 +0800 Subject: [PATCH] ldap debug bus removal (#45014) * ldap debug bus removal * linter --- pkg/api/admin_users_test.go | 34 ++--- pkg/api/api.go | 2 +- pkg/api/ldap_debug.go | 15 ++- pkg/api/ldap_debug_test.go | 119 ++++-------------- pkg/api/org.go | 4 +- pkg/services/live/live.go | 3 +- .../login/loginservice/loginservice.go | 2 +- .../login/loginservice/loginservice_mock.go | 39 ++++++ pkg/services/login/userprotection.go | 1 - pkg/services/sqlstore/mockstore/mockstore.go | 6 + pkg/services/sqlstore/org.go | 4 +- pkg/services/sqlstore/org_test.go | 8 +- pkg/services/sqlstore/store.go | 1 + 13 files changed, 99 insertions(+), 139 deletions(-) create mode 100644 pkg/services/login/loginservice/loginservice_mock.go diff --git a/pkg/api/admin_users_test.go b/pkg/api/admin_users_test.go index 7b2bdbb37c6..31c43653ff5 100644 --- a/pkg/api/admin_users_test.go +++ b/pkg/api/admin_users_test.go @@ -2,7 +2,6 @@ package api import ( "context" - "errors" "fmt" "testing" @@ -13,7 +12,7 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/auth" - "github.com/grafana/grafana/pkg/services/login" + "github.com/grafana/grafana/pkg/services/login/loginservice" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/setting" @@ -453,8 +452,13 @@ func adminCreateUserScenario(t *testing.T, desc string, url string, routePattern t.Cleanup(bus.ClearBusHandlers) hs := HTTPServer{ - Bus: bus.GetBus(), - Login: fakeLoginService{expected: cmd}, + Bus: bus.GetBus(), + Login: loginservice.LoginServiceMock{ + ExpectedUserForm: cmd, + NoExistingOrgId: nonExistingOrgID, + AlreadyExitingLogin: existingTestLogin, + GeneratedUserId: testUserID, + }, } sc := setupScenarioContext(t, url) @@ -471,25 +475,3 @@ func adminCreateUserScenario(t *testing.T, desc string, url string, routePattern fn(sc) }) } - -type fakeLoginService struct { - login.Service - expected dtos.AdminCreateUserForm -} - -func (s fakeLoginService) CreateUser(cmd models.CreateUserCommand) (*models.User, error) { - if cmd.OrgId == nonExistingOrgID { - return nil, models.ErrOrgNotFound - } - - if cmd.Login == existingTestLogin { - return nil, models.ErrUserAlreadyExists - } - - if s.expected.Login == cmd.Login && s.expected.Email == cmd.Email && - s.expected.Password == cmd.Password && s.expected.Name == cmd.Name && s.expected.OrgId == cmd.OrgId { - return &models.User{Id: testUserID}, nil - } - - return nil, errors.New("unexpected cmd") -} diff --git a/pkg/api/api.go b/pkg/api/api.go index e3f253981e4..efe96c92e30 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -238,7 +238,7 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Post("/orgs", authorizeInOrg(reqSignedIn, acmiddleware.UseGlobalOrg, ac.EvalPermission(ActionOrgsCreate)), quota("org"), routing.Wrap(hs.CreateOrg)) // search all orgs - apiRoute.Get("/orgs", authorizeInOrg(reqGrafanaAdmin, acmiddleware.UseGlobalOrg, ac.EvalPermission(ActionOrgsRead)), routing.Wrap(SearchOrgs)) + apiRoute.Get("/orgs", authorizeInOrg(reqGrafanaAdmin, acmiddleware.UseGlobalOrg, ac.EvalPermission(ActionOrgsRead)), routing.Wrap(hs.SearchOrgs)) // orgs (admin routes) apiRoute.Group("/orgs/:orgId", func(orgsRoute routing.RouteRegister) { diff --git a/pkg/api/ldap_debug.go b/pkg/api/ldap_debug.go index ad223cdb66f..f59277c0d7a 100644 --- a/pkg/api/ldap_debug.go +++ b/pkg/api/ldap_debug.go @@ -8,12 +8,12 @@ import ( "strconv" "github.com/grafana/grafana/pkg/api/response" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/ldap" "github.com/grafana/grafana/pkg/services/multildap" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" ) @@ -64,7 +64,7 @@ type LDAPServerDTO struct { } // FetchOrgs fetches the organization(s) information by executing a single query to the database. Then, populating the DTO with the information retrieved. -func (user *LDAPUserDTO) FetchOrgs(ctx context.Context) error { +func (user *LDAPUserDTO) FetchOrgs(ctx context.Context, sqlstore sqlstore.Store) error { orgIds := []int64{} for _, or := range user.OrgRoles { @@ -74,7 +74,7 @@ func (user *LDAPUserDTO) FetchOrgs(ctx context.Context) error { q := &models.SearchOrgsQuery{} q.Ids = orgIds - if err := bus.Dispatch(ctx, q); err != nil { + if err := sqlstore.SearchOrgs(ctx, q); err != nil { return err } @@ -171,7 +171,7 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) response.Respon query := models.GetUserByIdQuery{Id: userId} - if err := bus.Dispatch(c.Req.Context(), &query); err != nil { // validate the userId exists + if err := hs.SQLStore.GetUserById(c.Req.Context(), &query); err != nil { // validate the userId exists if errors.Is(err, models.ErrUserNotFound) { return response.Error(404, models.ErrUserNotFound.Error(), nil) } @@ -180,8 +180,7 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) response.Respon } authModuleQuery := &models.GetAuthInfoQuery{UserId: query.Result.Id, AuthModule: models.AuthModuleLDAP} - - if err := bus.Dispatch(c.Req.Context(), authModuleQuery); err != nil { // validate the userId comes from LDAP + if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), authModuleQuery); err != nil { // validate the userId comes from LDAP if errors.Is(err, models.ErrUserNotFound) { return response.Error(404, models.ErrUserNotFound.Error(), nil) } @@ -223,7 +222,7 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) response.Respon SignupAllowed: hs.Cfg.LDAPAllowSignup, } - err = bus.Dispatch(c.Req.Context(), upsertCmd) + err = hs.Login.UpsertUser(c.Req.Context(), upsertCmd) if err != nil { return response.Error(http.StatusInternalServerError, "Failed to update the user", err) } @@ -306,7 +305,7 @@ func (hs *HTTPServer) GetUserFromLDAP(c *models.ReqContext) response.Response { u.OrgRoles = orgRoles ldapLogger.Debug("mapping org roles", "orgsRoles", u.OrgRoles) - err = u.FetchOrgs(c.Req.Context()) + err = u.FetchOrgs(c.Req.Context(), hs.SQLStore) if err != nil { return response.Error(http.StatusBadRequest, "An organization was not found - Please verify your LDAP configuration", err) } diff --git a/pkg/api/ldap_debug_test.go b/pkg/api/ldap_debug_test.go index 9cb6e339723..15c66cc13a4 100644 --- a/pkg/api/ldap_debug_test.go +++ b/pkg/api/ldap_debug_test.go @@ -9,6 +9,9 @@ import ( "testing" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/login/loginservice" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" @@ -53,7 +56,7 @@ func (m *LDAPMock) User(login string) (*models.ExternalUserInfo, ldap.ServerConf // GetUserFromLDAP tests // *** -func getUserFromLDAPContext(t *testing.T, requestURL string) *scenarioContext { +func getUserFromLDAPContext(t *testing.T, requestURL string, searchOrgRst []*models.OrgDTO) *scenarioContext { t.Helper() sc := setupScenarioContext(t, requestURL) @@ -62,7 +65,7 @@ func getUserFromLDAPContext(t *testing.T, requestURL string) *scenarioContext { setting.LDAPEnabled = true t.Cleanup(func() { setting.LDAPEnabled = origLDAP }) - hs := &HTTPServer{Cfg: setting.NewCfg(), ldapGroups: ldap.ProvideGroupsService()} + hs := &HTTPServer{Cfg: setting.NewCfg(), ldapGroups: ldap.ProvideGroupsService(), SQLStore: &mockstore.SQLStoreMock{ExpectedSearchOrgList: searchOrgRst}} sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response { sc.context = c @@ -90,7 +93,7 @@ func TestGetUserFromLDAPAPIEndpoint_UserNotFound(t *testing.T) { userSearchResult = nil - sc := getUserFromLDAPContext(t, "/api/admin/ldap/user-that-does-not-exist") + sc := getUserFromLDAPContext(t, "/api/admin/ldap/user-that-does-not-exist", []*models.OrgDTO{}) require.Equal(t, sc.resp.Code, http.StatusNotFound) assert.JSONEq(t, "{\"message\":\"No user was found in the LDAP server(s) with that username\"}", sc.resp.Body.String()) @@ -132,11 +135,6 @@ func TestGetUserFromLDAPAPIEndpoint_OrgNotfound(t *testing.T) { {Id: 1, Name: "Main Org."}, } - bus.AddHandler("test", func(ctx context.Context, query *models.SearchOrgsQuery) error { - query.Result = mockOrgSearchResult - return nil - }) - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { return &ldap.Config{}, nil } @@ -145,7 +143,7 @@ func TestGetUserFromLDAPAPIEndpoint_OrgNotfound(t *testing.T) { return &LDAPMock{} } - sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe") + sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe", mockOrgSearchResult) require.Equal(t, http.StatusBadRequest, sc.resp.Code) @@ -194,11 +192,6 @@ func TestGetUserFromLDAPAPIEndpoint(t *testing.T) { {Id: 1, Name: "Main Org."}, } - bus.AddHandler("test", func(ctx context.Context, query *models.SearchOrgsQuery) error { - query.Result = mockOrgSearchResult - return nil - }) - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { return &ldap.Config{}, nil } @@ -207,7 +200,7 @@ func TestGetUserFromLDAPAPIEndpoint(t *testing.T) { return &LDAPMock{} } - sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe") + sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe", mockOrgSearchResult) assert.Equal(t, sc.resp.Code, http.StatusOK) @@ -269,11 +262,6 @@ func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) { {Id: 1, Name: "Main Org."}, } - bus.AddHandler("test", func(ctx context.Context, query *models.SearchOrgsQuery) error { - query.Result = mockOrgSearchResult - return nil - }) - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { return &ldap.Config{}, nil } @@ -282,7 +270,7 @@ func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) { return &LDAPMock{} } - sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe") + sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe", mockOrgSearchResult) require.Equal(t, sc.resp.Code, http.StatusOK) @@ -376,7 +364,7 @@ func TestGetLDAPStatusAPIEndpoint(t *testing.T) { // PostSyncUserWithLDAP tests // *** -func postSyncUserWithLDAPContext(t *testing.T, requestURL string, preHook func(*testing.T, *scenarioContext)) *scenarioContext { +func postSyncUserWithLDAPContext(t *testing.T, requestURL string, preHook func(*testing.T, *scenarioContext), sqlstoremock sqlstore.Store) *scenarioContext { t.Helper() sc := setupScenarioContext(t, requestURL) @@ -390,6 +378,9 @@ func postSyncUserWithLDAPContext(t *testing.T, requestURL string, preHook func(* hs := &HTTPServer{ Cfg: sc.cfg, AuthTokenService: auth.NewFakeUserAuthTokenService(), + SQLStore: sqlstoremock, + Login: loginservice.LoginServiceMock{}, + authInfoService: &mockAuthInfoService{}, } sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response { @@ -412,6 +403,8 @@ func postSyncUserWithLDAPContext(t *testing.T, requestURL string, preHook func(* } func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) { + sqlstoremock := mockstore.SQLStoreMock{} + sqlstoremock.ExpectedUser = &models.User{Login: "ldap-daniel", Id: 34} sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34", func(t *testing.T, sc *scenarioContext) { getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { return &ldap.Config{}, nil @@ -424,26 +417,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) { userSearchResult = &models.ExternalUserInfo{ Login: "ldap-daniel", } - - bus.AddHandler("test", func(ctx context.Context, cmd *models.UpsertUserCommand) error { - require.Equal(t, "ldap-daniel", cmd.ExternalUser.Login) - return nil - }) - - bus.AddHandler("test", func(ctx context.Context, q *models.GetUserByIdQuery) error { - require.Equal(t, q.Id, int64(34)) - - q.Result = &models.User{Login: "ldap-daniel", Id: 34} - return nil - }) - - bus.AddHandler("test", func(ctx context.Context, q *models.GetAuthInfoQuery) error { - require.Equal(t, q.UserId, int64(34)) - require.Equal(t, q.AuthModule, models.AuthModuleLDAP) - - return nil - }) - }) + }, &sqlstoremock) assert.Equal(t, http.StatusOK, sc.resp.Code) @@ -457,6 +431,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) { } func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) { + sqlstoremock := mockstore.SQLStoreMock{ExpectedError: models.ErrUserNotFound} sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34", func(t *testing.T, sc *scenarioContext) { getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { return &ldap.Config{}, nil @@ -465,13 +440,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) { newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { return &LDAPMock{} } - - bus.AddHandler("test", func(ctx context.Context, q *models.GetUserByIdQuery) error { - require.Equal(t, q.Id, int64(34)) - - return models.ErrUserNotFound - }) - }) + }, &sqlstoremock) assert.Equal(t, http.StatusNotFound, sc.resp.Code) @@ -485,6 +454,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) { } func TestPostSyncUserWithLDAPAPIEndpoint_WhenGrafanaAdmin(t *testing.T) { + sqlstoremock := mockstore.SQLStoreMock{ExpectedUser: &models.User{Login: "ldap-daniel", Id: 34}} sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34", func(t *testing.T, sc *scenarioContext) { getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { return &ldap.Config{}, nil @@ -497,22 +467,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenGrafanaAdmin(t *testing.T) { userSearchError = multildap.ErrDidNotFindUser sc.cfg.AdminUser = "ldap-daniel" - - bus.AddHandler("test", func(ctx context.Context, q *models.GetUserByIdQuery) error { - require.Equal(t, q.Id, int64(34)) - - q.Result = &models.User{Login: "ldap-daniel", Id: 34} - return nil - }) - - bus.AddHandler("test", func(ctx context.Context, q *models.GetAuthInfoQuery) error { - require.Equal(t, q.UserId, int64(34)) - require.Equal(t, q.AuthModule, models.AuthModuleLDAP) - - return nil - }) - }) - + }, &sqlstoremock) assert.Equal(t, http.StatusBadRequest, sc.resp.Code) expected := ` @@ -526,6 +481,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenGrafanaAdmin(t *testing.T) { } func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) { + sqlstoremock := mockstore.SQLStoreMock{ExpectedUser: &models.User{Login: "ldap-daniel", Id: 34}} sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34", func(t *testing.T, sc *scenarioContext) { getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { return &ldap.Config{}, nil @@ -537,18 +493,6 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) { userSearchResult = nil - bus.AddHandler("test", func(ctx context.Context, cmd *models.UpsertUserCommand) error { - require.Equal(t, "ldap-daniel", cmd.ExternalUser.Login) - return nil - }) - - bus.AddHandler("test", func(ctx context.Context, q *models.GetUserByIdQuery) error { - require.Equal(t, q.Id, int64(34)) - - q.Result = &models.User{Login: "ldap-daniel", Id: 34} - return nil - }) - bus.AddHandler("test", func(ctx context.Context, q *models.GetExternalUserInfoByLoginQuery) error { assert.Equal(t, "ldap-daniel", q.LoginOrEmail) q.Result = &models.ExternalUserInfo{IsDisabled: true, UserId: 34} @@ -560,7 +504,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) { assert.Equal(t, 34, cmd.UserId) return nil }) - }) + }, &sqlstoremock) assert.Equal(t, http.StatusBadRequest, sc.resp.Code) @@ -670,8 +614,10 @@ func TestLDAP_AccessControl(t *testing.T) { cfg := setting.NewCfg() cfg.LDAPEnabled = true - - sc, _ := setupAccessControlScenarioContext(t, cfg, test.url, test.permissions) + sc, hs := setupAccessControlScenarioContext(t, cfg, test.url, test.permissions) + hs.SQLStore = &mockstore.SQLStoreMock{ExpectedUser: &models.User{}} + hs.authInfoService = &mockAuthInfoService{} + hs.Login = &loginservice.LoginServiceMock{} sc.resp = httptest.NewRecorder() sc.req, err = http.NewRequest(test.method, test.url, nil) assert.NoError(t, err) @@ -683,19 +629,6 @@ func TestLDAP_AccessControl(t *testing.T) { return &LDAPMock{} } - bus.AddHandler("test", func(ctx context.Context, q *models.GetUserByIdQuery) error { - q.Result = &models.User{} - return nil - }) - - bus.AddHandler("test", func(ctx context.Context, q *models.GetAuthInfoQuery) error { - return nil - }) - - bus.AddHandler("test", func(ctx context.Context, cmd *models.UpsertUserCommand) error { - return nil - }) - sc.exec() assert.Equal(t, test.expectedCode, sc.resp.Code) }) diff --git a/pkg/api/org.go b/pkg/api/org.go index 7aeea8e5c81..fa2b4ce944f 100644 --- a/pkg/api/org.go +++ b/pkg/api/org.go @@ -207,7 +207,7 @@ func (hs *HTTPServer) DeleteOrgByID(c *models.ReqContext) response.Response { return response.Success("Organization deleted") } -func SearchOrgs(c *models.ReqContext) response.Response { +func (hs *HTTPServer) SearchOrgs(c *models.ReqContext) response.Response { perPage := c.QueryInt("perpage") if perPage <= 0 { perPage = 1000 @@ -222,7 +222,7 @@ func SearchOrgs(c *models.ReqContext) response.Response { Limit: perPage, } - if err := sqlstore.SearchOrgs(c.Req.Context(), &query); err != nil { + if err := hs.SQLStore.SearchOrgs(c.Req.Context(), &query); err != nil { return response.Error(500, "Failed to search orgs", err) } diff --git a/pkg/services/live/live.go b/pkg/services/live/live.go index 927f0871538..f61b36ffc75 100644 --- a/pkg/services/live/live.go +++ b/pkg/services/live/live.go @@ -206,7 +206,8 @@ func ProvideService(plugCtxProvider *plugincontext.Provider, cfg *setting.Cfg, r // This can be unreasonable to have in production scenario with many // organizations. orgQuery := &models.SearchOrgsQuery{} - err := sqlstore.SearchOrgs(context.Background(), orgQuery) + + err := sqlStore.SearchOrgs(context.Background(), orgQuery) if err != nil { return nil, fmt.Errorf("can't get org list: %w", err) } diff --git a/pkg/services/login/loginservice/loginservice.go b/pkg/services/login/loginservice/loginservice.go index e567014a410..d3f9ffe01b1 100644 --- a/pkg/services/login/loginservice/loginservice.go +++ b/pkg/services/login/loginservice/loginservice.go @@ -16,7 +16,7 @@ var ( logger = log.New("login.ext_user") ) -func ProvideService(sqlStore *sqlstore.SQLStore, bus bus.Bus, quotaService *quota.QuotaService, authInfoService login.AuthInfoService) *Implementation { +func ProvideService(sqlStore sqlstore.Store, bus bus.Bus, quotaService *quota.QuotaService, authInfoService login.AuthInfoService) *Implementation { s := &Implementation{ SQLStore: sqlStore, Bus: bus, diff --git a/pkg/services/login/loginservice/loginservice_mock.go b/pkg/services/login/loginservice/loginservice_mock.go new file mode 100644 index 00000000000..46752034e02 --- /dev/null +++ b/pkg/services/login/loginservice/loginservice_mock.go @@ -0,0 +1,39 @@ +package loginservice + +import ( + "context" + "errors" + + "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/login" +) + +type LoginServiceMock struct { + login.Service + ExpectedUserForm dtos.AdminCreateUserForm + NoExistingOrgId int64 + AlreadyExitingLogin string + GeneratedUserId int64 +} + +func (s LoginServiceMock) CreateUser(cmd models.CreateUserCommand) (*models.User, error) { + if cmd.OrgId == s.NoExistingOrgId { + return nil, models.ErrOrgNotFound + } + + if cmd.Login == s.AlreadyExitingLogin { + return nil, models.ErrUserAlreadyExists + } + + if s.ExpectedUserForm.Login == cmd.Login && s.ExpectedUserForm.Email == cmd.Email && + s.ExpectedUserForm.Password == cmd.Password && s.ExpectedUserForm.Name == cmd.Name && s.ExpectedUserForm.OrgId == cmd.OrgId { + return &models.User{Id: s.GeneratedUserId}, nil + } + + return nil, errors.New("unexpected cmd") +} + +func (s LoginServiceMock) UpsertUser(ctx context.Context, cmd *models.UpsertUserCommand) error { + return nil +} diff --git a/pkg/services/login/userprotection.go b/pkg/services/login/userprotection.go index 1bc65f9bb83..63b64d1b690 100644 --- a/pkg/services/login/userprotection.go +++ b/pkg/services/login/userprotection.go @@ -16,7 +16,6 @@ type Store interface { SetAuthInfo(ctx context.Context, cmd *models.SetAuthInfoCommand) error UpdateAuthInfo(ctx context.Context, cmd *models.UpdateAuthInfoCommand) error DeleteAuthInfo(ctx context.Context, cmd *models.DeleteAuthInfoCommand) error - GetUserById(id int64) (bool, *models.User, error) GetUser(user *models.User) (bool, error) } diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index 31919457d80..940eb842249 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -27,6 +27,7 @@ type SQLStoreMock struct { ExpectedOrgListResponse OrgListResponse ExpectedDashboardSnapshot *models.DashboardSnapshot ExpectedTeamsByUser []*models.TeamDTO + ExpectedSearchOrgList []*models.OrgDTO ExpectedError error } @@ -628,3 +629,8 @@ func (m *SQLStoreMock) ExpireOldUserInvites(ctx context.Context, cmd *models.Exp func (m *SQLStoreMock) GetDBHealthQuery(ctx context.Context, query *models.GetDBHealthQuery) error { return m.ExpectedError } + +func (m *SQLStoreMock) SearchOrgs(ctx context.Context, query *models.SearchOrgsQuery) error { + query.Result = m.ExpectedSearchOrgList + return m.ExpectedError +} diff --git a/pkg/services/sqlstore/org.go b/pkg/services/sqlstore/org.go index cf90e4b3b6b..a373b81fe5b 100644 --- a/pkg/services/sqlstore/org.go +++ b/pkg/services/sqlstore/org.go @@ -21,11 +21,11 @@ func (ss *SQLStore) addOrgQueryAndCommandHandlers() { bus.AddHandler("sql", ss.UpdateOrg) bus.AddHandler("sql", ss.UpdateOrgAddress) bus.AddHandler("sql", GetOrgByName) - bus.AddHandler("sql", SearchOrgs) + bus.AddHandler("sql", ss.SearchOrgs) bus.AddHandler("sql", ss.DeleteOrg) } -func SearchOrgs(ctx context.Context, query *models.SearchOrgsQuery) error { +func (ss *SQLStore) SearchOrgs(ctx context.Context, query *models.SearchOrgsQuery) error { query.Result = make([]*models.OrgDTO, 0) sess := x.Table("org") if query.Query != "" { diff --git a/pkg/services/sqlstore/org_test.go b/pkg/services/sqlstore/org_test.go index 2d3c5ffb103..95f3056e35f 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/pkg/services/sqlstore/org_test.go @@ -32,7 +32,7 @@ func TestAccountDataAccess(t *testing.T) { } query := &models.SearchOrgsQuery{Ids: ids} - err = SearchOrgs(context.Background(), query) + err = sqlStore.SearchOrgs(context.Background(), query) require.NoError(t, err) require.Equal(t, len(query.Result), 3) @@ -48,7 +48,7 @@ func TestAccountDataAccess(t *testing.T) { t.Run("Should be able to search with defaults", func(t *testing.T) { query := &models.SearchOrgsQuery{} - err := SearchOrgs(context.Background(), query) + err := sqlStore.SearchOrgs(context.Background(), query) require.NoError(t, err) require.Equal(t, len(query.Result), 3) @@ -56,7 +56,7 @@ func TestAccountDataAccess(t *testing.T) { t.Run("Should be able to limit search", func(t *testing.T) { query := &models.SearchOrgsQuery{Limit: 1} - err := SearchOrgs(context.Background(), query) + err := sqlStore.SearchOrgs(context.Background(), query) require.NoError(t, err) require.Equal(t, len(query.Result), 1) @@ -64,7 +64,7 @@ func TestAccountDataAccess(t *testing.T) { t.Run("Should be able to limit and paginate search", func(t *testing.T) { query := &models.SearchOrgsQuery{Limit: 2, Page: 1} - err := SearchOrgs(context.Background(), query) + err := sqlStore.SearchOrgs(context.Background(), query) require.NoError(t, err) require.Equal(t, len(query.Result), 1) diff --git a/pkg/services/sqlstore/store.go b/pkg/services/sqlstore/store.go index 7adb1ea8ea8..1ce44bbb161 100644 --- a/pkg/services/sqlstore/store.go +++ b/pkg/services/sqlstore/store.go @@ -151,4 +151,5 @@ type Store interface { GetTempUserByCode(ctx context.Context, query *models.GetTempUserByCodeQuery) error ExpireOldUserInvites(ctx context.Context, cmd *models.ExpireTempUsersCommand) error GetDBHealthQuery(ctx context.Context, query *models.GetDBHealthQuery) error + SearchOrgs(ctx context.Context, query *models.SearchOrgsQuery) error }