From 8a2c3680317986447187f4912a6d68447116e11f Mon Sep 17 00:00:00 2001 From: Yuriy Tseretyan Date: Fri, 1 Apr 2022 09:39:59 -0400 Subject: [PATCH] check that user is authorized to create\update silences (#47163) --- pkg/services/ngalert/api/api.go | 2 +- pkg/services/ngalert/api/api_alertmanager.go | 14 ++ .../ngalert/api/api_alertmanager_test.go | 187 +++++++++++++++++- 3 files changed, 198 insertions(+), 5 deletions(-) diff --git a/pkg/services/ngalert/api/api.go b/pkg/services/ngalert/api/api.go index 464cab974d8..843fa582914 100644 --- a/pkg/services/ngalert/api/api.go +++ b/pkg/services/ngalert/api/api.go @@ -86,7 +86,7 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) { api.RegisterAlertmanagerApiEndpoints(NewForkedAM( api.DatasourceCache, NewLotexAM(proxy, logger), - &AlertmanagerSrv{store: api.AlertingStore, mam: api.MultiOrgAlertmanager, secrets: api.SecretsService, log: logger}, + &AlertmanagerSrv{store: api.AlertingStore, mam: api.MultiOrgAlertmanager, secrets: api.SecretsService, log: logger, ac: api.AccessControl}, ), m) // Register endpoints for proxying to Prometheus-compatible backends. api.RegisterPrometheusApiEndpoints(NewForkedProm( diff --git a/pkg/services/ngalert/api/api_alertmanager.go b/pkg/services/ngalert/api/api_alertmanager.go index 7089671d96d..047a637c32c 100644 --- a/pkg/services/ngalert/api/api_alertmanager.go +++ b/pkg/services/ngalert/api/api_alertmanager.go @@ -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" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/notifier" @@ -32,6 +33,7 @@ type AlertmanagerSrv struct { secrets secrets.Service store AlertingStore log log.Logger + ac accesscontrol.AccessControl } type UnknownReceiverError struct { @@ -130,6 +132,18 @@ func (srv AlertmanagerSrv) RouteCreateSilence(c *models.ReqContext, postableSile return errResp } + action := accesscontrol.ActionAlertingInstanceUpdate + if postableSilence.ID == "" { + action = accesscontrol.ActionAlertingInstanceCreate + } + if !accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqOrgAdminOrEditor, accesscontrol.EvalPermission(action)) { + errAction := "update" + if postableSilence.ID == "" { + errAction = "create" + } + return ErrResp(http.StatusUnauthorized, fmt.Errorf("user is not authorized to %s silences", errAction), "") + } + silenceID, err := am.CreateSilence(&postableSilence) if err != nil { if errors.Is(err, notifier.ErrSilenceNotFound) { diff --git a/pkg/services/ngalert/api/api_alertmanager_test.go b/pkg/services/ngalert/api/api_alertmanager_test.go index b9f1470906c..13943adc9ed 100644 --- a/pkg/services/ngalert/api/api_alertmanager_test.go +++ b/pkg/services/ngalert/api/api_alertmanager_test.go @@ -2,15 +2,20 @@ package api import ( "context" + "math/rand" "net/http" "testing" "time" + "github.com/go-openapi/strfmt" + amv2 "github.com/prometheus/alertmanager/api/v2/models" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/accesscontrol" + 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/metrics" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -18,6 +23,7 @@ import ( "github.com/grafana/grafana/pkg/services/secrets/fakes" secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" ) @@ -151,7 +157,7 @@ func TestStatusForTestReceivers(t *testing.T) { } func TestAlertmanagerConfig(t *testing.T) { - sut := createSut(t) + sut := createSut(t, nil) t.Run("assert 404 Not Found when applying config to nonexistent org", func(t *testing.T) { rc := models.ReqContext{ @@ -187,7 +193,7 @@ func TestAlertmanagerConfig(t *testing.T) { }) t.Run("assert 202 when alertmanager to configure is not ready", func(t *testing.T) { - sut := createSut(t) + sut := createSut(t, nil) rc := models.ReqContext{ Context: &web.Context{ Req: &http.Request{}, @@ -204,7 +210,139 @@ func TestAlertmanagerConfig(t *testing.T) { }) } -func createSut(t *testing.T) AlertmanagerSrv { +func TestRouteCreateSilence(t *testing.T) { + tesCases := []struct { + name string + silence func() apimodels.PostableSilence + accessControl func() accesscontrol.AccessControl + role models.RoleType + expectedStatus int + }{ + { + name: "new silence, fine-grained access control is enabled, not authorized", + silence: silenceGen(withEmptyID), + accessControl: func() accesscontrol.AccessControl { + return acMock.New() + }, + expectedStatus: http.StatusUnauthorized, + }, + { + name: "new silence, fine-grained access control is enabled, authorized", + silence: silenceGen(withEmptyID), + accessControl: func() accesscontrol.AccessControl { + return acMock.New().WithPermissions([]*accesscontrol.Permission{ + {Action: accesscontrol.ActionAlertingInstanceCreate}, + }) + }, + expectedStatus: http.StatusAccepted, + }, + { + name: "new silence, fine-grained access control is disabled, Viewer", + silence: silenceGen(withEmptyID), + accessControl: func() accesscontrol.AccessControl { + return acMock.New().WithDisabled() + }, + role: models.ROLE_VIEWER, + expectedStatus: http.StatusUnauthorized, + }, + { + name: "new silence, fine-grained access control is disabled, Editor", + silence: silenceGen(withEmptyID), + accessControl: func() accesscontrol.AccessControl { + return acMock.New().WithDisabled() + }, + role: models.ROLE_EDITOR, + expectedStatus: http.StatusAccepted, + }, + { + name: "new silence, fine-grained access control is disabled, Admin", + silence: silenceGen(withEmptyID), + accessControl: func() accesscontrol.AccessControl { + return acMock.New().WithDisabled() + }, + role: models.ROLE_ADMIN, + expectedStatus: http.StatusAccepted, + }, + { + name: "update silence, fine-grained access control is enabled, not authorized", + silence: silenceGen(), + accessControl: func() accesscontrol.AccessControl { + return acMock.New() + }, + expectedStatus: http.StatusUnauthorized, + }, + { + name: "update silence, fine-grained access control is enabled, authorized", + silence: silenceGen(), + accessControl: func() accesscontrol.AccessControl { + return acMock.New().WithPermissions([]*accesscontrol.Permission{ + {Action: accesscontrol.ActionAlertingInstanceUpdate}, + }) + }, + expectedStatus: http.StatusAccepted, + }, + { + name: "update silence, fine-grained access control is disabled, Viewer", + silence: silenceGen(), + accessControl: func() accesscontrol.AccessControl { + return acMock.New().WithDisabled() + }, + role: models.ROLE_VIEWER, + expectedStatus: http.StatusUnauthorized, + }, + { + name: "update silence, fine-grained access control is disabled, Editor", + silence: silenceGen(), + accessControl: func() accesscontrol.AccessControl { + return acMock.New().WithDisabled() + }, + role: models.ROLE_EDITOR, + expectedStatus: http.StatusAccepted, + }, + { + name: "update silence, fine-grained access control is disabled, Admin", + silence: silenceGen(), + accessControl: func() accesscontrol.AccessControl { + return acMock.New().WithDisabled() + }, + role: models.ROLE_ADMIN, + expectedStatus: http.StatusAccepted, + }, + } + + for _, tesCase := range tesCases { + t.Run(tesCase.name, func(t *testing.T) { + ac := tesCase.accessControl() + sut := createSut(t, ac) + + rc := models.ReqContext{ + Context: &web.Context{ + Req: &http.Request{}, + }, + SignedInUser: &models.SignedInUser{ + OrgRole: tesCase.role, + OrgId: 1, + }, + } + + silence := tesCase.silence() + + if silence.ID != "" { + alertmanagerFor, err := sut.mam.AlertmanagerFor(1) + require.NoError(t, err) + silence.ID = "" + newID, err := alertmanagerFor.CreateSilence(&silence) + require.NoError(t, err) + silence.ID = newID + } + + response := sut.RouteCreateSilence(&rc, silence) + require.Equal(t, tesCase.expectedStatus, response.Status()) + }) + } +} + +func createSut(t *testing.T, accessControl accesscontrol.AccessControl) AlertmanagerSrv { t.Helper() mam := createMultiOrgAlertmanager(t) @@ -213,7 +351,10 @@ func createSut(t *testing.T) AlertmanagerSrv { store.Setup(2) store.Setup(3) secrets := fakes.NewFakeSecretsService() - return AlertmanagerSrv{mam: mam, store: store, secrets: secrets} + if accessControl == nil { + accessControl = acMock.New().WithDisabled() + } + return AlertmanagerSrv{mam: mam, store: store, secrets: secrets, ac: accessControl} } func createAmConfigRequest(t *testing.T) apimodels.PostableUserConfig { @@ -277,3 +418,41 @@ var brokenConfig = ` }] } }` + +func silenceGen(mutatorFuncs ...func(*apimodels.PostableSilence)) func() apimodels.PostableSilence { + return func() apimodels.PostableSilence { + testString := util.GenerateShortUID() + isEqual := rand.Int()%2 == 0 + isRegex := rand.Int()%2 == 0 + value := util.GenerateShortUID() + if isRegex { + value = ".*" + util.GenerateShortUID() + } + + matchers := amv2.Matchers{&amv2.Matcher{Name: &testString, IsEqual: &isEqual, IsRegex: &isRegex, Value: &value}} + comment := util.GenerateShortUID() + starts := strfmt.DateTime(timeNow().Add(-time.Duration(rand.Int63n(9)+1) * time.Second)) + ends := strfmt.DateTime(timeNow().Add(time.Duration(rand.Int63n(9)+1) * time.Second)) + createdBy := "User-" + util.GenerateShortUID() + s := apimodels.PostableSilence{ + ID: util.GenerateShortUID(), + Silence: amv2.Silence{ + Comment: &comment, + CreatedBy: &createdBy, + EndsAt: &ends, + Matchers: matchers, + StartsAt: &starts, + }, + } + + for _, mutator := range mutatorFuncs { + mutator(&s) + } + + return s + } +} + +func withEmptyID(silence *apimodels.PostableSilence) { + silence.ID = "" +}