apply security patch: release-11.3.7/404-202504151210.patch

pkg/services/org/orgimpl/store.go

@@ -681,6 +681,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 = ?",
@@ -727,7 +736,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

pkg/services/org/orgimpl/store_test.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/apimachinery/identity"
 	"github.com/grafana/grafana/pkg/infra/db"
+	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/org"
@@ -38,6 +39,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) {
@@ -258,6 +260,7 @@ func TestIntegrationOrgUserDataAccess(t *testing.T) {
 	orgUserStore := sqlStore{
 		db:      ss,
 		dialect: ss.GetDialect(),
+		log:     log.NewNopLogger(),
 	}
 
 	t.Run("org user inserted", func(t *testing.T) {
@@ -333,7 +336,7 @@ func TestIntegrationOrgUserDataAccess(t *testing.T) {
 		ss, cfg := db.InitTestDBWithCfg(t)
 		_, usrSvc := createOrgAndUserSvc(t, 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 +463,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)
@@ -545,6 +557,7 @@ func TestIntegrationSQLStore_AddOrgUser(t *testing.T) {
 	orgUserStore := sqlStore{
 		db:      store,
 		dialect: store.GetDialect(),
+		log:     log.NewNopLogger(),
 	}
 	orgSvc, usrSvc := createOrgAndUserSvc(t, store, cfg)
 
@@ -610,6 +623,7 @@ func TestIntegration_SQLStore_GetOrgUsers(t *testing.T) {
 	orgUserStore := sqlStore{
 		db:      store,
 		dialect: store.GetDialect(),
+		log:     log.NewNopLogger(),
 	}
 	cfg.IsEnterprise = true
 	defer func() {
@@ -728,6 +742,7 @@ func TestIntegration_SQLStore_GetOrgUsers_PopulatesCorrectly(t *testing.T) {
 	orgUserStore := sqlStore{
 		db:      store,
 		dialect: store.GetDialect(),
+		log:     log.NewNopLogger(),
 	}
 	_, usrSvc := createOrgAndUserSvc(t, store, cfg)
 
@@ -789,6 +804,7 @@ func TestIntegration_SQLStore_SearchOrgUsers(t *testing.T) {
 	orgUserStore := sqlStore{
 		db:      store,
 		dialect: store.GetDialect(),
+		log:     log.NewNopLogger(),
 	}
 	// orgUserStore.cfg.Skip
 	orgSvc, userSvc := createOrgAndUserSvc(t, store, cfg)
@@ -865,12 +881,18 @@ func TestIntegration_SQLStore_RemoveOrgUser(t *testing.T) {
 	orgUserStore := sqlStore{
 		db:      store,
 		dialect: store.GetDialect(),
+		log:     log.NewNopLogger(),
 	}
+
 	orgSvc, usrSvc := createOrgAndUserSvc(t, 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",
@@ -879,28 +901,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{
+	viewer, err := usrSvc.Create(context.Background(), &user.CreateUserCommand{
-		Login:        "user",
+		Login:        "viewer",
-		OrgID:        1,
 		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,
+		OrgID:  o.ID,
-		UserID: 2,
+		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,
+		UserID:                   viewer.ID,
-		OrgID:                    1,
+		OrgID:                    o.ID,
-		ShouldDeleteOrphanedUser: false,
+		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) {