diff --git a/pkg/services/ngalert/accesscontrol/fakes/rules.go b/pkg/services/ngalert/accesscontrol/fakes/rules.go index d7ddcda79a7..9d6b5918921 100644 --- a/pkg/services/ngalert/accesscontrol/fakes/rules.go +++ b/pkg/services/ngalert/accesscontrol/fakes/rules.go @@ -3,8 +3,9 @@ package fakes import ( "context" - "github.com/grafana/grafana/pkg/services/accesscontrol" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/auth/identity" + "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/store" ) @@ -15,18 +16,19 @@ type Call struct { } type FakeRuleService struct { - HasAccessFunc func(context.Context, identity.Requester, accesscontrol.Evaluator) (bool, error) - HasAccessOrErrorFunc func(context.Context, identity.Requester, accesscontrol.Evaluator, func() string) error + HasAccessFunc func(context.Context, identity.Requester, ac.Evaluator) (bool, error) + HasAccessOrErrorFunc func(context.Context, identity.Requester, ac.Evaluator, func() string) error AuthorizeDatasourceAccessForRuleFunc func(context.Context, identity.Requester, *models.AlertRule) error AuthorizeDatasourceAccessForRuleGroupFunc func(context.Context, identity.Requester, models.RulesGroup) error HasAccessToRuleGroupFunc func(context.Context, identity.Requester, models.RulesGroup) (bool, error) AuthorizeAccessToRuleGroupFunc func(context.Context, identity.Requester, models.RulesGroup) error + AuthorizeAccessInFolderFunc func(context.Context, identity.Requester, accesscontrol.Namespaced) error AuthorizeRuleChangesFunc func(context.Context, identity.Requester, *store.GroupDelta) error Calls []Call } -func (s *FakeRuleService) HasAccess(ctx context.Context, user identity.Requester, evaluator accesscontrol.Evaluator) (bool, error) { +func (s *FakeRuleService) HasAccess(ctx context.Context, user identity.Requester, evaluator ac.Evaluator) (bool, error) { s.Calls = append(s.Calls, Call{"HasAccess", []interface{}{ctx, user, evaluator}}) if s.HasAccessFunc != nil { return s.HasAccessFunc(ctx, user, evaluator) @@ -34,7 +36,7 @@ func (s *FakeRuleService) HasAccess(ctx context.Context, user identity.Requester return false, nil } -func (s *FakeRuleService) HasAccessOrError(ctx context.Context, user identity.Requester, evaluator accesscontrol.Evaluator, action func() string) error { +func (s *FakeRuleService) HasAccessOrError(ctx context.Context, user identity.Requester, evaluator ac.Evaluator, action func() string) error { s.Calls = append(s.Calls, Call{"HasAccessOrError", []interface{}{ctx, user, evaluator, action}}) if s.HasAccessOrErrorFunc != nil { return s.HasAccessOrErrorFunc(ctx, user, evaluator, action) @@ -74,6 +76,14 @@ func (s *FakeRuleService) AuthorizeAccessToRuleGroup(ctx context.Context, user i return nil } +func (s *FakeRuleService) AuthorizeAccessInFolder(ctx context.Context, user identity.Requester, namespaced accesscontrol.Namespaced) error { + s.Calls = append(s.Calls, Call{"AuthorizeAccessInFolder", []interface{}{ctx, user, namespaced}}) + if s.AuthorizeAccessInFolderFunc != nil { + return s.AuthorizeAccessInFolderFunc(ctx, user, namespaced) + } + return nil +} + func (s *FakeRuleService) AuthorizeRuleChanges(ctx context.Context, user identity.Requester, change *store.GroupDelta) error { s.Calls = append(s.Calls, Call{"AuthorizeRuleGroupWrite", []interface{}{ctx, user, change}}) if s.AuthorizeRuleChangesFunc != nil { diff --git a/pkg/services/ngalert/accesscontrol/rules.go b/pkg/services/ngalert/accesscontrol/rules.go index 8bb0769b1ef..35b2246e091 100644 --- a/pkg/services/ngalert/accesscontrol/rules.go +++ b/pkg/services/ngalert/accesscontrol/rules.go @@ -31,6 +31,10 @@ func NewRuleService(ac accesscontrol.AccessControl) *RuleService { } } +type Namespaced interface { + GetNamespaceUID() string +} + // getReadFolderAccessEvaluator constructs accesscontrol.Evaluator that checks all permissions required to read rules in specific folder func getReadFolderAccessEvaluator(folderUID string) accesscontrol.Evaluator { return accesscontrol.EvalAll( @@ -96,7 +100,7 @@ func (r *RuleService) AuthorizeDatasourceAccessForRuleGroup(ctx context.Context, }) } -// AuthorizeAccessToRuleGroup checks that the identity.Requester has permissions to all rules, which means that it has permissions to: +// HasAccessToRuleGroup 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 // Returns false if the requester does not have enough permissions, and error if something went wrong during the permission evaluation. @@ -108,7 +112,7 @@ func (r *RuleService) HasAccessToRuleGroup(ctx context.Context, user identity.Re // 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 -// Returns error if at least one permissions is missing or if something went wrong during the permission evaluation +// Returns error if at least one permission 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 { @@ -121,6 +125,28 @@ func (r *RuleService) AuthorizeAccessToRuleGroup(ctx context.Context, user ident }) } +// HasAccessInFolder checks that the identity.Requester has permissions to read alert rules in the given folder, +// which requires the following permissions: +// - ("folders:read") read the folder +// - ("alert.rules:read") read alert rules in the folder +// Returns false if the requester does not have enough permissions, and error if something went wrong during the permission evaluation. +func (r *RuleService) HasAccessInFolder(ctx context.Context, user identity.Requester, rule Namespaced) (bool, error) { + eval := accesscontrol.EvalAll(getReadFolderAccessEvaluator(rule.GetNamespaceUID())) + return r.HasAccess(ctx, user, eval) +} + +// AuthorizeAccessInFolder checks that the identity.Requester has permissions to read alert rules in the given folder, +// which requires the following permissions: +// - ("folders:read") read the folder +// - ("alert.rules:read") read alert rules in the folder +// Returns error if at least one permission is missing or if something went wrong during the permission evaluation +func (r *RuleService) AuthorizeAccessInFolder(ctx context.Context, user identity.Requester, rule Namespaced) error { + eval := accesscontrol.EvalAll(getReadFolderAccessEvaluator(rule.GetNamespaceUID())) + return r.HasAccessOrError(ctx, user, eval, func() string { + return fmt.Sprintf("access rules in folder '%s'", rule.GetNamespaceUID()) + }) +} + // AuthorizeRuleChanges analyzes changes in the rule group, and checks whether the changes are authorized. // NOTE: if there are rules for deletion, and the user does not have access to data sources that a rule uses, the rule is removed from the list. // If the user is not authorized to perform the changes the function returns ErrAuthorization with a description of what action is not authorized. diff --git a/pkg/services/ngalert/api/api.go b/pkg/services/ngalert/api/api.go index 16268df7e1a..9ab3a02da64 100644 --- a/pkg/services/ngalert/api/api.go +++ b/pkg/services/ngalert/api/api.go @@ -45,6 +45,7 @@ type RuleAccessControlService interface { AuthorizeRuleChanges(ctx context.Context, user identity.Requester, change *store.GroupDelta) error AuthorizeDatasourceAccessForRule(ctx context.Context, user identity.Requester, rule *models.AlertRule) error AuthorizeDatasourceAccessForRuleGroup(ctx context.Context, user identity.Requester, rules models.RulesGroup) error + AuthorizeAccessInFolder(ctx context.Context, user identity.Requester, namespaced accesscontrol.Namespaced) error } // API handlers. diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index 5a0142be735..94dece2a4ae 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -639,28 +639,22 @@ func shouldValidate(delta store.RuleDelta) bool { return false } -// getAuthorizedRuleByUid fetches all rules in group to which the specified rule belongs, and checks whether the user is authorized to access the group. -// A user is authorized to access a group of rules only when it has permission to query all data sources used by all rules in this group. +// getAuthorizedRuleByUid fetches the rule by uid and checks whether the user is authorized to read it. // Returns rule identified by provided UID or ErrAuthorization if user is not authorized to access the rule. func (srv RulerSrv) getAuthorizedRuleByUid(ctx context.Context, c *contextmodel.ReqContext, ruleUID string) (ngmodels.AlertRule, error) { - q := ngmodels.GetAlertRulesGroupByRuleUIDQuery{ + q := ngmodels.GetAlertRuleByUIDQuery{ UID: ruleUID, OrgID: c.SignedInUser.GetOrgID(), } var err error - rules, err := srv.store.GetAlertRulesGroupByRuleUID(ctx, &q) + rule, err := srv.store.GetAlertRuleByUID(ctx, &q) if err != nil { return ngmodels.AlertRule{}, err } - if err := srv.authz.AuthorizeAccessToRuleGroup(ctx, c.SignedInUser, rules); err != nil { + if err := srv.authz.AuthorizeAccessInFolder(ctx, c.SignedInUser, rule); err != nil { return ngmodels.AlertRule{}, err } - for _, rule := range rules { - if rule.UID == ruleUID { - return *rule, nil - } - } - return ngmodels.AlertRule{}, ngmodels.ErrAlertRuleNotFound + return *rule, nil } // getAuthorizedRuleGroup fetches rules that belong to the specified models.AlertRuleGroupKey and validate user's authorization. diff --git a/pkg/services/ngalert/api/persist.go b/pkg/services/ngalert/api/persist.go index 698d78e3703..8b0894fd6a3 100644 --- a/pkg/services/ngalert/api/persist.go +++ b/pkg/services/ngalert/api/persist.go @@ -16,6 +16,7 @@ type RuleStore interface { 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) + GetAlertRuleByUID(ctx context.Context, query *ngmodels.GetAlertRuleByUIDQuery) (*ngmodels.AlertRule, error) GetAlertRulesGroupByRuleUID(ctx context.Context, query *ngmodels.GetAlertRulesGroupByRuleUIDQuery) ([]*ngmodels.AlertRule, error) ListAlertRules(ctx context.Context, query *ngmodels.ListAlertRulesQuery) (ngmodels.RulesGroup, error) diff --git a/pkg/services/ngalert/api/testing.go b/pkg/services/ngalert/api/testing.go index fd55598cac3..d1098754257 100644 --- a/pkg/services/ngalert/api/testing.go +++ b/pkg/services/ngalert/api/testing.go @@ -9,8 +9,9 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/data" - "github.com/grafana/grafana/pkg/services/accesscontrol" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/auth/identity" + "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" "github.com/grafana/grafana/pkg/services/ngalert/eval" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/state" @@ -107,16 +108,16 @@ type recordingAccessControlFake struct { Disabled bool EvaluateRecordings []struct { User *user.SignedInUser - Evaluator accesscontrol.Evaluator + Evaluator ac.Evaluator } - Callback func(user *user.SignedInUser, evaluator accesscontrol.Evaluator) (bool, error) + Callback func(user *user.SignedInUser, evaluator ac.Evaluator) (bool, error) } -func (a *recordingAccessControlFake) Evaluate(ctx context.Context, ur identity.Requester, evaluator accesscontrol.Evaluator) (bool, error) { +func (a *recordingAccessControlFake) Evaluate(ctx context.Context, ur identity.Requester, evaluator ac.Evaluator) (bool, error) { u := ur.(*user.SignedInUser) a.EvaluateRecordings = append(a.EvaluateRecordings, struct { User *user.SignedInUser - Evaluator accesscontrol.Evaluator + Evaluator ac.Evaluator }{User: u, Evaluator: evaluator}) if a.Callback == nil { return false, nil @@ -124,7 +125,7 @@ func (a *recordingAccessControlFake) Evaluate(ctx context.Context, ur identity.R return a.Callback(u, evaluator) } -func (a *recordingAccessControlFake) RegisterScopeAttributeResolver(prefix string, resolver accesscontrol.ScopeAttributeResolver) { +func (a *recordingAccessControlFake) RegisterScopeAttributeResolver(prefix string, resolver ac.ScopeAttributeResolver) { // TODO implement me panic("implement me") } @@ -133,7 +134,7 @@ func (a *recordingAccessControlFake) IsDisabled() bool { return a.Disabled } -var _ accesscontrol.AccessControl = &recordingAccessControlFake{} +var _ ac.AccessControl = &recordingAccessControlFake{} type fakeRuleAccessControlService struct { } @@ -146,6 +147,10 @@ func (f fakeRuleAccessControlService) AuthorizeAccessToRuleGroup(ctx context.Con return nil } +func (f fakeRuleAccessControlService) AuthorizeAccessInFolder(ctx context.Context, user identity.Requester, namespaced accesscontrol.Namespaced) error { + return nil +} + func (f fakeRuleAccessControlService) AuthorizeRuleChanges(ctx context.Context, user identity.Requester, change *store.GroupDelta) error { return nil } diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index 89254bb7ac8..a8a9a4a8c1e 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -14,9 +14,10 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - alertingModels "github.com/grafana/alerting/models" "github.com/grafana/grafana-plugin-sdk-go/data" + alertingModels "github.com/grafana/alerting/models" + "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util/cmputil" @@ -297,6 +298,10 @@ func (s AlertRulesSorter) Len() int { return len(s.rules) } func (s AlertRulesSorter) Swap(i, j int) { s.rules[i], s.rules[j] = s.rules[j], s.rules[i] } func (s AlertRulesSorter) Less(i, j int) bool { return s.by(s.rules[i], s.rules[j]) } +func (alertRule *AlertRule) GetNamespaceUID() string { + return alertRule.NamespaceUID +} + // GetDashboardUID returns the DashboardUID or "". func (alertRule *AlertRule) GetDashboardUID() string { if alertRule.DashboardUID != nil { diff --git a/pkg/services/ngalert/provisioning/accesscontrol.go b/pkg/services/ngalert/provisioning/accesscontrol.go index 33305fec79a..fa0eb499e61 100644 --- a/pkg/services/ngalert/provisioning/accesscontrol.go +++ b/pkg/services/ngalert/provisioning/accesscontrol.go @@ -5,6 +5,7 @@ import ( ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/auth/identity" + "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/store" ) @@ -12,6 +13,7 @@ import ( type RuleAccessControlService interface { HasAccess(ctx context.Context, user identity.Requester, evaluator ac.Evaluator) (bool, error) AuthorizeAccessToRuleGroup(ctx context.Context, user identity.Requester, rules models.RulesGroup) error + AuthorizeAccessInFolder(ctx context.Context, user identity.Requester, namespaced accesscontrol.Namespaced) error AuthorizeRuleChanges(ctx context.Context, user identity.Requester, change *store.GroupDelta) error } @@ -27,6 +29,21 @@ type provisioningRuleAccessControl struct { var _ ruleAccessControlService = &provisioningRuleAccessControl{} +// AuthorizeRuleRead authorizes the read access to a rule for a user. +// It first checks if the user has permission to read all rules. If yes, it bypasses the authorization. +// If not, it calls the RuleAccessControlService to authorize access to the rule. +// It returns an error if the authorization fails or if there is an error during permission check. +func (p *provisioningRuleAccessControl) AuthorizeRuleRead(ctx context.Context, user identity.Requester, rule *models.AlertRule) error { + can, err := p.CanReadAllRules(ctx, user) + if err != nil { + return err + } + if !can { + return p.RuleAccessControlService.AuthorizeAccessInFolder(ctx, user, rule) + } + return nil +} + // AuthorizeRuleGroupRead authorizes the read access to a group of rules for a user. // It first checks if the user has permission to read all rules. If yes, it bypasses the authorization. // If not, it calls the RuleAccessControlService to authorize access to the rule group. diff --git a/pkg/services/ngalert/provisioning/accesscontrol_test.go b/pkg/services/ngalert/provisioning/accesscontrol_test.go index c5f678e65a3..fc712290c6c 100644 --- a/pkg/services/ngalert/provisioning/accesscontrol_test.go +++ b/pkg/services/ngalert/provisioning/accesscontrol_test.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/auth/identity" + accesscontrol2 "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol/fakes" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/store" @@ -163,6 +164,82 @@ func TestAuthorizeAccessToRuleGroup(t *testing.T) { }) } +func TestAuthorizeAccessToRule(t *testing.T) { + testUser := &user.SignedInUser{} + rule := models.RuleGen.Generate() + + t.Run("should return nil when user has provisioning permissions", func(t *testing.T) { + rs := &fakes.FakeRuleService{} + provisioner := provisioningRuleAccessControl{ + RuleAccessControlService: rs, + } + + rs.HasAccessFunc = func(ctx context.Context, user identity.Requester, evaluator accesscontrol.Evaluator) (bool, error) { + return true, nil + } + + err := provisioner.AuthorizeRuleRead(context.Background(), testUser, &rule) + require.NoError(t, err) + + require.Len(t, rs.Calls, 1) + require.Equal(t, "HasAccess", rs.Calls[0].MethodName) + assert.Equal(t, accesscontrol.EvalAny( + accesscontrol.EvalPermission(accesscontrol.ActionAlertingProvisioningRead), + accesscontrol.EvalPermission(accesscontrol.ActionAlertingProvisioningReadSecrets), + accesscontrol.EvalPermission(accesscontrol.ActionAlertingRulesProvisioningRead), + ).GoString(), rs.Calls[0].Arguments[2].(accesscontrol.Evaluator).GoString()) + assert.Equal(t, testUser, rs.Calls[0].Arguments[1]) + }) + + t.Run("should call upstream method if no provisioning permissions", func(t *testing.T) { + rs := &fakes.FakeRuleService{} + provisioner := provisioningRuleAccessControl{ + RuleAccessControlService: rs, + } + + rs.HasAccessFunc = func(ctx context.Context, user identity.Requester, evaluator accesscontrol.Evaluator) (bool, error) { + return false, nil + } + rs.AuthorizeAccessInFolderFunc = func(ctx context.Context, requester identity.Requester, namespaced accesscontrol2.Namespaced) error { + return nil + } + + err := provisioner.AuthorizeRuleRead(context.Background(), testUser, &rule) + require.NoError(t, err) + + require.Len(t, rs.Calls, 2) + require.Equal(t, "HasAccess", rs.Calls[0].MethodName) + require.Equal(t, "AuthorizeAccessInFolder", rs.Calls[1].MethodName) + require.Equal(t, &rule, rs.Calls[1].Arguments[2]) + }) + + t.Run("should propagate error", func(t *testing.T) { + rs := &fakes.FakeRuleService{} + provisioner := provisioningRuleAccessControl{ + RuleAccessControlService: rs, + } + + expected := errors.New("test1") + rs.HasAccessFunc = func(ctx context.Context, user identity.Requester, evaluator accesscontrol.Evaluator) (bool, error) { + return false, expected + } + + err := provisioner.AuthorizeRuleRead(context.Background(), testUser, &rule) + require.ErrorIs(t, err, expected) + + rs.HasAccessFunc = func(ctx context.Context, user identity.Requester, evaluator accesscontrol.Evaluator) (bool, error) { + return false, nil + } + expected = errors.New("test2") + rs.AuthorizeAccessInFolderFunc = func(ctx context.Context, requester identity.Requester, rule accesscontrol2.Namespaced) error { + return expected + } + + err = provisioner.AuthorizeRuleRead(context.Background(), testUser, &rule) + require.ErrorIs(t, err, expected) + }) +} + func TestAuthorizeRuleChanges(t *testing.T) { testUser := &user.SignedInUser{} change := &store.GroupDelta{} diff --git a/pkg/services/ngalert/provisioning/alert_rules.go b/pkg/services/ngalert/provisioning/alert_rules.go index df227f061eb..b8ac2c0a026 100644 --- a/pkg/services/ngalert/provisioning/alert_rules.go +++ b/pkg/services/ngalert/provisioning/alert_rules.go @@ -21,6 +21,7 @@ import ( type ruleAccessControlService interface { AuthorizeRuleGroupRead(ctx context.Context, user identity.Requester, rules models.RulesGroup) error AuthorizeRuleGroupWrite(ctx context.Context, user identity.Requester, change *store.GroupDelta) error + AuthorizeRuleRead(ctx context.Context, user identity.Requester, rule *models.AlertRule) error // CanReadAllRules returns true if the user has full access to read rules via provisioning API and bypass regular checks CanReadAllRules(ctx context.Context, user identity.Requester) (bool, error) // CanWriteAllRules returns true if the user has full access to write rules via provisioning API and bypass regular checks @@ -119,49 +120,19 @@ func (service *AlertRuleService) GetAlertRules(ctx context.Context, user identit } func (service *AlertRuleService) getAlertRuleAuthorized(ctx context.Context, user identity.Requester, ruleUID string) (models.AlertRule, error) { - // check if the user can read all rules. If it cannot, pull the entire group and verify access to the entire group. - can, err := service.authz.CanReadAllRules(ctx, user) - if err != nil { - return models.AlertRule{}, err - } - // if user has blanket access to all rules, just read a single rule from database - if can { - query := &models.GetAlertRuleByUIDQuery{ - OrgID: user.GetOrgID(), - UID: ruleUID, - } - rule, err := service.ruleStore.GetAlertRuleByUID(ctx, query) - if err != nil { - return models.AlertRule{}, err - } - if rule == nil { - return models.AlertRule{}, models.ErrAlertRuleNotFound - } - return *rule, nil - } - - // if user does not have privilege to access all rules, check that the user can read this rule by fetching entire group and - // checking that user has access to it. - q := &models.GetAlertRulesGroupByRuleUIDQuery{ + q := models.GetAlertRuleByUIDQuery{ UID: ruleUID, OrgID: user.GetOrgID(), } - group, err := service.ruleStore.GetAlertRulesGroupByRuleUID(ctx, q) + var err error + rule, err := service.ruleStore.GetAlertRuleByUID(ctx, &q) if err != nil { return models.AlertRule{}, err } - if len(group) == 0 { - return models.AlertRule{}, models.ErrAlertRuleNotFound - } - if err := service.authz.AuthorizeRuleGroupRead(ctx, user, group); err != nil { + if err := service.authz.AuthorizeRuleRead(ctx, user, rule); err != nil { return models.AlertRule{}, err } - for _, rule := range group { - if rule.UID == ruleUID { - return *rule, nil - } - } - return models.AlertRule{}, models.ErrAlertRuleNotFound + return *rule, nil } func (service *AlertRuleService) GetAlertRule(ctx context.Context, user identity.Requester, ruleUID string) (models.AlertRule, models.Provenance, error) { diff --git a/pkg/services/ngalert/provisioning/alert_rules_test.go b/pkg/services/ngalert/provisioning/alert_rules_test.go index d035f5b78bf..9ae0fbcaaae 100644 --- a/pkg/services/ngalert/provisioning/alert_rules_test.go +++ b/pkg/services/ngalert/provisioning/alert_rules_test.go @@ -1008,112 +1008,47 @@ func TestGetAlertRule(t *testing.T) { return service, ruleStore, provenanceStore, ac } - t.Run("when user cannot read all rules", func(t *testing.T) { - t.Run("should authorize access to entire group", func(t *testing.T) { - service, _, _, ac := initServiceWithData(t) + t.Run("should authorize access to rule", func(t *testing.T) { + service, _, _, ac := initServiceWithData(t) - ac.CanReadAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) { - return false, nil - } - - expected := errors.New("test") - ac.AuthorizeAccessToRuleGroupFunc = func(ctx context.Context, user identity.Requester, r models.RulesGroup) error { - assert.Equal(t, u, user) - assert.EqualValues(t, rules, r) - return expected - } - - _, _, err := service.GetAlertRule(context.Background(), u, rule.UID) - require.Error(t, err) - require.Equal(t, expected, err) - - assert.Len(t, ac.Calls, 2) - assert.Equal(t, "CanReadAllRules", ac.Calls[0].Method) - assert.Equal(t, "AuthorizeRuleGroupRead", ac.Calls[1].Method) - - ac.Calls = nil - ac.AuthorizeAccessToRuleGroupFunc = func(ctx context.Context, user identity.Requester, rules models.RulesGroup) error { - return nil - } - - actual, provenance, err := service.GetAlertRule(context.Background(), u, rule.UID) - require.NoError(t, err) - assert.Equal(t, *rule, actual) - assert.Equal(t, expectedProvenance, provenance) - }) - - t.Run("should return ErrAlertRuleNotFound if rule does not exist", func(t *testing.T) { - service, ruleStore, _, ac := initServiceWithData(t) - ac.CanReadAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) { - return false, nil - } - - _, _, err := service.GetAlertRule(context.Background(), u, "no-rule-uid") - require.ErrorIs(t, err, models.ErrAlertRuleNotFound) - - assert.Len(t, ac.Calls, 1) - assert.Equal(t, "CanReadAllRules", ac.Calls[0].Method) - require.IsType(t, ruleStore.RecordedOps[0], models.GetAlertRulesGroupByRuleUIDQuery{}) - query := ruleStore.RecordedOps[0].(models.GetAlertRulesGroupByRuleUIDQuery) - assert.Equal(t, models.GetAlertRulesGroupByRuleUIDQuery{ - OrgID: orgID, - UID: "no-rule-uid", - }, query) - }) - }) - - t.Run("when user can read all rules", func(t *testing.T) { - t.Run("should query rule by UID and do not check any permissions", func(t *testing.T) { - service, ruleStore, _, ac := initServiceWithData(t) - ac.CanReadAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) { - assert.Equal(t, u, user) - return true, nil - } - - actual, provenance, err := service.GetAlertRule(context.Background(), u, rule.UID) - require.NoError(t, err) - assert.Equal(t, *rule, actual) - assert.Equal(t, expectedProvenance, provenance) - - assert.Len(t, ac.Calls, 1) - assert.Equal(t, "CanReadAllRules", ac.Calls[0].Method) - - require.Len(t, ruleStore.RecordedOps, 1) - require.IsType(t, ruleStore.RecordedOps[0], models.GetAlertRuleByUIDQuery{}) - query := ruleStore.RecordedOps[0].(models.GetAlertRuleByUIDQuery) - assert.Equal(t, models.GetAlertRuleByUIDQuery{ - OrgID: rule.OrgID, - UID: rule.UID, - }, query) - }) - - t.Run("should return ErrAlertRuleNotFound if rule does not exist", func(t *testing.T) { - service, _, _, ac := initServiceWithData(t) - ac.CanReadAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) { - return true, nil - } - - _, _, err := service.GetAlertRule(context.Background(), u, "no-rule-uid") - require.ErrorIs(t, err, models.ErrAlertRuleNotFound) - }) - }) - - t.Run("return error immediately when CanReadAllRules returns error", func(t *testing.T) { - service, ruleStore, _, ac := initServiceWithData(t) - - expectedErr := errors.New("test") - ac.CanReadAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) { - return false, expectedErr + expected := errors.New("test") + ac.AuthorizeAccessInFolderFunc = func(ctx context.Context, user identity.Requester, namespaced accesscontrol.Namespaced) error { + assert.Equal(t, u, user) + assert.EqualValues(t, rule, namespaced) + return expected } _, _, err := service.GetAlertRule(context.Background(), u, rule.UID) require.Error(t, err) - require.Equal(t, expectedErr, err) + require.Equal(t, expected, err) assert.Len(t, ac.Calls, 1) - assert.Equal(t, "CanReadAllRules", ac.Calls[0].Method) + assert.Equal(t, "AuthorizeRuleRead", ac.Calls[0].Method) - assert.Empty(t, ruleStore.RecordedOps) + ac.Calls = nil + ac.AuthorizeAccessInFolderFunc = func(ctx context.Context, user identity.Requester, namespaced accesscontrol.Namespaced) error { + return nil + } + + actual, provenance, err := service.GetAlertRule(context.Background(), u, rule.UID) + require.NoError(t, err) + assert.Equal(t, *rule, actual) + assert.Equal(t, expectedProvenance, provenance) + }) + + t.Run("should return ErrAlertRuleNotFound if rule does not exist", func(t *testing.T) { + service, ruleStore, _, ac := initServiceWithData(t) + + _, _, err := service.GetAlertRule(context.Background(), u, "no-rule-uid") + require.ErrorIs(t, err, models.ErrAlertRuleNotFound) + + assert.Len(t, ac.Calls, 0) + require.IsType(t, ruleStore.RecordedOps[0], models.GetAlertRuleByUIDQuery{}) + query := ruleStore.RecordedOps[0].(models.GetAlertRuleByUIDQuery) + assert.Equal(t, models.GetAlertRuleByUIDQuery{ + OrgID: orgID, + UID: "no-rule-uid", + }, query) }) } diff --git a/pkg/services/ngalert/provisioning/testing.go b/pkg/services/ngalert/provisioning/testing.go index 62e6128319e..3b669c0256b 100644 --- a/pkg/services/ngalert/provisioning/testing.go +++ b/pkg/services/ngalert/provisioning/testing.go @@ -9,6 +9,7 @@ import ( mock "github.com/stretchr/testify/mock" "github.com/grafana/grafana/pkg/services/auth/identity" + "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/notifier" "github.com/grafana/grafana/pkg/services/ngalert/store" @@ -160,6 +161,7 @@ type fakeRuleAccessControlService struct { mu sync.Mutex Calls []call AuthorizeAccessToRuleGroupFunc func(ctx context.Context, user identity.Requester, rules models.RulesGroup) error + AuthorizeAccessInFolderFunc func(ctx context.Context, user identity.Requester, namespaced accesscontrol.Namespaced) error AuthorizeRuleChangesFunc func(ctx context.Context, user identity.Requester, change *store.GroupDelta) error CanReadAllRulesFunc func(ctx context.Context, user identity.Requester) (bool, error) CanWriteAllRulesFunc func(ctx context.Context, user identity.Requester) (bool, error) @@ -185,6 +187,14 @@ func (s *fakeRuleAccessControlService) AuthorizeRuleGroupRead(ctx context.Contex return nil } +func (s *fakeRuleAccessControlService) AuthorizeRuleRead(ctx context.Context, user identity.Requester, rule *models.AlertRule) error { + s.RecordCall("AuthorizeRuleRead", ctx, user, rule) + if s.AuthorizeAccessInFolderFunc != nil { + return s.AuthorizeAccessInFolderFunc(ctx, user, rule) + } + return nil +} + func (s *fakeRuleAccessControlService) AuthorizeRuleGroupWrite(ctx context.Context, user identity.Requester, change *store.GroupDelta) error { s.RecordCall("AuthorizeRuleGroupWrite", ctx, user, change) if s.AuthorizeRuleChangesFunc != nil { diff --git a/pkg/services/ngalert/tests/fakes/rules.go b/pkg/services/ngalert/tests/fakes/rules.go index 1f91c52fcd9..3bc41ab7758 100644 --- a/pkg/services/ngalert/tests/fakes/rules.go +++ b/pkg/services/ngalert/tests/fakes/rules.go @@ -140,7 +140,7 @@ func (f *RuleStore) GetAlertRuleByUID(_ context.Context, q *models.GetAlertRuleB return rule, nil } } - return nil, nil + return nil, models.ErrAlertRuleNotFound } func (f *RuleStore) GetAlertRulesGroupByRuleUID(_ context.Context, q *models.GetAlertRulesGroupByRuleUIDQuery) ([]*models.AlertRule, error) {