ServiceAccounts: Remove permissions to service account when it is deleted (#93877)

* Service account: clean up permissions related to service accounts when deleted

* Add migration for deleting orphaned service account permissions

Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>

pkg/services/serviceaccounts/manager/service.go

@@ -26,7 +26,9 @@ const (
 )
 
 type ServiceAccountsService struct {
-	acService         accesscontrol.Service
+	acService   accesscontrol.Service
+	permissions accesscontrol.ServiceAccountPermissionsService
+
 	store             store
 	log               log.Logger
 	backgroundLog     log.Logger
@@ -44,7 +46,8 @@ func ProvideServiceAccountsService(
 	kvStore kvstore.KVStore,
 	userService user.Service,
 	orgService org.Service,
-	accesscontrolService accesscontrol.Service,
+	acService accesscontrol.Service,
+	permissions accesscontrol.ServiceAccountPermissionsService,
 ) (*ServiceAccountsService, error) {
 	serviceAccountsStore := database.ProvideServiceAccountsStore(
 		cfg,
@@ -55,13 +58,14 @@ func ProvideServiceAccountsService(
 		orgService,
 	)
 	s := &ServiceAccountsService{
-		acService:     accesscontrolService,
+		acService:     acService,
+		permissions:   permissions,
 		store:         serviceAccountsStore,
 		log:           log.New("serviceaccounts"),
 		backgroundLog: log.New("serviceaccounts.background"),
 	}
 
-	if err := RegisterRoles(accesscontrolService); err != nil {
+	if err := RegisterRoles(acService); err != nil {
 		s.log.Error("Failed to register roles", "error", err)
 	}
 
@@ -179,7 +183,10 @@ func (sa *ServiceAccountsService) DeleteServiceAccount(ctx context.Context, orgI
 	if err := sa.store.DeleteServiceAccount(ctx, orgID, serviceAccountID); err != nil {
 		return err
 	}
-	return sa.acService.DeleteUserPermissions(ctx, orgID, serviceAccountID)
+	if err := sa.acService.DeleteUserPermissions(ctx, orgID, serviceAccountID); err != nil {
+		return err
+	}
+	return sa.permissions.DeleteResourcePermissions(ctx, orgID, fmt.Sprintf("%d", serviceAccountID))
 }
 
 func (sa *ServiceAccountsService) EnableServiceAccount(ctx context.Context, orgID, serviceAccountID int64, enable bool) error {

pkg/services/serviceaccounts/manager/service_test.go

@@ -119,7 +119,8 @@ func (f *SecretsCheckerFake) CheckTokens(ctx context.Context) error {
 func TestProvideServiceAccount_DeleteServiceAccount(t *testing.T) {
 	storeMock := newServiceAccountStoreFake()
 	acSvc := actest.FakeService{}
-	svc := ServiceAccountsService{acSvc, storeMock, log.New("test"), log.New("background.test"), &SecretsCheckerFake{}, false, 0}
+	pSvc := &actest.FakePermissionsService{}
+	svc := ServiceAccountsService{acSvc, pSvc, storeMock, log.NewNopLogger(), log.NewNopLogger(), &SecretsCheckerFake{}, false, 0}
 	testOrgId := 1
 
 	t.Run("should create service account", func(t *testing.T) {

pkg/services/serviceaccounts/manager/stats_test.go

@@ -14,8 +14,9 @@ import (
 
 func Test_UsageStats(t *testing.T) {
 	acSvc := actest.FakeService{}
+	pSvc := actest.FakePermissionsService{}
 	storeMock := newServiceAccountStoreFake()
-	svc := ServiceAccountsService{acSvc, storeMock, log.New("test"), log.New("background-test"), &SecretsCheckerFake{}, true, 5}
+	svc := ServiceAccountsService{acSvc, &pSvc, storeMock, log.NewNopLogger(), log.NewNopLogger(), &SecretsCheckerFake{}, true, 5}
 	err := svc.DeleteServiceAccount(context.Background(), 1, 1)
 	require.NoError(t, err)
 

pkg/services/sqlstore/migrations/accesscontrol/orphaned.go

@@ -0,0 +1,96 @@
+package accesscontrol
+
+import (
+	"fmt"
+	"strconv"
+	"strings"
+
+	"xorm.io/xorm"
+
+	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
+)
+
+const (
+	orphanedServiceAccountsPermissions = "delete orphaned service account permissions"
+)
+
+func AddOrphanedMigrations(mg *migrator.Migrator) {
+	mg.AddMigration(orphanedServiceAccountsPermissions, &orphanedServiceAccountPermissions{})
+}
+
+var _ migrator.CodeMigration = new(alertingScopeRemovalMigrator)
+
+type orphanedServiceAccountPermissions struct {
+	migrator.MigrationBase
+}
+
+func (m *orphanedServiceAccountPermissions) SQL(dialect migrator.Dialect) string {
+	return CodeMigrationSQL
+}
+
+func (m *orphanedServiceAccountPermissions) Exec(sess *xorm.Session, mg *migrator.Migrator) error {
+	var idents []string
+
+	// find all permissions that are scopes directly to a service account
+	err := sess.SQL("SELECT DISTINCT p.identifier FROM permission AS p WHERE p.kind  = 'serviceaccounts' AND NOT p.identifier = '*'").Find(&idents)
+	if err != nil {
+		return fmt.Errorf("failed to fetch permissinos scoped to service accounts: %w", err)
+	}
+
+	ids := make([]int64, 0, len(idents))
+	for _, id := range idents {
+		id, err := strconv.ParseInt(id, 10, 64)
+		if err == nil {
+			ids = append(ids, id)
+		}
+	}
+
+	if len(ids) == 0 {
+		return nil
+	}
+
+	// Then find all existing service accounts
+	raw := "SELECT u.id FROM " + mg.Dialect.Quote("user") + " AS u WHERE u.is_service_account AND u.id IN(?" + strings.Repeat(",?", len(ids)-1) + ")"
+	args := make([]any, 0, len(ids))
+	for _, id := range ids {
+		args = append(args, id)
+	}
+
+	var existingIDs []int64
+	err = sess.SQL(raw, args...).Find(&existingIDs)
+	if err != nil {
+		return fmt.Errorf("failed to fetch existing service accounts: %w", err)
+	}
+
+	existing := make(map[int64]struct{}, len(existingIDs))
+	for _, id := range existingIDs {
+		existing[id] = struct{}{}
+	}
+
+	// filter out orphaned permissions
+	var orphaned []string
+	for _, id := range ids {
+		if _, ok := existing[id]; !ok {
+			orphaned = append(orphaned, strconv.FormatInt(id, 10))
+		}
+	}
+
+	if len(orphaned) == 0 {
+		return nil
+	}
+
+	// delete all orphaned permissions
+	rawDelete := "DELETE FROM permission AS p WHERE p.kind = 'serviceaccounts' AND p.identifier IN(?" + strings.Repeat(",?", len(orphaned)-1) + ")"
+	deleteArgs := make([]any, 0, len(orphaned)+1)
+	deleteArgs = append(deleteArgs, rawDelete)
+	for _, id := range orphaned {
+		deleteArgs = append(deleteArgs, id)
+	}
+
+	_, err = sess.Exec(deleteArgs...)
+	if err != nil {
+		return fmt.Errorf("failed to delete orphaned service accounts: %w", err)
+	}
+
+	return nil
+}

pkg/services/sqlstore/migrations/migrations.go

@@ -131,6 +131,8 @@ func (oss *OSSMigrations) AddMigration(mg *Migrator) {
 	ualert.AddReceiverActionScopesMigration(mg)
 
 	ualert.AddRuleMetadata(mg)
+
+	accesscontrol.AddOrphanedMigrations(mg)
 }
 
 func addStarMigrations(mg *Migrator) {