From 099055e8a592b55eea51549a2a329f70aef9de36 Mon Sep 17 00:00:00 2001 From: Matthew Jacobson Date: Fri, 4 Oct 2024 16:52:38 -0400 Subject: [PATCH] Alerting: Verify receiver permission read on rule create/update (#94286) * Alerting: Verify receiver permission read on rule create/update --- pkg/services/ngalert/accesscontrol/rules.go | 31 +++- .../ngalert/accesscontrol/rules_test.go | 145 +++++++++++++----- pkg/services/ngalert/models/notifications.go | 4 + pkg/services/ngalert/models/receivers.go | 5 + .../ngalert/notifier/legacy_storage/compat.go | 4 +- 5 files changed, 149 insertions(+), 40 deletions(-) diff --git a/pkg/services/ngalert/accesscontrol/rules.go b/pkg/services/ngalert/accesscontrol/rules.go index 812cc5816eb..8aa64c479e9 100644 --- a/pkg/services/ngalert/accesscontrol/rules.go +++ b/pkg/services/ngalert/accesscontrol/rules.go @@ -2,6 +2,7 @@ package accesscontrol import ( "fmt" + "slices" "golang.org/x/net/context" @@ -23,11 +24,17 @@ const ( type RuleService struct { genericService + notificationSettingsAuth notificationSettingsAuth +} + +type notificationSettingsAuth interface { + AuthorizeRead(context.Context, identity.Requester, *models.NotificationSettings) error } func NewRuleService(ac accesscontrol.AccessControl) *RuleService { return &RuleService{ - genericService{ac: ac}, + genericService: genericService{ac: ac}, + notificationSettingsAuth: NewReceiverAccess[*models.NotificationSettings](ac, true), } } @@ -196,6 +203,10 @@ func (r *RuleService) AuthorizeRuleChanges(ctx context.Context, user identity.Re }); err != nil { return err } + + if err := r.authorizeNotificationSettings(ctx, user, rule); err != nil { + 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. @@ -237,6 +248,24 @@ func (r *RuleService) AuthorizeRuleChanges(ctx context.Context, user identity.Re } updateAuthorized = true } + + if !slices.EqualFunc(rule.Existing.NotificationSettings, rule.New.NotificationSettings, func(settings models.NotificationSettings, settings2 models.NotificationSettings) bool { + return settings.Equals(&settings2) + }) { + if err := r.authorizeNotificationSettings(ctx, user, rule.New); err != nil { + return err + } + } + } + return nil +} + +// authorizeNotificationSettings checks if the user has access to all receivers that are used by the rule's notification settings. +func (r *RuleService) authorizeNotificationSettings(ctx context.Context, user identity.Requester, rule *models.AlertRule) error { + for _, ns := range rule.NotificationSettings { + if err := r.notificationSettingsAuth.AuthorizeRead(ctx, user, &ns); err != nil { + return err + } } return nil } diff --git a/pkg/services/ngalert/accesscontrol/rules_test.go b/pkg/services/ngalert/accesscontrol/rules_test.go index cc85799ba3c..8c823763308 100644 --- a/pkg/services/ngalert/accesscontrol/rules_test.go +++ b/pkg/services/ngalert/accesscontrol/rules_test.go @@ -13,10 +13,14 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" + "github.com/grafana/grafana/pkg/services/authz/zanzana" "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" "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/services/ngalert/notifier/legacy_storage" "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/util" @@ -75,6 +79,22 @@ func getDatasourceScopesForRules(rules models.RulesGroup) []string { return result } +func getReceiverScopesForRules(rules models.RulesGroup) []string { + scopesMap := map[string]struct{}{} + var result []string + for _, rule := range rules { + for _, ns := range rule.NotificationSettings { + scope := ScopeReceiversProvider.GetResourceScopeUID(legacy_storage.NameToUid(ns.Receiver)) + if _, ok := scopesMap[scope]; ok { + continue + } + result = append(result, scope) + scopesMap[scope] = struct{}{} + } + } + return result +} + func mapUpdates(updates []store.RuleDelta, mapFunc func(store.RuleDelta) *models.AlertRule) models.RulesGroup { result := make(models.RulesGroup, 0, len(updates)) for _, update := range updates { @@ -111,12 +131,6 @@ func TestAuthorizeRuleChanges(t *testing.T) { } }, permissions: func(c *store.GroupDelta) map[string][]string { - var scopes []string - for _, rule := range c.New { - for _, query := range rule.Data { - scopes = append(scopes, datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID)) - } - } return map[string][]string{ ruleCreate: { namespaceIdScope, @@ -127,7 +141,8 @@ func TestAuthorizeRuleChanges(t *testing.T) { dashboards.ActionFoldersRead: { namespaceIdScope, }, - datasources.ActionQuery: scopes, + datasources.ActionQuery: getDatasourceScopesForRules(c.New), + accesscontrol.ActionAlertingReceiversRead: getReceiverScopesForRules(c.New), } }, }, @@ -313,6 +328,85 @@ func TestAuthorizeRuleChanges(t *testing.T) { } }, }, + { + name: "if there are new rules that have notification settings it should check access to all receivers", + changes: func() *store.GroupDelta { + receiverName := "test-receiver" + genWithNotificationSettings := genWithGroupKey.With(gen.WithNotificationSettingsGen(models.NotificationSettingsGen(models.NSMuts.WithReceiver(receiverName)))) + return &store.GroupDelta{ + GroupKey: groupKey, + New: genWithNotificationSettings.GenerateManyRef(1, 5), + Update: nil, + Delete: nil, + } + }, + permissions: func(c *store.GroupDelta) map[string][]string { + return map[string][]string{ + ruleCreate: { + namespaceIdScope, + }, + ruleRead: { + namespaceIdScope, + }, + dashboards.ActionFoldersRead: { + namespaceIdScope, + }, + datasources.ActionQuery: getDatasourceScopesForRules(c.New), + accesscontrol.ActionAlertingReceiversRead: getReceiverScopesForRules(c.New), + } + }, + }, + { + name: "if there are rules that modify notification settings it should check access to all receivers", + changes: func() *store.GroupDelta { + receiverName := "test-receiver" + genWithNotificationSettings := genWithGroupKey.With(gen.WithNotificationSettingsGen(models.NotificationSettingsGen(models.NSMuts.WithReceiver(receiverName)))) + rules1 := genWithNotificationSettings.GenerateManyRef(1, 5) + rules := genWithNotificationSettings.GenerateManyRef(1, 5) + updates := make([]store.RuleDelta, 0, len(rules)) + + for _, rule := range rules { + cp := models.CopyRule(rule) + for i := range cp.NotificationSettings { + cp.NotificationSettings[i].Receiver = "new-receiver" + } + updates = append(updates, store.RuleDelta{ + Existing: rule, + New: cp, + Diff: nil, + }) + } + + return &store.GroupDelta{ + GroupKey: groupKey, + AffectedGroups: map[models.AlertRuleGroupKey]models.RulesGroup{ + groupKey: append(rules, rules1...), + }, + New: nil, + Update: updates, + Delete: nil, + } + }, + permissions: func(c *store.GroupDelta) map[string][]string { + return map[string][]string{ + ruleRead: { + namespaceIdScope, + }, + dashboards.ActionFoldersRead: { + namespaceIdScope, + }, + ruleUpdate: { + namespaceIdScope, + }, + datasources.ActionQuery: getDatasourceScopesForRules(mapUpdates(c.Update, func(update store.RuleDelta) *models.AlertRule { + return update.New + })), + accesscontrol.ActionAlertingReceiversRead: getReceiverScopesForRules(mapUpdates(c.Update, func(update store.RuleDelta) *models.AlertRule { + return update.New + })), + } + }, + }, } for _, testCase := range testCases { @@ -325,9 +419,7 @@ func TestAuthorizeRuleChanges(t *testing.T) { permissionCombinations = permissionCombinations[0 : len(permissionCombinations)-1] // exclude all permissions for _, missing := range permissionCombinations { ac := &recordingAccessControlFake{} - srv := RuleService{ - genericService{ac: ac}, - } + srv := NewRuleService(ac) err := srv.AuthorizeRuleChanges(context.Background(), createUserWithPermissions(missing), groupChanges) assert.Errorf(t, err, "expected error because less permissions than expected were provided. Provided: %v; Expected: %v; Diff: %v", missing, permissions, cmp.Diff(permissions, missing)) @@ -335,19 +427,10 @@ func TestAuthorizeRuleChanges(t *testing.T) { } }) - ac := &recordingAccessControlFake{ - Callback: func(user identity.Requester, evaluator accesscontrol.Evaluator) (bool, error) { - response := evaluator.Evaluate(user.GetPermissions()) - require.Truef(t, response, "provided permissions [%v] is not enough for requested permissions [%s]", permissions, evaluator.GoString()) - return response, nil - }, - } - srv := RuleService{ - genericService{ac: ac}, - } + ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()) + srv := NewRuleService(ac) err := srv.AuthorizeRuleChanges(context.Background(), createUserWithPermissions(permissions), groupChanges) require.NoError(t, err) - require.NotEmptyf(t, ac.EvaluateRecordings, "evaluation function is expected to be called but it was not.") }) } } @@ -387,9 +470,7 @@ func TestCheckDatasourcePermissionsForRule(t *testing.T) { } ac := &recordingAccessControlFake{} - svc := RuleService{ - genericService{ac: ac}, - } + svc := NewRuleService(ac) eval := svc.AuthorizeDatasourceAccessForRule(context.Background(), createUserWithPermissions(permissions), rule) @@ -403,9 +484,7 @@ func TestCheckDatasourcePermissionsForRule(t *testing.T) { return false, nil }, } - svc := RuleService{ - genericService{ac: ac}, - } + svc := NewRuleService(ac) result := svc.AuthorizeDatasourceAccessForRule(context.Background(), createUserWithPermissions(nil), rule) @@ -426,9 +505,7 @@ func Test_authorizeAccessToRuleGroup(t *testing.T) { dashboards.ActionFoldersRead: namespaceScopes, } ac := &recordingAccessControlFake{} - svc := RuleService{ - genericService{ac: ac}, - } + svc := NewRuleService(ac) result := svc.AuthorizeAccessToRuleGroup(context.Background(), createUserWithPermissions(permissions), rules) @@ -443,9 +520,7 @@ func Test_authorizeAccessToRuleGroup(t *testing.T) { rules := genWithFolder.GenerateManyRef(1, 5) ac := &recordingAccessControlFake{} - svc := RuleService{ - genericService{ac: ac}, - } + svc := NewRuleService(ac) result := svc.AuthorizeAccessToRuleGroup(context.Background(), createUserWithPermissions(map[string][]string{}), rules) @@ -456,9 +531,7 @@ func Test_authorizeAccessToRuleGroup(t *testing.T) { func TestCanReadAllRules(t *testing.T) { ac := &recordingAccessControlFake{} - svc := RuleService{ - genericService{ac: ac}, - } + svc := NewRuleService(ac) testCases := []struct { permissions map[string][]string diff --git a/pkg/services/ngalert/models/notifications.go b/pkg/services/ngalert/models/notifications.go index 602d21acfa7..d4ed80813fc 100644 --- a/pkg/services/ngalert/models/notifications.go +++ b/pkg/services/ngalert/models/notifications.go @@ -35,6 +35,10 @@ type NotificationSettings struct { MuteTimeIntervals []string `json:"mute_time_intervals,omitempty"` } +func (s *NotificationSettings) GetUID() string { + return NameToUid(s.Receiver) +} + // NormalizedGroupBy returns a consistent and ordered GroupBy. // - If the GroupBy is empty, it returns nil so that the parent group can be inherited. // - If the GroupBy contains the special label '...', it returns only '...'. diff --git a/pkg/services/ngalert/models/receivers.go b/pkg/services/ngalert/models/receivers.go index 160c07b6b05..31ffe697e70 100644 --- a/pkg/services/ngalert/models/receivers.go +++ b/pkg/services/ngalert/models/receivers.go @@ -2,6 +2,7 @@ package models import ( "context" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -559,6 +560,10 @@ func (r *Receiver) GetUID() string { return r.UID } +func NameToUid(name string) string { + return base64.RawURLEncoding.EncodeToString([]byte(name)) +} + func (r *Receiver) Fingerprint() string { sum := newFingerprint() diff --git a/pkg/services/ngalert/notifier/legacy_storage/compat.go b/pkg/services/ngalert/notifier/legacy_storage/compat.go index 8cd0084471a..72252c270f8 100644 --- a/pkg/services/ngalert/notifier/legacy_storage/compat.go +++ b/pkg/services/ngalert/notifier/legacy_storage/compat.go @@ -11,9 +11,7 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/models" ) -func NameToUid(name string) string { - return base64.RawURLEncoding.EncodeToString([]byte(name)) -} +var NameToUid = models.NameToUid func UidToName(uid string) (string, error) { data, err := base64.RawURLEncoding.DecodeString(uid)