[v10.2.x] Alerting: Fix deleting rules in a folder with matching UID …

…in another organization (#79011)

Alerting: Fix deleting rules in a folder with matching UID in another organization (#78258)

* Remove usage of obsolete function for deleting alert rules under folder

* Apply suggestion from code review

* Update tests

(cherry picked from commit 6d4625a)

pkg/api/apierrors/folder.go

@@ -18,8 +18,7 @@ func ToFolderErrorResponse(err error) response.Response {
 	if errors.Is(err, dashboards.ErrFolderTitleEmpty) ||
 		errors.Is(err, dashboards.ErrDashboardTypeMismatch) ||
 		errors.Is(err, dashboards.ErrDashboardInvalidUid) ||
-		errors.Is(err, dashboards.ErrDashboardUidTooLong) ||
-		errors.Is(err, dashboards.ErrFolderContainsAlertRules) {
+		errors.Is(err, dashboards.ErrDashboardUidTooLong) {
 		return response.Error(400, err.Error(), nil)
 	}
 

pkg/services/dashboards/database/database.go

@@ -721,10 +721,6 @@ func (d *dashboardStore) deleteDashboard(cmd *dashboards.DeleteDashboardCommand,
 		if err != nil {
 			return err
 		}
-
-		if err := deleteFolderAlertRules(sess, dashboard, cmd.ForceDeleteFolderRules); err != nil {
-			return err
-		}
 	} else {
 		if err := d.deleteResourcePermissions(sess, dashboard.OrgID, ac.GetResourceScopeUID("dashboards", dashboard.UID)); err != nil {
 			return err
@@ -819,33 +815,6 @@ func (d *dashboardStore) deleteChildrenDashboardAssociations(sess *db.Session, d
 	return nil
 }
 
-func deleteFolderAlertRules(sess *db.Session, dashboard dashboards.Dashboard, forceDeleteFolderAlertRules bool) error {
-	var existingRuleID int64
-	exists, err := sess.Table("alert_rule").Where("namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)", dashboard.ID).Cols("id").Get(&existingRuleID)
-	if err != nil {
-		return err
-	}
-	if exists {
-		if !forceDeleteFolderAlertRules {
-			return fmt.Errorf("folder cannot be deleted: %w", dashboards.ErrFolderContainsAlertRules)
-		}
-
-		// Delete all rules under this folder.
-		deleteNGAlertsByFolder := []string{
-			"DELETE FROM alert_rule WHERE namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)",
-			"DELETE FROM alert_rule_version WHERE rule_namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)",
-		}
-
-		for _, sql := range deleteNGAlertsByFolder {
-			_, err := sess.Exec(sql, dashboard.ID)
-			if err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
 func createEntityEvent(dashboard *dashboards.Dashboard, eventType store.EntityEventType) *store.EntityEvent {
 	var entityEvent *store.EntityEvent
 	if dashboard.IsFolder {

pkg/services/dashboards/database/database_test.go

@@ -3,7 +3,6 @@ package database
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"testing"
 	"time"
@@ -255,45 +254,6 @@ func TestIntegrationDashboardDataAccess(t *testing.T) {
 		require.NoError(t, err)
 	})
 
-	t.Run("Should be not able to delete a dashboard if force delete rules is disabled", func(t *testing.T) {
-		setup()
-		deleteCmd := &dashboards.DeleteDashboardCommand{ID: savedFolder.ID, ForceDeleteFolderRules: false}
-		err := dashboardStore.DeleteDashboard(context.Background(), deleteCmd)
-		require.True(t, errors.Is(err, dashboards.ErrFolderContainsAlertRules))
-	})
-
-	t.Run("Should be able to delete a dashboard folder and its children if force delete rules is enabled", func(t *testing.T) {
-		setup()
-		deleteCmd := &dashboards.DeleteDashboardCommand{ID: savedFolder.ID, ForceDeleteFolderRules: true}
-		err := dashboardStore.DeleteDashboard(context.Background(), deleteCmd)
-		require.NoError(t, err)
-
-		query := dashboards.FindPersistedDashboardsQuery{
-			OrgId:        1,
-			FolderIds:    []int64{savedFolder.ID},
-			SignedInUser: &user.SignedInUser{},
-		}
-
-		res, err := dashboardStore.FindDashboards(context.Background(), &query)
-		require.NoError(t, err)
-		require.Equal(t, len(res), 0)
-
-		err = sqlStore.WithDbSession(context.Background(), func(sess *db.Session) error {
-			var existingRuleID int64
-			exists, err := sess.Table("alert_rule").Where("namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)", savedFolder.ID).Cols("id").Get(&existingRuleID)
-			require.NoError(t, err)
-			require.False(t, exists)
-
-			var existingRuleVersionID int64
-			exists, err = sess.Table("alert_rule_version").Where("rule_namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)", savedFolder.ID).Cols("id").Get(&existingRuleVersionID)
-			require.NoError(t, err)
-			require.False(t, exists)
-
-			return nil
-		})
-		require.NoError(t, err)
-	})
-
 	t.Run("Should return error if no dashboard is found for update when dashboard id is greater than zero", func(t *testing.T) {
 		cmd := dashboards.SaveDashboardCommand{
 			OrgID:     1,

pkg/services/dashboards/errors.go

@@ -117,14 +117,13 @@ var (
 		Status:     "not-found",
 	}
 
-	ErrFolderNotFound           = errors.New("folder not found")
-	ErrFolderVersionMismatch    = errors.New("the folder has been changed by someone else")
-	ErrFolderTitleEmpty         = errors.New("folder title cannot be empty")
-	ErrFolderWithSameUIDExists  = errors.New("a folder/dashboard with the same uid already exists")
-	ErrFolderInvalidUID         = errors.New("invalid uid for folder provided")
-	ErrFolderSameNameExists     = errors.New("a folder or dashboard in the general folder with the same name already exists")
-	ErrFolderAccessDenied       = errors.New("access denied to folder")
-	ErrFolderContainsAlertRules = errors.New("folder contains alert rules")
+	ErrFolderNotFound          = errors.New("folder not found")
+	ErrFolderVersionMismatch   = errors.New("the folder has been changed by someone else")
+	ErrFolderTitleEmpty        = errors.New("folder title cannot be empty")
+	ErrFolderWithSameUIDExists = errors.New("a folder/dashboard with the same uid already exists")
+	ErrFolderInvalidUID        = errors.New("invalid uid for folder provided")
+	ErrFolderSameNameExists    = errors.New("a folder or dashboard in the general folder with the same name already exists")
+	ErrFolderAccessDenied      = errors.New("access denied to folder")
 )
 
 // DashboardErr represents a dashboard error.

pkg/services/dashboards/service/dashboard_service_test.go

@@ -206,7 +206,7 @@ func TestDashboardService(t *testing.T) {
 
 		t.Run("Given non provisioned dashboard", func(t *testing.T) {
 			t.Run("DeleteProvisionedDashboard should delete the dashboard", func(t *testing.T) {
-				args := &dashboards.DeleteDashboardCommand{OrgID: 1, ID: 1, ForceDeleteFolderRules: false}
+				args := &dashboards.DeleteDashboardCommand{OrgID: 1, ID: 1}
 				fakeStore.On("DeleteDashboard", mock.Anything, args).Return(nil).Once()
 				err := service.DeleteProvisionedDashboard(context.Background(), 1, 1)
 				require.NoError(t, err)

pkg/services/folder/folderimpl/folder.go

@@ -540,8 +540,8 @@ func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) e
 			return folder.ErrInternal.Errorf("failed to fetch subfolders from dashboard store: %w", err)
 		}
 
-		for _, folder := range result {
-			dashFolder, ok := dashFolders[folder]
+		for _, f := range result {
+			dashFolder, ok := dashFolders[f]
 			if !ok {
 				return err
 			}
@@ -550,6 +550,19 @@ func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) e
 				if err := s.deleteChildrenInFolder(ctx, dashFolder.OrgID, dashFolder.UID, cmd.SignedInUser); err != nil {
 					return err
 				}
+			} else {
+				alertRuleSrv, ok := s.registry[entity.StandardKindAlertRule]
+				if !ok {
+					return folder.ErrInternal.Errorf("no alert rule service found in registry")
+				}
+				alertRulesInFolder, err := alertRuleSrv.CountInFolder(ctx, dashFolder.OrgID, dashFolder.UID, cmd.SignedInUser)
+				if err != nil {
+					s.log.Error("failed to count alert rules in folder", "error", err)
+					return err
+				}
+				if alertRulesInFolder > 0 {
+					return folder.ErrFolderNotEmpty.Errorf("folder contains %d alert rules", alertRulesInFolder)
+				}
 			}
 
 			err = s.legacyDelete(ctx, cmd, dashFolder)

pkg/services/folder/folderimpl/folder_test.go

@@ -74,6 +74,16 @@ func TestIntegrationFolderService(t *testing.T) {
 		cfg := setting.NewCfg()
 		features := featuremgmt.WithFeatures()
 
+		ac := acmock.New().WithPermissions([]accesscontrol.Permission{
+			{Action: accesscontrol.ActionAlertingRuleDelete, Scope: dashboards.ScopeFoldersAll},
+		})
+		alertingStore := ngstore.DBstore{
+			SQLStore:      db,
+			Cfg:           cfg.UnifiedAlerting,
+			Logger:        log.New("test-alerting-store"),
+			AccessControl: ac,
+		}
+
 		service := &Service{
 			cfg:                  cfg,
 			log:                  log.New("test-folder-service"),
@@ -84,8 +94,11 @@ func TestIntegrationFolderService(t *testing.T) {
 			bus:                  bus.ProvideBus(tracing.InitializeTracerForTest()),
 			db:                   db,
 			accessControl:        acimpl.ProvideAccessControl(cfg),
+			registry:             make(map[string]folder.RegistryService),
 		}
 
+		require.NoError(t, service.RegisterService(alertingStore))
+
 		t.Run("Given user has no permissions", func(t *testing.T) {
 			origNewGuardian := guardian.New
 			guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{})
@@ -363,8 +376,10 @@ func TestIntegrationNestedFolderService(t *testing.T) {
 
 	signedInUser := user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{
 		orgID: {
-			dashboards.ActionFoldersCreate: {},
-			dashboards.ActionFoldersWrite:  {dashboards.ScopeFoldersAll}},
+			dashboards.ActionFoldersCreate:         {},
+			dashboards.ActionFoldersWrite:          {dashboards.ScopeFoldersAll},
+			accesscontrol.ActionAlertingRuleDelete: {dashboards.ScopeFoldersAll},
+		},
 	}}
 	createCmd := folder.CreateFolderCommand{
 		OrgID:        orgID,
@@ -568,7 +583,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
 				prefix:       "flagon-noforce",
 				depth:        3,
 				forceDelete:  false,
-				deletionErr:  dashboards.ErrFolderContainsAlertRules,
+				deletionErr:  folder.ErrFolderNotEmpty,
 				desc:         "With nested folder feature flag on and no force deletion of rules",
 			},
 			{
@@ -587,7 +602,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
 				prefix:       "flagoff-noforce",
 				depth:        1,
 				forceDelete:  false,
-				deletionErr:  dashboards.ErrFolderContainsAlertRules,
+				deletionErr:  folder.ErrFolderNotEmpty,
 				desc:         "With nested folder feature flag off and no force deletion of rules",
 			},
 		}

pkg/services/folder/model.go

@@ -16,6 +16,7 @@ var ErrDatabaseError = errutil.Internal("folder.database-error")
 var ErrInternal = errutil.Internal("folder.internal")
 var ErrCircularReference = errutil.BadRequest("folder.circular-reference", errutil.WithPublicMessage("Circular reference detected"))
 var ErrTargetRegistrySrvConflict = errutil.Internal("folder.target-registry-srv-conflict")
+var ErrFolderNotEmpty = errutil.BadRequest("folder.not-empty", errutil.WithPublicMessage("Folder cannot be deleted: folder is not empty"))
 
 const (
 	GeneralFolderUID     = "general"

pkg/services/ngalert/migration/store/testing.go

@@ -68,6 +68,9 @@ func NewTestMigrationStore(t *testing.T, sqlStore *sqlstore.SQLStore, cfg *setti
 	require.NoError(t, err)
 	folderService := folderimpl.ProvideService(ac, bus, cfg, dashboardStore, folderStore, sqlStore, features)
 
+	err = folderService.RegisterService(alertingStore)
+	require.NoError(t, err)
+
 	folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions(
 		features, routeRegister, sqlStore, ac, license, dashboardStore, folderService, acSvc, teamSvc, userSvc)
 	require.NoError(t, err)

pkg/services/ngalert/store/alert_rule.go

@@ -9,6 +9,7 @@ import (
 	"github.com/google/uuid"
 
 	"github.com/grafana/grafana/pkg/infra/db"
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/auth/identity"
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/folder"
@@ -590,6 +591,17 @@ func (st DBstore) GetAlertRulesForScheduling(ctx context.Context, query *ngmodel
 
 // DeleteInFolder deletes the rules contained in a given folder along with their associated data.
 func (st DBstore) DeleteInFolder(ctx context.Context, orgID int64, folderUID string, user identity.Requester) error {
+	evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAlertingRuleDelete, dashboards.ScopeFoldersProvider.GetResourceScopeName(folderUID))
+	canSave, err := st.AccessControl.Evaluate(ctx, user, evaluator)
+	if err != nil {
+		st.Logger.Error("Failed to evaluate access control", "error", err)
+		return err
+	}
+	if !canSave {
+		st.Logger.Error("user is not allowed to delete alert rules in folder", "folder", folderUID, "user")
+		return dashboards.ErrFolderAccessDenied
+	}
+
 	rules, err := st.ListAlertRules(ctx, &ngmodels.ListAlertRulesQuery{
 		OrgID:         orgID,
 		NamespaceUIDs: []string{folderUID},

pkg/services/ngalert/store/alert_rule_test.go

@@ -14,6 +14,8 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/log/logtest"
 	"github.com/grafana/grafana/pkg/infra/tracing"
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/folder"
 	"github.com/grafana/grafana/pkg/services/folder/folderimpl"
@@ -26,6 +28,7 @@ import (
 	"golang.org/x/exp/rand"
 
 	"github.com/grafana/grafana/pkg/infra/db"
+	acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
@@ -476,12 +479,23 @@ func TestIntegration_DeleteInFolder(t *testing.T) {
 	}
 	rule := createRule(t, store, nil)
 
-	err := store.DeleteInFolder(context.Background(), rule.OrgID, rule.NamespaceUID, nil)
-	require.NoError(t, err)
+	t.Run("should not be able to delete folder without permissions to delete rules", func(t *testing.T) {
+		store.AccessControl = acmock.New()
+		err := store.DeleteInFolder(context.Background(), rule.OrgID, rule.NamespaceUID, &user.SignedInUser{})
+		require.ErrorIs(t, err, dashboards.ErrFolderAccessDenied)
+	})
 
-	c, err := store.CountInFolder(context.Background(), rule.OrgID, rule.NamespaceUID, nil)
-	require.NoError(t, err)
-	require.Equal(t, int64(0), c)
+	t.Run("should be able to delete folder with permissions to delete rules", func(t *testing.T) {
+		store.AccessControl = acmock.New().WithPermissions([]accesscontrol.Permission{
+			{Action: accesscontrol.ActionAlertingRuleDelete, Scope: dashboards.ScopeFoldersAll},
+		})
+		err := store.DeleteInFolder(context.Background(), rule.OrgID, rule.NamespaceUID, &user.SignedInUser{})
+		require.NoError(t, err)
+
+		c, err := store.CountInFolder(context.Background(), rule.OrgID, rule.NamespaceUID, &user.SignedInUser{})
+		require.NoError(t, err)
+		require.Equal(t, int64(0), c)
+	})
 }
 
 func TestIntegration_GetNamespaceByUID(t *testing.T) {

pkg/tests/api/alerting/api_alertmanager_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/expr"
+	"github.com/grafana/grafana/pkg/util/errutil"
 
 	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
@@ -799,7 +800,10 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) {
 		b, err := io.ReadAll(resp.Body)
 		require.NoError(t, err)
 		require.Equal(t, http.StatusBadRequest, resp.StatusCode)
-		require.JSONEq(t, `{"message":"folder cannot be deleted: folder contains alert rules"}`, string(b))
+		var errutilErr errutil.PublicError
+		err = json.Unmarshal(b, &errutilErr)
+		require.NoError(t, err)
+		assert.Equal(t, "Folder cannot be deleted: folder is not empty", errutilErr.Message)
 	}
 
 	// Next, the editor can delete the folder if forceDeleteRules is true.