public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

User: Clean up update functions (#86341)

Browse Source 
* User: remove unused function

* User: Remove UpdatePermissions and support IsGrafanaAdmin flag in Update function instead

* User: Remove Disable function and use Update instead
This commit is contained in:
Karl Persson
2024-04-16 16:33:50 +02:00
committed by GitHub
parent 8c8885ef23
commit 0f06120b56
14 changed files with 91 additions and 267 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/user/userimpl/store.go
+17 -44
View File
@@ -34,11 +34,8 @@ type store interface {
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
BatchDisableUsers(context.Context, *user.BatchDisableUsersCommand) error
Disable(context.Context, *user.DisableUserCommand) error
Search(context.Context, *user.SearchUsersQuery) (*user.SearchUserQueryResult, error)
Count(ctx context.Context) (int64, error)
CountUserAccountsWithEmptyRole(ctx context.Context) (int64, error)
}
@@ -317,11 +314,21 @@ func (ss *sqlStore) Update(ctx context.Context, cmd *user.UpdateUserCommand) err
q := sess.ID(cmd.UserID).Where(ss.notServiceAccountFilter())
if cmd.IsDisabled != nil {
sess.UseBool("is_disabled")
user.IsDisabled = *cmd.IsDisabled
}
if cmd.EmailVerified != nil {
q.UseBool("email_verified")
user.EmailVerified = *cmd.EmailVerified
}
if cmd.IsGrafanaAdmin != nil {
q.UseBool("is_admin")
user.IsAdmin = *cmd.IsGrafanaAdmin
}
if _, err := q.Update(&user); err != nil {
return err
}
@@ -330,6 +337,13 @@ func (ss *sqlStore) Update(ctx context.Context, cmd *user.UpdateUserCommand) err
return err
}
if cmd.IsGrafanaAdmin != nil && !*cmd.IsGrafanaAdmin {
// validate that after update there is at least one server admin
if err := validateOneAdminLeft(ctx, sess); err != nil {
return err
}
}
sess.PublishAfterCommit(&events.UserUpdated{
Timestamp: user.Created,
Id: user.ID,
@@ -476,28 +490,6 @@ func (ss *sqlStore) SetHelpFlag(ctx context.Context, cmd *user.SetUserHelpFlagCo
})
}
// 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 *db.Session) 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
})
}
func (ss *sqlStore) Count(ctx context.Context) (int64, error) {
type result struct {
Count int64
@@ -570,25 +562,6 @@ func (ss *sqlStore) BatchDisableUsers(ctx context.Context, cmd *user.BatchDisabl
})
}
func (ss *sqlStore) Disable(ctx context.Context, cmd *user.DisableUserCommand) error {
return ss.db.WithDbSession(ctx, func(dbSess *db.Session) 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),
pkg/services/user/userimpl/store_test.go
+45 -40
View File
@@ -479,54 +479,47 @@ func TestIntegrationUserDataAccess(t *testing.T) {
t.Run("Testing DB - grafana admin users", func(t *testing.T) {
ss := db.InitTestDB(t)
_, usrSvc := createOrgAndUserSvc(t, ss, ss.Cfg)
createUserCmd := user.CreateUserCommand{
Email: fmt.Sprint("admin", "@test.com"),
usr, err := usrSvc.Create(context.Background(), &user.CreateUserCommand{
Email: "admin@test.com",
Name: "admin",
Login: "admin",
IsAdmin: true,
}
usr, err := usrSvc.Create(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)
// Cannot make user non grafana admin if it is the last one
err = userStore.Update(context.Background(), &user.UpdateUserCommand{
UserID: usr.ID,
IsGrafanaAdmin: boolPtr(false),
})
require.ErrorIs(t, user.ErrLastGrafanaAdmin, err)
query := user.GetUserByIDQuery{ID: usr.ID}
queryResult, getUserError := userStore.GetByID(context.Background(), query.ID)
require.Nil(t, getUserError)
require.True(t, queryResult.IsAdmin)
usr, err = userStore.GetByID(context.Background(), usr.ID)
require.NoError(t, err)
require.True(t, usr.IsAdmin)
// One user
const email = "user@test.com"
const username = "user"
createUserCmd = user.CreateUserCommand{
Email: email,
Name: "user",
Login: username,
}
_, err = usrSvc.Create(context.Background(), &createUserCmd)
require.Nil(t, err)
// Create another admin user
_, err = usrSvc.Create(context.Background(), &user.CreateUserCommand{
Email: "admin2@test.com",
Name: "admin2",
Login: "admin2",
IsAdmin: true,
})
require.NoError(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 = usrSvc.Create(context.Background(), &createUserCmd)
require.Equal(t, user.ErrUserAlreadyExists, err)
// Now first admin user should be able to be downgraded
err = userStore.Update(context.Background(), &user.UpdateUserCommand{
UserID: usr.ID,
IsGrafanaAdmin: boolPtr(false),
})
require.NoError(t, err)
// 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 = usrSvc.Create(context.Background(), &createUserCmd)
require.Equal(t, user.ErrUserAlreadyExists, err)
updated, err := userStore.GetByID(context.Background(), usr.ID)
require.NoError(t, err)
require.False(t, updated.IsAdmin)
require.Equal(t, usr.Email, updated.Email)
require.Equal(t, usr.Login, updated.Login)
require.Equal(t, usr.Name, updated.Name)
})
t.Run("GetProfile", func(t *testing.T) {
@@ -787,8 +780,16 @@ func TestIntegrationUserDataAccess(t *testing.T) {
Updated: time.Now(),
})
require.NoError(t, err)
err = userStore.Disable(context.Background(), &user.DisableUserCommand{UserID: id})
err = userStore.Update(context.Background(), &user.UpdateUserCommand{
UserID: id,
IsDisabled: boolPtr(true),
})
require.NoError(t, err)
usr, err := userStore.GetByID(context.Background(), id)
require.NoError(t, err)
require.True(t, usr.IsDisabled)
})
t.Run("Testing DB - multiple users", func(t *testing.T) {
@@ -1041,3 +1042,7 @@ func createOrgAndUserSvc(t *testing.T, store db.DB, cfg *setting.Cfg) (org.Servi
return orgService, usrSvc
}
func boolPtr(b bool) *bool {
return &b
}
pkg/services/user/userimpl/user.go
-34
View File
@@ -9,7 +9,6 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/models/roletype"
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/quota"
@@ -319,47 +318,14 @@ func (s *Service) GetSignedInUser(ctx context.Context, query *user.GetSignedInUs
return signedInUser, err
}
func (s *Service) NewAnonymousSignedInUser(ctx context.Context) (*user.SignedInUser, error) {
if !s.cfg.AnonymousEnabled {
return nil, fmt.Errorf("anonymous access is disabled")
}
usr := &user.SignedInUser{
IsAnonymous: true,
OrgRole: roletype.RoleType(s.cfg.AnonymousOrgRole),
}
if s.cfg.AnonymousOrgName == "" {
return usr, nil
}
getOrg := org.GetOrgByNameQuery{Name: s.cfg.AnonymousOrgName}
anonymousOrg, err := s.orgService.GetByName(ctx, &getOrg)
if err != nil {
return nil, err
}
usr.OrgID = anonymousOrg.ID
usr.OrgName = anonymousOrg.Name
return usr, nil
}
func (s *Service) Search(ctx context.Context, query *user.SearchUsersQuery) (*user.SearchUserQueryResult, error) {
return s.store.Search(ctx, query)
}
func (s *Service) Disable(ctx context.Context, cmd *user.DisableUserCommand) error {
return s.store.Disable(ctx, cmd)
}
func (s *Service) BatchDisableUsers(ctx context.Context, cmd *user.BatchDisableUsersCommand) error {
return s.store.BatchDisableUsers(ctx, cmd)
}
func (s *Service) UpdatePermissions(ctx context.Context, userID int64, isAdmin bool) error {
return s.store.UpdatePermissions(ctx, userID, isAdmin)
}
func (s *Service) SetUserHelpFlag(ctx context.Context, cmd *user.SetUserHelpFlagCommand) error {
return s.store.SetHelpFlag(ctx, cmd)
}
pkg/services/user/userimpl/user_test.go
-40
View File
@@ -10,7 +10,6 @@ import (
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/models/roletype"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgtest"
"github.com/grafana/grafana/pkg/services/team/teamtest"
@@ -127,41 +126,6 @@ func TestUserService(t *testing.T) {
assert.Equal(t, query2.OrgID, result2.OrgID)
})
t.Run("NewAnonymousSignedInUser", func(t *testing.T) {
t.Run("should error when anonymous access is disabled", func(t *testing.T) {
userService.cfg = setting.NewCfg()
userService.cfg.AnonymousEnabled = false
_, err := userService.NewAnonymousSignedInUser(context.Background())
require.Error(t, err)
})
t.Run("should return user when anonymous access is enabled and org is not set", func(t *testing.T) {
userService.cfg = setting.NewCfg()
userService.cfg.AnonymousEnabled = true
u, err := userService.NewAnonymousSignedInUser(context.Background())
require.NoError(t, err)
require.Equal(t, true, u.IsAnonymous)
require.Equal(t, int64(0), u.UserID)
require.Equal(t, "", u.OrgName)
require.Equal(t, roletype.RoleType(""), u.OrgRole)
})
t.Run("should return user with org info when anonymous access is enabled and org is set", func(t *testing.T) {
userService.cfg = setting.NewCfg()
userService.cfg.AnonymousEnabled = true
userService.cfg.AnonymousOrgName = "anonymous"
userService.cfg.AnonymousOrgRole = "anonymous"
orgService.ExpectedOrg = &org.Org{Name: "anonymous", ID: 123}
u, err := userService.NewAnonymousSignedInUser(context.Background())
require.NoError(t, err)
require.Equal(t, true, u.IsAnonymous)
require.Equal(t, int64(0), u.UserID)
require.Equal(t, orgService.ExpectedOrg.ID, u.OrgID)
require.Equal(t, orgService.ExpectedOrg.Name, u.OrgName)
require.Equal(t, roletype.RoleType(userService.cfg.AnonymousOrgRole), u.OrgRole)
})
})
t.Run("Can set using org", func(t *testing.T) {
cmd := user.SetUsingOrgCommand{UserID: 2, OrgID: 1}
orgService.ExpectedUserOrgDTO = []*org.UserOrgDTO{{OrgID: 1}}
@@ -302,10 +266,6 @@ func (f *FakeUserStore) BatchDisableUsers(ctx context.Context, cmd *user.BatchDi
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
}