From 5167c557602d8933cdb754a400827fd1cede0560 Mon Sep 17 00:00:00 2001 From: idafurjes <36131195+idafurjes@users.noreply.github.com> Date: Wed, 5 Oct 2022 09:34:36 +0200 Subject: [PATCH] Chore: Move methods from sqlstore to user store (#56305) * Copy sqlstore methods to suer store * Adjust ProvideService signatures in test * Add xorm tags and tests for search * Remove methods from sqlstore * fix lint in tests --- pkg/api/common_test.go | 2 +- pkg/api/org_users_test.go | 12 +- pkg/api/user_test.go | 2 +- .../resourcepermissions/service_test.go | 2 +- .../guardian/accesscontrol_guardian_test.go | 2 +- pkg/services/searchusers/searchusers.go | 2 +- pkg/services/serviceaccounts/api/api_test.go | 2 +- pkg/services/sqlstore/org_test.go | 101 ---- pkg/services/sqlstore/user.go | 155 ------ pkg/services/sqlstore/user_test.go | 321 ------------ pkg/services/user/model.go | 10 +- pkg/services/user/userimpl/store.go | 160 ++++++ pkg/services/user/userimpl/store_test.go | 471 ++++++++++++++++++ pkg/services/user/userimpl/user.go | 61 +-- pkg/services/user/userimpl/user_test.go | 23 +- 15 files changed, 670 insertions(+), 656 deletions(-) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index acfcbda561c..d2bba2fd666 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -398,7 +398,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, teamimpl.ProvideService(db, cfg), localcache.ProvideService()) + userSvc = userimpl.ProvideService(db, nil, cfg, 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 9d251827249..1d560352dff 100644 --- a/pkg/api/org_users_test.go +++ b/pkg/api/org_users_test.go @@ -390,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), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), + hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), ) hs.orgService = orgimpl.ProvideService(hs.SQLStore, cfg) }) @@ -494,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), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), + hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), ) hs.orgService = orgimpl.ProvideService(hs.SQLStore, cfg) }) @@ -599,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), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), + hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), ) }) @@ -718,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), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), setting.NewCfg()), localcache.ProvideService(), + hs.SQLStore, nil, setting.NewCfg(), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), setting.NewCfg()), localcache.ProvideService(), ) }) setInitCtxSignedInViewer(sc.initCtx) @@ -836,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), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), + hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), ) hs.orgService = orgimpl.ProvideService(hs.SQLStore, cfg) }) @@ -963,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), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), + hs.SQLStore, nil, cfg, 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 7ba6733e56f..204e9ce67ab 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, nil, nil) + hs.userService = userimpl.ProvideService(sqlStore, nil, sc.cfg, nil, nil) sc.handlerFunc = hs.GetUserByID diff --git a/pkg/services/accesscontrol/resourcepermissions/service_test.go b/pkg/services/accesscontrol/resourcepermissions/service_test.go index 77a8e491aa0..ee71359dc2c 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, teamimpl.ProvideService(sql, cfg), nil) + userSvc := userimpl.ProvideService(sql, nil, cfg, 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 daaccf4005c..ebc859e1fc0 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, nil, nil) + userSvc := userimpl.ProvideService(store, nil, store.Cfg, nil, nil) folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions( setting.NewCfg(), routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, ac, teamSvc, userSvc) diff --git a/pkg/services/searchusers/searchusers.go b/pkg/services/searchusers/searchusers.go index fbf994536af..4e57c678036 100644 --- a/pkg/services/searchusers/searchusers.go +++ b/pkg/services/searchusers/searchusers.go @@ -101,7 +101,7 @@ func (s *OSSService) SearchUser(c *models.ReqContext) (*user.SearchUserQueryResu } for _, user := range res.Users { - user.AvatarUrl = dtos.GetGravatarUrl(user.Email) + user.AvatarURL = dtos.GetGravatarUrl(user.Email) user.AuthLabels = make([]string, 0) if user.AuthModule != nil && len(user.AuthModule) > 0 { for _, authModule := range user.AuthModule { diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index 5146ef96e5d..6df108c3b21 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, teamimpl.ProvideService(sqlStore, cfg), nil) + userSvc := userimpl.ProvideService(sqlStore, nil, cfg, 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/org_test.go b/pkg/services/sqlstore/org_test.go index aaf97abc65c..484f1b90bee 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/pkg/services/sqlstore/org_test.go @@ -10,7 +10,6 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" dashver "github.com/grafana/grafana/pkg/services/dashboardversion" "github.com/grafana/grafana/pkg/services/org" @@ -129,21 +128,6 @@ func TestIntegrationAccountDataAccess(t *testing.T) { require.Equal(t, query.Result.Login, "ac1") }) - t.Run("Can search users", func(t *testing.T) { - query := models.SearchUsersQuery{Query: "", SignedInUser: &user.SignedInUser{ - OrgID: 1, - Permissions: map[int64]map[string][]string{ - 1: {accesscontrol.ActionUsersRead: {accesscontrol.ScopeGlobalUsersAll}}, - }, - }} - err := sqlStore.SearchUsers(context.Background(), &query) - - require.NoError(t, err) - require.Len(t, query.Result.Users, 2) - require.Equal(t, query.Result.Users[0].Email, "ac1@test.com") - require.Equal(t, query.Result.Users[1].Email, "ac2@test.com") - }) - t.Run("Given an added org user", func(t *testing.T) { cmd := models.AddOrgUserCommand{ OrgId: ac1.OrgID, @@ -359,88 +343,3 @@ func updateDashboardACL(t *testing.T, sqlStore *SQLStore, dashboardID int64, ite }) return err } - -// This function was copied from pkg/services/dashboards/database to circumvent -// import cycles. When this org-related code is refactored into a service the -// tests can the real GetDashboardACLInfoList functions -func getDashboardACLInfoList(s *SQLStore, query *models.GetDashboardACLInfoListQuery) error { - outerErr := s.WithDbSession(context.Background(), func(dbSession *DBSession) error { - query.Result = make([]*models.DashboardACLInfoDTO, 0) - falseStr := dialect.BooleanStr(false) - - if query.DashboardID == 0 { - sql := `SELECT - da.id, - da.org_id, - da.dashboard_id, - da.user_id, - da.team_id, - da.permission, - da.role, - da.created, - da.updated, - '' as user_login, - '' as user_email, - '' as team, - '' as title, - '' as slug, - '' as uid,` + - falseStr + ` AS is_folder,` + - falseStr + ` AS inherited - FROM dashboard_acl as da - WHERE da.dashboard_id = -1` - return dbSession.SQL(sql).Find(&query.Result) - } - - rawSQL := ` - -- get permissions for the dashboard and its parent folder - SELECT - da.id, - da.org_id, - da.dashboard_id, - da.user_id, - da.team_id, - da.permission, - da.role, - da.created, - da.updated, - u.login AS user_login, - u.email AS user_email, - ug.name AS team, - ug.email AS team_email, - d.title, - d.slug, - d.uid, - d.is_folder, - CASE WHEN (da.dashboard_id = -1 AND d.folder_id > 0) OR da.dashboard_id = d.folder_id THEN ` + dialect.BooleanStr(true) + ` ELSE ` + falseStr + ` END AS inherited - FROM dashboard as d - LEFT JOIN dashboard folder on folder.id = d.folder_id - LEFT JOIN dashboard_acl AS da ON - da.dashboard_id = d.id OR - da.dashboard_id = d.folder_id OR - ( - -- include default permissions --> - da.org_id = -1 AND ( - (folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR - (folder.id IS NULL AND d.has_acl = ` + falseStr + `) - ) - ) - LEFT JOIN ` + dialect.Quote("user") + ` AS u ON u.id = da.user_id - LEFT JOIN team ug on ug.id = da.team_id - WHERE d.org_id = ? AND d.id = ? AND da.id IS NOT NULL - ORDER BY da.id ASC - ` - - return dbSession.SQL(rawSQL, query.OrgID, query.DashboardID).Find(&query.Result) - }) - - if outerErr != nil { - return outerErr - } - - for _, p := range query.Result { - p.PermissionName = p.Permission.String() - } - - return nil -} diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 18b7d342025..edabcf712e4 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -463,161 +463,6 @@ func getTeamSelectSQLBase(filteredUsers []string) string { ` FROM team as team ` } -func (ss *SQLStore) SearchUsers(ctx context.Context, query *models.SearchUsersQuery) error { - return ss.WithDbSession(ctx, func(dbSess *DBSession) error { - query.Result = models.SearchUserQueryResult{ - Users: make([]*models.UserSearchHitDTO, 0), - } - - queryWithWildcards := "%" + query.Query + "%" - - whereConditions := make([]string, 0) - whereParams := make([]interface{}, 0) - sess := dbSess.Table("user").Alias("u") - - whereConditions = append(whereConditions, "u.is_service_account = ?") - whereParams = append(whereParams, dialect.BooleanStr(false)) - - // Join with only most recent auth module - joinCondition := `( - SELECT id from user_auth - WHERE user_auth.user_id = u.id - ORDER BY user_auth.created DESC ` - joinCondition = "user_auth.id=" + joinCondition + dialect.Limit(1) + ")" - sess.Join("LEFT", "user_auth", joinCondition) - if query.OrgId > 0 { - whereConditions = append(whereConditions, "org_id = ?") - whereParams = append(whereParams, query.OrgId) - } - - // user only sees the users for which it has read permissions - if !ac.IsDisabled(ss.Cfg) { - acFilter, err := ac.Filter(query.SignedInUser, "u.id", "global.users:id:", ac.ActionUsersRead) - if err != nil { - return err - } - whereConditions = append(whereConditions, acFilter.Where) - whereParams = append(whereParams, acFilter.Args...) - } - - if query.Query != "" { - whereConditions = append(whereConditions, "(email "+dialect.LikeStr()+" ? OR name "+dialect.LikeStr()+" ? OR login "+dialect.LikeStr()+" ?)") - whereParams = append(whereParams, queryWithWildcards, queryWithWildcards, queryWithWildcards) - } - - if query.IsDisabled != nil { - whereConditions = append(whereConditions, "is_disabled = ?") - whereParams = append(whereParams, query.IsDisabled) - } - - if query.AuthModule != "" { - whereConditions = append(whereConditions, `auth_module=?`) - whereParams = append(whereParams, query.AuthModule) - } - - if len(whereConditions) > 0 { - sess.Where(strings.Join(whereConditions, " AND "), whereParams...) - } - - for _, filter := range query.Filters { - if jc := filter.JoinCondition(); jc != nil { - sess.Join(jc.Operator, jc.Table, jc.Params) - } - if ic := filter.InCondition(); ic != nil { - sess.In(ic.Condition, ic.Params) - } - if wc := filter.WhereCondition(); wc != nil { - sess.Where(wc.Condition, wc.Params) - } - } - - if query.Limit > 0 { - offset := query.Limit * (query.Page - 1) - sess.Limit(query.Limit, offset) - } - - sess.Cols("u.id", "u.email", "u.name", "u.login", "u.is_admin", "u.is_disabled", "u.last_seen_at", "user_auth.auth_module") - sess.Asc("u.login", "u.email") - if err := sess.Find(&query.Result.Users); err != nil { - return err - } - - // get total - user := user.User{} - countSess := dbSess.Table("user").Alias("u") - - // Join with user_auth table if users filtered by auth_module - if query.AuthModule != "" { - countSess.Join("LEFT", "user_auth", joinCondition) - } - - if len(whereConditions) > 0 { - countSess.Where(strings.Join(whereConditions, " AND "), whereParams...) - } - - for _, filter := range query.Filters { - if jc := filter.JoinCondition(); jc != nil { - countSess.Join(jc.Operator, jc.Table, jc.Params) - } - if ic := filter.InCondition(); ic != nil { - countSess.In(ic.Condition, ic.Params) - } - if wc := filter.WhereCondition(); wc != nil { - countSess.Where(wc.Condition, wc.Params) - } - } - - count, err := countSess.Count(&user) - query.Result.TotalCount = count - - for _, user := range query.Result.Users { - user.LastSeenAtAge = util.GetAgeString(user.LastSeenAt) - } - - return err - }) -} - -func (ss *SQLStore) DisableUser(ctx context.Context, cmd *models.DisableUserCommand) error { - return ss.WithDbSession(ctx, func(dbSess *DBSession) error { - usr := user.User{} - sess := dbSess.Table("user") - - if has, err := sess.ID(cmd.UserId).Where(NotServiceAccountFilter(ss)).Get(&usr); err != nil { - return err - } else if !has { - return user.ErrUserNotFound - } - - usr.IsDisabled = cmd.IsDisabled - sess.UseBool("is_disabled") - - _, err := sess.ID(cmd.UserId).Update(&usr) - return err - }) -} - -func (ss *SQLStore) BatchDisableUsers(ctx context.Context, cmd *models.BatchDisableUsersCommand) error { - return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { - userIds := cmd.UserIds - - if len(userIds) == 0 { - return nil - } - - user_id_params := strings.Repeat(",?", len(userIds)-1) - disableSQL := "UPDATE " + dialect.Quote("user") + " SET is_disabled=? WHERE Id IN (?" + user_id_params + ")" - - disableParams := []interface{}{disableSQL, cmd.IsDisabled} - for _, v := range userIds { - disableParams = append(disableParams, v) - } - - _, err := sess.Where(NotServiceAccountFilter(ss)).Exec(disableParams...) - return err - }) -} - func (ss *SQLStore) DeleteUser(ctx context.Context, cmd *models.DeleteUserCommand) error { return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { return deleteUserInTransaction(ss, sess, cmd) diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index e06d4b77c94..0586dac022f 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -6,9 +6,7 @@ import ( "testing" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -17,10 +15,6 @@ func TestIntegrationUserDataAccess(t *testing.T) { t.Skip("skipping integration test") } ss := InitTestDB(t) - usr := &user.SignedInUser{ - OrgID: 1, - Permissions: map[int64]map[string][]string{1: {"users:read": {"global.users:*"}}}, - } t.Run("Testing DB - creates and loads disabled user", func(t *testing.T) { ss = InitTestDB(t) @@ -91,321 +85,6 @@ func TestIntegrationUserDataAccess(t *testing.T) { require.Equal(t, err, models.ErrOrgNotFound) }) - t.Run("Testing DB - multiple users", func(t *testing.T) { - ss = InitTestDB(t) - - 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, - } - }) - - // Return the first page of users and a total count - query := models.SearchUsersQuery{Query: "", Page: 1, Limit: 3, SignedInUser: usr} - err := ss.SearchUsers(context.Background(), &query) - - require.Nil(t, err) - require.Len(t, query.Result.Users, 3) - require.EqualValues(t, query.Result.TotalCount, 5) - - // Return the second page of users and a total count - query = models.SearchUsersQuery{Query: "", Page: 2, Limit: 3, SignedInUser: usr} - err = ss.SearchUsers(context.Background(), &query) - - require.Nil(t, err) - require.Len(t, query.Result.Users, 2) - require.EqualValues(t, query.Result.TotalCount, 5) - - // Return list of users matching query on user name - query = models.SearchUsersQuery{Query: "use", Page: 1, Limit: 3, SignedInUser: usr} - err = ss.SearchUsers(context.Background(), &query) - - require.Nil(t, err) - require.Len(t, query.Result.Users, 3) - require.EqualValues(t, query.Result.TotalCount, 5) - - query = models.SearchUsersQuery{Query: "ser1", Page: 1, Limit: 3, SignedInUser: usr} - err = ss.SearchUsers(context.Background(), &query) - - require.Nil(t, err) - require.Len(t, query.Result.Users, 1) - require.EqualValues(t, query.Result.TotalCount, 1) - - query = models.SearchUsersQuery{Query: "USER1", Page: 1, Limit: 3, SignedInUser: usr} - err = ss.SearchUsers(context.Background(), &query) - - require.Nil(t, err) - require.Len(t, query.Result.Users, 1) - require.EqualValues(t, query.Result.TotalCount, 1) - - query = models.SearchUsersQuery{Query: "idontexist", Page: 1, Limit: 3, SignedInUser: usr} - err = ss.SearchUsers(context.Background(), &query) - - require.Nil(t, err) - require.Len(t, query.Result.Users, 0) - require.EqualValues(t, query.Result.TotalCount, 0) - - // Return list of users matching query on email - query = models.SearchUsersQuery{Query: "ser1@test.com", Page: 1, Limit: 3, SignedInUser: usr} - err = ss.SearchUsers(context.Background(), &query) - - require.Nil(t, err) - require.Len(t, query.Result.Users, 1) - require.EqualValues(t, query.Result.TotalCount, 1) - - // Return list of users matching query on login name - query = models.SearchUsersQuery{Query: "loginuser1", Page: 1, Limit: 3, SignedInUser: usr} - err = ss.SearchUsers(context.Background(), &query) - - require.Nil(t, err) - require.Len(t, query.Result.Users, 1) - require.EqualValues(t, query.Result.TotalCount, 1) - }) - - t.Run("Testing DB - return list users based on their is_disabled flag", func(t *testing.T) { - ss = InitTestDB(t) - 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: i%2 == 0, - } - }) - - isDisabled := false - query := models.SearchUsersQuery{IsDisabled: &isDisabled, SignedInUser: usr} - err := ss.SearchUsers(context.Background(), &query) - require.Nil(t, err) - - require.Len(t, query.Result.Users, 2) - - first, third := false, false - for _, user := range query.Result.Users { - if user.Name == "user1" { - first = true - } - - if user.Name == "user3" { - third = true - } - } - - require.True(t, first) - require.True(t, third) - - // Re-init DB - ss = InitTestDB(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) - - // When the user is deleted - err = ss.DeleteUser(context.Background(), &models.DeleteUserCommand{UserId: users[1].ID}) - require.Nil(t, err) - - query1 := &models.GetOrgUsersQuery{OrgId: users[0].OrgID, User: usr} - err = ss.GetOrgUsersForTest(context.Background(), query1) - require.Nil(t, err) - - require.Len(t, query1.Result, 1) - - permQuery := &models.GetDashboardACLInfoListQuery{DashboardID: 1, OrgID: users[0].OrgID} - err = getDashboardACLInfoList(ss, permQuery) - require.Nil(t, err) - - require.Len(t, permQuery.Result, 0) - - // A user is an org member and has been assigned permissions - // Re-init DB - ss = InitTestDB(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() - - query3 := &models.GetSignedInUserQuery{OrgId: users[1].OrgID, UserId: users[1].ID} - err = ss.GetSignedInUserWithCacheCtx(context.Background(), query3) - require.Nil(t, err) - require.NotNil(t, query3.Result) - require.Equal(t, query3.OrgId, users[1].OrgID) - err = ss.SetUsingOrg(context.Background(), &models.SetUsingOrgCommand{UserId: users[1].ID, OrgId: users[0].OrgID}) - require.Nil(t, err) - query4 := &models.GetSignedInUserQuery{OrgId: 0, UserId: users[1].ID} - err = ss.GetSignedInUserWithCacheCtx(context.Background(), query4) - require.Nil(t, err) - require.NotNil(t, query4.Result) - require.Equal(t, query4.Result.OrgID, users[0].OrgID) - - cacheKey := newSignedInUserCacheKey(query4.Result.OrgID, query4.UserId) - _, found := ss.CacheService.Get(cacheKey) - require.True(t, found) - - disableCmd := models.BatchDisableUsersCommand{ - UserIds: []int64{users[0].ID, users[1].ID, users[2].ID, users[3].ID, users[4].ID}, - IsDisabled: true, - } - - err = ss.BatchDisableUsers(context.Background(), &disableCmd) - require.Nil(t, err) - - isDisabled = true - query5 := &models.SearchUsersQuery{IsDisabled: &isDisabled, SignedInUser: usr} - err = ss.SearchUsers(context.Background(), query5) - - require.Nil(t, err) - require.EqualValues(t, query5.Result.TotalCount, 5) - - // the user is deleted - err = ss.DeleteUser(context.Background(), &models.DeleteUserCommand{UserId: users[1].ID}) - require.Nil(t, err) - - // delete connected org users and permissions - query2 := &models.GetOrgUsersQuery{OrgId: users[0].OrgID} - err = ss.GetOrgUsersForTest(context.Background(), query2) - require.Nil(t, err) - - require.Len(t, query2.Result, 1) - - permQuery = &models.GetDashboardACLInfoListQuery{DashboardID: 1, OrgID: users[0].OrgID} - err = getDashboardACLInfoList(ss, permQuery) - require.Nil(t, err) - - require.Len(t, permQuery.Result, 0) - }) - - t.Run("Testing DB - return list of users that the SignedInUser has permission to read", func(t *testing.T) { - ss := InitTestDB(t) - 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), - } - }) - - testUser := &user.SignedInUser{ - OrgID: 1, - Permissions: map[int64]map[string][]string{1: {"users:read": {"global.users:id:1", "global.users:id:3"}}}, - } - query := models.SearchUsersQuery{SignedInUser: testUser} - err := ss.SearchUsers(context.Background(), &query) - assert.Nil(t, err) - assert.Len(t, query.Result.Users, 2) - }) - - ss = InitTestDB(t) - - t.Run("Testing DB - enable all users", 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: true, - } - }) - - disableCmd := models.BatchDisableUsersCommand{ - UserIds: []int64{users[0].ID, users[1].ID, users[2].ID, users[3].ID, users[4].ID}, - IsDisabled: false, - } - - err := ss.BatchDisableUsers(context.Background(), &disableCmd) - require.Nil(t, err) - - isDisabled := false - query := &models.SearchUsersQuery{IsDisabled: &isDisabled, SignedInUser: usr} - err = ss.SearchUsers(context.Background(), query) - - require.Nil(t, err) - require.EqualValues(t, query.Result.TotalCount, 5) - }) - - ss = InitTestDB(t) - - t.Run("Testing DB - disable only specific users", 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, - } - }) - - userIdsToDisable := []int64{} - for i := 0; i < 3; i++ { - userIdsToDisable = append(userIdsToDisable, users[i].ID) - } - disableCmd := models.BatchDisableUsersCommand{ - UserIds: userIdsToDisable, - IsDisabled: true, - } - - err := ss.BatchDisableUsers(context.Background(), &disableCmd) - require.Nil(t, err) - - query := models.SearchUsersQuery{SignedInUser: usr} - err = ss.SearchUsers(context.Background(), &query) - - require.Nil(t, err) - require.EqualValues(t, query.Result.TotalCount, 5) - for _, user := range query.Result.Users { - shouldBeDisabled := false - - // Check if user id is in the userIdsToDisable list - for _, disabledUserId := range userIdsToDisable { - if user.Id == disabledUserId { - require.True(t, user.IsDisabled) - shouldBeDisabled = true - } - } - - // Otherwise user shouldn't be disabled - if !shouldBeDisabled { - require.False(t, user.IsDisabled) - } - } - }) - ss = InitTestDB(t) t.Run("Testing DB - search users", func(t *testing.T) { diff --git a/pkg/services/user/model.go b/pkg/services/user/model.go index a16e20786e0..962810e6aed 100644 --- a/pkg/services/user/model.go +++ b/pkg/services/user/model.go @@ -104,7 +104,7 @@ type SetUsingOrgCommand struct { type SearchUsersQuery struct { SignedInUser *SignedInUser - OrgID int64 + OrgID int64 `xorm:"org_id"` Query string Page int Limit int @@ -122,11 +122,11 @@ type SearchUserQueryResult struct { } type UserSearchHitDTO struct { - ID int64 `json:"id"` + ID int64 `json:"id" xorm:"id"` Name string `json:"name"` Login string `json:"login"` Email string `json:"email"` - AvatarUrl string `json:"avatarUrl"` + AvatarURL string `json:"avatarUrl" xorm:"avatar_url"` IsAdmin bool `json:"isAdmin"` IsDisabled bool `json:"isDisabled"` LastSeenAt time.Time `json:"lastSeenAt"` @@ -171,12 +171,12 @@ func (auth *AuthModuleConversion) ToDB() ([]byte, error) { } type DisableUserCommand struct { - UserID int64 + UserID int64 `xorm:"user_id"` IsDisabled bool } type BatchDisableUsersCommand struct { - UserIDs []int64 + UserIDs []int64 `xorm:"user_ids"` IsDisabled bool } diff --git a/pkg/services/user/userimpl/store.go b/pkg/services/user/userimpl/store.go index 24e94e38af0..76d83242802 100644 --- a/pkg/services/user/userimpl/store.go +++ b/pkg/services/user/userimpl/store.go @@ -9,11 +9,13 @@ import ( "github.com/grafana/grafana/pkg/events" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/db" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" ) type store interface { @@ -33,6 +35,9 @@ type store interface { GetProfile(context.Context, *user.GetUserProfileQuery) (*user.UserProfileDTO, error) SetHelpFlag(context.Context, *user.SetUserHelpFlagCommand) error UpdatePermissions(context.Context, int64, bool) error + BatchDisableUsers(context.Context, *user.BatchDisableUsersCommand) error + Disable(context.Context, *user.DisableUserCommand) error + Search(context.Context, *user.SearchUsersQuery) (*user.SearchUserQueryResult, error) } type sqlStore struct { @@ -469,3 +474,158 @@ func validateOneAdminLeft(ctx context.Context, sess *sqlstore.DBSession) error { return nil } + +func (ss *sqlStore) BatchDisableUsers(ctx context.Context, cmd *user.BatchDisableUsersCommand) error { + return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + userIds := cmd.UserIDs + + if len(userIds) == 0 { + return nil + } + + user_id_params := strings.Repeat(",?", len(userIds)-1) + disableSQL := "UPDATE " + ss.dialect.Quote("user") + " SET is_disabled=? WHERE Id IN (?" + user_id_params + ")" + + disableParams := []interface{}{disableSQL, cmd.IsDisabled} + for _, v := range userIds { + disableParams = append(disableParams, v) + } + + _, err := sess.Where(ss.notServiceAccountFilter()).Exec(disableParams...) + return err + }) +} + +func (ss *sqlStore) Disable(ctx context.Context, cmd *user.DisableUserCommand) error { + return ss.db.WithDbSession(ctx, func(dbSess *sqlstore.DBSession) error { + usr := user.User{} + sess := dbSess.Table("user") + + if has, err := sess.ID(cmd.UserID).Where(ss.notServiceAccountFilter()).Get(&usr); err != nil { + return err + } else if !has { + return user.ErrUserNotFound + } + + usr.IsDisabled = cmd.IsDisabled + sess.UseBool("is_disabled") + + _, err := sess.ID(cmd.UserID).Update(&usr) + return err + }) +} + +func (ss *sqlStore) Search(ctx context.Context, query *user.SearchUsersQuery) (*user.SearchUserQueryResult, error) { + result := user.SearchUserQueryResult{ + Users: make([]*user.UserSearchHitDTO, 0), + } + err := ss.db.WithDbSession(ctx, func(dbSess *sqlstore.DBSession) error { + queryWithWildcards := "%" + query.Query + "%" + + whereConditions := make([]string, 0) + whereParams := make([]interface{}, 0) + sess := dbSess.Table("user").Alias("u") + + whereConditions = append(whereConditions, "u.is_service_account = ?") + whereParams = append(whereParams, ss.dialect.BooleanStr(false)) + + // Join with only most recent auth module + joinCondition := `( + SELECT id from user_auth + WHERE user_auth.user_id = u.id + ORDER BY user_auth.created DESC ` + joinCondition = "user_auth.id=" + joinCondition + ss.dialect.Limit(1) + ")" + sess.Join("LEFT", "user_auth", joinCondition) + if query.OrgID > 0 { + whereConditions = append(whereConditions, "org_id = ?") + whereParams = append(whereParams, query.OrgID) + } + + // user only sees the users for which it has read permissions + if !accesscontrol.IsDisabled(ss.cfg) { + acFilter, err := accesscontrol.Filter(query.SignedInUser, "u.id", "global.users:id:", accesscontrol.ActionUsersRead) + if err != nil { + return err + } + whereConditions = append(whereConditions, acFilter.Where) + whereParams = append(whereParams, acFilter.Args...) + } + + if query.Query != "" { + whereConditions = append(whereConditions, "(email "+ss.dialect.LikeStr()+" ? OR name "+ss.dialect.LikeStr()+" ? OR login "+ss.dialect.LikeStr()+" ?)") + whereParams = append(whereParams, queryWithWildcards, queryWithWildcards, queryWithWildcards) + } + + if query.IsDisabled != nil { + whereConditions = append(whereConditions, "is_disabled = ?") + whereParams = append(whereParams, query.IsDisabled) + } + + if query.AuthModule != "" { + whereConditions = append(whereConditions, `auth_module=?`) + whereParams = append(whereParams, query.AuthModule) + } + + if len(whereConditions) > 0 { + sess.Where(strings.Join(whereConditions, " AND "), whereParams...) + } + + for _, filter := range query.Filters { + if jc := filter.JoinCondition(); jc != nil { + sess.Join(jc.Operator, jc.Table, jc.Params) + } + if ic := filter.InCondition(); ic != nil { + sess.In(ic.Condition, ic.Params) + } + if wc := filter.WhereCondition(); wc != nil { + sess.Where(wc.Condition, wc.Params) + } + } + + if query.Limit > 0 { + offset := query.Limit * (query.Page - 1) + sess.Limit(query.Limit, offset) + } + + sess.Cols("u.id", "u.email", "u.name", "u.login", "u.is_admin", "u.is_disabled", "u.last_seen_at", "user_auth.auth_module") + sess.Asc("u.login", "u.email") + if err := sess.Find(&result.Users); err != nil { + return err + } + + // get total + user := user.User{} + countSess := dbSess.Table("user").Alias("u") + + // Join with user_auth table if users filtered by auth_module + if query.AuthModule != "" { + countSess.Join("LEFT", "user_auth", joinCondition) + } + + if len(whereConditions) > 0 { + countSess.Where(strings.Join(whereConditions, " AND "), whereParams...) + } + + for _, filter := range query.Filters { + if jc := filter.JoinCondition(); jc != nil { + countSess.Join(jc.Operator, jc.Table, jc.Params) + } + if ic := filter.InCondition(); ic != nil { + countSess.In(ic.Condition, ic.Params) + } + if wc := filter.WhereCondition(); wc != nil { + countSess.Where(wc.Condition, wc.Params) + } + } + + count, err := countSess.Count(&user) + result.TotalCount = count + + for _, user := range result.Users { + user.LastSeenAtAge = util.GetAgeString(user.LastSeenAt) + } + + return err + }) + return &result, err +} diff --git a/pkg/services/user/userimpl/store_test.go b/pkg/services/user/userimpl/store_test.go index f0a88d0de1c..f6b1413ab3f 100644 --- a/pkg/services/user/userimpl/store_test.go +++ b/pkg/services/user/userimpl/store_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/models" + "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/migrator" @@ -24,6 +25,10 @@ func TestIntegrationUserDataAccess(t *testing.T) { ss := sqlstore.InitTestDB(t) userStore := ProvideStore(ss, setting.NewCfg()) + usr := &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{1: {"users:read": {"global.users:*"}}}, + } t.Run("user not found", func(t *testing.T) { _, err := userStore.Get(context.Background(), @@ -311,6 +316,373 @@ func TestIntegrationUserDataAccess(t *testing.T) { err := userStore.SetHelpFlag(context.Background(), &user.SetUserHelpFlagCommand{UserID: 1, HelpFlags1: user.HelpFlags1(1)}) require.NoError(t, err) }) + + t.Run("Testing DB - return list users based on their is_disabled flag", func(t *testing.T) { + ss = sqlstore.InitTestDB(t) + 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: i%2 == 0, + } + }) + + isDisabled := false + query := user.SearchUsersQuery{IsDisabled: &isDisabled, SignedInUser: usr} + result, err := userStore.Search(context.Background(), &query) + require.Nil(t, err) + + require.Len(t, result.Users, 2) + + first, third := false, false + for _, user := range result.Users { + if user.Name == "user1" { + first = true + } + + if user.Name == "user3" { + third = true + } + } + + require.True(t, first) + require.True(t, third) + + // Re-init DB + ss = sqlstore.InitTestDB(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) + + // When the user is deleted + err = ss.DeleteUser(context.Background(), &models.DeleteUserCommand{UserId: users[1].ID}) + require.Nil(t, err) + + query1 := &org.GetOrgUsersQuery{OrgID: users[0].OrgID, User: usr} + query1Result, err := userStore.getOrgUsersForTest(context.Background(), query1) + require.Nil(t, err) + + require.Len(t, query1Result, 1) + + permQuery := &models.GetDashboardACLInfoListQuery{DashboardID: 1, OrgID: users[0].OrgID} + err = userStore.getDashboardACLInfoList(permQuery) + require.Nil(t, err) + + require.Len(t, permQuery.Result, 0) + + // A user is an org member and has been assigned permissions + // Re-init DB + ss = sqlstore.InitTestDB(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() + + query3 := &models.GetSignedInUserQuery{OrgId: users[1].OrgID, UserId: users[1].ID} + err = ss.GetSignedInUserWithCacheCtx(context.Background(), query3) + require.Nil(t, err) + require.NotNil(t, query3.Result) + require.Equal(t, query3.OrgId, users[1].OrgID) + err = ss.SetUsingOrg(context.Background(), &models.SetUsingOrgCommand{UserId: users[1].ID, OrgId: users[0].OrgID}) + require.Nil(t, err) + query4 := &models.GetSignedInUserQuery{OrgId: 0, UserId: users[1].ID} + err = ss.GetSignedInUserWithCacheCtx(context.Background(), query4) + require.Nil(t, err) + require.NotNil(t, query4.Result) + require.Equal(t, query4.Result.OrgID, users[0].OrgID) + + cacheKey := newSignedInUserCacheKey(query4.Result.OrgID, query4.UserId) + _, found := ss.CacheService.Get(cacheKey) + require.True(t, found) + + disableCmd := user.BatchDisableUsersCommand{ + UserIDs: []int64{users[0].ID, users[1].ID, users[2].ID, users[3].ID, users[4].ID}, + IsDisabled: true, + } + + err = userStore.BatchDisableUsers(context.Background(), &disableCmd) + require.Nil(t, err) + + isDisabled = true + query5 := &user.SearchUsersQuery{IsDisabled: &isDisabled, SignedInUser: usr} + query5Result, err := userStore.Search(context.Background(), query5) + require.Nil(t, err) + require.EqualValues(t, query5Result.TotalCount, 5) + + // the user is deleted + err = ss.DeleteUser(context.Background(), &models.DeleteUserCommand{UserId: users[1].ID}) + require.Nil(t, err) + + // delete connected org users and permissions + query2 := &org.GetOrgUsersQuery{OrgID: users[0].OrgID} + query2Result, err := userStore.getOrgUsersForTest(context.Background(), query2) + require.Nil(t, err) + + require.Len(t, query2Result, 1) + + permQuery = &models.GetDashboardACLInfoListQuery{DashboardID: 1, OrgID: users[0].OrgID} + err = userStore.getDashboardACLInfoList(permQuery) + require.Nil(t, err) + + require.Len(t, permQuery.Result, 0) + }) + + t.Run("Testing DB - return list of users that the SignedInUser has permission to read", func(t *testing.T) { + ss := sqlstore.InitTestDB(t) + 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), + } + }) + + testUser := &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{1: {"users:read": {"global.users:id:1", "global.users:id:3"}}}, + } + query := user.SearchUsersQuery{SignedInUser: testUser} + queryResult, err := userStore.Search(context.Background(), &query) + assert.Nil(t, err) + assert.Len(t, queryResult.Users, 2) + }) + + ss = sqlstore.InitTestDB(t) + + t.Run("Testing DB - enable all users", 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: true, + } + }) + + disableCmd := user.BatchDisableUsersCommand{ + UserIDs: []int64{users[0].ID, users[1].ID, users[2].ID, users[3].ID, users[4].ID}, + IsDisabled: false, + } + + err := userStore.BatchDisableUsers(context.Background(), &disableCmd) + require.Nil(t, err) + + isDisabled := false + query := &user.SearchUsersQuery{IsDisabled: &isDisabled, SignedInUser: usr} + queryResult, err := userStore.Search(context.Background(), query) + + require.Nil(t, err) + require.EqualValues(t, queryResult.TotalCount, 5) + }) + + t.Run("Can search users", func(t *testing.T) { + ss = sqlstore.InitTestDB(t) + userStore.cfg.AutoAssignOrg = false + + ac1cmd := user.CreateUserCommand{Login: "ac1", Email: "ac1@test.com", Name: "ac1 name"} + ac2cmd := user.CreateUserCommand{Login: "ac2", Email: "ac2@test.com", Name: "ac2 name", IsAdmin: true} + serviceaccountcmd := user.CreateUserCommand{Login: "serviceaccount", Email: "service@test.com", Name: "serviceaccount name", IsAdmin: true, IsServiceAccount: true} + + _, err := ss.CreateUser(context.Background(), ac1cmd) + require.NoError(t, err) + _, err = ss.CreateUser(context.Background(), ac2cmd) + require.NoError(t, err) + // user only used for making sure we filter out the service accounts + _, err = ss.CreateUser(context.Background(), serviceaccountcmd) + require.NoError(t, err) + query := user.SearchUsersQuery{Query: "", SignedInUser: &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{ + 1: {accesscontrol.ActionUsersRead: {accesscontrol.ScopeGlobalUsersAll}}, + }, + }} + queryResult, err := userStore.Search(context.Background(), &query) + require.NoError(t, err) + require.Len(t, queryResult.Users, 2) + require.Equal(t, queryResult.Users[0].Email, "ac1@test.com") + require.Equal(t, queryResult.Users[1].Email, "ac2@test.com") + }) + + ss = sqlstore.InitTestDB(t) + + t.Run("Testing DB - disable only specific users", 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, + } + }) + + userIdsToDisable := []int64{} + for i := 0; i < 3; i++ { + userIdsToDisable = append(userIdsToDisable, users[i].ID) + } + disableCmd := user.BatchDisableUsersCommand{ + UserIDs: userIdsToDisable, + IsDisabled: true, + } + + err := userStore.BatchDisableUsers(context.Background(), &disableCmd) + require.Nil(t, err) + + query := user.SearchUsersQuery{SignedInUser: usr} + queryResult, err := userStore.Search(context.Background(), &query) + require.Nil(t, err) + require.EqualValues(t, queryResult.TotalCount, 5) + for _, user := range queryResult.Users { + shouldBeDisabled := false + + // Check if user id is in the userIdsToDisable list + for _, disabledUserId := range userIdsToDisable { + fmt.Println(user.ID, disabledUserId) + if user.ID == disabledUserId { + require.True(t, user.IsDisabled) + shouldBeDisabled = true + } + } + + // Otherwise user shouldn't be disabled + if !shouldBeDisabled { + require.False(t, user.IsDisabled) + } + } + }) + + ss = sqlstore.InitTestDB(t) + + t.Run("Testing DB - search users", func(t *testing.T) { + // Since previous tests were destructive + 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, + } + }) + }) + + t.Run("Disable user", func(t *testing.T) { + id, err := userStore.Insert(context.Background(), &user.User{ + Name: "user111", + Created: time.Now(), + Updated: time.Now(), + }) + require.NoError(t, err) + err = userStore.Disable(context.Background(), &user.DisableUserCommand{UserID: id}) + require.NoError(t, err) + }) + + t.Run("Testing DB - multiple users", func(t *testing.T) { + ss = sqlstore.InitTestDB(t) + + 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, + } + }) + + // Return the first page of users and a total count + query := user.SearchUsersQuery{Query: "", Page: 1, Limit: 3, SignedInUser: usr} + queryResult, err := userStore.Search(context.Background(), &query) + + require.Nil(t, err) + require.Len(t, queryResult.Users, 3) + require.EqualValues(t, queryResult.TotalCount, 5) + + // Return the second page of users and a total count + query = user.SearchUsersQuery{Query: "", Page: 2, Limit: 3, SignedInUser: usr} + queryResult, err = userStore.Search(context.Background(), &query) + + require.Nil(t, err) + require.Len(t, queryResult.Users, 2) + require.EqualValues(t, queryResult.TotalCount, 5) + + // Return list of users matching query on user name + query = user.SearchUsersQuery{Query: "use", Page: 1, Limit: 3, SignedInUser: usr} + queryResult, err = userStore.Search(context.Background(), &query) + + require.Nil(t, err) + require.Len(t, queryResult.Users, 3) + require.EqualValues(t, queryResult.TotalCount, 5) + + query = user.SearchUsersQuery{Query: "ser1", Page: 1, Limit: 3, SignedInUser: usr} + queryResult, err = userStore.Search(context.Background(), &query) + + require.Nil(t, err) + require.Len(t, queryResult.Users, 1) + require.EqualValues(t, queryResult.TotalCount, 1) + + query = user.SearchUsersQuery{Query: "USER1", Page: 1, Limit: 3, SignedInUser: usr} + queryResult, err = userStore.Search(context.Background(), &query) + + require.Nil(t, err) + require.Len(t, queryResult.Users, 1) + require.EqualValues(t, queryResult.TotalCount, 1) + + query = user.SearchUsersQuery{Query: "idontexist", Page: 1, Limit: 3, SignedInUser: usr} + queryResult, err = userStore.Search(context.Background(), &query) + + require.Nil(t, err) + require.Len(t, queryResult.Users, 0) + require.EqualValues(t, queryResult.TotalCount, 0) + + // Return list of users matching query on email + query = user.SearchUsersQuery{Query: "ser1@test.com", Page: 1, Limit: 3, SignedInUser: usr} + queryResult, err = userStore.Search(context.Background(), &query) + + require.Nil(t, err) + require.Len(t, queryResult.Users, 1) + require.EqualValues(t, queryResult.TotalCount, 1) + + // Return list of users matching query on login name + query = user.SearchUsersQuery{Query: "loginuser1", Page: 1, Limit: 3, SignedInUser: usr} + queryResult, err = userStore.Search(context.Background(), &query) + + require.Nil(t, err) + require.Len(t, queryResult.Users, 1) + require.EqualValues(t, queryResult.TotalCount, 1) + }) } func TestIntegrationUserUpdate(t *testing.T) { @@ -430,3 +802,102 @@ func updateDashboardACL(t *testing.T, sqlStore *sqlstore.SQLStore, dashboardID i }) return err } + +func (ss *sqlStore) getOrgUsersForTest(ctx context.Context, query *org.GetOrgUsersQuery) ([]*org.OrgUserDTO, error) { + result := make([]*org.OrgUserDTO, 0) + err := ss.db.WithDbSession(ctx, func(dbSess *sqlstore.DBSession) error { + sess := dbSess.Table("org_user") + sess.Join("LEFT ", ss.dialect.Quote("user"), fmt.Sprintf("org_user.user_id=%s.id", ss.dialect.Quote("user"))) + sess.Where("org_user.org_id=?", query.OrgID) + sess.Cols("org_user.org_id", "org_user.user_id", "user.email", "user.login", "org_user.role") + + err := sess.Find(&result) + return err + }) + return result, err +} + +// This function was copied from pkg/services/dashboards/database to circumvent +// import cycles. When this org-related code is refactored into a service the +// tests can the real GetDashboardACLInfoList functions +func (ss *sqlStore) getDashboardACLInfoList(query *models.GetDashboardACLInfoListQuery) error { + outerErr := ss.db.WithDbSession(context.Background(), func(dbSession *sqlstore.DBSession) error { + query.Result = make([]*models.DashboardACLInfoDTO, 0) + falseStr := ss.dialect.BooleanStr(false) + + if query.DashboardID == 0 { + sql := `SELECT + da.id, + da.org_id, + da.dashboard_id, + da.user_id, + da.team_id, + da.permission, + da.role, + da.created, + da.updated, + '' as user_login, + '' as user_email, + '' as team, + '' as title, + '' as slug, + '' as uid,` + + falseStr + ` AS is_folder,` + + falseStr + ` AS inherited + FROM dashboard_acl as da + WHERE da.dashboard_id = -1` + return dbSession.SQL(sql).Find(&query.Result) + } + + rawSQL := ` + -- get permissions for the dashboard and its parent folder + SELECT + da.id, + da.org_id, + da.dashboard_id, + da.user_id, + da.team_id, + da.permission, + da.role, + da.created, + da.updated, + u.login AS user_login, + u.email AS user_email, + ug.name AS team, + ug.email AS team_email, + d.title, + d.slug, + d.uid, + d.is_folder, + CASE WHEN (da.dashboard_id = -1 AND d.folder_id > 0) OR da.dashboard_id = d.folder_id THEN ` + ss.dialect.BooleanStr(true) + ` ELSE ` + falseStr + ` END AS inherited + FROM dashboard as d + LEFT JOIN dashboard folder on folder.id = d.folder_id + LEFT JOIN dashboard_acl AS da ON + da.dashboard_id = d.id OR + da.dashboard_id = d.folder_id OR + ( + -- include default permissions --> + da.org_id = -1 AND ( + (folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR + (folder.id IS NULL AND d.has_acl = ` + falseStr + `) + ) + ) + LEFT JOIN ` + ss.dialect.Quote("user") + ` AS u ON u.id = da.user_id + LEFT JOIN team ug on ug.id = da.team_id + WHERE d.org_id = ? AND d.id = ? AND da.id IS NOT NULL + ORDER BY da.id ASC + ` + + return dbSession.SQL(rawSQL, query.OrgID, query.DashboardID).Find(&query.Result) + }) + + if outerErr != nil { + return outerErr + } + + for _, p := range query.Result { + p.PermissionName = p.Permission.String() + } + + return nil +} diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index 31133c4f152..add60b72963 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -10,7 +10,6 @@ import ( "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" @@ -23,16 +22,13 @@ type Service struct { orgService org.Service teamService team.Service cacheService *localcache.CacheService - // TODO remove sqlstore - sqlStore *sqlstore.SQLStore - cfg *setting.Cfg + cfg *setting.Cfg } func ProvideService( db db.DB, orgService org.Service, cfg *setting.Cfg, - ss *sqlstore.SQLStore, teamService team.Service, cacheService *localcache.CacheService, ) user.Service { @@ -41,7 +37,6 @@ func ProvideService( store: &store, orgService: orgService, cfg: cfg, - sqlStore: ss, teamService: teamService, cacheService: cacheService, } @@ -259,64 +254,16 @@ func (s *Service) GetSignedInUser(ctx context.Context, query *user.GetSignedInUs return signedInUser, err } -// TODO: remove wrapper around sqlstore func (s *Service) Search(ctx context.Context, query *user.SearchUsersQuery) (*user.SearchUserQueryResult, error) { - var usrSeschHitDTOs []*user.UserSearchHitDTO - q := &models.SearchUsersQuery{ - SignedInUser: query.SignedInUser, - Query: query.Query, - OrgId: query.OrgID, - Page: query.Page, - Limit: query.Limit, - AuthModule: query.AuthModule, - Filters: query.Filters, - IsDisabled: query.IsDisabled, - } - err := s.sqlStore.SearchUsers(ctx, q) - if err != nil { - return nil, err - } - for _, usrSearch := range q.Result.Users { - usrSeschHitDTOs = append(usrSeschHitDTOs, &user.UserSearchHitDTO{ - ID: usrSearch.Id, - Login: usrSearch.Login, - Email: usrSearch.Email, - Name: usrSearch.Name, - AvatarUrl: usrSearch.AvatarUrl, - IsDisabled: usrSearch.IsDisabled, - IsAdmin: usrSearch.IsAdmin, - LastSeenAt: usrSearch.LastSeenAt, - LastSeenAtAge: usrSearch.LastSeenAtAge, - AuthLabels: usrSearch.AuthLabels, - AuthModule: user.AuthModuleConversion(usrSearch.AuthModule), - }) - } - - res := &user.SearchUserQueryResult{ - Users: usrSeschHitDTOs, - TotalCount: q.Result.TotalCount, - Page: q.Result.Page, - PerPage: q.Result.PerPage, - } - return res, nil + return s.store.Search(ctx, query) } -// TODO: remove wrapper around sqlstore func (s *Service) Disable(ctx context.Context, cmd *user.DisableUserCommand) error { - q := &models.DisableUserCommand{ - UserId: cmd.UserID, - IsDisabled: cmd.IsDisabled, - } - return s.sqlStore.DisableUser(ctx, q) + return s.store.Disable(ctx, cmd) } -// TODO: remove wrapper around sqlstore func (s *Service) BatchDisableUsers(ctx context.Context, cmd *user.BatchDisableUsersCommand) error { - c := &models.BatchDisableUsersCommand{ - UserIds: cmd.UserIDs, - IsDisabled: cmd.IsDisabled, - } - return s.sqlStore.BatchDisableUsers(ctx, c) + return s.store.BatchDisableUsers(ctx, cmd) } func (s *Service) UpdatePermissions(ctx context.Context, userID int64, isAdmin bool) error { diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index 7fd094eab83..c6611fe2ae5 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -129,11 +129,12 @@ func TestUserService(t *testing.T) { } type FakeUserStore struct { - ExpectedUser *user.User - ExpectedSignedInUser *user.SignedInUser - ExpectedUserProfile *user.UserProfileDTO - ExpectedError error - ExpectedDeleteUserError error + ExpectedUser *user.User + ExpectedSignedInUser *user.SignedInUser + ExpectedUserProfile *user.UserProfileDTO + ExpectedSearchUserQueryResult *user.SearchUserQueryResult + ExpectedError error + ExpectedDeleteUserError error } func newUserStoreFake() *FakeUserStore { @@ -203,3 +204,15 @@ func (f *FakeUserStore) SetHelpFlag(ctx context.Context, cmd *user.SetUserHelpFl func (f *FakeUserStore) UpdatePermissions(ctx context.Context, userID int64, isAdmin bool) error { return f.ExpectedError } + +func (f *FakeUserStore) BatchDisableUsers(ctx context.Context, cmd *user.BatchDisableUsersCommand) error { + return f.ExpectedError +} + +func (f *FakeUserStore) Disable(ctx context.Context, cmd *user.DisableUserCommand) error { + return f.ExpectedError +} + +func (f *FakeUserStore) Search(ctx context.Context, query *user.SearchUsersQuery) (*user.SearchUserQueryResult, error) { + return f.ExpectedSearchUserQueryResult, f.ExpectedError +}