Chore: delete user check 10.4.19 (#105795)

check user is admin when deleting from org

Co-authored-by: Serge Zaitsev <serge.zaitsev@grafana.com>
This commit is contained in:
Kevin Yu
2025-05-22 11:14:32 +02:00
committed by GitHub
co-authored by Serge Zaitsev
parent 3578bab394
commit 626f72f5f4
2 changed files with 129 additions and 10 deletions
+10 -1
View File
@@ -683,6 +683,15 @@ func (ss *sqlStore) RemoveOrgUser(ctx context.Context, cmd *org.RemoveOrgUserCom
return user.ErrUserNotFound
}
// check if user belongs to org
var orgUser org.OrgUser
if exists, err := sess.Where("org_id=? AND user_id=?", cmd.OrgID, cmd.UserID).Get(&orgUser); err != nil {
return err
} else if !exists {
ss.log.Debug("User not in org, nothing to do", "user_id", cmd.UserID, "org_id", cmd.OrgID)
return nil
}
deletes := []string{
"DELETE FROM org_user WHERE org_id=? and user_id=?",
"DELETE FROM dashboard_acl WHERE org_id=? and user_id = ?",
@@ -729,7 +738,7 @@ func (ss *sqlStore) RemoveOrgUser(ctx context.Context, cmd *org.RemoveOrgUserCom
return err
}
}
} else if cmd.ShouldDeleteOrphanedUser {
} else if cmd.ShouldDeleteOrphanedUser && !usr.IsAdmin {
// no other orgs, delete the full user
if err := ss.deleteUserInTransaction(sess, &user.DeleteUserCommand{UserID: usr.ID}); err != nil {
return err
+119 -9
View File
@@ -11,6 +11,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/org"
@@ -37,6 +38,7 @@ func TestIntegrationOrgDataAccess(t *testing.T) {
orgStore := sqlStore{
db: ss,
dialect: ss.GetDialect(),
log: log.NewNopLogger(),
}
t.Run("org not found", func(t *testing.T) {
@@ -204,6 +206,7 @@ func TestIntegrationOrgDataAccess(t *testing.T) {
orgStore = sqlStore{
db: ss,
dialect: ss.GetDialect(),
log: log.NewNopLogger(),
}
ids := []int64{}
@@ -258,6 +261,7 @@ func TestIntegrationOrgUserDataAccess(t *testing.T) {
db: ss,
dialect: ss.GetDialect(),
cfg: setting.NewCfg(),
log: log.NewNopLogger(),
}
t.Run("org user inserted", func(t *testing.T) {
@@ -333,7 +337,7 @@ func TestIntegrationOrgUserDataAccess(t *testing.T) {
ss := db.InitTestDB(t)
_, usrSvc := createOrgAndUserSvc(t, ss, ss.Cfg)
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}
ac2cmd := &user.CreateUserCommand{Login: "ac2", Email: "ac2@test.com", Name: "ac2 name"}
ac1, err := usrSvc.Create(context.Background(), ac1cmd)
require.NoError(t, err)
ac2, err := usrSvc.Create(context.Background(), ac2cmd)
@@ -460,6 +464,15 @@ func TestIntegrationOrgUserDataAccess(t *testing.T) {
err := orgUserStore.Delete(context.Background(), &org.DeleteOrgCommand{ID: ac2.OrgID})
require.NoError(t, err)
// make sure ac2 is in ac1 org
cmd := org.AddOrgUserCommand{
OrgID: ac1.OrgID,
UserID: ac2.ID,
Role: org.RoleViewer,
}
err = orgUserStore.AddOrgUser(context.Background(), &cmd)
require.NoError(t, err)
// remove ac2 user from ac1 org
remCmd := org.RemoveOrgUserCommand{OrgID: ac1.OrgID, UserID: ac2.ID, ShouldDeleteOrphanedUser: true}
err = orgUserStore.RemoveOrgUser(context.Background(), &remCmd)
@@ -543,6 +556,7 @@ func TestIntegrationSQLStore_AddOrgUser(t *testing.T) {
db: store,
dialect: store.GetDialect(),
cfg: setting.NewCfg(),
log: log.NewNopLogger(),
}
orgSvc, usrSvc := createOrgAndUserSvc(t, store, store.Cfg)
@@ -609,6 +623,7 @@ func TestIntegration_SQLStore_GetOrgUsers(t *testing.T) {
db: store,
dialect: store.GetDialect(),
cfg: setting.NewCfg(),
log: log.NewNopLogger(),
}
orgUserStore.cfg.IsEnterprise = true
defer func() {
@@ -729,6 +744,7 @@ func TestIntegration_SQLStore_GetOrgUsers_PopulatesCorrectly(t *testing.T) {
db: store,
dialect: store.GetDialect(),
cfg: setting.NewCfg(),
log: log.NewNopLogger(),
}
_, usrSvc := createOrgAndUserSvc(t, store, store.Cfg)
@@ -791,6 +807,7 @@ func TestIntegration_SQLStore_SearchOrgUsers(t *testing.T) {
db: store,
dialect: store.GetDialect(),
cfg: setting.NewCfg(),
log: log.NewNopLogger(),
}
// orgUserStore.cfg.Skip
orgSvc, userSvc := createOrgAndUserSvc(t, store, store.Cfg)
@@ -868,12 +885,17 @@ func TestIntegration_SQLStore_RemoveOrgUser(t *testing.T) {
db: store,
dialect: store.GetDialect(),
cfg: setting.NewCfg(),
log: log.NewNopLogger(),
}
orgSvc, usrSvc := createOrgAndUserSvc(t, store, store.Cfg)
o, err := orgSvc.CreateWithMember(context.Background(), &org.CreateOrgCommand{Name: MainOrgName})
require.NoError(t, err)
// create 2nd org
o2, err := orgSvc.CreateWithMember(context.Background(), &org.CreateOrgCommand{Name: "test org 2"})
require.NoError(t, err)
// create org and admin
_, err = usrSvc.Create(context.Background(), &user.CreateUserCommand{
Login: "admin",
@@ -882,28 +904,116 @@ func TestIntegration_SQLStore_RemoveOrgUser(t *testing.T) {
require.NoError(t, err)
// create a user with no org
_, err = usrSvc.Create(context.Background(), &user.CreateUserCommand{
Login: "user",
OrgID: 1,
viewer, err := usrSvc.Create(context.Background(), &user.CreateUserCommand{
Login: "viewer",
SkipOrgSetup: true,
})
require.NoError(t, err)
// create a user with no org
viewer2, err := usrSvc.Create(context.Background(), &user.CreateUserCommand{
Login: "viewer2",
SkipOrgSetup: true,
})
require.NoError(t, err)
// create a user with no org
viewer3, err := usrSvc.Create(context.Background(), &user.CreateUserCommand{
Login: "viewer3",
SkipOrgSetup: true,
})
require.NoError(t, err)
// create an admin user with no org
admin, err := usrSvc.Create(context.Background(), &user.CreateUserCommand{
Login: "serverAdmin",
SkipOrgSetup: true,
IsAdmin: true,
})
require.NoError(t, err)
// assign the user to the org
err = orgUserStore.AddOrgUser(context.Background(), &org.AddOrgUserCommand{
Role: "Viewer",
OrgID: 1,
UserID: 2,
OrgID: o.ID,
UserID: viewer.ID,
})
require.NoError(t, err)
// assign the admin user to the org
err = orgUserStore.AddOrgUser(context.Background(), &org.AddOrgUserCommand{
Role: "Admin",
OrgID: o.ID,
UserID: admin.ID,
})
require.NoError(t, err)
// assign the viewer3 user to the 2nd org
err = orgUserStore.AddOrgUser(context.Background(), &org.AddOrgUserCommand{
Role: "Viewer",
OrgID: o2.ID,
UserID: viewer3.ID,
})
require.NoError(t, err)
// remove the user org
err = orgUserStore.RemoveOrgUser(context.Background(), &org.RemoveOrgUserCommand{
UserID: 2,
OrgID: 1,
ShouldDeleteOrphanedUser: false,
UserID: viewer.ID,
OrgID: o.ID,
ShouldDeleteOrphanedUser: true,
})
require.NoError(t, err)
// remove the admin user
err = orgUserStore.RemoveOrgUser(context.Background(), &org.RemoveOrgUserCommand{
UserID: admin.ID,
OrgID: o.ID,
ShouldDeleteOrphanedUser: true,
})
require.NoError(t, err)
// remove the viewer3 user from first org they don't belong to
err = orgUserStore.RemoveOrgUser(context.Background(), &org.RemoveOrgUserCommand{
UserID: viewer3.ID,
OrgID: o.ID,
ShouldDeleteOrphanedUser: true,
})
require.NoError(t, err)
// remove the viewer2 user from first org they don't belong to
err = orgUserStore.RemoveOrgUser(context.Background(), &org.RemoveOrgUserCommand{
UserID: viewer2.ID,
OrgID: o.ID,
ShouldDeleteOrphanedUser: true,
})
require.NoError(t, err)
// verify the user is deleted
_, err = usrSvc.GetByID(context.Background(), &user.GetUserByIDQuery{
ID: viewer.ID,
})
require.ErrorIs(t, err, user.ErrUserNotFound)
// verify the admin user is not deleted
usr, err := usrSvc.GetByID(context.Background(), &user.GetUserByIDQuery{
ID: admin.ID,
})
require.NoError(t, err)
assert.NotNil(t, usr)
// verify the viewer2 user is not deleted
_, err = usrSvc.GetByID(context.Background(), &user.GetUserByIDQuery{
ID: viewer2.ID,
})
require.NoError(t, err)
assert.NotNil(t, usr)
// verify the viewer3 user is not deleted
_, err = usrSvc.GetByID(context.Background(), &user.GetUserByIDQuery{
ID: viewer3.ID,
})
require.NoError(t, err)
assert.NotNil(t, usr)
}
func createOrgAndUserSvc(t *testing.T, store db.DB, cfg *setting.Cfg) (org.Service, user.Service) {