Alerting: Disallow invalid rule namespace UIDs in provisioning API (#83938)

* Disallow invalid rule namespace UIDs in provisioning

Reject requests with rules that reference a nonexistent folder or have an empty folder uid

pkg/services/ngalert/accesscontrol.go

@@ -179,6 +179,10 @@ var (
 				{
 					Action: accesscontrol.ActionAlertingProvisioningWrite, // organization scope
 				},
+				{
+					Action: dashboards.ActionFoldersRead,
+					Scope:  dashboards.ScopeFoldersAll,
+				},
 			},
 		},
 		Grants: []string{string(org.RoleAdmin)},

pkg/services/ngalert/api/api_provisioning.go

@@ -60,7 +60,7 @@ type MuteTimingService interface {
 type AlertRuleService interface {
 	GetAlertRules(ctx context.Context, orgID int64) ([]*alerting_models.AlertRule, map[string]alerting_models.Provenance, error)
 	GetAlertRule(ctx context.Context, orgID int64, ruleUID string) (alerting_models.AlertRule, alerting_models.Provenance, error)
-	CreateAlertRule(ctx context.Context, rule alerting_models.AlertRule, provenance alerting_models.Provenance, userID int64) (alerting_models.AlertRule, error)
+	CreateAlertRule(ctx context.Context, user identity.Requester, rule alerting_models.AlertRule, provenance alerting_models.Provenance) (alerting_models.AlertRule, error)
 	UpdateAlertRule(ctx context.Context, rule alerting_models.AlertRule, provenance alerting_models.Provenance) (alerting_models.AlertRule, error)
 	DeleteAlertRule(ctx context.Context, orgID int64, ruleUID string, provenance alerting_models.Provenance) error
 	GetRuleGroup(ctx context.Context, orgID int64, folder, group string) (alerting_models.AlertRuleGroup, error)
@@ -334,8 +334,7 @@ func (srv *ProvisioningSrv) RoutePostAlertRule(c *contextmodel.ReqContext, ar de
 		return ErrResp(http.StatusBadRequest, err, "")
 	}
 	provenance := determineProvenance(c)
-	userID, _ := identity.UserIdentifier(c.SignedInUser.GetNamespacedID())
+	createdAlertRule, err := srv.alertRules.CreateAlertRule(c.Req.Context(), c.SignedInUser, upstreamModel, alerting_models.Provenance(provenance))
-	createdAlertRule, err := srv.alertRules.CreateAlertRule(c.Req.Context(), upstreamModel, alerting_models.Provenance(provenance), userID)
 	if errors.Is(err, alerting_models.ErrAlertRuleFailedValidation) {
 		return ErrResp(http.StatusBadRequest, err, "")
 	}

pkg/services/ngalert/api/api_provisioning_test.go

@@ -26,6 +26,8 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/dashboards"
+	"github.com/grafana/grafana/pkg/services/folder"
+	"github.com/grafana/grafana/pkg/services/folder/foldertest"
 	"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/notifier"
@@ -271,6 +273,39 @@ func TestProvisioningApi(t *testing.T) {
 				require.NotEmpty(t, response.Body())
 				require.Contains(t, string(response.Body()), "invalid alert rule")
 			})
+
+			t.Run("POST returns 400 when folderUID not set", func(t *testing.T) {
+				sut := createProvisioningSrvSut(t)
+				rc := createTestRequestCtx()
+				rule := createTestAlertRule("rule", 1)
+				rule.FolderUID = ""
+
+				response := sut.RoutePostAlertRule(&rc, rule)
+
+				require.Equal(t, 400, response.Status())
+				require.NotEmpty(t, response.Body())
+				require.Contains(t, string(response.Body()), "invalid alert rule")
+				require.Contains(t, string(response.Body()), "folderUID must be set")
+			})
+
+			t.Run("POST returns 400 if folder does not exist", func(t *testing.T) {
+				testEnv := createTestEnv(t, testConfig)
+				// Create a fake folder service that will return an error when trying to get a folder.
+				folderService := foldertest.NewFakeService()
+				folderService.ExpectedError = dashboards.ErrFolderNotFound
+				testEnv.folderService = folderService
+				sut := createProvisioningSrvSutFromEnv(t, &testEnv)
+
+				rc := createTestRequestCtx()
+				rule := createTestAlertRule("rule", 1)
+
+				response := sut.RoutePostAlertRule(&rc, rule)
+
+				require.Equal(t, 400, response.Status())
+				require.NotEmpty(t, response.Body())
+				require.Contains(t, string(response.Body()), "invalid alert rule")
+				require.Contains(t, string(response.Body()), "folder does not exist")
+			})
 		})
 
 		t.Run("exist in non-default orgs", func(t *testing.T) {
@@ -1571,6 +1606,7 @@ type testEnvironment struct {
 	secrets          secrets.Service
 	log              log.Logger
 	store            store.DBstore
+	folderService    folder.Service
 	dashboardService dashboards.DashboardService
 	configs          provisioning.AMConfigStore
 	xact             provisioning.TransactionManager
@@ -1602,12 +1638,18 @@ func createTestEnv(t *testing.T, testConfig string) testEnvironment {
 			AlertmanagerConfiguration: string(raw),
 		})
 	sqlStore := db.InitTestDB(t)
+
+	// init folder service with default folder
+	folderService := foldertest.NewFakeService()
+	folderService.ExpectedFolder = &folder.Folder{}
+
 	store := store.DBstore{
 		Logger:   log,
 		SQLStore: sqlStore,
 		Cfg: setting.UnifiedAlertingSettings{
 			BaseInterval: time.Second * 10,
 		},
+		FolderService: folderService,
 	}
 	quotas := &provisioning.MockQuotaChecker{}
 	quotas.EXPECT().LimitOK()
@@ -1637,6 +1679,7 @@ func createTestEnv(t *testing.T, testConfig string) testEnvironment {
 		log:              log,
 		configs:          configs,
 		store:            store,
+		folderService:    folderService,
 		dashboardService: dashboardService,
 		xact:             xact,
 		prov:             prov,
@@ -1662,7 +1705,7 @@ func createProvisioningSrvSutFromEnv(t *testing.T, env *testEnvironment) Provisi
 		contactPointService: provisioning.NewContactPointService(env.configs, env.secrets, env.prov, env.xact, receiverSvc, env.log, env.store),
 		templates:           provisioning.NewTemplateService(env.configs, env.prov, env.xact, env.log),
 		muteTimings:         provisioning.NewMuteTimingService(env.configs, env.prov, env.xact, env.log),
-		alertRules:          provisioning.NewAlertRuleService(env.store, env.prov, env.dashboardService, env.quotas, env.xact, 60, 10, 100, env.log, &provisioning.NotificationSettingsValidatorProviderFake{}),
+		alertRules:          provisioning.NewAlertRuleService(env.store, env.prov, env.folderService, env.dashboardService, env.quotas, env.xact, 60, 10, 100, env.log, &provisioning.NotificationSettingsValidatorProviderFake{}),
 	}
 }
 

pkg/services/ngalert/api/persist.go

@@ -14,6 +14,7 @@ type RuleStore interface {
 	// by returning map[string]struct{} instead of map[string]*folder.Folder
 	GetUserVisibleNamespaces(context.Context, int64, identity.Requester) (map[string]*folder.Folder, error)
 	GetNamespaceByUID(ctx context.Context, uid string, orgID int64, user identity.Requester) (*folder.Folder, error)
+
 	GetAlertRulesGroupByRuleUID(ctx context.Context, query *ngmodels.GetAlertRulesGroupByRuleUIDQuery) ([]*ngmodels.AlertRule, error)
 	ListAlertRules(ctx context.Context, query *ngmodels.ListAlertRulesQuery) (ngmodels.RulesGroup, error)
 

pkg/services/ngalert/ngalert.go

@@ -329,7 +329,7 @@ func (ng *AlertNG) init() error {
 	contactPointService := provisioning.NewContactPointService(ng.store, ng.SecretsService, ng.store, ng.store, receiverService, ng.Log, ng.store)
 	templateService := provisioning.NewTemplateService(ng.store, ng.store, ng.store, ng.Log)
 	muteTimingService := provisioning.NewMuteTimingService(ng.store, ng.store, ng.store, ng.Log)
-	alertRuleService := provisioning.NewAlertRuleService(ng.store, ng.store, ng.dashboardService, ng.QuotaService, ng.store,
+	alertRuleService := provisioning.NewAlertRuleService(ng.store, ng.store, ng.folderService, ng.dashboardService, ng.QuotaService, ng.store,
 		int64(ng.Cfg.UnifiedAlerting.DefaultRuleEvaluationInterval.Seconds()),
 		int64(ng.Cfg.UnifiedAlerting.BaseInterval.Seconds()),
 		ng.Cfg.UnifiedAlerting.RulesPerRuleGroupLimit, ng.Log, notifier.NewNotificationSettingsValidationService(ng.store))

pkg/services/ngalert/provisioning/alert_rules.go

@@ -7,7 +7,9 @@ import (
 	"time"
 
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/services/auth/identity"
 	"github.com/grafana/grafana/pkg/services/dashboards"
+	"github.com/grafana/grafana/pkg/services/folder"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/notifier"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
@@ -25,6 +27,7 @@ type AlertRuleService struct {
 	rulesPerRuleGroupLimit int64
 	ruleStore              RuleStore
 	provenanceStore        ProvisioningStore
+	folderService          folder.Service
 	dashboardService       dashboards.DashboardService
 	quotas                 QuotaChecker
 	xact                   TransactionManager
@@ -34,6 +37,7 @@ type AlertRuleService struct {
 
 func NewAlertRuleService(ruleStore RuleStore,
 	provenanceStore ProvisioningStore,
+	folderService folder.Service,
 	dashboardService dashboards.DashboardService,
 	quotas QuotaChecker,
 	xact TransactionManager,
@@ -49,6 +53,7 @@ func NewAlertRuleService(ruleStore RuleStore,
 		rulesPerRuleGroupLimit: rulesPerRuleGroupLimit,
 		ruleStore:              ruleStore,
 		provenanceStore:        provenanceStore,
+		folderService:          folderService,
 		dashboardService:       dashboardService,
 		quotas:                 quotas,
 		xact:                   xact,
@@ -127,7 +132,7 @@ func (service *AlertRuleService) GetAlertRuleWithFolderTitle(ctx context.Context
 // CreateAlertRule creates a new alert rule. This function will ignore any
 // interval that is set in the rule struct and use the already existing group
 // interval or the default one.
-func (service *AlertRuleService) CreateAlertRule(ctx context.Context, rule models.AlertRule, provenance models.Provenance, userID int64) (models.AlertRule, error) {
+func (service *AlertRuleService) CreateAlertRule(ctx context.Context, user identity.Requester, rule models.AlertRule, provenance models.Provenance) (models.AlertRule, error) {
 	if rule.UID == "" {
 		rule.UID = util.GenerateShortUID()
 	} else if err := util.ValidateUID(rule.UID); err != nil {
@@ -145,6 +150,9 @@ func (service *AlertRuleService) CreateAlertRule(ctx context.Context, rule model
 	if err != nil {
 		return models.AlertRule{}, err
 	}
+	if err = service.ensureRuleNamespace(ctx, user, rule); err != nil {
+		return models.AlertRule{}, err
+	}
 	rule.Updated = time.Now()
 	if len(rule.NotificationSettings) > 0 {
 		validator, err := service.nsValidatorProvider.Validator(ctx, rule.OrgID)
@@ -176,6 +184,11 @@ func (service *AlertRuleService) CreateAlertRule(ctx context.Context, rule model
 			return errors.New("couldn't find newly created id")
 		}
 
+		// default to 0 if there is no user
+		userID := int64(0)
+		if user != nil {
+			userID, _ = identity.UserIdentifier(user.GetNamespacedID())
+		}
 		if err = service.checkLimitsTransactionCtx(ctx, rule.OrgID, userID); err != nil {
 			return err
 		}
@@ -639,3 +652,32 @@ func (service *AlertRuleService) checkGroupLimits(group models.AlertRuleGroup) e
 
 	return nil
 }
+
+// ensureRuleNamespace ensures that the rule has a valid namespace UID.
+// If the rule does not have a namespace UID or the namespace (folder) does not exist it will return an error.
+func (service *AlertRuleService) ensureRuleNamespace(ctx context.Context, user identity.Requester, rule models.AlertRule) error {
+	if rule.NamespaceUID == "" {
+		return fmt.Errorf("%w: folderUID must be set", models.ErrAlertRuleFailedValidation)
+	}
+
+	if user == nil {
+		// user is nil when this is called during file provisioning,
+		// which already creates the folder if it does not exist
+		return nil
+	}
+
+	// ensure the namespace exists
+	_, err := service.folderService.Get(ctx, &folder.GetFolderQuery{
+		OrgID:        rule.OrgID,
+		UID:          &rule.NamespaceUID,
+		SignedInUser: user,
+	})
+	if err != nil {
+		if errors.Is(err, dashboards.ErrFolderNotFound) {
+			return fmt.Errorf("%w: folder does not exist", models.ErrAlertRuleFailedValidation)
+		}
+		return err
+	}
+
+	return nil
+}

pkg/services/ngalert/provisioning/alert_rules_test.go

@@ -15,6 +15,8 @@ import (
 
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/services/folder"
+	"github.com/grafana/grafana/pkg/services/folder/foldertest"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
 	"github.com/grafana/grafana/pkg/setting"
@@ -42,7 +44,7 @@ func TestAlertRuleService(t *testing.T) {
 	t.Run("alert rule group should be updated correctly", func(t *testing.T) {
 		rule := dummyRule("test#3", orgID)
 		rule.RuleGroup = "a"
-		rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)
+		rule, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone)
 		require.NoError(t, err)
 		require.Equal(t, int64(60), rule.IntervalSeconds)
 
@@ -59,7 +61,7 @@ func TestAlertRuleService(t *testing.T) {
 		var orgID int64 = 2
 		rule := dummyRule("test#1", orgID)
 		rule.NamespaceUID = "123abc"
-		rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)
+		rule, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone)
 		require.NoError(t, err)
 
 		rule.NamespaceUID = "abc123"
@@ -68,6 +70,20 @@ func TestAlertRuleService(t *testing.T) {
 		require.NoError(t, err)
 	})
 
+	t.Run("group update should propagate folderUID from group to rules", func(t *testing.T) {
+		ruleService := createAlertRuleService(t)
+		group := createDummyGroup("namespace-test", 1)
+		group.Rules[0].NamespaceUID = ""
+
+		err := ruleService.ReplaceRuleGroup(context.Background(), 1, group, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+
+		readGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "namespace-test")
+		require.NoError(t, err)
+		require.NotEmpty(t, readGroup.Rules)
+		require.Equal(t, "my-namespace", readGroup.Rules[0].NamespaceUID)
+	})
+
 	t.Run("group creation should propagate group title correctly", func(t *testing.T) {
 		group := createDummyGroup("group-test-3", orgID)
 		group.Rules[0].RuleGroup = "something different"
@@ -86,7 +102,7 @@ func TestAlertRuleService(t *testing.T) {
 	t.Run("alert rule should get interval from existing rule group", func(t *testing.T) {
 		rule := dummyRule("test#4", orgID)
 		rule.RuleGroup = "b"
-		rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)
+		rule, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone)
 		require.NoError(t, err)
 
 		var interval int64 = 120
@@ -95,7 +111,7 @@ func TestAlertRuleService(t *testing.T) {
 
 		rule = dummyRule("test#4-1", orgID)
 		rule.RuleGroup = "b"
-		rule, err = ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)
+		rule, err = ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone)
 		require.NoError(t, err)
 		require.Equal(t, interval, rule.IntervalSeconds)
 	})
@@ -112,7 +128,7 @@ func TestAlertRuleService(t *testing.T) {
 		rule.UID = ruleUID
 		rule.RuleGroup = ruleGroup
 		rule.NamespaceUID = namespaceUID
-		_, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)
+		_, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone)
 		require.NoError(t, err)
 
 		rule, _, err = ruleService.GetAlertRule(context.Background(), orgID, ruleUID)
@@ -420,7 +436,7 @@ func TestAlertRuleService(t *testing.T) {
 			t.Run(test.name, func(t *testing.T) {
 				var orgID int64 = 1
 				rule := dummyRule(t.Name(), orgID)
-				rule, err := ruleService.CreateAlertRule(context.Background(), rule, test.from, 0)
+				rule, err := ruleService.CreateAlertRule(context.Background(), nil, rule, test.from)
 				require.NoError(t, err)
 
 				_, err = ruleService.UpdateAlertRule(context.Background(), rule, test.to)
@@ -501,7 +517,7 @@ func TestAlertRuleService(t *testing.T) {
 		checker.EXPECT().LimitExceeded()
 		ruleService.quotas = checker
 
-		_, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#1", orgID), models.ProvenanceNone, 0)
+		_, err := ruleService.CreateAlertRule(context.Background(), nil, dummyRule("test#1", orgID), models.ProvenanceNone)
 
 		require.ErrorIs(t, err, models.ErrQuotaReached)
 	})
@@ -524,13 +540,13 @@ func TestCreateAlertRule(t *testing.T) {
 	var orgID int64 = 1
 
 	t.Run("should return the created id", func(t *testing.T) {
-		rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#1", orgID), models.ProvenanceNone, 0)
+		rule, err := ruleService.CreateAlertRule(context.Background(), nil, dummyRule("test#1", orgID), models.ProvenanceNone)
 		require.NoError(t, err)
 		require.NotEqual(t, 0, rule.ID, "expected to get the created id and not the zero value")
 	})
 
 	t.Run("should set the right provenance", func(t *testing.T) {
-		rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#2", orgID), models.ProvenanceAPI, 0)
+		rule, err := ruleService.CreateAlertRule(context.Background(), nil, dummyRule("test#2", orgID), models.ProvenanceAPI)
 		require.NoError(t, err)
 
 		_, provenance, err := ruleService.GetAlertRule(context.Background(), orgID, rule.UID)
@@ -542,14 +558,14 @@ func TestCreateAlertRule(t *testing.T) {
 		t.Run("return error if it is not valid UID", func(t *testing.T) {
 			rule := dummyRule("test#3", orgID)
 			rule.UID = strings.Repeat("1", util.MaxUIDLength+1)
-			rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)
+			rule, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone)
 			require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation)
 		})
 		t.Run("should create a new rule with this UID", func(t *testing.T) {
 			rule := dummyRule("test#3", orgID)
 			uid := util.GenerateShortUID()
 			rule.UID = uid
-			created, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)
+			created, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone)
 			require.NoError(t, err)
 			require.Equal(t, uid, created.UID)
 			_, _, err = ruleService.GetAlertRule(context.Background(), orgID, uid)
@@ -561,13 +577,19 @@ func TestCreateAlertRule(t *testing.T) {
 func createAlertRuleService(t *testing.T) AlertRuleService {
 	t.Helper()
 	sqlStore := db.InitTestDB(t)
+	folderService := foldertest.NewFakeService()
+	folderService.ExpectedFolder = &folder.Folder{
+		UID: "default-namespace",
+	}
 	store := store.DBstore{
 		SQLStore: sqlStore,
 		Cfg: setting.UnifiedAlertingSettings{
 			BaseInterval: time.Second * 10,
 		},
-		Logger: log.NewNopLogger(),
+		Logger:        log.NewNopLogger(),
+		FolderService: folderService,
 	}
+	// store := fakes.NewRuleStore(t)
 	quotas := MockQuotaChecker{}
 	quotas.EXPECT().LimitOK()
 	return AlertRuleService{
@@ -578,6 +600,7 @@ func createAlertRuleService(t *testing.T) AlertRuleService {
 		log:                    log.New("testing"),
 		baseIntervalSeconds:    10,
 		defaultIntervalSeconds: 60,
+		folderService:          folderService,
 	}
 }
 

pkg/services/provisioning/alerting/rules_provisioner.go

@@ -85,9 +85,9 @@ func (prov *defaultAlertRuleProvisioner) provisionRule(
 		return err
 	} else if err != nil {
 		prov.logger.Debug("creating rule", "uid", rule.UID, "org", rule.OrgID)
-		// 0 is passed as userID as then the quota logic will only check for
+		// a nil user is passed in as then the quota logic will only check for
-		// the organization quota, as we don't have any user scope here.
+		// the organization quota since we don't have any user scope here.
-		_, err = prov.ruleService.CreateAlertRule(ctx, rule, alert_models.ProvenanceFile, 0)
+		_, err = prov.ruleService.CreateAlertRule(ctx, nil, rule, alert_models.ProvenanceFile)
 	} else {
 		prov.logger.Debug("updating rule", "uid", rule.UID, "org", rule.OrgID)
 		_, err = prov.ruleService.UpdateAlertRule(ctx, rule, alert_models.ProvenanceFile)

pkg/services/provisioning/provisioning.go

@@ -275,6 +275,7 @@ func (ps *ProvisioningServiceImpl) ProvisionAlerting(ctx context.Context) error
 	ruleService := provisioning.NewAlertRuleService(
 		st,
 		st,
+		ps.folderService,
 		ps.dashboardService,
 		ps.quotaService,
 		ps.SQLStore,