From a863a4d95d02d8158d89ea01b15fcbc7aae3ce1e Mon Sep 17 00:00:00 2001 From: idafurjes <36131195+idafurjes@users.noreply.github.com> Date: Tue, 4 Oct 2022 12:17:55 +0200 Subject: [PATCH] Chore: Copy user methods over to user store (#56000) * Chore: Copy user methods over to user store * Fix some tests and bugs * Add some more tests * Move tests to user store * Move back the tests * Add some tests --- pkg/api/common_test.go | 2 +- pkg/api/org_users_test.go | 16 +-- pkg/api/user_test.go | 2 +- pkg/services/accesscontrol/middleware.go | 1 + .../resourcepermissions/service_test.go | 2 +- .../guardian/accesscontrol_guardian_test.go | 2 +- pkg/services/serviceaccounts/api/api_test.go | 2 +- pkg/services/sqlstore/store.go | 1 - pkg/services/sqlstore/user.go | 2 + pkg/services/team/teamtest/team.go | 4 + pkg/services/user/model.go | 4 +- pkg/services/user/userimpl/store.go | 76 +++++++++++++ pkg/services/user/userimpl/store_test.go | 76 +++++++++++++ pkg/services/user/userimpl/user.go | 105 +++++++++++++----- pkg/services/user/userimpl/user_test.go | 53 ++++++++- 15 files changed, 304 insertions(+), 44 deletions(-) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index c33fd64fb00..81a92a86c61 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -394,7 +394,7 @@ func setupHTTPServerWithCfgDb( acService, err = acimpl.ProvideService(cfg, db, routeRegister, localcache.ProvideService()) require.NoError(t, err) ac = acimpl.ProvideAccessControl(cfg) - userSvc = userimpl.ProvideService(db, nil, cfg, db) + userSvc = userimpl.ProvideService(db, nil, cfg, db, teamimpl.ProvideService(db, cfg), localcache.ProvideService()) } teamPermissionService, err := ossaccesscontrol.ProvideTeamPermissions(cfg, routeRegister, db, ac, license, acService, teamService, userSvc) require.NoError(t, err) diff --git a/pkg/api/org_users_test.go b/pkg/api/org_users_test.go index eb108229481..9d251827249 100644 --- a/pkg/api/org_users_test.go +++ b/pkg/api/org_users_test.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -22,6 +23,7 @@ import ( "github.com/grafana/grafana/pkg/services/org/orgtest" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" + "github.com/grafana/grafana/pkg/services/team/teamimpl" "github.com/grafana/grafana/pkg/services/temp_user/tempuserimpl" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/userimpl" @@ -388,7 +390,7 @@ func TestGetOrgUsersAPIEndpoint_AccessControlMetadata(t *testing.T) { cfg.RBACEnabled = tc.enableAccessControl sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) { hs.userService = userimpl.ProvideService( - hs.SQLStore, nil, cfg, hs.SQLStore.(*sqlstore.SQLStore), + hs.SQLStore, nil, cfg, hs.SQLStore.(*sqlstore.SQLStore), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), ) hs.orgService = orgimpl.ProvideService(hs.SQLStore, cfg) }) @@ -471,7 +473,7 @@ func TestGetOrgUsersAPIEndpoint_AccessControl(t *testing.T) { targetOrg: 2, }, { - name: "org admin can get users in his org", + name: "org admin can get users in their org", enableAccessControl: true, expectedCode: http.StatusOK, expectedUserCount: 2, @@ -492,7 +494,7 @@ func TestGetOrgUsersAPIEndpoint_AccessControl(t *testing.T) { cfg.RBACEnabled = tc.enableAccessControl sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) { hs.userService = userimpl.ProvideService( - hs.SQLStore, nil, cfg, hs.SQLStore.(*sqlstore.SQLStore), + hs.SQLStore, nil, cfg, hs.SQLStore.(*sqlstore.SQLStore), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), ) hs.orgService = orgimpl.ProvideService(hs.SQLStore, cfg) }) @@ -597,7 +599,7 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) { cfg.RBACEnabled = tc.enableAccessControl sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) { hs.userService = userimpl.ProvideService( - hs.SQLStore, nil, cfg, hs.SQLStore.(*sqlstore.SQLStore), + hs.SQLStore, nil, cfg, hs.SQLStore.(*sqlstore.SQLStore), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), ) }) @@ -716,7 +718,7 @@ func TestOrgUsersAPIEndpointWithSetPerms_AccessControl(t *testing.T) { sc := setupHTTPServer(t, true, func(hs *HTTPServer) { hs.tempUserService = tempuserimpl.ProvideService(hs.SQLStore) hs.userService = userimpl.ProvideService( - hs.SQLStore, nil, setting.NewCfg(), hs.SQLStore.(*sqlstore.SQLStore), + hs.SQLStore, nil, setting.NewCfg(), hs.SQLStore.(*sqlstore.SQLStore), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), setting.NewCfg()), localcache.ProvideService(), ) }) setInitCtxSignedInViewer(sc.initCtx) @@ -834,7 +836,7 @@ func TestPatchOrgUsersAPIEndpoint_AccessControl(t *testing.T) { cfg.RBACEnabled = tc.enableAccessControl sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) { hs.userService = userimpl.ProvideService( - hs.SQLStore, nil, cfg, hs.SQLStore.(*sqlstore.SQLStore), + hs.SQLStore, nil, cfg, hs.SQLStore.(*sqlstore.SQLStore), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), ) hs.orgService = orgimpl.ProvideService(hs.SQLStore, cfg) }) @@ -961,7 +963,7 @@ func TestDeleteOrgUsersAPIEndpoint_AccessControl(t *testing.T) { cfg.RBACEnabled = tc.enableAccessControl sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) { hs.userService = userimpl.ProvideService( - hs.SQLStore, nil, cfg, hs.SQLStore.(*sqlstore.SQLStore), + hs.SQLStore, nil, cfg, hs.SQLStore.(*sqlstore.SQLStore), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), ) hs.orgService = orgimpl.ProvideService(hs.SQLStore, cfg) }) diff --git a/pkg/api/user_test.go b/pkg/api/user_test.go index 2bf71390b10..7ba6733e56f 100644 --- a/pkg/api/user_test.go +++ b/pkg/api/user_test.go @@ -68,7 +68,7 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { } user, err := sqlStore.CreateUser(context.Background(), createUserCmd) require.Nil(t, err) - hs.userService = userimpl.ProvideService(sqlStore, nil, sc.cfg, sqlStore) + hs.userService = userimpl.ProvideService(sqlStore, nil, sc.cfg, sqlStore, nil, nil) sc.handlerFunc = hs.GetUserByID diff --git a/pkg/services/accesscontrol/middleware.go b/pkg/services/accesscontrol/middleware.go index 75845018f8a..4b7c0b1fb50 100644 --- a/pkg/services/accesscontrol/middleware.go +++ b/pkg/services/accesscontrol/middleware.go @@ -93,6 +93,7 @@ func newID() string { } type OrgIDGetter func(c *models.ReqContext) (int64, error) + type userCache interface { GetSignedInUserWithCacheCtx(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) } diff --git a/pkg/services/accesscontrol/resourcepermissions/service_test.go b/pkg/services/accesscontrol/resourcepermissions/service_test.go index b069a4fac4c..77a8e491aa0 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/service_test.go @@ -224,7 +224,7 @@ func setupTestEnvironment(t *testing.T, permissions []accesscontrol.Permission, sql := sqlstore.InitTestDB(t) cfg := setting.NewCfg() teamSvc := teamimpl.ProvideService(sql, cfg) - userSvc := userimpl.ProvideService(sql, nil, cfg, sql) + userSvc := userimpl.ProvideService(sql, nil, cfg, sql, teamimpl.ProvideService(sql, cfg), nil) license := licensingtest.NewFakeLicensing() license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe() mock := accesscontrolmock.New().WithPermissions(permissions) diff --git a/pkg/services/guardian/accesscontrol_guardian_test.go b/pkg/services/guardian/accesscontrol_guardian_test.go index c7bacd63911..daaccf4005c 100644 --- a/pkg/services/guardian/accesscontrol_guardian_test.go +++ b/pkg/services/guardian/accesscontrol_guardian_test.go @@ -604,7 +604,7 @@ func setupAccessControlGuardianTest(t *testing.T, uid string, permissions []acce license := licensingtest.NewFakeLicensing() license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe() teamSvc := teamimpl.ProvideService(store, store.Cfg) - userSvc := userimpl.ProvideService(store, nil, store.Cfg, store) + userSvc := userimpl.ProvideService(store, nil, store.Cfg, store, nil, nil) folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions( setting.NewCfg(), routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, ac, teamSvc, userSvc) diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index a77782f3c83..5146ef96e5d 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/pkg/services/serviceaccounts/api/api_test.go @@ -284,7 +284,7 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, sqlStore *sqlstore.SQLStore, saStore serviceaccounts.Store) (*web.Mux, *ServiceAccountsAPI) { cfg := setting.NewCfg() teamSvc := teamimpl.ProvideService(sqlStore, cfg) - userSvc := userimpl.ProvideService(sqlStore, nil, cfg, sqlStore) + userSvc := userimpl.ProvideService(sqlStore, nil, cfg, sqlStore, teamimpl.ProvideService(sqlStore, cfg), nil) saPermissionService, err := ossaccesscontrol.ProvideServiceAccountPermissions( cfg, routing.NewRouteRegister(), sqlStore, acmock, &licensing.OSSLicensingService{}, saStore, acmock, teamSvc, userSvc) require.NoError(t, err) diff --git a/pkg/services/sqlstore/store.go b/pkg/services/sqlstore/store.go index fab61c5fbd9..d6824bf7fc4 100644 --- a/pkg/services/sqlstore/store.go +++ b/pkg/services/sqlstore/store.go @@ -24,7 +24,6 @@ type Store interface { GetOrgById(context.Context, *models.GetOrgByIdQuery) error GetOrgByNameHandler(ctx context.Context, query *models.GetOrgByNameQuery) error CreateUser(ctx context.Context, cmd user.CreateUserCommand) (*user.User, error) - SetUsingOrg(ctx context.Context, cmd *models.SetUsingOrgCommand) error GetUserProfile(ctx context.Context, query *models.GetUserProfileQuery) error GetUserOrgList(ctx context.Context, query *models.GetUserOrgListQuery) error GetSignedInUser(ctx context.Context, query *models.GetSignedInUserQuery) error diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 71090e630ee..da4cd92f6d1 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -201,6 +201,7 @@ func (ss *SQLStore) GetUserById(ctx context.Context, query *models.GetUserByIdQu }) } +// deprecated method, use only for tests func (ss *SQLStore) SetUsingOrg(ctx context.Context, cmd *models.SetUsingOrgCommand) error { getOrgsForUserCmd := &models.GetUserOrgListQuery{UserId: cmd.UserId} if err := ss.GetUserOrgList(ctx, getOrgsForUserCmd); err != nil { @@ -301,6 +302,7 @@ func newSignedInUserCacheKey(orgID, userID int64) string { return fmt.Sprintf("signed-in-user-%d-%d", userID, orgID) } +// deprecated method, use only for tests func (ss *SQLStore) GetSignedInUserWithCacheCtx(ctx context.Context, query *models.GetSignedInUserQuery) error { cacheKey := newSignedInUserCacheKey(query.OrgId, query.UserId) if cached, found := ss.CacheService.Get(cacheKey); found { diff --git a/pkg/services/team/teamtest/team.go b/pkg/services/team/teamtest/team.go index 0314a757333..84f8c1b2aef 100644 --- a/pkg/services/team/teamtest/team.go +++ b/pkg/services/team/teamtest/team.go @@ -13,6 +13,10 @@ type FakeService struct { ExpectedError error } +func NewFakeService() *FakeService { + return &FakeService{} +} + func (s *FakeService) CreateTeam(name, email string, orgID int64) (models.Team, error) { return s.ExpectedTeam, s.ExpectedError } diff --git a/pkg/services/user/model.go b/pkg/services/user/model.go index d999b8d4f7f..0f966c354e3 100644 --- a/pkg/services/user/model.go +++ b/pkg/services/user/model.go @@ -186,10 +186,10 @@ type SetUserHelpFlagCommand struct { } type GetSignedInUserQuery struct { - UserID int64 + UserID int64 `xorm:"user_id"` Login string Email string - OrgID int64 + OrgID int64 `xorm:"org_id"` } type SignedInUser struct { diff --git a/pkg/services/user/userimpl/store.go b/pkg/services/user/userimpl/store.go index f2ed0c5c8dd..fdc1faeadce 100644 --- a/pkg/services/user/userimpl/store.go +++ b/pkg/services/user/userimpl/store.go @@ -3,6 +3,7 @@ package userimpl import ( "context" "fmt" + "strconv" "strings" "time" @@ -27,6 +28,8 @@ type store interface { Update(context.Context, *user.UpdateUserCommand) error ChangePassword(context.Context, *user.ChangeUserPasswordCommand) error UpdateLastSeenAt(context.Context, *user.UpdateUserLastSeenAtCommand) error + GetSignedInUser(context.Context, *user.GetSignedInUserQuery) (*user.SignedInUser, error) + UpdateUser(context.Context, *user.User) error } type sqlStore struct { @@ -311,3 +314,76 @@ func (ss *sqlStore) UpdateLastSeenAt(ctx context.Context, cmd *user.UpdateUserLa return err }) } + +func (ss *sqlStore) GetSignedInUser(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) { + var signedInUser user.SignedInUser + err := ss.db.WithDbSession(ctx, func(dbSess *sqlstore.DBSession) error { + orgId := "u.org_id" + if query.OrgID > 0 { + orgId = strconv.FormatInt(query.OrgID, 10) + } + + var rawSQL = `SELECT + u.id as user_id, + u.is_admin as is_grafana_admin, + u.email as email, + u.login as login, + u.name as name, + u.is_disabled as is_disabled, + u.help_flags1 as help_flags1, + u.last_seen_at as last_seen_at, + (SELECT COUNT(*) FROM org_user where org_user.user_id = u.id) as org_count, + user_auth.auth_module as external_auth_module, + user_auth.auth_id as external_auth_id, + org.name as org_name, + org_user.role as org_role, + org.id as org_id + FROM ` + ss.dialect.Quote("user") + ` as u + LEFT OUTER JOIN user_auth on user_auth.user_id = u.id + LEFT OUTER JOIN org_user on org_user.org_id = ` + orgId + ` and org_user.user_id = u.id + LEFT OUTER JOIN org on org.id = org_user.org_id ` + + sess := dbSess.Table("user") + sess = sess.Context(ctx) + switch { + case query.UserID > 0: + sess.SQL(rawSQL+"WHERE u.id=?", query.UserID) + case query.Login != "": + if ss.cfg.CaseInsensitiveLogin { + sess.SQL(rawSQL+"WHERE LOWER(u.login)=LOWER(?)", query.Login) + } else { + sess.SQL(rawSQL+"WHERE u.login=?", query.Login) + } + case query.Email != "": + if ss.cfg.CaseInsensitiveLogin { + sess.SQL(rawSQL+"WHERE LOWER(u.email)=LOWER(?)", query.Email) + } else { + sess.SQL(rawSQL+"WHERE u.email=?", query.Email) + } + } + has, err := sess.Get(&signedInUser) + if err != nil { + return err + } else if !has { + return user.ErrUserNotFound + } + + if signedInUser.OrgRole == "" { + signedInUser.OrgID = -1 + signedInUser.OrgName = "Org missing" + } + + if signedInUser.ExternalAuthModule != "oauth_grafana_com" { + signedInUser.ExternalAuthID = "" + } + return nil + }) + return &signedInUser, err +} + +func (ss *sqlStore) UpdateUser(ctx context.Context, user *user.User) error { + return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + _, err := sess.ID(user.ID).Update(user) + return err + }) +} diff --git a/pkg/services/user/userimpl/store_test.go b/pkg/services/user/userimpl/store_test.go index bf1c8cef735..062fe28fa63 100644 --- a/pkg/services/user/userimpl/store_test.go +++ b/pkg/services/user/userimpl/store_test.go @@ -6,8 +6,11 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/services/user" @@ -205,6 +208,44 @@ func TestIntegrationUserDataAccess(t *testing.T) { err := userStore.UpdateLastSeenAt(context.Background(), &user.UpdateUserLastSeenAtCommand{}) require.NoError(t, err) }) + + t.Run("get signed in user", func(t *testing.T) { + users := createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand { + return &user.CreateUserCommand{ + Email: fmt.Sprint("user", i, "@test.com"), + Name: fmt.Sprint("user", i), + Login: fmt.Sprint("loginuser", i), + IsDisabled: false, + } + }) + err := ss.AddOrgUser(context.Background(), &models.AddOrgUserCommand{ + LoginOrEmail: users[1].Login, Role: org.RoleViewer, + OrgId: users[0].OrgID, UserId: users[1].ID, + }) + require.Nil(t, err) + + err = updateDashboardACL(t, ss, 1, &models.DashboardACL{ + DashboardID: 1, OrgID: users[0].OrgID, UserID: users[1].ID, + Permission: models.PERMISSION_EDIT, + }) + require.Nil(t, err) + + ss.CacheService.Flush() + + query := &user.GetSignedInUserQuery{OrgID: users[1].OrgID, UserID: users[1].ID} + result, err := userStore.GetSignedInUser(context.Background(), query) + require.NoError(t, err) + require.Equal(t, result.Email, "user1@test.com") + }) + + t.Run("update user", func(t *testing.T) { + err := userStore.UpdateUser(context.Background(), &user.User{ID: 1, Name: "testtestest", Login: "loginloginlogin"}) + require.NoError(t, err) + result, err := userStore.GetByID(context.Background(), 1) + require.NoError(t, err) + assert.Equal(t, result.Name, "testtestest") + assert.Equal(t, result.Login, "loginloginlogin") + }) } func TestIntegrationUserUpdate(t *testing.T) { @@ -289,3 +330,38 @@ func createFiveTestUsers(t *testing.T, sqlStore *sqlstore.SQLStore, fn func(i in return users } + +// TODO: Use FakeDashboardStore when org has its own service +func updateDashboardACL(t *testing.T, sqlStore *sqlstore.SQLStore, dashboardID int64, items ...*models.DashboardACL) error { + t.Helper() + + err := sqlStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + _, err := sess.Exec("DELETE FROM dashboard_acl WHERE dashboard_id=?", dashboardID) + if err != nil { + return fmt.Errorf("deleting from dashboard_acl failed: %w", err) + } + + for _, item := range items { + item.Created = time.Now() + item.Updated = time.Now() + if item.UserID == 0 && item.TeamID == 0 && (item.Role == nil || !item.Role.IsValid()) { + return models.ErrDashboardACLInfoMissing + } + + if item.DashboardID == 0 { + return models.ErrDashboardPermissionDashboardEmpty + } + + sess.Nullable("user_id", "team_id") + if _, err := sess.Insert(item); err != nil { + return err + } + } + + // Update dashboard HasACL flag + dashboard := models.Dashboard{HasACL: true} + _, err = sess.Cols("has_acl").Where("id=?", dashboardID).Update(&dashboard) + return err + }) + return err +} diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index 1e1a9df869e..6bb0dd03bd8 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -3,20 +3,26 @@ package userimpl import ( "context" "errors" + "fmt" "time" + "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/models" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/db" + "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) type Service struct { - store store - orgService org.Service + store store + orgService org.Service + teamService team.Service + cacheService *localcache.CacheService // TODO remove sqlstore sqlStore *sqlstore.SQLStore cfg *setting.Cfg @@ -27,13 +33,17 @@ func ProvideService( orgService org.Service, cfg *setting.Cfg, ss *sqlstore.SQLStore, + teamService team.Service, + cacheService *localcache.CacheService, ) user.Service { store := ProvideStore(db, cfg) return &Service{ - store: &store, - orgService: orgService, - cfg: cfg, - sqlStore: ss, + store: &store, + orgService: orgService, + cfg: cfg, + sqlStore: ss, + teamService: teamService, + cacheService: cacheService, } } @@ -172,40 +182,81 @@ func (s *Service) UpdateLastSeenAt(ctx context.Context, cmd *user.UpdateUserLast return s.store.UpdateLastSeenAt(ctx, cmd) } -// TODO: remove wrapper around sqlstore func (s *Service) SetUsingOrg(ctx context.Context, cmd *user.SetUsingOrgCommand) error { - q := &models.SetUsingOrgCommand{ - UserId: cmd.UserID, - OrgId: cmd.OrgID, + getOrgsForUserCmd := &org.GetUserOrgListQuery{UserID: cmd.UserID} + orgsForUser, err := s.orgService.GetUserOrgList(ctx, getOrgsForUserCmd) + if err != nil { + return err } - return s.sqlStore.SetUsingOrg(ctx, q) + + valid := false + for _, other := range orgsForUser { + if other.OrgID == cmd.OrgID { + valid = true + } + } + if !valid { + return fmt.Errorf("user does not belong to org") + } + return s.store.UpdateUser(ctx, &user.User{ + ID: cmd.UserID, + OrgID: cmd.OrgID, + }) } -// TODO: remove wrapper around sqlstore func (s *Service) GetSignedInUserWithCacheCtx(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) { - q := &models.GetSignedInUserQuery{ - UserId: query.UserID, - Login: query.Login, - Email: query.Email, - OrgId: query.OrgID, + var signedInUser *user.SignedInUser + cacheKey := newSignedInUserCacheKey(query.OrgID, query.UserID) + if cached, found := s.cacheService.Get(cacheKey); found { + cachedUser := cached.(user.SignedInUser) + signedInUser = &cachedUser + return signedInUser, nil } - err := s.sqlStore.GetSignedInUserWithCacheCtx(ctx, q) + + result, err := s.GetSignedInUser(ctx, query) if err != nil { return nil, err } - return q.Result, nil + + cacheKey = newSignedInUserCacheKey(result.OrgID, query.UserID) + s.cacheService.Set(cacheKey, *result, time.Second*5) + return result, nil +} + +func newSignedInUserCacheKey(orgID, userID int64) string { + return fmt.Sprintf("signed-in-user-%d-%d", userID, orgID) } -// TODO: remove wrapper around sqlstore func (s *Service) GetSignedInUser(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) { - q := &models.GetSignedInUserQuery{ - UserId: query.UserID, - Login: query.Login, - Email: query.Email, - OrgId: query.OrgID, + signedInUser, err := s.store.GetSignedInUser(ctx, query) + if err != nil { + return nil, err } - err := s.sqlStore.GetSignedInUser(ctx, q) - return q.Result, err + + // tempUser is used to retrieve the teams for the signed in user for internal use. + tempUser := &user.SignedInUser{ + OrgID: signedInUser.OrgID, + Permissions: map[int64]map[string][]string{ + signedInUser.OrgID: { + ac.ActionTeamsRead: {ac.ScopeTeamsAll}, + }, + }, + } + getTeamsByUserQuery := &models.GetTeamsByUserQuery{ + OrgId: signedInUser.OrgID, + UserId: signedInUser.UserID, + SignedInUser: tempUser, + } + err = s.teamService.GetTeamsByUser(ctx, getTeamsByUserQuery) + if err != nil { + return nil, err + } + + signedInUser.Teams = make([]int64, len(getTeamsByUserQuery.Result)) + for i, t := range getTeamsByUserQuery.Result { + signedInUser.Teams[i] = t.Id + } + return signedInUser, err } // TODO: remove wrapper around sqlstore diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index 4bf87408b3e..cee31ad23a9 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -5,10 +5,14 @@ import ( "errors" "testing" + "github.com/grafana/grafana/pkg/infra/localcache" + "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org/orgtest" + "github.com/grafana/grafana/pkg/services/team/teamtest" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -16,8 +20,9 @@ func TestUserService(t *testing.T) { userStore := newUserStoreFake() orgService := orgtest.NewOrgServiceFake() userService := Service{ - store: userStore, - orgService: orgService, + store: userStore, + orgService: orgService, + cacheService: localcache.ProvideService(), } t.Run("create user", func(t *testing.T) { @@ -37,6 +42,7 @@ func TestUserService(t *testing.T) { require.NoError(t, err) require.Equal(t, "login", u.Login) require.Equal(t, "name", u.Name) + require.Equal(t, "email", u.Email) }) @@ -86,10 +92,45 @@ func TestUserService(t *testing.T) { _, err := userService.GetByID(context.Background(), &query) require.Error(t, err) }) + + t.Run("Testing DB - return list users based on their is_disabled flag", func(t *testing.T) { + userStore := newUserStoreFake() + orgService := orgtest.NewOrgServiceFake() + userService := Service{ + store: userStore, + orgService: orgService, + cacheService: localcache.ProvideService(), + teamService: teamtest.NewFakeService(), + } + usr := &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{1: {"users:read": {"global.users:*"}}}, + } + + usr2 := &user.SignedInUser{ + OrgID: 0, + Permissions: map[int64]map[string][]string{1: {"users:read": {"global.users:*"}}}, + } + + query1 := &user.GetSignedInUserQuery{OrgID: 1, UserID: 1} + userStore.ExpectedSignedInUser = usr + orgService.ExpectedUserOrgDTO = []*org.UserOrgDTO{{OrgID: 0}, {OrgID: 1}} + result, err := userService.GetSignedInUserWithCacheCtx(context.Background(), query1) + require.Nil(t, err) + require.NotNil(t, result) + assert.Equal(t, query1.OrgID, result.OrgID) + userStore.ExpectedSignedInUser = usr2 + query2 := &user.GetSignedInUserQuery{OrgID: 0, UserID: 1} + result2, err := userService.GetSignedInUserWithCacheCtx(context.Background(), query2) + require.Nil(t, err) + require.NotNil(t, result2) + assert.Equal(t, query2.OrgID, result2.OrgID) + }) } type FakeUserStore struct { ExpectedUser *user.User + ExpectedSignedInUser *user.SignedInUser ExpectedError error ExpectedDeleteUserError error } @@ -141,3 +182,11 @@ func (f *FakeUserStore) ChangePassword(ctx context.Context, cmd *user.ChangeUser func (f *FakeUserStore) UpdateLastSeenAt(ctx context.Context, cmd *user.UpdateUserLastSeenAtCommand) error { return f.ExpectedError } + +func (f *FakeUserStore) GetSignedInUser(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) { + return f.ExpectedSignedInUser, f.ExpectedError +} + +func (f *FakeUserStore) UpdateUser(ctx context.Context, user *user.User) error { + return f.ExpectedError +}