diff --git a/pkg/api/admin_users.go b/pkg/api/admin_users.go index 120bb17f416..7daf81be95c 100644 --- a/pkg/api/admin_users.go +++ b/pkg/api/admin_users.go @@ -159,7 +159,7 @@ func (hs *HTTPServer) AdminUpdateUserPermissions(c *models.ReqContext) response. return response.Error(http.StatusBadRequest, "id is invalid", err) } - err = hs.userService.UpdatePermissions(userID, form.IsGrafanaAdmin) + err = hs.userService.UpdatePermissions(c.Req.Context(), userID, form.IsGrafanaAdmin) if err != nil { if errors.Is(err, user.ErrLastGrafanaAdmin) { return response.Error(400, user.ErrLastGrafanaAdmin.Error(), nil) diff --git a/pkg/services/login/loginservice/loginservice.go b/pkg/services/login/loginservice/loginservice.go index cc7d05711dc..ee3538239cf 100644 --- a/pkg/services/login/loginservice/loginservice.go +++ b/pkg/services/login/loginservice/loginservice.go @@ -148,7 +148,7 @@ func (ls *Implementation) UpsertUser(ctx context.Context, cmd *models.UpsertUser // Sync isGrafanaAdmin permission if extUser.IsGrafanaAdmin != nil && *extUser.IsGrafanaAdmin != cmd.Result.IsAdmin { - if errPerms := ls.userService.UpdatePermissions(cmd.Result.ID, *extUser.IsGrafanaAdmin); errPerms != nil { + if errPerms := ls.userService.UpdatePermissions(ctx, cmd.Result.ID, *extUser.IsGrafanaAdmin); errPerms != nil { return errPerms } } diff --git a/pkg/services/sqlstore/store.go b/pkg/services/sqlstore/store.go index d6824bf7fc4..7d0eb608a2e 100644 --- a/pkg/services/sqlstore/store.go +++ b/pkg/services/sqlstore/store.go @@ -27,8 +27,6 @@ type Store interface { GetUserProfile(ctx context.Context, query *models.GetUserProfileQuery) error GetUserOrgList(ctx context.Context, query *models.GetUserOrgListQuery) error GetSignedInUser(ctx context.Context, query *models.GetSignedInUserQuery) error - UpdateUserPermissions(userID int64, isAdmin bool) error - SetUserHelpFlag(ctx context.Context, cmd *models.SetUserHelpFlagCommand) error WithDbSession(ctx context.Context, callback DBTransactionFunc) error WithNewDbSession(ctx context.Context, callback DBTransactionFunc) error GetOrgQuotaByTarget(ctx context.Context, query *models.GetOrgQuotaByTargetQuery) error diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index da4cd92f6d1..18b7d342025 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -702,52 +702,3 @@ func UserDeletions() []string { } return deletes } - -// UpdateUserPermissions sets the user Server Admin flag -func (ss *SQLStore) UpdateUserPermissions(userID int64, isAdmin bool) error { - return ss.WithTransactionalDbSession(context.Background(), func(sess *DBSession) error { - var user user.User - if _, err := sess.ID(userID).Where(NotServiceAccountFilter(ss)).Get(&user); err != nil { - return err - } - - user.IsAdmin = isAdmin - sess.UseBool("is_admin") - _, err := sess.ID(user.ID).Update(&user) - if err != nil { - return err - } - // validate that after update there is at least one server admin - if err := validateOneAdminLeft(sess); err != nil { - return err - } - return nil - }) -} - -func (ss *SQLStore) SetUserHelpFlag(ctx context.Context, cmd *models.SetUserHelpFlagCommand) error { - return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { - user := user.User{ - ID: cmd.UserId, - HelpFlags1: cmd.HelpFlags1, - Updated: TimeNow(), - } - - _, err := sess.ID(cmd.UserId).Cols("help_flags1").Update(&user) - return err - }) -} - -// validateOneAdminLeft validate that there is an admin user left -func validateOneAdminLeft(sess *DBSession) error { - count, err := sess.Where("is_admin=?", true).Count(&user.User{}) - if err != nil { - return err - } - - if count == 0 { - return user.ErrLastGrafanaAdmin - } - - return nil -} diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index 2094a8263c5..e06d4b77c94 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -419,61 +419,6 @@ func TestIntegrationUserDataAccess(t *testing.T) { } }) }) - - t.Run("Testing DB - grafana admin users", func(t *testing.T) { - ss = InitTestDB(t) - - createUserCmd := user.CreateUserCommand{ - Email: fmt.Sprint("admin", "@test.com"), - Name: "admin", - Login: "admin", - IsAdmin: true, - } - usr, err := ss.CreateUser(context.Background(), createUserCmd) - require.Nil(t, err) - - // Cannot make themselves a non-admin - updatePermsError := ss.UpdateUserPermissions(usr.ID, false) - - require.Equal(t, updatePermsError, user.ErrLastGrafanaAdmin) - - query := models.GetUserByIdQuery{Id: usr.ID} - getUserError := ss.GetUserById(context.Background(), &query) - require.Nil(t, getUserError) - - require.True(t, query.Result.IsAdmin) - - // One user - const email = "user@test.com" - const username = "user" - createUserCmd = user.CreateUserCommand{ - Email: email, - Name: "user", - Login: username, - } - _, err = ss.CreateUser(context.Background(), createUserCmd) - require.Nil(t, err) - - // When trying to create a new user with the same email, an error is returned - createUserCmd = user.CreateUserCommand{ - Email: email, - Name: "user2", - Login: "user2", - SkipOrgSetup: true, - } - _, err = ss.CreateUser(context.Background(), createUserCmd) - require.Equal(t, err, user.ErrUserAlreadyExists) - - // When trying to create a new user with the same login, an error is returned - createUserCmd = user.CreateUserCommand{ - Email: "user2@test.com", - Name: "user2", - Login: username, - SkipOrgSetup: true, - } - _, err = ss.CreateUser(context.Background(), createUserCmd) - require.Equal(t, err, user.ErrUserAlreadyExists) - }) } func (ss *SQLStore) GetOrgUsersForTest(ctx context.Context, query *models.GetOrgUsersQuery) error { diff --git a/pkg/services/user/model.go b/pkg/services/user/model.go index 0f966c354e3..a16e20786e0 100644 --- a/pkg/services/user/model.go +++ b/pkg/services/user/model.go @@ -182,7 +182,7 @@ type BatchDisableUsersCommand struct { type SetUserHelpFlagCommand struct { HelpFlags1 HelpFlags1 - UserID int64 + UserID int64 `xorm:"user_id"` } type GetSignedInUserQuery struct { diff --git a/pkg/services/user/user.go b/pkg/services/user/user.go index c718018783c..ecf19c6bfc4 100644 --- a/pkg/services/user/user.go +++ b/pkg/services/user/user.go @@ -19,7 +19,7 @@ type Service interface { Search(context.Context, *SearchUsersQuery) (*SearchUserQueryResult, error) Disable(context.Context, *DisableUserCommand) error BatchDisableUsers(context.Context, *BatchDisableUsersCommand) error - UpdatePermissions(int64, bool) error + UpdatePermissions(context.Context, int64, bool) error SetUserHelpFlag(context.Context, *SetUserHelpFlagCommand) error - GetProfile(context.Context, *GetUserProfileQuery) (UserProfileDTO, error) + GetProfile(context.Context, *GetUserProfileQuery) (*UserProfileDTO, error) } diff --git a/pkg/services/user/userimpl/store.go b/pkg/services/user/userimpl/store.go index fdc1faeadce..24e94e38af0 100644 --- a/pkg/services/user/userimpl/store.go +++ b/pkg/services/user/userimpl/store.go @@ -30,6 +30,9 @@ type store interface { UpdateLastSeenAt(context.Context, *user.UpdateUserLastSeenAtCommand) error GetSignedInUser(context.Context, *user.GetSignedInUserQuery) (*user.SignedInUser, error) UpdateUser(context.Context, *user.User) error + GetProfile(context.Context, *user.GetUserProfileQuery) (*user.UserProfileDTO, error) + SetHelpFlag(context.Context, *user.SetUserHelpFlagCommand) error + UpdatePermissions(context.Context, int64, bool) error } type sqlStore struct { @@ -387,3 +390,82 @@ func (ss *sqlStore) UpdateUser(ctx context.Context, user *user.User) error { return err }) } + +func (ss *sqlStore) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (*user.UserProfileDTO, error) { + var usr user.User + var userProfile user.UserProfileDTO + err := ss.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { + has, err := sess.ID(query.UserID).Where(ss.notServiceAccountFilter()).Get(&usr) + + if err != nil { + return err + } else if !has { + return user.ErrUserNotFound + } + + userProfile = user.UserProfileDTO{ + ID: usr.ID, + Name: usr.Name, + Email: usr.Email, + Login: usr.Login, + Theme: usr.Theme, + IsGrafanaAdmin: usr.IsAdmin, + IsDisabled: usr.IsDisabled, + OrgID: usr.OrgID, + UpdatedAt: usr.Updated, + CreatedAt: usr.Created, + } + + return err + }) + return &userProfile, err +} + +func (ss *sqlStore) SetHelpFlag(ctx context.Context, cmd *user.SetUserHelpFlagCommand) error { + return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + user := user.User{ + ID: cmd.UserID, + HelpFlags1: cmd.HelpFlags1, + Updated: sqlstore.TimeNow(), + } + + _, err := sess.ID(cmd.UserID).Cols("help_flags1").Update(&user) + return err + }) +} + +// UpdatePermissions sets the user Server Admin flag +func (ss *sqlStore) UpdatePermissions(ctx context.Context, userID int64, isAdmin bool) error { + return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + var user user.User + if _, err := sess.ID(userID).Where(ss.notServiceAccountFilter()).Get(&user); err != nil { + return err + } + + user.IsAdmin = isAdmin + sess.UseBool("is_admin") + _, err := sess.ID(user.ID).Update(&user) + if err != nil { + return err + } + // validate that after update there is at least one server admin + if err := validateOneAdminLeft(ctx, sess); err != nil { + return err + } + return nil + }) +} + +// validateOneAdminLeft validate that there is an admin user left +func validateOneAdminLeft(ctx context.Context, sess *sqlstore.DBSession) error { + count, err := sess.Where("is_admin=?", true).Count(&user.User{}) + if err != nil { + return err + } + + if count == 0 { + return user.ErrLastGrafanaAdmin + } + + return nil +} diff --git a/pkg/services/user/userimpl/store_test.go b/pkg/services/user/userimpl/store_test.go index 062fe28fa63..f0a88d0de1c 100644 --- a/pkg/services/user/userimpl/store_test.go +++ b/pkg/services/user/userimpl/store_test.go @@ -246,6 +246,71 @@ func TestIntegrationUserDataAccess(t *testing.T) { assert.Equal(t, result.Name, "testtestest") assert.Equal(t, result.Login, "loginloginlogin") }) + + t.Run("Testing DB - grafana admin users", func(t *testing.T) { + ss = sqlstore.InitTestDB(t) + + createUserCmd := user.CreateUserCommand{ + Email: fmt.Sprint("admin", "@test.com"), + Name: "admin", + Login: "admin", + IsAdmin: true, + } + usr, err := ss.CreateUser(context.Background(), createUserCmd) + require.Nil(t, err) + + // Cannot make themselves a non-admin + updatePermsError := userStore.UpdatePermissions(context.Background(), usr.ID, false) + + require.Equal(t, user.ErrLastGrafanaAdmin, updatePermsError) + + query := models.GetUserByIdQuery{Id: usr.ID} + getUserError := ss.GetUserById(context.Background(), &query) + require.Nil(t, getUserError) + + require.True(t, query.Result.IsAdmin) + + // One user + const email = "user@test.com" + const username = "user" + createUserCmd = user.CreateUserCommand{ + Email: email, + Name: "user", + Login: username, + } + _, err = ss.CreateUser(context.Background(), createUserCmd) + require.Nil(t, err) + + // When trying to create a new user with the same email, an error is returned + createUserCmd = user.CreateUserCommand{ + Email: email, + Name: "user2", + Login: "user2", + SkipOrgSetup: true, + } + _, err = ss.CreateUser(context.Background(), createUserCmd) + require.Equal(t, err, user.ErrUserAlreadyExists) + + // When trying to create a new user with the same login, an error is returned + createUserCmd = user.CreateUserCommand{ + Email: "user2@test.com", + Name: "user2", + Login: username, + SkipOrgSetup: true, + } + _, err = ss.CreateUser(context.Background(), createUserCmd) + require.Equal(t, err, user.ErrUserAlreadyExists) + }) + + t.Run("GetProfile", func(t *testing.T) { + _, err := userStore.GetProfile(context.Background(), &user.GetUserProfileQuery{UserID: 1}) + require.NoError(t, err) + }) + + t.Run("SetHelpFlag", func(t *testing.T) { + err := userStore.SetHelpFlag(context.Background(), &user.SetUserHelpFlagCommand{UserID: 1, HelpFlags1: user.HelpFlags1(1)}) + require.NoError(t, err) + }) } func TestIntegrationUserUpdate(t *testing.T) { diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index 6bb0dd03bd8..31133c4f152 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -319,44 +319,15 @@ func (s *Service) BatchDisableUsers(ctx context.Context, cmd *user.BatchDisableU return s.sqlStore.BatchDisableUsers(ctx, c) } -// TODO: remove wrapper around sqlstore -func (s *Service) UpdatePermissions(userID int64, isAdmin bool) error { - return s.sqlStore.UpdateUserPermissions(userID, isAdmin) +func (s *Service) UpdatePermissions(ctx context.Context, userID int64, isAdmin bool) error { + return s.store.UpdatePermissions(ctx, userID, isAdmin) } -// TODO: remove wrapper around sqlstore func (s *Service) SetUserHelpFlag(ctx context.Context, cmd *user.SetUserHelpFlagCommand) error { - c := &models.SetUserHelpFlagCommand{ - UserId: cmd.UserID, - HelpFlags1: cmd.HelpFlags1, - } - return s.sqlStore.SetUserHelpFlag(ctx, c) + return s.store.SetHelpFlag(ctx, cmd) } -// TODO: remove wrapper around sqlstore -func (s *Service) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (user.UserProfileDTO, error) { - q := &models.GetUserProfileQuery{ - UserId: query.UserID, - } - err := s.sqlStore.GetUserProfile(ctx, q) - if err != nil { - return user.UserProfileDTO{}, err - } - result := user.UserProfileDTO{ - ID: q.Result.Id, - Email: q.Result.Email, - Name: q.Result.Name, - Login: q.Result.Login, - Theme: q.Result.Theme, - OrgID: q.Result.OrgId, - IsGrafanaAdmin: q.Result.IsGrafanaAdmin, - IsDisabled: q.Result.IsDisabled, - IsExternal: q.Result.IsExternal, - AuthLabels: q.Result.AuthLabels, - UpdatedAt: q.Result.UpdatedAt, - CreatedAt: q.Result.CreatedAt, - AvatarUrl: q.Result.AvatarUrl, - AccessControl: q.Result.AccessControl, - } - return result, nil +func (s *Service) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (*user.UserProfileDTO, error) { + result, err := s.store.GetProfile(ctx, query) + return result, err } diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index cee31ad23a9..7fd094eab83 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -131,6 +131,7 @@ func TestUserService(t *testing.T) { type FakeUserStore struct { ExpectedUser *user.User ExpectedSignedInUser *user.SignedInUser + ExpectedUserProfile *user.UserProfileDTO ExpectedError error ExpectedDeleteUserError error } @@ -190,3 +191,15 @@ func (f *FakeUserStore) GetSignedInUser(ctx context.Context, query *user.GetSign func (f *FakeUserStore) UpdateUser(ctx context.Context, user *user.User) error { return f.ExpectedError } + +func (f *FakeUserStore) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (*user.UserProfileDTO, error) { + return f.ExpectedUserProfile, f.ExpectedError +} + +func (f *FakeUserStore) SetHelpFlag(ctx context.Context, cmd *user.SetUserHelpFlagCommand) error { + return f.ExpectedError +} + +func (f *FakeUserStore) UpdatePermissions(ctx context.Context, userID int64, isAdmin bool) error { + return f.ExpectedError +} diff --git a/pkg/services/user/usertest/fake.go b/pkg/services/user/usertest/fake.go index 862b4ac4e88..a21c6ca4a87 100644 --- a/pkg/services/user/usertest/fake.go +++ b/pkg/services/user/usertest/fake.go @@ -12,7 +12,7 @@ type FakeUserService struct { ExpectedError error ExpectedSetUsingOrgError error ExpectedSearchUsers user.SearchUserQueryResult - ExpectedUSerProfileDTO user.UserProfileDTO + ExpectedUserProfileDTO *user.UserProfileDTO GetSignedInUserFn func(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) } @@ -83,7 +83,7 @@ func (f *FakeUserService) BatchDisableUsers(ctx context.Context, cmd *user.Batch return f.ExpectedError } -func (f *FakeUserService) UpdatePermissions(userID int64, isAdmin bool) error { +func (f *FakeUserService) UpdatePermissions(ctx context.Context, userID int64, isAdmin bool) error { return f.ExpectedError } @@ -91,6 +91,6 @@ func (f *FakeUserService) SetUserHelpFlag(ctx context.Context, cmd *user.SetUser return f.ExpectedError } -func (f *FakeUserService) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (user.UserProfileDTO, error) { - return f.ExpectedUSerProfileDTO, f.ExpectedError +func (f *FakeUserService) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (*user.UserProfileDTO, error) { + return f.ExpectedUserProfileDTO, f.ExpectedError }