From 7cec741baee6f038b09f6aea768fc3e526bbc8da Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Wed, 15 Nov 2023 11:54:54 -0500 Subject: [PATCH] Alerting: Extract alerting rules authorization logic to a service (#77006) * extract alerting authorization logic to separate package * convert authorization logic to service --- pkg/services/ngalert/accesscontrol/models.go | 9 + pkg/services/ngalert/accesscontrol/rules.go | 152 ++++++ .../ngalert/accesscontrol/rules_test.go | 458 ++++++++++++++++++ pkg/services/ngalert/accesscontrol/testing.go | 39 ++ pkg/services/ngalert/api/api.go | 18 +- pkg/services/ngalert/api/api_prometheus.go | 8 +- .../ngalert/api/api_prometheus_test.go | 9 +- pkg/services/ngalert/api/api_ruler.go | 29 +- pkg/services/ngalert/api/api_ruler_export.go | 3 +- pkg/services/ngalert/api/api_ruler_test.go | 3 +- pkg/services/ngalert/api/api_testing.go | 22 +- pkg/services/ngalert/api/api_testing_test.go | 21 +- pkg/services/ngalert/api/authorization.go | 118 ----- .../ngalert/api/authorization_test.go | 433 ----------------- pkg/services/ngalert/api/errors.go | 3 +- pkg/services/ngalert/api/testing.go | 13 + pkg/services/ngalert/models/testing.go | 8 + 17 files changed, 734 insertions(+), 612 deletions(-) create mode 100644 pkg/services/ngalert/accesscontrol/models.go create mode 100644 pkg/services/ngalert/accesscontrol/rules.go create mode 100644 pkg/services/ngalert/accesscontrol/rules_test.go create mode 100644 pkg/services/ngalert/accesscontrol/testing.go diff --git a/pkg/services/ngalert/accesscontrol/models.go b/pkg/services/ngalert/accesscontrol/models.go new file mode 100644 index 00000000000..1f0403a18f7 --- /dev/null +++ b/pkg/services/ngalert/accesscontrol/models.go @@ -0,0 +1,9 @@ +package accesscontrol + +import ( + "errors" +) + +var ( + ErrAuthorization = errors.New("user is not authorized") +) diff --git a/pkg/services/ngalert/accesscontrol/rules.go b/pkg/services/ngalert/accesscontrol/rules.go new file mode 100644 index 00000000000..e6c40df8d98 --- /dev/null +++ b/pkg/services/ngalert/accesscontrol/rules.go @@ -0,0 +1,152 @@ +package accesscontrol + +import ( + "fmt" + + "golang.org/x/net/context" + + "github.com/grafana/grafana/pkg/expr" + "github.com/grafana/grafana/pkg/infra/log" + "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/datasources" + "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/services/ngalert/store" +) + +const ( + ruleCreate = accesscontrol.ActionAlertingRuleCreate + ruleRead = accesscontrol.ActionAlertingRuleRead + ruleUpdate = accesscontrol.ActionAlertingRuleUpdate + ruleDelete = accesscontrol.ActionAlertingRuleDelete +) + +var logger = log.New("ngalert.accesscontrol") + +type RuleService struct { + ac accesscontrol.AccessControl +} + +func NewRuleService(ac accesscontrol.AccessControl) *RuleService { + return &RuleService{ + ac: ac, + } +} + +// HasAccess returns true if the user has all permissions specified by the evaluator +func (r *RuleService) HasAccess(ctx context.Context, user identity.Requester, evaluator accesscontrol.Evaluator) bool { + result, err := r.ac.Evaluate(ctx, user, evaluator) + if err != nil { // this is how accesscontrol.HasAccess works. //TODO change when AuthorizeDatasourceAccessForRule can return errors + logger.FromContext(ctx).Error("Failed to evaluate access control", "error", err) + return false + } + return result +} + +// AuthorizeDatasourceAccessForRule checks that user has access to all data sources declared by the rule +func (r *RuleService) AuthorizeDatasourceAccessForRule(ctx context.Context, user identity.Requester, rule *models.AlertRule) bool { + for _, query := range rule.Data { + if query.QueryType == expr.DatasourceType || query.DatasourceUID == expr.DatasourceUID || query. + DatasourceUID == expr.OldDatasourceUID { + continue + } + if !r.HasAccess(ctx, user, accesscontrol.EvalPermission(datasources.ActionQuery, datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID))) { + return false + } + } + return true +} + +// AuthorizeAccessToRuleGroup checks all rules against AuthorizeDatasourceAccessForRule and exits on the first negative result +func (r *RuleService) AuthorizeAccessToRuleGroup(ctx context.Context, user identity.Requester, rules models.RulesGroup) bool { + for _, rule := range rules { + if !r.AuthorizeDatasourceAccessForRule(ctx, user, rule) { + return false + } + } + return true +} + +// 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. +// Return changes that the user is authorized to perform or ErrAuthorization +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] + if ok { // not ok can be when user creates a new rule group or moves existing alerts to a new group + if !r.AuthorizeAccessToRuleGroup(ctx, user, rules) { // if user is not authorized to do operation in the group that is being changed + return fmt.Errorf("%w to change group %s because it does not have access to one or many rules in this group", ErrAuthorization, change.GroupKey.RuleGroup) + } + } else if len(change.Delete) > 0 { + // 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 changes in rule group %s. Detected %d deletes but group was not provided", change.GroupKey.RuleGroup, len(change.Delete)) + } + + if len(change.Delete) > 0 { + allowed := r.HasAccess(ctx, user, accesscontrol.EvalPermission(ruleDelete, namespaceScope)) + if !allowed { + return fmt.Errorf("%w to delete alert rules that belong to folder %s", ErrAuthorization, change.GroupKey.NamespaceUID) + } + for _, rule := range change.Delete { + if !r.AuthorizeDatasourceAccessForRule(ctx, user, rule) { + return fmt.Errorf("%w to delete an alert rule '%s' because the user does not have read permissions for one or many datasources the rule uses", ErrAuthorization, rule.UID) + } + } + } + + var addAuthorized, updateAuthorized bool + + if len(change.New) > 0 { + addAuthorized = r.HasAccess(ctx, user, accesscontrol.EvalPermission(ruleCreate, namespaceScope)) + if !addAuthorized { + return fmt.Errorf("%w to create alert rules in the folder %s", ErrAuthorization, change.GroupKey.NamespaceUID) + } + for _, rule := range change.New { + if !r.AuthorizeDatasourceAccessForRule(ctx, user, rule) { + return fmt.Errorf("%w to create a new alert rule '%s' because the user does not have read permissions for one or many datasources the rule uses", ErrAuthorization, rule.Title) + } + } + } + + for _, rule := range change.Update { + if !r.AuthorizeDatasourceAccessForRule(ctx, user, rule.New) { + return fmt.Errorf("%w to update alert rule '%s' (UID: %s) because the user does not have read permissions for one or many datasources the rule uses", ErrAuthorization, rule.Existing.Title, rule.Existing.UID) + } + + // Check if the rule is moved from one folder to the current. If yes, then the user must have the authorization to delete rules from the source folder and add rules to the target folder. + if rule.Existing.NamespaceUID != rule.New.NamespaceUID { + allowed := r.HasAccess(ctx, user, accesscontrol.EvalPermission(ruleDelete, dashboards.ScopeFoldersProvider.GetResourceScopeUID(rule.Existing.NamespaceUID))) + if !allowed { + return fmt.Errorf("%w to delete alert rules from folder UID %s", ErrAuthorization, rule.Existing.NamespaceUID) + } + + if !addAuthorized { + addAuthorized = r.HasAccess(ctx, user, accesscontrol.EvalPermission(ruleCreate, namespaceScope)) + if !addAuthorized { + return fmt.Errorf("%w to create alert rules in the folder '%s'", ErrAuthorization, change.GroupKey.NamespaceUID) + } + } + } else if !updateAuthorized { // if it is false then the authorization was not checked. If it is true then the user is authorized to update rules + updateAuthorized = r.HasAccess(ctx, user, accesscontrol.EvalPermission(ruleUpdate, namespaceScope)) + if !updateAuthorized { + return fmt.Errorf("%w to update alert rules that belong to folder %s", ErrAuthorization, change.GroupKey.NamespaceUID) + } + } + + if rule.Existing.NamespaceUID != rule.New.NamespaceUID || rule.Existing.RuleGroup != rule.New.RuleGroup { + key := rule.Existing.GetGroupKey() + rules, ok = change.AffectedGroups[key] + if !ok { + // 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) + } + if !r.AuthorizeAccessToRuleGroup(ctx, user, rules) { + return fmt.Errorf("%w to move rule %s between two different groups because user does not have access to the source group %s", ErrAuthorization, rule.Existing.UID, rule.Existing.RuleGroup) + } + } + } + return nil +} diff --git a/pkg/services/ngalert/accesscontrol/rules_test.go b/pkg/services/ngalert/accesscontrol/rules_test.go new file mode 100644 index 00000000000..c05628bc41d --- /dev/null +++ b/pkg/services/ngalert/accesscontrol/rules_test.go @@ -0,0 +1,458 @@ +package accesscontrol + +import ( + "context" + "math" + "math/rand" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/expr" + "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/datasources" + "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/services/ngalert/store" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/util" +) + +func createAllCombinationsOfPermissions(permissions map[string][]string) []map[string][]string { + type actionscope struct { + action string + scope string + } + + var flattenPermissions []actionscope + for action, scopes := range permissions { + for _, scope := range scopes { + flattenPermissions = append(flattenPermissions, actionscope{ + action, + scope, + }) + } + } + + l := len(flattenPermissions) + // this is all possible combinations of the permissions + var permissionCombinations []map[string][]string + for bit := uint(0); bit < uint(math.Pow(2, float64(l))); bit++ { + var tuple []actionscope + for idx := 0; idx < l; idx++ { + if (bit>>idx)&1 == 1 { + tuple = append(tuple, flattenPermissions[idx]) + } + } + + combination := make(map[string][]string) + for _, perm := range tuple { + combination[perm.action] = append(combination[perm.action], perm.scope) + } + + permissionCombinations = append(permissionCombinations, combination) + } + return permissionCombinations +} + +func getDatasourceScopesForRules(rules models.RulesGroup) []string { + scopesMap := map[string]struct{}{} + var result []string + for _, rule := range rules { + for _, query := range rule.Data { + scope := datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID) + 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 { + result = append(result, mapFunc(update)) + } + return result +} + +func createUserWithPermissions(permissions map[string][]string) identity.Requester { + return &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{ + 1: permissions, + }} +} + +func TestAuthorizeRuleChanges(t *testing.T) { + groupKey := models.GenerateGroupKey(rand.Int63()) + namespaceIdScope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(groupKey.NamespaceUID) + + testCases := []struct { + name string + changes func() *store.GroupDelta + permissions func(c *store.GroupDelta) map[string][]string + }{ + { + name: "if there are rules to add it should check create action and query for datasource", + changes: func() *store.GroupDelta { + return &store.GroupDelta{ + GroupKey: groupKey, + New: models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(models.WithGroupKey(groupKey))), + Update: nil, + Delete: nil, + } + }, + 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, + }, + datasources.ActionQuery: scopes, + } + }, + }, + { + name: "if there are rules to delete it should check delete action and query for datasource", + changes: func() *store.GroupDelta { + rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(models.WithGroupKey(groupKey))) + rules2 := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(models.WithGroupKey(groupKey))) + return &store.GroupDelta{ + GroupKey: groupKey, + AffectedGroups: map[models.AlertRuleGroupKey]models.RulesGroup{ + groupKey: append(rules, rules2...), + }, + New: nil, + Update: nil, + Delete: rules2, + } + }, + permissions: func(c *store.GroupDelta) map[string][]string { + return map[string][]string{ + ruleDelete: { + namespaceIdScope, + }, + datasources.ActionQuery: getDatasourceScopesForRules(c.AffectedGroups[c.GroupKey]), + } + }, + }, + { + name: "if there are rules to update within the same namespace it should check update action and access to datasource", + changes: func() *store.GroupDelta { + rules1 := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(models.WithGroupKey(groupKey))) + rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(models.WithGroupKey(groupKey))) + updates := make([]store.RuleDelta, 0, len(rules)) + + for _, rule := range rules { + cp := models.CopyRule(rule) + cp.Data = []models.AlertQuery{models.GenerateAlertQuery()} + 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 { + scopes := getDatasourceScopesForRules(append(c.AffectedGroups[c.GroupKey], mapUpdates(c.Update, func(update store.RuleDelta) *models.AlertRule { + return update.New + })...)) + return map[string][]string{ + ruleUpdate: { + namespaceIdScope, + }, + datasources.ActionQuery: scopes, + } + }, + }, + { + name: "if there are rules that are moved between namespaces it should check delete+add action and access to group where rules come from", + changes: func() *store.GroupDelta { + rules1 := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(models.WithGroupKey(groupKey))) + rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(models.WithGroupKey(groupKey))) + + targetGroupKey := models.GenerateGroupKey(groupKey.OrgID) + + updates := make([]store.RuleDelta, 0, len(rules)) + for _, rule := range rules { + cp := models.CopyRule(rule) + models.WithGroupKey(targetGroupKey)(cp) + cp.Data = []models.AlertQuery{ + models.GenerateAlertQuery(), + } + + updates = append(updates, store.RuleDelta{ + Existing: rule, + New: cp, + }) + } + + return &store.GroupDelta{ + GroupKey: targetGroupKey, + AffectedGroups: map[models.AlertRuleGroupKey]models.RulesGroup{ + groupKey: append(rules, rules1...), + }, + New: nil, + Update: updates, + Delete: nil, + } + }, + permissions: func(c *store.GroupDelta) map[string][]string { + dsScopes := getDatasourceScopesForRules( + append(append(append(c.AffectedGroups[c.GroupKey], + mapUpdates(c.Update, func(update store.RuleDelta) *models.AlertRule { + return update.New + })..., + ), mapUpdates(c.Update, func(update store.RuleDelta) *models.AlertRule { + return update.Existing + })...), c.AffectedGroups[groupKey]...), + ) + + var deleteScopes []string + for key := range c.AffectedGroups { + deleteScopes = append(deleteScopes, dashboards.ScopeFoldersProvider.GetResourceScopeUID(key.NamespaceUID)) + } + + return map[string][]string{ + ruleDelete: deleteScopes, + ruleCreate: { + dashboards.ScopeFoldersProvider.GetResourceScopeUID(c.GroupKey.NamespaceUID), + }, + datasources.ActionQuery: dsScopes, + } + }, + }, + { + name: "if there are rules that are moved between groups in the same namespace it should check update action and access to all groups (source+target)", + changes: func() *store.GroupDelta { + targetGroupKey := models.AlertRuleGroupKey{ + OrgID: groupKey.OrgID, + NamespaceUID: groupKey.NamespaceUID, + RuleGroup: util.GenerateShortUID(), + } + sourceGroup := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(models.WithGroupKey(groupKey))) + targetGroup := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(models.WithGroupKey(targetGroupKey))) + + updates := make([]store.RuleDelta, 0, len(sourceGroup)) + toCopy := len(sourceGroup) + if toCopy > 1 { + toCopy = rand.Intn(toCopy-1) + 1 + } + for i := 0; i < toCopy; i++ { + rule := sourceGroup[0] + cp := models.CopyRule(rule) + models.WithGroupKey(targetGroupKey)(cp) + cp.Data = []models.AlertQuery{ + models.GenerateAlertQuery(), + } + + updates = append(updates, store.RuleDelta{ + Existing: rule, + New: cp, + }) + } + + return &store.GroupDelta{ + GroupKey: targetGroupKey, + AffectedGroups: map[models.AlertRuleGroupKey]models.RulesGroup{ + groupKey: sourceGroup, + targetGroupKey: targetGroup, + }, + New: nil, + Update: updates, + Delete: nil, + } + }, + permissions: func(c *store.GroupDelta) map[string][]string { + scopes := make(map[string]struct{}) + for _, update := range c.Update { + for _, query := range update.New.Data { + scopes[datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID)] = struct{}{} + } + for _, query := range update.Existing.Data { + scopes[datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID)] = struct{}{} + } + } + for _, rules := range c.AffectedGroups { + for _, rule := range rules { + for _, query := range rule.Data { + scopes[datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID)] = struct{}{} + } + } + } + + dsScopes := make([]string, 0, len(scopes)) + for key := range scopes { + dsScopes = append(dsScopes, key) + } + + return map[string][]string{ + ruleUpdate: { + dashboards.ScopeFoldersProvider.GetResourceScopeUID(c.GroupKey.NamespaceUID), + }, + datasources.ActionQuery: dsScopes, + } + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + groupChanges := testCase.changes() + permissions := testCase.permissions(groupChanges) + + t.Run("should fail with insufficient permissions", func(t *testing.T) { + permissionCombinations := createAllCombinationsOfPermissions(permissions) + permissionCombinations = permissionCombinations[0 : len(permissionCombinations)-1] // exclude all permissions + for _, missing := range permissionCombinations { + ac := &recordingAccessControlFake{} + srv := RuleService{ + ac: ac, + } + err := srv.AuthorizeRuleChanges(context.Background(), createUserWithPermissions(missing), groupChanges) + require.Errorf(t, err, "expected error because less permissions than expected were provided. Provided: %v; Expected: %v", missing, permissions) + require.ErrorIs(t, err, ErrAuthorization) + require.NotEmptyf(t, ac.EvaluateRecordings, "Access control was supposed to be called but it was not") + } + }) + + 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{ + ac: 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.") + }) + } +} + +func TestCheckDatasourcePermissionsForRule(t *testing.T) { + rule := models.AlertRuleGen()() + + expressionByType := models.GenerateAlertQuery() + expressionByType.QueryType = expr.DatasourceType + expressionByUID := models.GenerateAlertQuery() + expressionByUID.DatasourceUID = expr.DatasourceUID + + var data []models.AlertQuery + var scopes []string + expectedExecutions := rand.Intn(3) + 2 + for i := 0; i < expectedExecutions; i++ { + q := models.GenerateAlertQuery() + scopes = append(scopes, datasources.ScopeProvider.GetResourceScopeUID(q.DatasourceUID)) + data = append(data, q) + } + + data = append(data, expressionByType, expressionByUID) + rand.Shuffle(len(data), func(i, j int) { + data[j], data[i] = data[i], data[j] + }) + + rule.Data = data + + t.Run("should check only expressions", func(t *testing.T) { + permissions := map[string][]string{ + datasources.ActionQuery: scopes, + } + + ac := &recordingAccessControlFake{} + svc := RuleService{ + ac: ac, + } + + eval := svc.AuthorizeDatasourceAccessForRule(context.Background(), createUserWithPermissions(permissions), rule) + + require.True(t, eval) + require.Len(t, ac.EvaluateRecordings, expectedExecutions) + }) + + t.Run("should return on first negative evaluation", func(t *testing.T) { + ac := &recordingAccessControlFake{ + Callback: func(user identity.Requester, evaluator accesscontrol.Evaluator) (bool, error) { + return false, nil + }, + } + svc := RuleService{ + ac: ac, + } + + eval := svc.AuthorizeDatasourceAccessForRule(context.Background(), createUserWithPermissions(nil), rule) + + require.False(t, eval) + require.Len(t, ac.EvaluateRecordings, 1) + }) +} + +func Test_authorizeAccessToRuleGroup(t *testing.T) { + t.Run("should return true if user has access to all datasources of all rules in group", func(t *testing.T) { + rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen()) + var scopes []string + for _, rule := range rules { + for _, query := range rule.Data { + scopes = append(scopes, datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID)) + } + } + permissions := map[string][]string{ + datasources.ActionQuery: scopes, + } + ac := &recordingAccessControlFake{} + svc := RuleService{ + ac: ac, + } + + result := svc.AuthorizeAccessToRuleGroup(context.Background(), createUserWithPermissions(permissions), rules) + + require.True(t, result) + 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()) + var scopes []string + for _, rule := range rules { + for _, query := range rule.Data { + scopes = append(scopes, datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID)) + } + } + permissions := map[string][]string{ + datasources.ActionQuery: scopes, + } + + rule := models.AlertRuleGen()() + rules = append(rules, rule) + + ac := &recordingAccessControlFake{} + + svc := RuleService{ + ac: ac, + } + + result := svc.AuthorizeAccessToRuleGroup(context.Background(), createUserWithPermissions(permissions), rules) + + require.False(t, result) + }) +} diff --git a/pkg/services/ngalert/accesscontrol/testing.go b/pkg/services/ngalert/accesscontrol/testing.go new file mode 100644 index 00000000000..6e99264f7e1 --- /dev/null +++ b/pkg/services/ngalert/accesscontrol/testing.go @@ -0,0 +1,39 @@ +package accesscontrol + +import ( + "context" + + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/auth/identity" +) + +type recordingAccessControlFake struct { + Disabled bool + EvaluateRecordings []struct { + Permissions map[string][]string + Evaluator accesscontrol.Evaluator + } + Callback func(user identity.Requester, evaluator accesscontrol.Evaluator) (bool, error) +} + +func (a *recordingAccessControlFake) Evaluate(_ context.Context, ur identity.Requester, evaluator accesscontrol.Evaluator) (bool, error) { + a.EvaluateRecordings = append(a.EvaluateRecordings, struct { + Permissions map[string][]string + Evaluator accesscontrol.Evaluator + }{Permissions: ur.GetPermissions(), Evaluator: evaluator}) + if a.Callback == nil { + return evaluator.Evaluate(ur.GetPermissions()), nil + } + return a.Callback(ur, evaluator) +} + +func (a *recordingAccessControlFake) RegisterScopeAttributeResolver(prefix string, resolver accesscontrol.ScopeAttributeResolver) { + // TODO implement me + panic("implement me") +} + +func (a *recordingAccessControlFake) IsDisabled() bool { + return a.Disabled +} + +var _ accesscontrol.AccessControl = &recordingAccessControlFake{} diff --git a/pkg/services/ngalert/api/api.go b/pkg/services/ngalert/api/api.go index a64825f2704..400d667674c 100644 --- a/pkg/services/ngalert/api/api.go +++ b/pkg/services/ngalert/api/api.go @@ -8,10 +8,12 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" - "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/datasourceproxy" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" "github.com/grafana/grafana/pkg/services/ngalert/backtesting" "github.com/grafana/grafana/pkg/services/ngalert/eval" "github.com/grafana/grafana/pkg/services/ngalert/metrics" @@ -37,6 +39,11 @@ type AlertingStore interface { GetLatestAlertmanagerConfiguration(ctx context.Context, query *models.GetLatestAlertmanagerConfigurationQuery) (*models.AlertConfiguration, error) } +type RuleAccessControlService interface { + AuthorizeAccessToRuleGroup(ctx context.Context, user identity.Requester, rules models.RulesGroup) bool + AuthorizeRuleChanges(ctx context.Context, user identity.Requester, change *store.GroupDelta) error +} + // API handlers. type API struct { Cfg *setting.Cfg @@ -52,7 +59,7 @@ type API struct { DataProxy *datasourceproxy.DataSourceProxyService MultiOrgAlertmanager *notifier.MultiOrgAlertmanager StateManager *state.Manager - AccessControl accesscontrol.AccessControl + AccessControl ac.AccessControl Policies *provisioning.NotificationPolicyService ContactPointService *provisioning.ContactPointService Templates *provisioning.TemplateService @@ -76,6 +83,7 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) { DataProxy: api.DataProxy, ac: api.AccessControl, } + ruleAuthzService := accesscontrol.NewRuleService(api.AccessControl) // Register endpoints for proxying to Alertmanager-compatible backends. api.RegisterAlertmanagerApiEndpoints(NewForkingAM( @@ -87,7 +95,7 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) { api.RegisterPrometheusApiEndpoints(NewForkingProm( api.DatasourceCache, NewLotexProm(proxy, logger), - &PrometheusSrv{log: logger, manager: api.StateManager, store: api.RuleStore, ac: api.AccessControl}, + &PrometheusSrv{log: logger, manager: api.StateManager, store: api.RuleStore, authz: ruleAuthzService}, ), m) // Register endpoints for proxying to Cortex Ruler-compatible backends. api.RegisterRulerApiEndpoints(NewForkingRuler( @@ -101,7 +109,7 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) { xactManager: api.TransactionManager, log: logger, cfg: &api.Cfg.UnifiedAlerting, - ac: api.AccessControl, + authz: ruleAuthzService, }, ), m) api.RegisterTestingApiEndpoints(NewTestingApi( @@ -109,7 +117,7 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) { AlertingProxy: proxy, DatasourceCache: api.DatasourceCache, log: logger, - accessControl: api.AccessControl, + authz: ruleAuthzService, evaluator: api.EvaluatorFactory, cfg: &api.Cfg.UnifiedAlerting, backtesting: backtesting.NewEngine(api.AppUrl, api.EvaluatorFactory, api.Tracer), diff --git a/pkg/services/ngalert/api/api_prometheus.go b/pkg/services/ngalert/api/api_prometheus.go index c407650c5c6..51c7f3630fd 100644 --- a/pkg/services/ngalert/api/api_prometheus.go +++ b/pkg/services/ngalert/api/api_prometheus.go @@ -15,7 +15,6 @@ import ( "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/accesscontrol" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/folder" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" @@ -29,7 +28,7 @@ type PrometheusSrv struct { log log.Logger manager state.AlertInstanceManager store RuleStore - ac accesscontrol.AccessControl + authz RuleAccessControlService } const queryIncludeInternalLabels = "includeInternalLabels" @@ -213,9 +212,6 @@ func (srv PrometheusSrv) RouteGetRuleStatuses(c *contextmodel.ReqContext) respon ruleResponse.DiscoveryBase.ErrorType = apiv1.ErrServer return response.JSON(http.StatusInternalServerError, ruleResponse) } - hasAccess := func(evaluator accesscontrol.Evaluator) bool { - return accesscontrol.HasAccess(srv.ac, c)(evaluator) - } // Group rules together by Namespace and Rule Group. Rules are also grouped by Org ID, // but in this API all rules belong to the same organization. @@ -239,7 +235,7 @@ func (srv PrometheusSrv) RouteGetRuleStatuses(c *contextmodel.ReqContext) respon srv.log.Warn("Query returned rules that belong to folder the user does not have access to. All rules that belong to that namespace will not be added to the response", "folder_uid", groupKey.NamespaceUID) continue } - if !authorizeAccessToRuleGroup(rules, hasAccess) { + if !srv.authz.AuthorizeAccessToRuleGroup(c.Req.Context(), c.SignedInUser, rules) { continue } ruleGroup, totals := srv.toRuleGroup(groupKey, folder, rules, limitAlertsPerRule, withStatesFast, matchers, labelOptions) diff --git a/pkg/services/ngalert/api/api_prometheus_test.go b/pkg/services/ngalert/api/api_prometheus_test.go index 5a34b2f0b69..e08152d7441 100644 --- a/pkg/services/ngalert/api/api_prometheus_test.go +++ b/pkg/services/ngalert/api/api_prometheus_test.go @@ -15,13 +15,14 @@ import ( alertingModels "github.com/grafana/alerting/models" "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" - acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/folder" + "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/eval" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -502,7 +503,7 @@ func TestRouteGetRuleStatuses(t *testing.T) { log: log.NewNopLogger(), manager: fakeAIM, store: ruleStore, - ac: acmock.New(), + authz: &fakeRuleAccessControlService{}, } response := api.RouteGetRuleStatuses(c) @@ -546,7 +547,7 @@ func TestRouteGetRuleStatuses(t *testing.T) { log: log.NewNopLogger(), manager: fakeAIM, store: ruleStore, - ac: acimpl.ProvideAccessControl(setting.NewCfg()), + authz: &fakeRuleAccessControlService{}, } c := &contextmodel.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &user.SignedInUser{OrgID: orgID, Permissions: createPermissionsForRules(rules, orgID)}} @@ -1259,7 +1260,7 @@ func setupAPI(t *testing.T) (*fakes.RuleStore, *fakeAlertInstanceManager, Promet log: log.NewNopLogger(), manager: fakeAIM, store: fakeStore, - ac: acimpl.ProvideAccessControl(setting.NewCfg()), + authz: accesscontrol.NewRuleService(acimpl.ProvideAccessControl(setting.NewCfg())), } return fakeStore, fakeAIM, api diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index 04466c3e7e5..21727f4fe4d 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -13,10 +13,10 @@ import ( "github.com/grafana/grafana/pkg/api/apierrors" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/auth/identity" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/eval" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -39,8 +39,8 @@ type RulerSrv struct { QuotaService quota.Service log log.Logger cfg *setting.UnifiedAlertingSettings - ac accesscontrol.AccessControl conditionValidator ConditionValidator + authz RuleAccessControlService } var ( @@ -96,7 +96,7 @@ func (srv RulerSrv) RouteDeleteAlertRules(c *contextmodel.ReqContext, namespaceT return err } if totalGroups > 0 && len(deletionCandidates) == 0 { - return fmt.Errorf("%w to delete any existing rules in the namespace", ErrAuthorization) + return fmt.Errorf("%w to delete any existing rules in the namespace", accesscontrol.ErrAuthorization) } } rulesToDelete := make([]string, 0) @@ -131,7 +131,7 @@ func (srv RulerSrv) RouteDeleteAlertRules(c *contextmodel.ReqContext, namespaceT }) if err != nil { - if errors.Is(err, ErrAuthorization) { + if errors.Is(err, accesscontrol.ErrAuthorization) { return ErrResp(http.StatusUnauthorized, err, "failed to delete rule group") } if errors.Is(err, errProvisionedResource) { @@ -268,7 +268,6 @@ func (srv RulerSrv) RoutePostNameRulesConfig(c *contextmodel.ReqContext, ruleGro // All operations are performed in a single transaction func (srv RulerSrv) updateAlertRulesInGroup(c *contextmodel.ReqContext, groupKey ngmodels.AlertRuleGroupKey, rules []*ngmodels.AlertRuleWithOptionals) response.Response { var finalChanges *store.GroupDelta - hasAccess := accesscontrol.HasAccess(srv.ac, c) err := srv.xactManager.InTransaction(c.Req.Context(), func(tranCtx context.Context) error { userNamespace, id := c.SignedInUser.GetNamespacedID() logger := srv.log.New("namespace_uid", groupKey.NamespaceUID, "group", @@ -284,9 +283,7 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *contextmodel.ReqContext, groupKey return nil } - err = authorizeRuleChanges(groupChanges, func(evaluator accesscontrol.Evaluator) bool { - return hasAccess(evaluator) - }) + err = srv.authz.AuthorizeRuleChanges(c.Req.Context(), c.SignedInUser, groupChanges) if err != nil { return err } @@ -371,7 +368,7 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *contextmodel.ReqContext, groupKey return ErrResp(http.StatusBadRequest, err, "failed to update rule group") } else if errors.Is(err, ngmodels.ErrQuotaReached) { return ErrResp(http.StatusForbidden, err, "") - } else if errors.Is(err, ErrAuthorization) { + } else if errors.Is(err, accesscontrol.ErrAuthorization) { return ErrResp(http.StatusUnauthorized, err, "") } else if errors.Is(err, store.ErrOptimisticLock) { return ErrResp(http.StatusConflict, err, "") @@ -512,7 +509,6 @@ func validateQueries(ctx context.Context, groupChanges *store.GroupDelta, valida // 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. // 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) { - hasAccess := accesscontrol.HasAccess(srv.ac, c) q := ngmodels.GetAlertRulesGroupByRuleUIDQuery{ UID: ruleUID, OrgID: c.SignedInUser.GetOrgID(), @@ -522,8 +518,8 @@ func (srv RulerSrv) getAuthorizedRuleByUid(ctx context.Context, c *contextmodel. if err != nil { return ngmodels.AlertRule{}, err } - if !authorizeAccessToRuleGroup(rules, hasAccess) { - return ngmodels.AlertRule{}, fmt.Errorf("%w to access rules in this group", ErrAuthorization) + if !srv.authz.AuthorizeAccessToRuleGroup(ctx, c.SignedInUser, rules) { + return ngmodels.AlertRule{}, fmt.Errorf("%w to access rules in this group", accesscontrol.ErrAuthorization) } for _, rule := range rules { if rule.UID == ruleUID { @@ -537,8 +533,6 @@ func (srv RulerSrv) getAuthorizedRuleByUid(ctx context.Context, c *contextmodel. // 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. // Returns models.RuleGroup if authorization passed or ErrAuthorization if user is not authorized to access the rule. func (srv RulerSrv) getAuthorizedRuleGroup(ctx context.Context, c *contextmodel.ReqContext, ruleGroupKey ngmodels.AlertRuleGroupKey) (ngmodels.RulesGroup, error) { - hasAccess := accesscontrol.HasAccess(srv.ac, c) - q := ngmodels.ListAlertRulesQuery{ OrgID: ruleGroupKey.OrgID, NamespaceUIDs: []string{ruleGroupKey.NamespaceUID}, @@ -548,8 +542,8 @@ func (srv RulerSrv) getAuthorizedRuleGroup(ctx context.Context, c *contextmodel. if err != nil { return nil, err } - if !authorizeAccessToRuleGroup(rules, hasAccess) { - return nil, fmt.Errorf("%w to access rules in this group", ErrAuthorization) + if !srv.authz.AuthorizeAccessToRuleGroup(ctx, c.SignedInUser, rules) { + return nil, fmt.Errorf("%w to access rules in this group", accesscontrol.ErrAuthorization) } return rules, nil } @@ -558,7 +552,6 @@ func (srv RulerSrv) getAuthorizedRuleGroup(ctx context.Context, c *contextmodel. // 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. // Returns groups that user is authorized to access, and total count of groups returned by query func (srv RulerSrv) searchAuthorizedAlertRules(ctx context.Context, c *contextmodel.ReqContext, folderUIDs []string, dashboardUID string, panelID int64) (map[ngmodels.AlertRuleGroupKey]ngmodels.RulesGroup, int, error) { - hasAccess := accesscontrol.HasAccess(srv.ac, c) query := ngmodels.ListAlertRulesQuery{ OrgID: c.SignedInUser.GetOrgID(), NamespaceUIDs: folderUIDs, @@ -573,7 +566,7 @@ func (srv RulerSrv) searchAuthorizedAlertRules(ctx context.Context, c *contextmo byGroupKey := ngmodels.GroupByAlertRuleGroupKey(rules) totalGroups := len(byGroupKey) for groupKey, rulesGroup := range byGroupKey { - if !authorizeAccessToRuleGroup(rulesGroup, hasAccess) { + if !srv.authz.AuthorizeAccessToRuleGroup(ctx, c.SignedInUser, rulesGroup) { delete(byGroupKey, groupKey) } } diff --git a/pkg/services/ngalert/api/api_ruler_export.go b/pkg/services/ngalert/api/api_ruler_export.go index abe3a29ea04..ad895e7233d 100644 --- a/pkg/services/ngalert/api/api_ruler_export.go +++ b/pkg/services/ngalert/api/api_ruler_export.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana/pkg/api/response" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" + "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" ) @@ -147,7 +148,7 @@ func (srv RulerSrv) getRulesWithFolderTitleInFolders(c *contextmodel.ReqContext, } } if len(query.NamespaceUIDs) == 0 { - return nil, fmt.Errorf("%w access rules in the specified folders", ErrAuthorization) + return nil, fmt.Errorf("%w access rules in the specified folders", accesscontrol.ErrAuthorization) } } else { for _, folder := range folders { diff --git a/pkg/services/ngalert/api/api_ruler_test.go b/pkg/services/ngalert/api/api_ruler_test.go index 7299f4977ca..650a20cffe5 100644 --- a/pkg/services/ngalert/api/api_ruler_test.go +++ b/pkg/services/ngalert/api/api_ruler_test.go @@ -21,6 +21,7 @@ import ( contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/folder" + "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" apimodels "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/provisioning" @@ -602,7 +603,7 @@ func createService(store *fakes.RuleStore) *RulerSrv { cfg: &setting.UnifiedAlertingSettings{ BaseInterval: 10 * time.Second, }, - ac: acimpl.ProvideAccessControl(setting.NewCfg()), + authz: accesscontrol.NewRuleService(acimpl.ProvideAccessControl(setting.NewCfg())), } } diff --git a/pkg/services/ngalert/api/api_testing.go b/pkg/services/ngalert/api/api_testing.go index 9bb05852f0f..005bfba6c9e 100644 --- a/pkg/services/ngalert/api/api_testing.go +++ b/pkg/services/ngalert/api/api_testing.go @@ -17,11 +17,11 @@ import ( "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" - "github.com/grafana/grafana/pkg/services/accesscontrol" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "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/accesscontrol" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/backtesting" "github.com/grafana/grafana/pkg/services/ngalert/eval" @@ -35,7 +35,7 @@ type TestingApiSrv struct { *AlertingProxy DatasourceCache datasources.CacheService log log.Logger - accessControl accesscontrol.AccessControl + authz RuleAccessControlService evaluator eval.EvaluatorFactory cfg *setting.UnifiedAlertingSettings backtesting *backtesting.Engine @@ -64,10 +64,8 @@ func (srv TestingApiSrv) RouteTestGrafanaRuleConfig(c *contextmodel.ReqContext, return ErrResp(http.StatusBadRequest, err, "") } - if !authorizeDatasourceAccessForRule(rule, func(evaluator accesscontrol.Evaluator) bool { - return accesscontrol.HasAccess(srv.accessControl, c)(evaluator) - }) { - return errorToResponse(fmt.Errorf("%w to query one or many data sources used by the rule", ErrAuthorization)) + if !srv.authz.AuthorizeAccessToRuleGroup(c.Req.Context(), c.SignedInUser, ngmodels.RulesGroup{rule}) { + return errorToResponse(fmt.Errorf("%w to query one or many data sources used by the rule", accesscontrol.ErrAuthorization)) } evaluator, err := srv.evaluator.Create(eval.NewContext(c.Req.Context(), c.SignedInUser), rule.GetEvalCondition()) @@ -150,10 +148,8 @@ func (srv TestingApiSrv) RouteTestRuleConfig(c *contextmodel.ReqContext, body ap func (srv TestingApiSrv) RouteEvalQueries(c *contextmodel.ReqContext, cmd apimodels.EvalQueriesPayload) response.Response { queries := AlertQueriesFromApiAlertQueries(cmd.Data) - if !authorizeDatasourceAccessForRule(&ngmodels.AlertRule{Data: queries}, func(evaluator accesscontrol.Evaluator) bool { - return accesscontrol.HasAccess(srv.accessControl, c)(evaluator) - }) { - return ErrResp(http.StatusUnauthorized, fmt.Errorf("%w to query one or many data sources used by the rule", ErrAuthorization), "") + if !srv.authz.AuthorizeAccessToRuleGroup(c.Req.Context(), c.SignedInUser, ngmodels.RulesGroup{&ngmodels.AlertRule{Data: queries}}) { + return ErrResp(http.StatusUnauthorized, fmt.Errorf("%w to query one or many data sources used by the rule", accesscontrol.ErrAuthorization), "") } cond := ngmodels.Condition{ @@ -208,10 +204,8 @@ func (srv TestingApiSrv) BacktestAlertRule(c *contextmodel.ReqContext, cmd apimo } queries := AlertQueriesFromApiAlertQueries(cmd.Data) - if !authorizeDatasourceAccessForRule(&ngmodels.AlertRule{Data: queries}, func(evaluator accesscontrol.Evaluator) bool { - return accesscontrol.HasAccess(srv.accessControl, c)(evaluator) - }) { - return errorToResponse(fmt.Errorf("%w to query one or many data sources used by the rule", ErrAuthorization)) + if !srv.authz.AuthorizeAccessToRuleGroup(c.Req.Context(), c.SignedInUser, ngmodels.RulesGroup{&ngmodels.AlertRule{Data: queries}}) { + return errorToResponse(fmt.Errorf("%w to query one or many data sources used by the rule", accesscontrol.ErrAuthorization)) } rule := &ngmodels.AlertRule{ diff --git a/pkg/services/ngalert/api/api_testing_test.go b/pkg/services/ngalert/api/api_testing_test.go index f3e6924a2dc..4cfa5f45e8e 100644 --- a/pkg/services/ngalert/api/api_testing_test.go +++ b/pkg/services/ngalert/api/api_testing_test.go @@ -11,11 +11,12 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/tracing" - "github.com/grafana/grafana/pkg/services/accesscontrol" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" acMock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/datasources" fakes "github.com/grafana/grafana/pkg/services/datasources/fakes" + "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/eval" "github.com/grafana/grafana/pkg/services/ngalert/eval/eval_mocks" @@ -140,7 +141,7 @@ func TestRouteTestGrafanaRuleConfig(t *testing.T) { data1 := models.GenerateAlertQuery() data2 := models.GenerateAlertQuery() - ac := acMock.New().WithPermissions([]accesscontrol.Permission{ + ac := acMock.New().WithPermissions([]ac.Permission{ {Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(data1.DatasourceUID)}, }) @@ -162,7 +163,7 @@ func TestRouteTestGrafanaRuleConfig(t *testing.T) { data1 := models.GenerateAlertQuery() data2 := models.GenerateAlertQuery() - ac := acMock.New().WithPermissions([]accesscontrol.Permission{ + ac := acMock.New().WithPermissions([]ac.Permission{ {Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(data1.DatasourceUID)}, {Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(data2.DatasourceUID)}, }) @@ -211,13 +212,11 @@ func TestRouteEvalQueries(t *testing.T) { data1 := models.GenerateAlertQuery() data2 := models.GenerateAlertQuery() - ac := acMock.New().WithPermissions([]accesscontrol.Permission{ - {Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(data1.DatasourceUID)}, - }) - srv := &TestingApiSrv{ - accessControl: ac, - tracer: tracing.InitializeTracerForTest(), + authz: accesscontrol.NewRuleService(acMock.New().WithPermissions([]ac.Permission{ + {Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(data1.DatasourceUID)}, + })), + tracer: tracing.InitializeTracerForTest(), } response := srv.RouteEvalQueries(rc, definitions.EvalQueriesPayload{ @@ -234,7 +233,7 @@ func TestRouteEvalQueries(t *testing.T) { currentTime := time.Now() - ac := acMock.New().WithPermissions([]accesscontrol.Permission{ + ac := acMock.New().WithPermissions([]ac.Permission{ {Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(data1.DatasourceUID)}, {Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(data2.DatasourceUID)}, }) @@ -276,7 +275,7 @@ func createTestingApiSrv(t *testing.T, ds *fakes.FakeCacheService, ac *acMock.Mo return &TestingApiSrv{ DatasourceCache: ds, - accessControl: ac, + authz: accesscontrol.NewRuleService(ac), evaluator: evaluator, cfg: config(t), tracer: tracing.InitializeTracerForTest(), diff --git a/pkg/services/ngalert/api/authorization.go b/pkg/services/ngalert/api/authorization.go index cc7ca551937..5b5a44a151d 100644 --- a/pkg/services/ngalert/api/authorization.go +++ b/pkg/services/ngalert/api/authorization.go @@ -1,24 +1,16 @@ package api import ( - "errors" "fmt" "net/http" - "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/middleware" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/datasources" - ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" - "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/web" ) -var ( - ErrAuthorization = errors.New("user is not authorized") -) - //nolint:gocyclo func (api *API) authorize(method, path string) web.Handler { authorize := ac.Middleware(api.AccessControl) @@ -234,113 +226,3 @@ func (api *API) authorize(method, path string) web.Handler { panic(fmt.Sprintf("no authorization handler for method [%s] of endpoint [%s]", method, path)) } - -// authorizeDatasourceAccessForRule checks that user has access to all data sources declared by the rule -func authorizeDatasourceAccessForRule(rule *ngmodels.AlertRule, evaluator func(evaluator ac.Evaluator) bool) bool { - for _, query := range rule.Data { - if query.QueryType == expr.DatasourceType || query.DatasourceUID == expr.DatasourceUID || query. - DatasourceUID == expr. - OldDatasourceUID { - continue - } - if !evaluator(ac.EvalPermission(datasources.ActionQuery, datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID))) { - return false - } - } - return true -} - -// authorizeAccessToRuleGroup checks all rules against authorizeDatasourceAccessForRule and exits on the first negative result -func authorizeAccessToRuleGroup(rules []*ngmodels.AlertRule, evaluator func(evaluator ac.Evaluator) bool) bool { - for _, rule := range rules { - if !authorizeDatasourceAccessForRule(rule, evaluator) { - return false - } - } - return true -} - -// 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. -// Return changes that the user is authorized to perform or ErrAuthorization -func authorizeRuleChanges(change *store.GroupDelta, evaluator func(evaluator ac.Evaluator) bool) error { - namespaceScope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(change.GroupKey.NamespaceUID) - - rules, ok := 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 !authorizeAccessToRuleGroup(rules, evaluator) { // if user is not authorized to do operation in the group that is being changed - return fmt.Errorf("%w to change group %s because it does not have access to one or many rules in this group", ErrAuthorization, change.GroupKey.RuleGroup) - } - } else if len(change.Delete) > 0 { - // 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 changes in rule group %s. Detected %d deletes but group was not provided", change.GroupKey.RuleGroup, len(change.Delete)) - } - - if len(change.Delete) > 0 { - allowed := evaluator(ac.EvalPermission(ac.ActionAlertingRuleDelete, namespaceScope)) - if !allowed { - return fmt.Errorf("%w to delete alert rules that belong to folder %s", ErrAuthorization, change.GroupKey.NamespaceUID) - } - for _, rule := range change.Delete { - if !authorizeDatasourceAccessForRule(rule, evaluator) { - return fmt.Errorf("%w to delete an alert rule '%s' because the user does not have read permissions for one or many datasources the rule uses", ErrAuthorization, rule.UID) - } - } - } - - var addAuthorized, updateAuthorized bool - - if len(change.New) > 0 { - addAuthorized = evaluator(ac.EvalPermission(ac.ActionAlertingRuleCreate, namespaceScope)) - if !addAuthorized { - return fmt.Errorf("%w to create alert rules in the folder %s", ErrAuthorization, change.GroupKey.NamespaceUID) - } - for _, rule := range change.New { - dsAllowed := authorizeDatasourceAccessForRule(rule, evaluator) - if !dsAllowed { - return fmt.Errorf("%w to create a new alert rule '%s' because the user does not have read permissions for one or many datasources the rule uses", ErrAuthorization, rule.Title) - } - } - } - - for _, rule := range change.Update { - dsAllowed := authorizeDatasourceAccessForRule(rule.New, evaluator) - if !dsAllowed { - return fmt.Errorf("%w to update alert rule '%s' (UID: %s) because the user does not have read permissions for one or many datasources the rule uses", ErrAuthorization, rule.Existing.Title, rule.Existing.UID) - } - - // Check if the rule is moved from one folder to the current. If yes, then the user must have the authorization to delete rules from the source folder and add rules to the target folder. - if rule.Existing.NamespaceUID != rule.New.NamespaceUID { - allowed := evaluator(ac.EvalAll(ac.EvalPermission(ac.ActionAlertingRuleDelete, dashboards.ScopeFoldersProvider.GetResourceScopeUID(rule.Existing.NamespaceUID)))) - if !allowed { - return fmt.Errorf("%w to delete alert rules from folder UID %s", ErrAuthorization, rule.Existing.NamespaceUID) - } - - if !addAuthorized { - addAuthorized = evaluator(ac.EvalPermission(ac.ActionAlertingRuleCreate, namespaceScope)) - if !addAuthorized { - return fmt.Errorf("%w to create alert rules in the folder '%s'", ErrAuthorization, change.GroupKey.NamespaceUID) - } - } - } else if !updateAuthorized { // if it is false then the authorization was not checked. If it is true then the user is authorized to update rules - updateAuthorized = evaluator(ac.EvalPermission(ac.ActionAlertingRuleUpdate, namespaceScope)) - if !updateAuthorized { - return fmt.Errorf("%w to update alert rules that belong to folder %s", ErrAuthorization, change.GroupKey.NamespaceUID) - } - } - - if rule.Existing.NamespaceUID != rule.New.NamespaceUID || rule.Existing.RuleGroup != rule.New.RuleGroup { - key := rule.Existing.GetGroupKey() - rules, ok = change.AffectedGroups[key] - if !ok { - // 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) - } - if !authorizeAccessToRuleGroup(rules, evaluator) { - return fmt.Errorf("%w to move rule %s between two different groups because user does not have access to the source group %s", ErrAuthorization, rule.Existing.UID, rule.Existing.RuleGroup) - } - } - } - return nil -} diff --git a/pkg/services/ngalert/api/authorization_test.go b/pkg/services/ngalert/api/authorization_test.go index 01627fac49a..213be17fd70 100644 --- a/pkg/services/ngalert/api/authorization_test.go +++ b/pkg/services/ngalert/api/authorization_test.go @@ -1,8 +1,6 @@ package api import ( - "math" - "math/rand" "net/http" "os" "path/filepath" @@ -11,14 +9,7 @@ import ( "github.com/go-openapi/loads" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/expr" - ac "github.com/grafana/grafana/pkg/services/accesscontrol" acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" - "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/datasources" - "github.com/grafana/grafana/pkg/services/ngalert/models" - "github.com/grafana/grafana/pkg/services/ngalert/store" - "github.com/grafana/grafana/pkg/util" ) func TestAuthorize(t *testing.T) { @@ -70,427 +61,3 @@ func TestAuthorize(t *testing.T) { }) }) } - -func createAllCombinationsOfPermissions(permissions map[string][]string) []map[string][]string { - type actionscope struct { - action string - scope string - } - - var flattenPermissions []actionscope - for action, scopes := range permissions { - for _, scope := range scopes { - flattenPermissions = append(flattenPermissions, actionscope{ - action, - scope, - }) - } - } - - l := len(flattenPermissions) - // this is all possible combinations of the permissions - var permissionCombinations []map[string][]string - for bit := uint(0); bit < uint(math.Pow(2, float64(l))); bit++ { - var tuple []actionscope - for idx := 0; idx < l; idx++ { - if (bit>>idx)&1 == 1 { - tuple = append(tuple, flattenPermissions[idx]) - } - } - - combination := make(map[string][]string) - for _, perm := range tuple { - combination[perm.action] = append(combination[perm.action], perm.scope) - } - - permissionCombinations = append(permissionCombinations, combination) - } - return permissionCombinations -} - -func getDatasourceScopesForRules(rules models.RulesGroup) []string { - scopesMap := map[string]struct{}{} - var result []string - for _, rule := range rules { - for _, query := range rule.Data { - scope := datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID) - 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 { - result = append(result, mapFunc(update)) - } - return result -} - -func TestAuthorizeRuleChanges(t *testing.T) { - groupKey := models.GenerateGroupKey(rand.Int63()) - namespaceIdScope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(groupKey.NamespaceUID) - - testCases := []struct { - name string - changes func() *store.GroupDelta - permissions func(c *store.GroupDelta) map[string][]string - }{ - { - name: "if there are rules to add it should check create action and query for datasource", - changes: func() *store.GroupDelta { - return &store.GroupDelta{ - GroupKey: groupKey, - New: models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withGroupKey(groupKey))), - Update: nil, - Delete: nil, - } - }, - 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{ - ac.ActionAlertingRuleCreate: { - namespaceIdScope, - }, - datasources.ActionQuery: scopes, - } - }, - }, - { - name: "if there are rules to delete it should check delete action and query for datasource", - changes: func() *store.GroupDelta { - rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withGroupKey(groupKey))) - rules2 := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withGroupKey(groupKey))) - return &store.GroupDelta{ - GroupKey: groupKey, - AffectedGroups: map[models.AlertRuleGroupKey]models.RulesGroup{ - groupKey: append(rules, rules2...), - }, - New: nil, - Update: nil, - Delete: rules2, - } - }, - permissions: func(c *store.GroupDelta) map[string][]string { - return map[string][]string{ - ac.ActionAlertingRuleDelete: { - namespaceIdScope, - }, - datasources.ActionQuery: getDatasourceScopesForRules(c.AffectedGroups[c.GroupKey]), - } - }, - }, - { - name: "if there are rules to update within the same namespace it should check update action and access to datasource", - changes: func() *store.GroupDelta { - rules1 := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withGroupKey(groupKey))) - rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withGroupKey(groupKey))) - updates := make([]store.RuleDelta, 0, len(rules)) - - for _, rule := range rules { - cp := models.CopyRule(rule) - cp.Data = []models.AlertQuery{models.GenerateAlertQuery()} - 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 { - scopes := getDatasourceScopesForRules(append(c.AffectedGroups[c.GroupKey], mapUpdates(c.Update, func(update store.RuleDelta) *models.AlertRule { - return update.New - })...)) - return map[string][]string{ - ac.ActionAlertingRuleUpdate: { - namespaceIdScope, - }, - datasources.ActionQuery: scopes, - } - }, - }, - { - name: "if there are rules that are moved between namespaces it should check delete+add action and access to group where rules come from", - changes: func() *store.GroupDelta { - rules1 := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withGroupKey(groupKey))) - rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withGroupKey(groupKey))) - - targetGroupKey := models.GenerateGroupKey(groupKey.OrgID) - - updates := make([]store.RuleDelta, 0, len(rules)) - for _, rule := range rules { - cp := models.CopyRule(rule) - withGroupKey(targetGroupKey)(cp) - cp.Data = []models.AlertQuery{ - models.GenerateAlertQuery(), - } - - updates = append(updates, store.RuleDelta{ - Existing: rule, - New: cp, - }) - } - - return &store.GroupDelta{ - GroupKey: targetGroupKey, - AffectedGroups: map[models.AlertRuleGroupKey]models.RulesGroup{ - groupKey: append(rules, rules1...), - }, - New: nil, - Update: updates, - Delete: nil, - } - }, - permissions: func(c *store.GroupDelta) map[string][]string { - dsScopes := getDatasourceScopesForRules( - append(append(append(c.AffectedGroups[c.GroupKey], - mapUpdates(c.Update, func(update store.RuleDelta) *models.AlertRule { - return update.New - })..., - ), mapUpdates(c.Update, func(update store.RuleDelta) *models.AlertRule { - return update.Existing - })...), c.AffectedGroups[groupKey]...), - ) - - var deleteScopes []string - for key := range c.AffectedGroups { - deleteScopes = append(deleteScopes, dashboards.ScopeFoldersProvider.GetResourceScopeUID(key.NamespaceUID)) - } - - return map[string][]string{ - ac.ActionAlertingRuleDelete: deleteScopes, - ac.ActionAlertingRuleCreate: { - dashboards.ScopeFoldersProvider.GetResourceScopeUID(c.GroupKey.NamespaceUID), - }, - datasources.ActionQuery: dsScopes, - } - }, - }, - { - name: "if there are rules that are moved between groups in the same namespace it should check update action and access to all groups (source+target)", - changes: func() *store.GroupDelta { - targetGroupKey := models.AlertRuleGroupKey{ - OrgID: groupKey.OrgID, - NamespaceUID: groupKey.NamespaceUID, - RuleGroup: util.GenerateShortUID(), - } - sourceGroup := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withGroupKey(groupKey))) - targetGroup := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withGroupKey(targetGroupKey))) - - updates := make([]store.RuleDelta, 0, len(sourceGroup)) - toCopy := len(sourceGroup) - if toCopy > 1 { - toCopy = rand.Intn(toCopy-1) + 1 - } - for i := 0; i < toCopy; i++ { - rule := sourceGroup[0] - cp := models.CopyRule(rule) - withGroupKey(targetGroupKey)(cp) - cp.Data = []models.AlertQuery{ - models.GenerateAlertQuery(), - } - - updates = append(updates, store.RuleDelta{ - Existing: rule, - New: cp, - }) - } - - return &store.GroupDelta{ - GroupKey: targetGroupKey, - AffectedGroups: map[models.AlertRuleGroupKey]models.RulesGroup{ - groupKey: sourceGroup, - targetGroupKey: targetGroup, - }, - New: nil, - Update: updates, - Delete: nil, - } - }, - permissions: func(c *store.GroupDelta) map[string][]string { - scopes := make(map[string]struct{}) - for _, update := range c.Update { - for _, query := range update.New.Data { - scopes[datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID)] = struct{}{} - } - for _, query := range update.Existing.Data { - scopes[datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID)] = struct{}{} - } - } - for _, rules := range c.AffectedGroups { - for _, rule := range rules { - for _, query := range rule.Data { - scopes[datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID)] = struct{}{} - } - } - } - - dsScopes := make([]string, 0, len(scopes)) - for key := range scopes { - dsScopes = append(dsScopes, key) - } - - return map[string][]string{ - ac.ActionAlertingRuleUpdate: { - dashboards.ScopeFoldersProvider.GetResourceScopeUID(c.GroupKey.NamespaceUID), - }, - datasources.ActionQuery: dsScopes, - } - }, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - groupChanges := testCase.changes() - permissions := testCase.permissions(groupChanges) - - t.Run("should fail with insufficient permissions", func(t *testing.T) { - permissionCombinations := createAllCombinationsOfPermissions(permissions) - permissionCombinations = permissionCombinations[0 : len(permissionCombinations)-1] // exclude all permissions - for _, missing := range permissionCombinations { - executed := false - err := authorizeRuleChanges(groupChanges, func(evaluator ac.Evaluator) bool { - response := evaluator.Evaluate(missing) - executed = true - return response - }) - require.Errorf(t, err, "expected error because less permissions than expected were provided. Provided: %v; Expected: %v", missing, permissions) - require.ErrorIs(t, err, ErrAuthorization) - require.Truef(t, executed, "evaluation function is expected to be called but it was not.") - } - }) - - executed := false - - err := authorizeRuleChanges(groupChanges, func(evaluator ac.Evaluator) bool { - response := evaluator.Evaluate(permissions) - require.Truef(t, response, "provided permissions [%v] is not enough for requested permissions [%s]", permissions, evaluator.GoString()) - executed = true - return true - }) - require.NoError(t, err) - require.Truef(t, executed, "evaluation function is expected to be called but it was not.") - }) - } -} - -func TestCheckDatasourcePermissionsForRule(t *testing.T) { - rule := models.AlertRuleGen()() - - expressionByType := models.GenerateAlertQuery() - expressionByType.QueryType = expr.DatasourceType - expressionByUID := models.GenerateAlertQuery() - expressionByUID.DatasourceUID = expr.DatasourceUID - - var data []models.AlertQuery - var scopes []string - expectedExecutions := rand.Intn(3) + 2 - for i := 0; i < expectedExecutions; i++ { - q := models.GenerateAlertQuery() - scopes = append(scopes, datasources.ScopeProvider.GetResourceScopeUID(q.DatasourceUID)) - data = append(data, q) - } - - data = append(data, expressionByType, expressionByUID) - rand.Shuffle(len(data), func(i, j int) { - data[j], data[i] = data[i], data[j] - }) - - rule.Data = data - - t.Run("should check only expressions", func(t *testing.T) { - permissions := map[string][]string{ - datasources.ActionQuery: scopes, - } - - executed := 0 - - eval := authorizeDatasourceAccessForRule(rule, func(evaluator ac.Evaluator) bool { - response := evaluator.Evaluate(permissions) - require.Truef(t, response, "provided permissions [%v] is not enough for requested permissions [%s]", permissions, evaluator.GoString()) - executed++ - return true - }) - - require.True(t, eval) - require.Equal(t, expectedExecutions, executed) - }) - - t.Run("should return on first negative evaluation", func(t *testing.T) { - executed := 0 - - eval := authorizeDatasourceAccessForRule(rule, func(evaluator ac.Evaluator) bool { - executed++ - return false - }) - - require.False(t, eval) - require.Equal(t, 1, executed) - }) -} - -func Test_authorizeAccessToRuleGroup(t *testing.T) { - t.Run("should return true if user has access to all datasources of all rules in group", func(t *testing.T) { - rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen()) - var scopes []string - for _, rule := range rules { - for _, query := range rule.Data { - scopes = append(scopes, datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID)) - } - } - permissions := map[string][]string{ - datasources.ActionQuery: scopes, - } - - result := authorizeAccessToRuleGroup(rules, func(evaluator ac.Evaluator) bool { - response := evaluator.Evaluate(permissions) - require.Truef(t, response, "provided permissions [%v] is not enough for requested permissions [%s]", permissions, evaluator.GoString()) - return true - }) - - require.True(t, result) - }) - 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()) - var scopes []string - for _, rule := range rules { - for _, query := range rule.Data { - scopes = append(scopes, datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID)) - } - } - permissions := map[string][]string{ - datasources.ActionQuery: scopes, - } - - rule := models.AlertRuleGen()() - rules = append(rules, rule) - - result := authorizeAccessToRuleGroup(rules, func(evaluator ac.Evaluator) bool { - response := evaluator.Evaluate(permissions) - return response - }) - - require.False(t, result) - }) -} diff --git a/pkg/services/ngalert/api/errors.go b/pkg/services/ngalert/api/errors.go index 86cb7b04c66..7fe54f0b5bb 100644 --- a/pkg/services/ngalert/api/errors.go +++ b/pkg/services/ngalert/api/errors.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/services/datasources" + "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" ) @@ -34,7 +35,7 @@ func errorToResponse(err error) response.Response { if errors.Is(err, errUnexpectedDatasourceType) { return ErrResp(400, err, "") } - if errors.Is(err, ErrAuthorization) { + if errors.Is(err, accesscontrol.ErrAuthorization) { return ErrResp(401, err, "") } if errors.Is(err, errFolderAccess) { diff --git a/pkg/services/ngalert/api/testing.go b/pkg/services/ngalert/api/testing.go index 032bc108d1f..4fecc5d0f1a 100644 --- a/pkg/services/ngalert/api/testing.go +++ b/pkg/services/ngalert/api/testing.go @@ -12,7 +12,9 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/ngalert/eval" + "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/state" + "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/services/user" ) @@ -139,3 +141,14 @@ func (a *recordingAccessControlFake) IsDisabled() bool { } var _ accesscontrol.AccessControl = &recordingAccessControlFake{} + +type fakeRuleAccessControlService struct { +} + +func (f fakeRuleAccessControlService) AuthorizeAccessToRuleGroup(ctx context.Context, user identity.Requester, rules models.RulesGroup) bool { + return true +} + +func (f fakeRuleAccessControlService) AuthorizeRuleChanges(ctx context.Context, user identity.Requester, change *store.GroupDelta) error { + return nil +} diff --git a/pkg/services/ngalert/models/testing.go b/pkg/services/ngalert/models/testing.go index 280dfa30269..8e5a470b3c1 100644 --- a/pkg/services/ngalert/models/testing.go +++ b/pkg/services/ngalert/models/testing.go @@ -266,6 +266,14 @@ func WithQuery(query ...AlertQuery) AlertRuleMutator { } } +func WithGroupKey(groupKey AlertRuleGroupKey) AlertRuleMutator { + return func(rule *AlertRule) { + rule.RuleGroup = groupKey.RuleGroup + rule.OrgID = groupKey.OrgID + rule.NamespaceUID = groupKey.NamespaceUID + } +} + func GenerateAlertLabels(count int, prefix string) data.Labels { labels := make(data.Labels, count) for i := 0; i < count; i++ {