Alerting: Update rule access control to explicitly check for permissions "alert.rules:read" and "folders:read" (#78289)

* require "folders:read" and "alert.rules:read"  in all rules API requests (write and read). 

* add check for permissions "folders:read" and "alert.rules:read" to AuthorizeAccessToRuleGroup and HasAccessToRuleGroup

* check only access to datasource in rule testing API

---------

Co-authored-by: William Wernert <william.wernert@grafana.com>

pkg/services/ngalert/accesscontrol/rules.go

@@ -48,9 +48,27 @@ func (r *RuleService) HasAccessOrError(ctx context.Context, user identity.Reques
 	return nil
 }
 
-// getRulesReadEvaluator constructs accesscontrol.Evaluator that checks all permission required to read all provided rules
+// getReadFolderAccessEvaluator constructs accesscontrol.Evaluator that checks all permissions required to read rules in  specific folder
+func getReadFolderAccessEvaluator(folderUID string) accesscontrol.Evaluator {
+	return accesscontrol.EvalAll(
+		accesscontrol.EvalPermission(ruleRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(folderUID)),
+		accesscontrol.EvalPermission(dashboards.ActionFoldersRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(folderUID)),
+	)
+}
+
+// getRulesReadEvaluator constructs accesscontrol.Evaluator that checks all permissions required to access provided rules
 func (r *RuleService) getRulesReadEvaluator(rules ...*models.AlertRule) accesscontrol.Evaluator {
-	return r.getRulesQueryEvaluator(rules...)
+	added := make(map[string]struct{}, 1)
+	evals := make([]accesscontrol.Evaluator, 0, 1)
+	for _, rule := range rules {
+		if _, ok := added[rule.NamespaceUID]; ok {
+			continue
+		}
+		added[rule.NamespaceUID] = struct{}{}
+		evals = append(evals, getReadFolderAccessEvaluator(rule.NamespaceUID))
+	}
+	dsEvals := r.getRulesQueryEvaluator(rules...)
+	return accesscontrol.EvalAll(append(evals, dsEvals)...)
 }
 
 // getRulesQueryEvaluator constructs accesscontrol.Evaluator that checks all permissions to query data sources used by the provided rules
@@ -88,13 +106,21 @@ func (r *RuleService) AuthorizeDatasourceAccessForRule(ctx context.Context, user
 	})
 }
 
-// HasAccessToRuleGroup returns false if
+// AuthorizeAccessToRuleGroup checks that the identity.Requester has permissions to all rules, which means that it has permissions to:
+// - ("folders:read") read folders which contain the rules
+// - ("alert.rules:read") read alert rules in the folders
+// - ("datasources:query") query all data sources that rules refer to
+// Returns false if the requester does not have enough permissions, and error if something went wrong during the permission evaluation.
 func (r *RuleService) HasAccessToRuleGroup(ctx context.Context, user identity.Requester, rules models.RulesGroup) (bool, error) {
 	eval := r.getRulesReadEvaluator(rules...)
 	return r.HasAccess(ctx, user, eval)
 }
 
-// AuthorizeAccessToRuleGroup checks all rules against AuthorizeDatasourceAccessForRule and exits on the first negative result
+// AuthorizeAccessToRuleGroup checks that the identity.Requester has permissions to all rules, which means that it has permissions to:
+// - ("folders:read") read folders which contain the rules
+// - ("alert.rules:read") read alert rules in the folders
+// - ("datasources:query") query all data sources that rules refer to
+// Returns error if at least one permissions is missing or if something went wrong during the permission evaluation
 func (r *RuleService) AuthorizeAccessToRuleGroup(ctx context.Context, user identity.Requester, rules models.RulesGroup) error {
 	eval := r.getRulesReadEvaluator(rules...)
 	return r.HasAccessOrError(ctx, user, eval, func() string {
@@ -113,8 +139,8 @@ func (r *RuleService) AuthorizeAccessToRuleGroup(ctx context.Context, user ident
 func (r *RuleService) AuthorizeRuleChanges(ctx context.Context, user identity.Requester, change *store.GroupDelta) error {
 	namespaceScope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(change.GroupKey.NamespaceUID)
 
-	rules, ok := change.AffectedGroups[change.GroupKey]
+	rules, existingGroup := change.AffectedGroups[change.GroupKey]
-	if ok { // not ok can be when user creates a new rule group or moves existing alerts to a new group
+	if existingGroup { // not existingGroup can be when user creates a new rule group or moves existing alerts to a new group
 		if err := r.AuthorizeAccessToRuleGroup(ctx, user, rules); err != nil { // if user is not authorized to do operation in the group that is being changed
 			return err
 		}
@@ -153,6 +179,12 @@ func (r *RuleService) AuthorizeRuleChanges(ctx context.Context, user identity.Re
 				return err
 			}
 		}
+		if !existingGroup {
+			// create a new group, check that user has "read" access to that new group. Otherwise, it will not be able to read it back.
+			if err := r.AuthorizeAccessToRuleGroup(ctx, user, change.New); err != nil { // if user is not authorized to do operation in the group that is being changed
+				return err
+			}
+		}
 	}
 
 	for _, rule := range change.Update {
@@ -190,8 +222,8 @@ func (r *RuleService) AuthorizeRuleChanges(ctx context.Context, user identity.Re
 
 		if rule.Existing.NamespaceUID != rule.New.NamespaceUID || rule.Existing.RuleGroup != rule.New.RuleGroup {
 			key := rule.Existing.GetGroupKey()
-			rules, ok = change.AffectedGroups[key]
+			rules, existingGroup = change.AffectedGroups[key]
-			if !ok {
+			if !existingGroup {
 				// add a safeguard in the case of inconsistency. If user hit this then there is a bug in the calculating of changes struct
 				return fmt.Errorf("failed to authorize moving an alert rule %s between groups because unable to check access to group %s from which the rule is moved", rule.Existing.UID, rule.Existing.RuleGroup)
 			}

pkg/services/ngalert/accesscontrol/rules_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/auth/identity"
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/datasources"
+	"github.com/grafana/grafana/pkg/services/folder"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
 	"github.com/grafana/grafana/pkg/services/user"
@@ -118,6 +119,12 @@ func TestAuthorizeRuleChanges(t *testing.T) {
 					ruleCreate: {
 						namespaceIdScope,
 					},
+					ruleRead: {
+						namespaceIdScope,
+					},
+					dashboards.ActionFoldersRead: {
+						namespaceIdScope,
+					},
 					datasources.ActionQuery: scopes,
 				}
 			},
@@ -139,6 +146,12 @@ func TestAuthorizeRuleChanges(t *testing.T) {
 			},
 			permissions: func(c *store.GroupDelta) map[string][]string {
 				return map[string][]string{
+					ruleRead: {
+						namespaceIdScope,
+					},
+					dashboards.ActionFoldersRead: {
+						namespaceIdScope,
+					},
 					ruleDelete: {
 						namespaceIdScope,
 					},
@@ -178,6 +191,12 @@ func TestAuthorizeRuleChanges(t *testing.T) {
 					return update.New
 				})...))
 				return map[string][]string{
+					ruleRead: {
+						namespaceIdScope,
+					},
+					dashboards.ActionFoldersRead: {
+						namespaceIdScope,
+					},
 					ruleUpdate: {
 						namespaceIdScope,
 					},
@@ -307,6 +326,12 @@ func TestAuthorizeRuleChanges(t *testing.T) {
 				}
 
 				return map[string][]string{
+					ruleRead: {
+						dashboards.ScopeFoldersProvider.GetResourceScopeUID(c.GroupKey.NamespaceUID),
+					},
+					dashboards.ActionFoldersRead: {
+						dashboards.ScopeFoldersProvider.GetResourceScopeUID(c.GroupKey.NamespaceUID),
+					},
 					ruleUpdate: {
 						dashboards.ScopeFoldersProvider.GetResourceScopeUID(c.GroupKey.NamespaceUID),
 					},
@@ -378,6 +403,12 @@ func TestCheckDatasourcePermissionsForRule(t *testing.T) {
 
 	t.Run("should check only expressions", func(t *testing.T) {
 		permissions := map[string][]string{
+			ruleRead: {
+				dashboards.ScopeFoldersProvider.GetResourceScopeUID(rule.NamespaceUID),
+			},
+			dashboards.ActionFoldersRead: {
+				dashboards.ScopeFoldersProvider.GetResourceScopeUID(rule.NamespaceUID),
+			},
 			datasources.ActionQuery: scopes,
 		}
 
@@ -418,8 +449,14 @@ func Test_authorizeAccessToRuleGroup(t *testing.T) {
 				scopes = append(scopes, datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID))
 			}
 		}
+		namespaceScopes := make([]string, 0)
+		for _, rule := range rules {
+			namespaceScopes = append(namespaceScopes, dashboards.ScopeFoldersProvider.GetResourceScopeUID(rule.NamespaceUID))
+		}
 		permissions := map[string][]string{
-			datasources.ActionQuery: scopes,
+			ruleRead:                     namespaceScopes,
+			dashboards.ActionFoldersRead: namespaceScopes,
+			datasources.ActionQuery:      scopes,
 		}
 		ac := &recordingAccessControlFake{}
 		svc := RuleService{
@@ -432,7 +469,8 @@ func Test_authorizeAccessToRuleGroup(t *testing.T) {
 		require.NotEmpty(t, ac.EvaluateRecordings)
 	})
 	t.Run("should return false if user does not have access to at least one rule in group", func(t *testing.T) {
-		rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen())
+		f := &folder.Folder{UID: "test-folder"}
+		rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(models.WithNamespace(f)))
 		var scopes []string
 		for _, rule := range rules {
 			for _, query := range rule.Data {
@@ -440,10 +478,16 @@ func Test_authorizeAccessToRuleGroup(t *testing.T) {
 			}
 		}
 		permissions := map[string][]string{
+			ruleRead: {
+				dashboards.ScopeFoldersProvider.GetResourceScopeUID(f.UID),
+			},
+			dashboards.ActionFoldersRead: {
+				dashboards.ScopeFoldersProvider.GetResourceScopeUID(f.UID),
+			},
 			datasources.ActionQuery: scopes,
 		}
 
-		rule := models.AlertRuleGen()()
+		rule := models.AlertRuleGen(models.WithNamespace(f))()
 		rules = append(rules, rule)
 
 		ac := &recordingAccessControlFake{}

pkg/services/ngalert/api/api_ruler_export_test.go

@@ -15,7 +15,9 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"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/datasources"
 	folder2 "github.com/grafana/grafana/pkg/services/folder"
 	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
@@ -413,7 +415,9 @@ func TestExportRules(t *testing.T) {
 		t.Run(tc.title, func(t *testing.T) {
 			rc := createRequestContextWithPerms(orgID, map[int64]map[string][]string{
 				orgID: {
-					datasources.ActionQuery: []string{datasources.ScopeProvider.GetResourceScopeUID(accessQuery.DatasourceUID)},
+					dashboards.ActionFoldersRead:         []string{dashboards.ScopeFoldersProvider.GetResourceScopeUID(f1.UID), dashboards.ScopeFoldersProvider.GetResourceScopeUID(f2.UID)},
+					accesscontrol.ActionAlertingRuleRead: []string{dashboards.ScopeFoldersProvider.GetResourceScopeUID(f1.UID), dashboards.ScopeFoldersProvider.GetResourceScopeUID(f2.UID)},
+					datasources.ActionQuery:              []string{datasources.ScopeProvider.GetResourceScopeUID(accessQuery.DatasourceUID)},
 				},
 			}, nil)
 			rc.Req.Form = tc.params

pkg/services/ngalert/api/api_ruler_test.go

@@ -18,8 +18,10 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/infra/log"
+	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
+	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/folder"
@@ -237,7 +239,8 @@ func TestRouteGetNamespaceRulesConfig(t *testing.T) {
 		err := svc.provenanceStore.SetProvenance(context.Background(), rule, orgID, models.ProvenanceAPI)
 		require.NoError(t, err)
 
-		req := createRequestContext(orgID, nil)
+		perms := createPermissionsForRules(expectedRules, orgID)
+		req := createRequestContextWithPerms(orgID, perms, nil)
 		response := svc.RouteGetNamespaceRulesConfig(req, folder.UID)
 
 		require.Equal(t, http.StatusAccepted, response.Status())
@@ -271,7 +274,8 @@ func TestRouteGetNamespaceRulesConfig(t *testing.T) {
 		expectedRules := models.GenerateAlertRules(rand.Intn(5)+5, models.AlertRuleGen(withGroupKey(groupKey), models.WithUniqueGroupIndex()))
 		ruleStore.PutRule(context.Background(), expectedRules...)
 
-		req := createRequestContext(orgID, nil)
+		perms := createPermissionsForRules(expectedRules, orgID)
+		req := createRequestContextWithPerms(orgID, perms, nil)
 		response := createService(ruleStore).RouteGetNamespaceRulesConfig(req, folder.UID)
 
 		require.Equal(t, http.StatusAccepted, response.Status())
@@ -354,7 +358,8 @@ func TestRouteGetRulesConfig(t *testing.T) {
 		expectedRules := models.GenerateAlertRules(rand.Intn(5)+5, models.AlertRuleGen(withGroupKey(groupKey), models.WithUniqueGroupIndex()))
 		ruleStore.PutRule(context.Background(), expectedRules...)
 
-		req := createRequestContext(orgID, nil)
+		perms := createPermissionsForRules(expectedRules, orgID)
+		req := createRequestContextWithPerms(orgID, perms, nil)
 		response := createService(ruleStore).RouteGetRulesConfig(req)
 
 		require.Equal(t, http.StatusOK, response.Status())
@@ -437,7 +442,9 @@ func TestRouteGetRulesGroupConfig(t *testing.T) {
 		expectedRules := models.GenerateAlertRules(rand.Intn(5)+5, models.AlertRuleGen(withGroupKey(groupKey), models.WithUniqueGroupIndex()))
 		ruleStore.PutRule(context.Background(), expectedRules...)
 
-		req := createRequestContext(orgID, nil)
+		perms := createPermissionsForRules(expectedRules, orgID)
+		req := createRequestContextWithPerms(orgID, perms, nil)
+
 		response := createService(ruleStore).RouteGetRulesGroupConfig(req, folder.UID, groupKey.RuleGroup)
 
 		require.Equal(t, http.StatusAccepted, response.Status())
@@ -672,8 +679,15 @@ func createRequestContextWithPerms(orgID int64, permissions map[int64]map[string
 }
 
 func createPermissionsForRules(rules []*models.AlertRule, orgID int64) map[int64]map[string][]string {
+	ns := map[string]any{}
 	permissions := map[string][]string{}
 	for _, rule := range rules {
+		if _, ok := ns[rule.NamespaceUID]; !ok {
+			scope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(rule.NamespaceUID)
+			permissions[dashboards.ActionFoldersRead] = append(permissions[dashboards.ActionFoldersRead], scope)
+			permissions[ac.ActionAlertingRuleRead] = append(permissions[ac.ActionAlertingRuleRead], scope)
+			ns[rule.NamespaceUID] = struct{}{}
+		}
 		for _, query := range rule.Data {
 			permissions[datasources.ActionQuery] = append(permissions[datasources.ActionQuery], datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID))
 		}

pkg/services/ngalert/api/api_testing.go

@@ -73,7 +73,7 @@ func (srv TestingApiSrv) RouteTestGrafanaRuleConfig(c *contextmodel.ReqContext,
 		return ErrResp(http.StatusBadRequest, err, "")
 	}
 
-	if err := srv.authz.AuthorizeAccessToRuleGroup(c.Req.Context(), c.SignedInUser, ngmodels.RulesGroup{rule}); err != nil {
+	if err := srv.authz.AuthorizeDatasourceAccessForRule(c.Req.Context(), c.SignedInUser, rule); err != nil {
 		return response.ErrOrFallback(http.StatusInternalServerError, "failed to authorize access to rule group", err)
 	}
 
@@ -244,7 +244,7 @@ func (srv TestingApiSrv) BacktestAlertRule(c *contextmodel.ReqContext, cmd apimo
 	}
 
 	queries := AlertQueriesFromApiAlertQueries(cmd.Data)
-	if err := srv.authz.AuthorizeAccessToRuleGroup(c.Req.Context(), c.SignedInUser, ngmodels.RulesGroup{&ngmodels.AlertRule{Data: queries}}); err != nil {
+	if err := srv.authz.AuthorizeDatasourceAccessForRule(c.Req.Context(), c.SignedInUser, &ngmodels.AlertRule{Data: queries}); err != nil {
 		return errorToResponse(err)
 	}
 

pkg/services/ngalert/api/authorization.go

@@ -20,28 +20,43 @@ func (api *API) authorize(method, path string) web.Handler {
 	// Alert Rules
 
 	// Grafana Paths
-	case http.MethodDelete + "/api/ruler/grafana/api/v1/rules/{Namespace}/{Groupname}":
+	case http.MethodDelete + "/api/ruler/grafana/api/v1/rules/{Namespace}/{Groupname}",
-		eval = ac.EvalPermission(ac.ActionAlertingRuleDelete, dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace")))
+		http.MethodDelete + "/api/ruler/grafana/api/v1/rules/{Namespace}":
-	case http.MethodDelete + "/api/ruler/grafana/api/v1/rules/{Namespace}":
+		eval = ac.EvalAll(
-		eval = ac.EvalPermission(ac.ActionAlertingRuleDelete, dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace")))
+			ac.EvalPermission(ac.ActionAlertingRuleDelete, dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace"))),
+			ac.EvalPermission(ac.ActionAlertingRuleRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace"))),
+			ac.EvalPermission(dashboards.ActionFoldersRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace"))),
+		)
 	case http.MethodGet + "/api/ruler/grafana/api/v1/rules/{Namespace}/{Groupname}":
-		eval = ac.EvalPermission(ac.ActionAlertingRuleRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace")))
+		eval = ac.EvalAll(
+			ac.EvalPermission(ac.ActionAlertingRuleRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace"))),
+			ac.EvalPermission(dashboards.ActionFoldersRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace"))),
+		)
 	case http.MethodGet + "/api/ruler/grafana/api/v1/rules/{Namespace}":
-		eval = ac.EvalPermission(ac.ActionAlertingRuleRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace")))
+		eval = ac.EvalAll(
+			ac.EvalPermission(ac.ActionAlertingRuleRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace"))),
+			ac.EvalPermission(dashboards.ActionFoldersRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace"))),
+		)
 	case http.MethodGet + "/api/ruler/grafana/api/v1/rules",
 		http.MethodGet + "/api/ruler/grafana/api/v1/export/rules":
 		eval = ac.EvalPermission(ac.ActionAlertingRuleRead)
 	case http.MethodPost + "/api/ruler/grafana/api/v1/rules/{Namespace}/export":
 		scope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace"))
 		// more granular permissions are enforced by the handler via "authorizeRuleChanges"
-		eval = ac.EvalPermission(ac.ActionAlertingRuleRead, scope)
+		eval = ac.EvalAll(ac.EvalPermission(ac.ActionAlertingRuleRead, scope),
+			ac.EvalPermission(dashboards.ActionFoldersRead, scope),
+		)
 	case http.MethodPost + "/api/ruler/grafana/api/v1/rules/{Namespace}":
 		scope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.Parameter(":Namespace"))
 		// more granular permissions are enforced by the handler via "authorizeRuleChanges"
-		eval = ac.EvalAny(
+		eval = ac.EvalAll(
-			ac.EvalPermission(ac.ActionAlertingRuleUpdate, scope),
+			ac.EvalPermission(ac.ActionAlertingRuleRead, scope),
-			ac.EvalPermission(ac.ActionAlertingRuleCreate, scope),
+			ac.EvalPermission(dashboards.ActionFoldersRead, scope),
-			ac.EvalPermission(ac.ActionAlertingRuleDelete, scope),
+			ac.EvalAny(
+				ac.EvalPermission(ac.ActionAlertingRuleUpdate, scope),
+				ac.EvalPermission(ac.ActionAlertingRuleCreate, scope),
+				ac.EvalPermission(ac.ActionAlertingRuleDelete, scope),
+			),
 		)
 
 	// Grafana rule state history paths

pkg/tests/api/alerting/api_alertmanager_test.go

@@ -614,7 +614,7 @@ func TestIntegrationRulerAccess(t *testing.T) {
 			desc:            "viewer request should fail",
 			client:          newAlertingApiClient(grafanaListedAddr, "viewer", "viewer"),
 			expStatus:       http.StatusForbidden,
-			expectedMessage: `You'll need additional permissions to perform this action. Permissions needed: any of alert.rules:write, alert.rules:create, alert.rules:delete`,
+			expectedMessage: `You'll need additional permissions to perform this action. Permissions needed: all of alert.rules:read, folders:read, any of alert.rules:write, alert.rules:create, alert.rules:delete`,
 		},
 		{
 			desc:            "editor request should succeed",

pkg/tests/api/alerting/api_backtesting_test.go

@@ -125,7 +125,7 @@ func TestBacktesting(t *testing.T) {
 
 		t.Run("fail if can't query data sources", func(t *testing.T) {
 			status, body := testUserApiCli.SubmitRuleForBacktesting(t, queryRequest)
-			require.Contains(t, body, "user is not authorized to access rule group")
+			require.Contains(t, body, "user is not authorized to access one or many data sources")
 			require.Equalf(t, http.StatusForbidden, status, "Response: %s", body)
 		})
 	})