diff --git a/pkg/services/ngalert/api/api.go b/pkg/services/ngalert/api/api.go index 8910250bf9c..464cab974d8 100644 --- a/pkg/services/ngalert/api/api.go +++ b/pkg/services/ngalert/api/api.go @@ -104,7 +104,10 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) { scheduleService: api.Schedule, store: api.RuleStore, xactManager: api.TransactionManager, - log: logger, cfg: &api.Cfg.UnifiedAlerting}, + log: logger, + cfg: &api.Cfg.UnifiedAlerting, + ac: api.AccessControl, + }, ), m) api.RegisterTestingApiEndpoints(NewForkedTestingApi( &TestingApiSrv{ diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index b449e729480..a058a81fca5 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -7,6 +7,7 @@ import ( "net/http" "time" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/services/quota" @@ -34,6 +35,7 @@ type RulerSrv struct { scheduleService schedule.ScheduleService log log.Logger cfg *setting.UnifiedAlertingSettings + ac accesscontrol.AccessControl } var ( @@ -261,9 +263,9 @@ func (srv RulerSrv) RoutePostNameRulesConfig(c *models.ReqContext, ruleGroupConf } func (srv RulerSrv) updateAlertRulesInGroup(c *models.ReqContext, namespace *models.Folder, groupName string, rules []*ngmodels.AlertRule) response.Response { - // TODO add create rules authz logic - var groupChanges *changes = nil + hasAccess := accesscontrol.HasAccess(srv.ac, c) + err := srv.xactManager.InTransaction(c.Req.Context(), func(tranCtx context.Context) error { var err error groupChanges, err = calculateChanges(tranCtx, srv.store, c.SignedInUser.OrgId, namespace, groupName, rules) @@ -276,6 +278,15 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *models.ReqContext, namespace *mod return nil } + err = authorizeRuleChanges(namespace, groupChanges, func(evaluator accesscontrol.Evaluator) bool { + return hasAccess(accesscontrol.ReqOrgAdminOrEditor, evaluator) + }) + if err != nil { + return err + } + + srv.log.Debug("updating database with the changes", "group", groupName, "namespace", namespace.Uid, "add", len(groupChanges.New), "update", len(groupChanges.New), "delete", len(groupChanges.Delete)) + if len(groupChanges.Update) > 0 || len(groupChanges.New) > 0 { upsert := make([]store.UpsertRule, 0, len(groupChanges.Update)+len(groupChanges.New)) for _, update := range groupChanges.Update { @@ -291,7 +302,6 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *models.ReqContext, namespace *mod New: *rule, }) } - // TODO add update/delete authz logic err = srv.store.UpsertAlertRules(tranCtx, upsert) if err != nil { return fmt.Errorf("failed to add or update rules: %w", err) @@ -326,6 +336,8 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *models.ReqContext, namespace *mod return ErrResp(http.StatusBadRequest, err, "failed to update rule group") } else if errors.Is(err, errQuotaReached) { return ErrResp(http.StatusForbidden, err, "") + } else if errors.Is(err, ErrAuthorization) { + return ErrResp(http.StatusUnauthorized, err, "") } return ErrResp(http.StatusInternalServerError, err, "failed to update rule group") } diff --git a/pkg/services/ngalert/api/authorization.go b/pkg/services/ngalert/api/authorization.go index 2d0facab976..eda3a104b06 100644 --- a/pkg/services/ngalert/api/authorization.go +++ b/pkg/services/ngalert/api/authorization.go @@ -1,17 +1,26 @@ package api import ( + "errors" "fmt" "net/http" + "strconv" + "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/middleware" + "github.com/grafana/grafana/pkg/models" ac "github.com/grafana/grafana/pkg/services/accesscontrol" acmiddleware "github.com/grafana/grafana/pkg/services/accesscontrol/middleware" "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/web" ) +var ( + ErrAuthorization = errors.New("user is not authorized") +) + //nolint:gocyclo func (api *API) authorize(method, path string) web.Handler { authorize := acmiddleware.Middleware(api.AccessControl) @@ -162,3 +171,73 @@ func (api *API) authorize(method, path string) web.Handler { panic(fmt.Sprintf("no authorization handler for method [%s] of endpoint [%s]", method, path)) } + +// GetDatasourceScopesFromAlertRule extracts data source scopes from an alert rule +func getEvaluatorForAlertRule(rule *ngmodels.AlertRule) ac.Evaluator { + scopes := make([]ac.Evaluator, 0, len(rule.Data)) + for _, query := range rule.Data { + if query.QueryType == expr.DatasourceType || query.DatasourceUID == expr.OldDatasourceUID { + continue + } + scopes = append(scopes, ac.EvalPermission(datasources.ActionQuery, dashboards.ScopeFoldersProvider.GetResourceScopeUID(query.DatasourceUID))) + } + return ac.EvalAll(scopes...) +} + +// authorizeRuleChanges analyzes changes in the rule group, determines what actions the user is trying to perform and check whether those actions are authorized. +// If the user is not authorized to perform the changes the function returns ErrAuthorization with a description of what action is not authorized. If the evaluator function returns an error, the function returns it. +func authorizeRuleChanges(namespace *models.Folder, changes *changes, evaluator func(evaluator ac.Evaluator) bool) error { + namespaceScope := dashboards.ScopeFoldersProvider.GetResourceScope(strconv.FormatInt(namespace.Id, 10)) + if len(changes.Delete) > 0 { + allowed := evaluator(ac.EvalPermission(ac.ActionAlertingRuleDelete, namespaceScope)) + if !allowed { + return fmt.Errorf("%w user cannot delete alert rules that belong to folder %s", ErrAuthorization, namespace.Title) + } + } + + var addAuthorized, updateAuthorized bool + + if len(changes.New) > 0 { + addAuthorized = evaluator(ac.EvalPermission(ac.ActionAlertingRuleCreate, namespaceScope)) + if !addAuthorized { + return fmt.Errorf("%w user cannot create alert rules in the folder %s", ErrAuthorization, namespace.Title) + } + for _, rule := range changes.New { + dsAllowed := evaluator(getEvaluatorForAlertRule(rule)) + 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 changes.Update { + dsAllowed := evaluator(getEvaluatorForAlertRule(rule.New)) + 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, namespace.Title) + } + } + continue + } + + 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.EvalAll(ac.EvalPermission(ac.ActionAlertingRuleUpdate, namespaceScope))) + if !updateAuthorized { + return fmt.Errorf("%w to update alert rules that belong to folder %s", ErrAuthorization, namespace.Title) + } + } + } + return nil +} diff --git a/pkg/services/ngalert/api/authorization_test.go b/pkg/services/ngalert/api/authorization_test.go index 641856da079..cc0b7bc342b 100644 --- a/pkg/services/ngalert/api/authorization_test.go +++ b/pkg/services/ngalert/api/authorization_test.go @@ -1,15 +1,22 @@ package api import ( + "math/rand" "net/http" "os" "path/filepath" + "strconv" "testing" "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" ) func TestAuthorize(t *testing.T) { @@ -61,3 +68,197 @@ func TestAuthorize(t *testing.T) { }) }) } + +func TestAuthorizeRuleChanges(t *testing.T) { + namespace := randFolder() + namespaceIdScope := dashboards.ScopeFoldersProvider.GetResourceScope(strconv.FormatInt(namespace.Id, 10)) + + testCases := []struct { + name string + changes func() *changes + permissions func(c *changes) map[string][]string + }{ + { + name: "if there are rules to delete it should check delete action and access to data sources", + changes: func() *changes { + return &changes{ + New: nil, + Update: nil, + Delete: models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withNamespace(namespace))), + } + }, + permissions: func(c *changes) map[string][]string { + return map[string][]string{ + ac.ActionAlertingRuleDelete: { + namespaceIdScope, + }, + } + }, + }, + { + name: "if there are rules to add it should check create action and query for datasource", + changes: func() *changes { + return &changes{ + New: models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withNamespace(namespace))), + Update: nil, + Delete: nil, + } + }, + permissions: func(c *changes) map[string][]string { + var scopes []string + for _, rule := range c.New { + for _, query := range rule.Data { + scopes = append(scopes, dashboards.ScopeFoldersProvider.GetResourceScopeUID(query.DatasourceUID)) + } + } + return map[string][]string{ + ac.ActionAlertingRuleCreate: { + namespaceIdScope, + }, + datasources.ActionQuery: scopes, + } + }, + }, + { + name: "if there are rules to update within the same namespace it should check update action", + changes: func() *changes { + rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withNamespace(namespace))) + updates := make([]ruleUpdate, 0, len(rules)) + + for _, rule := range rules { + updates = append(updates, ruleUpdate{ + Existing: rule, + New: models.CopyRule(rule), + Diff: nil, + }) + } + + return &changes{ + New: nil, + Update: updates, + Delete: nil, + } + }, + permissions: func(c *changes) map[string][]string { + var scopes []string + for _, update := range c.Update { + for _, query := range update.New.Data { + scopes = append(scopes, dashboards.ScopeFoldersProvider.GetResourceScopeUID(query.DatasourceUID)) + } + } + + return map[string][]string{ + ac.ActionAlertingRuleUpdate: { + namespaceIdScope, + }, + datasources.ActionQuery: scopes, + } + }, + }, + { + name: "if there are rules that are moved between namespaces it should check update action", + changes: func() *changes { + rules := models.GenerateAlertRules(rand.Intn(4)+1, models.AlertRuleGen(withNamespace(namespace))) + updates := make([]ruleUpdate, 0, len(rules)) + + for _, rule := range rules { + cp := models.CopyRule(rule) + cp.NamespaceUID = rule.NamespaceUID + "other" + updates = append(updates, ruleUpdate{ + Existing: cp, + New: rule, + Diff: nil, + }) + } + + return &changes{ + New: nil, + Update: updates, + Delete: nil, + } + }, + permissions: func(c *changes) map[string][]string { + var scopes []string + for _, update := range c.Update { + for _, query := range update.New.Data { + scopes = append(scopes, dashboards.ScopeFoldersProvider.GetResourceScopeUID(query.DatasourceUID)) + } + } + return map[string][]string{ + ac.ActionAlertingRuleDelete: { + dashboards.ScopeFoldersProvider.GetResourceScopeUID(namespace.Uid + "other"), + }, + ac.ActionAlertingRuleCreate: { + namespaceIdScope, + }, + datasources.ActionQuery: scopes, + } + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + executed := false + + groupChanges := testCase.changes() + + err := authorizeRuleChanges(namespace, groupChanges, func(evaluator ac.Evaluator) bool { + response, err := evaluator.Evaluate(make(map[string][]string)) + require.False(t, response) + require.NoError(t, err) + executed = true + return false + }) + require.Error(t, err) + require.Truef(t, executed, "evaluation function is expected to be called but it was not.") + + permissions := testCase.permissions(groupChanges) + executed = false + err = authorizeRuleChanges(namespace, groupChanges, func(evaluator ac.Evaluator) bool { + response, err := evaluator.Evaluate(permissions) + require.Truef(t, response, "provided permissions [%v] is not enough for requested permissions [%s]", testCase.permissions, evaluator.GoString()) + require.NoError(t, err) + executed = true + return true + }) + require.NoError(t, err) + require.Truef(t, executed, "evaluation function is expected to be called but it was not.") + }) + } +} + +func TestGetEvaluatorForAlertRule(t *testing.T) { + t.Run("should not consider expressions", func(t *testing.T) { + rule := models.AlertRuleGen()() + + expressionByType := models.GenerateAlertQuery() + expressionByType.QueryType = expr.DatasourceType + expressionByUID := models.GenerateAlertQuery() + expressionByUID.DatasourceUID = expr.OldDatasourceUID + + var data []models.AlertQuery + var scopes []string + for i := 0; i < rand.Intn(3)+2; i++ { + q := models.GenerateAlertQuery() + scopes = append(scopes, dashboards.ScopeFoldersProvider.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 + + eval := getEvaluatorForAlertRule(rule) + + allowed, err := eval.Evaluate(map[string][]string{ + datasources.ActionQuery: scopes, + }) + + require.NoError(t, err) + require.True(t, allowed) + }) +}