Alerting: Add check for datasource permission in alert rule read API (#47087)

* add check for access to rule's data source in GET APIs

* use more general method GetAlertRules instead of GetNamespaceAlertRules.
* remove unused GetNamespaceAlertRules.

Tests:
* create a method to generate permissions for rules
* extract method to create RuleSrv
* add tests for RouteGetNamespaceRulesConfig
This commit is contained in:
Yuriy Tseretyan
2022-04-11 17:37:44 -04:00
committed by GitHub
parent 614fd04653
commit af9353caec
7 changed files with 203 additions and 80 deletions

View File

@ -96,7 +96,7 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) {
api.RegisterPrometheusApiEndpoints(NewForkedProm(
api.DatasourceCache,
NewLotexProm(proxy, logger),
&PrometheusSrv{log: logger, manager: api.StateManager, store: api.RuleStore},
&PrometheusSrv{log: logger, manager: api.StateManager, store: api.RuleStore, ac: api.AccessControl},
), m)
// Register endpoints for proxying to Cortex Ruler-compatible backends.
api.RegisterRulerApiEndpoints(NewForkedRuler(

View File

@ -13,6 +13,7 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/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"
@ -26,6 +27,7 @@ type PrometheusSrv struct {
log log.Logger
manager state.AlertInstanceManager
store store.RuleStore
ac accesscontrol.AccessControl
}
const queryIncludeInternalLabels = "includeInternalLabels"
@ -151,10 +153,16 @@ func (srv PrometheusSrv) RouteGetRuleStatuses(c *models.ReqContext) response.Res
ruleResponse.DiscoveryBase.ErrorType = apiv1.ErrServer
return response.JSON(http.StatusInternalServerError, ruleResponse)
}
hasAccess := func(evaluator accesscontrol.Evaluator) bool {
return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqSignedIn, evaluator)
}
groupMap := make(map[string]*apimodels.RuleGroup)
for _, rule := range alertRuleQuery.Result {
if !authorizeDatasourceAccessForRule(rule, hasAccess) {
continue
}
groupKey := rule.RuleGroup + "-" + rule.NamespaceUID
newGroup, ok := groupMap[groupKey]
if !ok {

View File

@ -4,21 +4,25 @@ import (
"context"
"encoding/json"
"fmt"
"math/rand"
"net/http"
"testing"
"time"
"github.com/grafana/grafana-plugin-sdk-go/data"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models"
acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
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"
"github.com/grafana/grafana/pkg/services/ngalert/state"
"github.com/grafana/grafana/pkg/services/ngalert/store"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
"github.com/grafana/grafana-plugin-sdk-go/data"
"github.com/stretchr/testify/require"
)
func Test_FormatValues(t *testing.T) {
@ -83,7 +87,7 @@ func TestRouteGetAlertStatuses(t *testing.T) {
orgID := int64(1)
t.Run("with no alerts", func(t *testing.T) {
_, _, api := setupAPI(t)
_, _, _, api := setupAPI(t)
req, err := http.NewRequest("GET", "/api/v1/alerts", nil)
require.NoError(t, err)
c := &models.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &models.SignedInUser{OrgId: orgID}}
@ -101,7 +105,7 @@ func TestRouteGetAlertStatuses(t *testing.T) {
})
t.Run("with two alerts", func(t *testing.T) {
_, fakeAIM, api := setupAPI(t)
_, fakeAIM, _, api := setupAPI(t)
fakeAIM.GenerateAlertInstances(1, util.GenerateShortUID(), 2)
req, err := http.NewRequest("GET", "/api/v1/alerts", nil)
require.NoError(t, err)
@ -143,7 +147,7 @@ func TestRouteGetAlertStatuses(t *testing.T) {
})
t.Run("with two firing alerts", func(t *testing.T) {
_, fakeAIM, api := setupAPI(t)
_, fakeAIM, _, api := setupAPI(t)
fakeAIM.GenerateAlertInstances(1, util.GenerateShortUID(), 2, withAlertingState())
req, err := http.NewRequest("GET", "/api/v1/alerts", nil)
require.NoError(t, err)
@ -185,7 +189,7 @@ func TestRouteGetAlertStatuses(t *testing.T) {
})
t.Run("with the inclusion of internal labels", func(t *testing.T) {
_, fakeAIM, api := setupAPI(t)
_, fakeAIM, _, api := setupAPI(t)
fakeAIM.GenerateAlertInstances(orgID, util.GenerateShortUID(), 2)
req, err := http.NewRequest("GET", "/api/v1/alerts?includeInternalLabels=true", nil)
require.NoError(t, err)
@ -251,10 +255,10 @@ func TestRouteGetRuleStatuses(t *testing.T) {
req, err := http.NewRequest("GET", "/api/v1/rules", nil)
require.NoError(t, err)
c := &models.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &models.SignedInUser{OrgId: orgID}}
c := &models.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &models.SignedInUser{OrgId: orgID}, IsSignedIn: true}
t.Run("with no rules", func(t *testing.T) {
_, _, api := setupAPI(t)
_, _, _, api := setupAPI(t)
r := api.RouteGetRuleStatuses(c)
require.JSONEq(t, `
@ -268,7 +272,7 @@ func TestRouteGetRuleStatuses(t *testing.T) {
})
t.Run("with a rule that only has one query", func(t *testing.T) {
fakeStore, fakeAIM, api := setupAPI(t)
fakeStore, fakeAIM, _, api := setupAPI(t)
generateRuleAndInstanceWithQuery(t, orgID, fakeAIM, fakeStore, withClassicConditionSingleQuery())
folder := fakeStore.Folders[orgID][0]
@ -315,13 +319,13 @@ func TestRouteGetRuleStatuses(t *testing.T) {
})
t.Run("with the inclusion of internal Labels", func(t *testing.T) {
fakeStore, fakeAIM, api := setupAPI(t)
fakeStore, fakeAIM, _, api := setupAPI(t)
generateRuleAndInstanceWithQuery(t, orgID, fakeAIM, fakeStore, withClassicConditionSingleQuery())
folder := fakeStore.Folders[orgID][0]
req, err := http.NewRequest("GET", "/api/v1/rules?includeInternalLabels=true", nil)
require.NoError(t, err)
c := &models.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &models.SignedInUser{OrgId: orgID}}
c := &models.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &models.SignedInUser{OrgId: orgID}, IsSignedIn: true}
r := api.RouteGetRuleStatuses(c)
require.Equal(t, http.StatusOK, r.Status())
@ -369,7 +373,7 @@ func TestRouteGetRuleStatuses(t *testing.T) {
})
t.Run("with a rule that has multiple queries", func(t *testing.T) {
fakeStore, fakeAIM, api := setupAPI(t)
fakeStore, fakeAIM, _, api := setupAPI(t)
generateRuleAndInstanceWithQuery(t, orgID, fakeAIM, fakeStore, withExpressionsMultiQuery())
folder := fakeStore.Folders[orgID][0]
@ -414,18 +418,59 @@ func TestRouteGetRuleStatuses(t *testing.T) {
}
`, folder.Title), string(r.Body()))
})
t.Run("when fine-grained access is enabled", func(t *testing.T) {
t.Run("should return only rules if the user can query all data sources", func(t *testing.T) {
ruleStore := store.NewFakeRuleStore(t)
fakeAIM := NewFakeAlertInstanceManager(t)
rules := ngmodels.GenerateAlertRules(rand.Intn(4)+2, ngmodels.AlertRuleGen(withOrgID(orgID)))
ruleStore.PutRule(context.Background(), rules...)
ruleStore.PutRule(context.Background(), ngmodels.GenerateAlertRules(rand.Intn(4)+2, ngmodels.AlertRuleGen(withOrgID(orgID)))...)
acMock := acmock.New().WithPermissions(createPermissionsForRules(rules))
api := PrometheusSrv{
log: log.NewNopLogger(),
manager: fakeAIM,
store: ruleStore,
ac: acMock,
}
response := api.RouteGetRuleStatuses(c)
require.Equal(t, http.StatusOK, response.Status())
result := &apimodels.RuleResponse{}
require.NoError(t, json.Unmarshal(response.Body(), result))
for _, group := range result.Data.RuleGroups {
grouploop:
for _, rule := range group.Rules {
for i, expected := range rules {
if rule.Name == expected.Title && group.Name == expected.RuleGroup {
rules = append(rules[:i], rules[i+1:]...)
continue grouploop
}
}
assert.Failf(t, "rule %s in a group %s was not found in expected", rule.Name, group.Name)
}
}
assert.Emptyf(t, rules, "not all expected rules were returned")
})
})
}
func setupAPI(t *testing.T) (*store.FakeRuleStore, *fakeAlertInstanceManager, PrometheusSrv) {
func setupAPI(t *testing.T) (*store.FakeRuleStore, *fakeAlertInstanceManager, *acmock.Mock, PrometheusSrv) {
fakeStore := store.NewFakeRuleStore(t)
fakeAIM := NewFakeAlertInstanceManager(t)
acMock := acmock.New().WithDisabled()
api := PrometheusSrv{
log: log.NewNopLogger(),
manager: fakeAIM,
store: fakeStore,
ac: acMock,
}
return fakeStore, fakeAIM, api
return fakeStore, fakeAIM, acMock, api
}
func generateRuleAndInstanceWithQuery(t *testing.T, orgID int64, fakeAIM *fakeAlertInstanceManager, fakeStore *store.FakeRuleStore, query func(r *ngmodels.AlertRule)) {

View File

@ -128,17 +128,25 @@ func (srv RulerSrv) RouteGetNamespaceRulesConfig(c *models.ReqContext) response.
return toNamespaceErrorResponse(err)
}
q := ngmodels.ListNamespaceAlertRulesQuery{
q := ngmodels.GetAlertRulesQuery{
OrgID: c.SignedInUser.OrgId,
NamespaceUID: namespace.Uid,
}
if err := srv.store.GetNamespaceAlertRules(c.Req.Context(), &q); err != nil {
if err := srv.store.GetAlertRules(c.Req.Context(), &q); err != nil {
return ErrResp(http.StatusInternalServerError, err, "failed to update rule group")
}
result := apimodels.NamespaceConfigResponse{}
ruleGroupConfigs := make(map[string]apimodels.GettableRuleGroupConfig)
hasAccess := func(evaluator accesscontrol.Evaluator) bool {
return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqSignedIn, evaluator)
}
for _, r := range q.Result {
if !authorizeDatasourceAccessForRule(r, hasAccess) {
continue
}
ruleGroupConfig, ok := ruleGroupConfigs[r.RuleGroup]
if !ok {
ruleGroupInterval := model.Duration(time.Duration(r.IntervalSeconds) * time.Second)
@ -181,7 +189,15 @@ func (srv RulerSrv) RouteGetRulegGroupConfig(c *models.ReqContext) response.Resp
var ruleGroupInterval model.Duration
ruleNodes := make([]apimodels.GettableExtendedRuleNode, 0, len(q.Result))
hasAccess := func(evaluator accesscontrol.Evaluator) bool {
return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqSignedIn, evaluator)
}
for _, r := range q.Result {
if !authorizeDatasourceAccessForRule(r, hasAccess) {
continue
}
ruleGroupInterval = model.Duration(time.Duration(r.IntervalSeconds) * time.Second)
ruleNodes = append(ruleNodes, toGettableExtendedRuleNode(*r, namespace.Id))
}
@ -234,7 +250,15 @@ func (srv RulerSrv) RouteGetRulesConfig(c *models.ReqContext) response.Response
}
configs := make(map[string]map[string]apimodels.GettableRuleGroupConfig)
hasAccess := func(evaluator accesscontrol.Evaluator) bool {
return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqSignedIn, evaluator)
}
for _, r := range q.Result {
if !authorizeDatasourceAccessForRule(r, hasAccess) {
continue
}
folder, ok := namespaceMap[r.NamespaceUID]
if !ok {
srv.log.Error("namespace not visible to the user", "user", c.SignedInUser.UserId, "namespace", r.NamespaceUID, "rule", r.UID)

View File

@ -2,12 +2,14 @@ package api
import (
"context"
"encoding/json"
"errors"
"math/rand"
"net/http"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
@ -16,6 +18,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol"
acMock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/datasources"
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/schedule"
"github.com/grafana/grafana/pkg/services/ngalert/store"
@ -264,19 +267,6 @@ func TestCalculateChanges(t *testing.T) {
}
func TestRouteDeleteAlertRules(t *testing.T) {
createService := func(ac *acMock.Mock, store *store.FakeRuleStore, scheduler schedule.ScheduleService) *RulerSrv {
return &RulerSrv{
xactManager: store,
store: store,
DatasourceCache: nil,
QuotaService: nil,
scheduleService: scheduler,
log: log.New("test"),
cfg: nil,
ac: ac,
}
}
getRecordedCommand := func(ruleStore *store.FakeRuleStore) []store.GenericRecordedQuery {
results := ruleStore.GetRecordedCommands(func(cmd interface{}) (interface{}, bool) {
c, ok := cmd.(store.GenericRecordedQuery)
@ -421,15 +411,7 @@ func TestRouteDeleteAlertRules(t *testing.T) {
scheduler := &schedule.FakeScheduleService{}
scheduler.On("DeleteAlertRule", mock.Anything)
var permissions []*accesscontrol.Permission
for _, rule := range rulesInFolder {
for _, query := range rule.Data {
permissions = append(permissions, &accesscontrol.Permission{
Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID),
})
}
}
ac := acMock.New().WithPermissions(permissions)
ac := acMock.New().WithPermissions(createPermissionsForRules(rulesInFolder))
request := createRequestContext(orgID, "None", map[string]string{
":Namespace": folder.Title,
})
@ -454,15 +436,7 @@ func TestRouteDeleteAlertRules(t *testing.T) {
scheduler := &schedule.FakeScheduleService{}
scheduler.On("DeleteAlertRule", mock.Anything)
var permissions []*accesscontrol.Permission
for _, rule := range authorizedRulesInFolder {
for _, query := range rule.Data {
permissions = append(permissions, &accesscontrol.Permission{
Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID),
})
}
}
ac := acMock.New().WithPermissions(permissions)
ac := acMock.New().WithPermissions(createPermissionsForRules(authorizedRulesInFolder))
request := createRequestContext(orgID, "None", map[string]string{
":Namespace": folder.Title,
})
@ -489,15 +463,7 @@ func TestRouteDeleteAlertRules(t *testing.T) {
scheduler := &schedule.FakeScheduleService{}
scheduler.On("DeleteAlertRule", mock.Anything)
var permissions []*accesscontrol.Permission
for _, rule := range authorizedRulesInGroup {
for _, query := range rule.Data {
permissions = append(permissions, &accesscontrol.Permission{
Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID),
})
}
}
ac := acMock.New().WithPermissions(permissions)
ac := acMock.New().WithPermissions(createPermissionsForRules(authorizedRulesInGroup))
request := createRequestContext(orgID, "None", map[string]string{
":Namespace": folder.Title,
":Groupname": groupName,
@ -510,11 +476,101 @@ func TestRouteDeleteAlertRules(t *testing.T) {
})
}
func TestRouteGetNamespaceRulesConfig(t *testing.T) {
t.Run("fine-grained access is enabled", func(t *testing.T) {
t.Run("should return rules for which user has access to data source", func(t *testing.T) {
orgID := rand.Int63()
folder := randFolder()
ruleStore := store.NewFakeRuleStore(t)
ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
expectedRules := models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))
ruleStore.PutRule(context.Background(), expectedRules...)
ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))...)
ac := acMock.New().WithPermissions(createPermissionsForRules(expectedRules))
response := createService(ac, ruleStore, nil).RouteGetNamespaceRulesConfig(createRequestContext(orgID, "", map[string]string{
":Namespace": folder.Title,
}))
require.Equal(t, http.StatusAccepted, response.Status())
result := &apimodels.NamespaceConfigResponse{}
require.NoError(t, json.Unmarshal(response.Body(), result))
require.NotNil(t, result)
for namespace, groups := range *result {
require.Equal(t, folder.Title, namespace)
for _, group := range groups {
grouploop:
for _, actualRule := range group.Rules {
for i, expected := range expectedRules {
if actualRule.GrafanaManagedAlert.UID == expected.UID {
expectedRules = append(expectedRules[:i], expectedRules[i+1:]...)
continue grouploop
}
}
assert.Failf(t, "rule in a group was not found in expected", "rule %s group %s", actualRule.GrafanaManagedAlert.Title, group.Name)
}
}
}
assert.Emptyf(t, expectedRules, "not all expected rules were returned")
})
})
t.Run("fine-grained access is disabled", func(t *testing.T) {
t.Run("should return all rules from folder", func(t *testing.T) {
orgID := rand.Int63()
folder := randFolder()
ruleStore := store.NewFakeRuleStore(t)
ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
expectedRules := models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))
ruleStore.PutRule(context.Background(), expectedRules...)
ac := acMock.New().WithDisabled()
response := createService(ac, ruleStore, nil).RouteGetNamespaceRulesConfig(createRequestContext(orgID, "", map[string]string{
":Namespace": folder.Title,
}))
require.Equal(t, http.StatusAccepted, response.Status())
result := &apimodels.NamespaceConfigResponse{}
require.NoError(t, json.Unmarshal(response.Body(), result))
require.NotNil(t, result)
for namespace, groups := range *result {
require.Equal(t, folder.Title, namespace)
for _, group := range groups {
grouploop:
for _, actualRule := range group.Rules {
for i, expected := range expectedRules {
if actualRule.GrafanaManagedAlert.UID == expected.UID {
expectedRules = append(expectedRules[:i], expectedRules[i+1:]...)
continue grouploop
}
}
assert.Failf(t, "rule in a group was not found in expected", "rule %s group %s", actualRule.GrafanaManagedAlert.Title, group.Name)
}
}
}
assert.Emptyf(t, expectedRules, "not all expected rules were returned")
})
})
}
func createService(ac *acMock.Mock, store *store.FakeRuleStore, scheduler schedule.ScheduleService) *RulerSrv {
return &RulerSrv{
xactManager: store,
store: store,
DatasourceCache: nil,
QuotaService: nil,
scheduleService: scheduler,
log: log.New("test"),
cfg: nil,
ac: ac,
}
}
func createRequestContext(orgID int64, role models2.RoleType, params map[string]string) *models2.ReqContext {
ctx := web.Context{Req: &http.Request{}}
ctx.Req = web.SetURLParams(ctx.Req, params)
return &models2.ReqContext{
IsSignedIn: true,
SignedInUser: &models2.SignedInUser{
OrgRole: role,
OrgId: orgID,
@ -523,6 +579,18 @@ func createRequestContext(orgID int64, role models2.RoleType, params map[string]
}
}
func createPermissionsForRules(rules []*models.AlertRule) []*accesscontrol.Permission {
var permissions []*accesscontrol.Permission
for _, rule := range rules {
for _, query := range rule.Data {
permissions = append(permissions, &accesscontrol.Permission{
Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(query.DatasourceUID),
})
}
}
return permissions
}
func withOrgID(orgId int64) func(rule *models.AlertRule) {
return func(rule *models.AlertRule) {
rule.OrgID = orgId

View File

@ -39,7 +39,6 @@ type RuleStore interface {
GetAlertRuleByUID(ctx context.Context, query *ngmodels.GetAlertRuleByUIDQuery) error
GetAlertRulesForScheduling(ctx context.Context, query *ngmodels.ListAlertRulesQuery) error
GetOrgAlertRules(ctx context.Context, query *ngmodels.ListAlertRulesQuery) error
GetNamespaceAlertRules(ctx context.Context, query *ngmodels.ListNamespaceAlertRulesQuery) error
GetAlertRules(ctx context.Context, query *ngmodels.GetAlertRulesQuery) error
GetUserVisibleNamespaces(context.Context, int64, *models.SignedInUser) (map[string]*models.Folder, error)
GetNamespaceByTitle(context.Context, string, int64, *models.SignedInUser, bool) (*models.Folder, error)
@ -229,21 +228,6 @@ func (st DBstore) GetOrgAlertRules(ctx context.Context, query *ngmodels.ListAler
})
}
// GetNamespaceAlertRules is a handler for retrieving namespace alert rules of specific organisation.
func (st DBstore) GetNamespaceAlertRules(ctx context.Context, query *ngmodels.ListNamespaceAlertRulesQuery) error {
return st.SQLStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
alertRules := make([]*ngmodels.AlertRule, 0)
// TODO rewrite using group by namespace_uid, rule_group
q := "SELECT * FROM alert_rule WHERE org_id = ? and namespace_uid = ?"
if err := sess.SQL(q, query.OrgID, query.NamespaceUID).Find(&alertRules); err != nil {
return err
}
query.Result = alertRules
return nil
})
}
// GetAlertRules is a handler for retrieving rule group alert rules of specific organisation.
func (st DBstore) GetAlertRules(ctx context.Context, query *ngmodels.GetAlertRulesQuery) error {
return st.SQLStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {

View File

@ -178,12 +178,6 @@ func (f *FakeRuleStore) GetOrgAlertRules(_ context.Context, q *models.ListAlertR
q.Result = rules
return nil
}
func (f *FakeRuleStore) GetNamespaceAlertRules(_ context.Context, q *models.ListNamespaceAlertRulesQuery) error {
f.mtx.Lock()
defer f.mtx.Unlock()
f.RecordedOps = append(f.RecordedOps, *q)
return nil
}
func (f *FakeRuleStore) GetAlertRules(_ context.Context, q *models.GetAlertRulesQuery) error {
f.mtx.Lock()
defer f.mtx.Unlock()