diff --git a/pkg/services/ngalert/api/api_provisioning.go b/pkg/services/ngalert/api/api_provisioning.go index bcaa879e790..4730203e2ae 100644 --- a/pkg/services/ngalert/api/api_provisioning.go +++ b/pkg/services/ngalert/api/api_provisioning.go @@ -135,7 +135,7 @@ func (srv *ProvisioningSrv) RouteGetContactPoints(c *contextmodel.ReqContext) re Name: c.Query("name"), OrgID: c.SignedInUser.GetOrgID(), } - cps, err := srv.contactPointService.GetContactPoints(c.Req.Context(), q, nil) + cps, err := srv.contactPointService.GetContactPoints(c.Req.Context(), q, c.SignedInUser) if err != nil { if errors.Is(err, provisioning.ErrPermissionDenied) { return ErrResp(http.StatusForbidden, err, "") diff --git a/pkg/services/ngalert/api/api_provisioning_test.go b/pkg/services/ngalert/api/api_provisioning_test.go index 84f6fa21a06..527561f077b 100644 --- a/pkg/services/ngalert/api/api_provisioning_test.go +++ b/pkg/services/ngalert/api/api_provisioning_test.go @@ -1473,9 +1473,26 @@ func TestProvisioningApi(t *testing.T) { } func TestProvisioningApiContactPointExport(t *testing.T) { + createTestEnv := func(t *testing.T, testConfig string) testEnvironment { + env := createTestEnv(t, testConfig) + env.ac = &recordingAccessControlFake{ + Callback: func(user *user.SignedInUser, evaluator accesscontrol.Evaluator) (bool, error) { + if strings.Contains(evaluator.String(), accesscontrol.ActionAlertingNotificationsRead) { + return true, nil + } + if strings.Contains(evaluator.String(), accesscontrol.ActionAlertingReceiversList) { + return true, nil + } + return false, nil + }, + } + return env + } + t.Run("contact point export", func(t *testing.T) { t.Run("are present, GET returns 200", func(t *testing.T) { - sut := createProvisioningSrvSut(t) + env := createTestEnv(t, testConfig) + sut := createProvisioningSrvSutFromEnv(t, &env) rc := createTestRequestCtx() response := sut.RouteGetContactPointsExport(&rc) @@ -1484,7 +1501,8 @@ func TestProvisioningApiContactPointExport(t *testing.T) { }) t.Run("accept header contains yaml, GET returns text yaml", func(t *testing.T) { - sut := createProvisioningSrvSut(t) + env := createTestEnv(t, testConfig) + sut := createProvisioningSrvSutFromEnv(t, &env) rc := createTestRequestCtx() rc.Context.Req.Header.Add("Accept", "application/yaml") @@ -1496,7 +1514,8 @@ func TestProvisioningApiContactPointExport(t *testing.T) { }) t.Run("accept header contains json, GET returns json", func(t *testing.T) { - sut := createProvisioningSrvSut(t) + env := createTestEnv(t, testConfig) + sut := createProvisioningSrvSutFromEnv(t, &env) rc := createTestRequestCtx() rc.Context.Req.Header.Add("Accept", "application/json") @@ -1508,7 +1527,8 @@ func TestProvisioningApiContactPointExport(t *testing.T) { }) t.Run("accept header contains json and yaml, GET returns json", func(t *testing.T) { - sut := createProvisioningSrvSut(t) + env := createTestEnv(t, testConfig) + sut := createProvisioningSrvSutFromEnv(t, &env) rc := createTestRequestCtx() rc.Context.Req.Header.Add("Accept", "application/json, application/yaml") @@ -1520,7 +1540,8 @@ func TestProvisioningApiContactPointExport(t *testing.T) { }) t.Run("query param download=true, GET returns content disposition attachment", func(t *testing.T) { - sut := createProvisioningSrvSut(t) + env := createTestEnv(t, testConfig) + sut := createProvisioningSrvSutFromEnv(t, &env) rc := createTestRequestCtx() rc.Context.Req.Form.Set("download", "true") @@ -1532,7 +1553,8 @@ func TestProvisioningApiContactPointExport(t *testing.T) { }) t.Run("query param download=false, GET returns empty content disposition", func(t *testing.T) { - sut := createProvisioningSrvSut(t) + env := createTestEnv(t, testConfig) + sut := createProvisioningSrvSutFromEnv(t, &env) rc := createTestRequestCtx() rc.Context.Req.Form.Set("download", "false") @@ -1544,7 +1566,8 @@ func TestProvisioningApiContactPointExport(t *testing.T) { }) t.Run("query param download not set, GET returns empty content disposition", func(t *testing.T) { - sut := createProvisioningSrvSut(t) + env := createTestEnv(t, testConfig) + sut := createProvisioningSrvSutFromEnv(t, &env) rc := createTestRequestCtx() response := sut.RouteGetContactPointsExport(&rc) diff --git a/pkg/services/ngalert/notifier/receiver_svc.go b/pkg/services/ngalert/notifier/receiver_svc.go index bef0e0cdd24..858d556da23 100644 --- a/pkg/services/ngalert/notifier/receiver_svc.go +++ b/pkg/services/ngalert/notifier/receiver_svc.go @@ -64,13 +64,7 @@ func NewReceiverService( } func (rs *ReceiverService) shouldDecrypt(ctx context.Context, user identity.Requester, reqDecrypt bool) (bool, error) { - // TODO: migrate to new permission - eval := accesscontrol.EvalAny( - accesscontrol.EvalPermission(accesscontrol.ActionAlertingReceiversReadSecrets), - accesscontrol.EvalPermission(accesscontrol.ActionAlertingProvisioningReadSecrets), - ) - - decryptAccess, err := rs.ac.Evaluate(ctx, user, eval) + decryptAccess, err := rs.hasReadDecrypted(ctx, user) if err != nil { return false, err } @@ -82,6 +76,31 @@ func (rs *ReceiverService) shouldDecrypt(ctx context.Context, user identity.Requ return decryptAccess && reqDecrypt, nil } +// hasReadDecrypted checks if the user has permission to read decrypted secure settings. +func (rs *ReceiverService) hasReadDecrypted(ctx context.Context, user identity.Requester) (bool, error) { + return rs.ac.Evaluate(ctx, user, accesscontrol.EvalAny( + accesscontrol.EvalPermission(accesscontrol.ActionAlertingReceiversReadSecrets), + accesscontrol.EvalPermission(accesscontrol.ActionAlertingProvisioningReadSecrets), // TODO: Add scope all when we implement FGAC. + )) +} + +// hasReadRedacted checks if the user has permission to read redacted secure settings. +func (rs *ReceiverService) hasReadRedacted(ctx context.Context, user identity.Requester) (bool, error) { + return rs.ac.Evaluate(ctx, user, accesscontrol.EvalAny( + accesscontrol.EvalPermission(accesscontrol.ActionAlertingProvisioningRead), + accesscontrol.EvalPermission(accesscontrol.ActionAlertingProvisioningReadSecrets), + accesscontrol.EvalPermission(accesscontrol.ActionAlertingNotificationsProvisioningRead), + accesscontrol.EvalPermission(accesscontrol.ActionAlertingNotificationsRead), + //accesscontrol.EvalPermission(accesscontrol.ActionAlertingReceiversRead, ScopeReceiversProvider.GetResourceAllScope()), // TODO: Add new permissions. + //accesscontrol.EvalPermission(accesscontrol.ActionAlertingReceiversReadSecrets, ScopeReceiversProvider.GetResourceAllScope(), + )) +} + +// hasList checks if the user has permission to list receivers. +func (rs *ReceiverService) hasList(ctx context.Context, user identity.Requester) (bool, error) { + return rs.ac.Evaluate(ctx, user, accesscontrol.EvalPermission(accesscontrol.ActionAlertingReceiversList)) +} + // GetReceiver returns a receiver by name. // The receiver's secure settings are decrypted if requested and the user has access to do so. func (rs *ReceiverService) GetReceiver(ctx context.Context, q models.GetReceiverQuery, user identity.Requester) (definitions.GettableApiReceiver, error) { @@ -144,12 +163,22 @@ func (rs *ReceiverService) GetReceivers(ctx context.Context, q models.GetReceive return nil, err } - eval := accesscontrol.EvalPermission(accesscontrol.ActionAlertingReceiversList) - listAccess, err := rs.ac.Evaluate(ctx, user, eval) + readRedactedAccess, err := rs.hasReadRedacted(ctx, user) if err != nil { return nil, err } + listAccess, err := rs.hasList(ctx, user) + if err != nil { + return nil, err + } + + // User doesn't have any permissions on the receivers. + // This is mostly a safeguard as it should not be possible with current API endpoints + middleware authentication. + if !listAccess && !readRedactedAccess { + return nil, ErrPermissionDenied + } + var output []definitions.GettableApiReceiver for i := q.Offset; i < len(cfg.AlertmanagerConfig.Receivers); i++ { r := cfg.AlertmanagerConfig.Receivers[i] @@ -163,7 +192,11 @@ func (rs *ReceiverService) GetReceivers(ctx context.Context, q models.GetReceive } decryptFn := rs.decryptOrRedact(ctx, decrypt, r.Name, "") - listOnly := !decrypt && listAccess + + // Only has permission to list. This reduces from: + // - Has List permission + // - Doesn't have ReadRedacted (or ReadDecrypted permission since it's a subset). + listOnly := !readRedactedAccess res, err := PostableToGettableApiReceiver(r, provenances, decryptFn, listOnly) if err != nil { diff --git a/pkg/services/ngalert/notifier/receiver_svc_test.go b/pkg/services/ngalert/notifier/receiver_svc_test.go index da367d576f1..25dbcc50952 100644 --- a/pkg/services/ngalert/notifier/receiver_svc_test.go +++ b/pkg/services/ngalert/notifier/receiver_svc_test.go @@ -13,7 +13,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" - "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/authz/zanzana" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" @@ -29,10 +28,16 @@ func TestReceiverService_GetReceiver(t *testing.T) { sqlStore := db.InitTestDB(t) secretsService := manager.SetupTestService(t, database.ProvideSecretsStore(sqlStore)) + redactedUser := &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{ + 1: { + accesscontrol.ActionAlertingProvisioningRead: nil, + }, + }} + t.Run("service gets receiver from AM config", func(t *testing.T) { sut := createReceiverServiceSut(t, secretsService) - Receiver, err := sut.GetReceiver(context.Background(), singleQ(1, "slack receiver"), nil) + Receiver, err := sut.GetReceiver(context.Background(), singleQ(1, "slack receiver"), redactedUser) require.NoError(t, err) require.Equal(t, "slack receiver", Receiver.Name) require.Len(t, Receiver.GrafanaManagedReceivers, 1) @@ -42,7 +47,7 @@ func TestReceiverService_GetReceiver(t *testing.T) { t.Run("service returns error when receiver does not exist", func(t *testing.T) { sut := createReceiverServiceSut(t, secretsService) - _, err := sut.GetReceiver(context.Background(), singleQ(1, "nonexistent"), nil) + _, err := sut.GetReceiver(context.Background(), singleQ(1, "nonexistent"), redactedUser) require.ErrorIs(t, err, ErrNotFound) }) } @@ -51,10 +56,16 @@ func TestReceiverService_GetReceivers(t *testing.T) { sqlStore := db.InitTestDB(t) secretsService := manager.SetupTestService(t, database.ProvideSecretsStore(sqlStore)) + redactedUser := &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{ + 1: { + accesscontrol.ActionAlertingProvisioningRead: nil, + }, + }} + t.Run("service gets receivers from AM config", func(t *testing.T) { sut := createReceiverServiceSut(t, secretsService) - Receivers, err := sut.GetReceivers(context.Background(), multiQ(1), nil) + Receivers, err := sut.GetReceivers(context.Background(), multiQ(1), redactedUser) require.NoError(t, err) require.Len(t, Receivers, 2) require.Equal(t, "grafana-default-email", Receivers[0].Name) @@ -64,7 +75,7 @@ func TestReceiverService_GetReceivers(t *testing.T) { t.Run("service filters receivers by name", func(t *testing.T) { sut := createReceiverServiceSut(t, secretsService) - Receivers, err := sut.GetReceivers(context.Background(), multiQ(1, "slack receiver"), nil) + Receivers, err := sut.GetReceivers(context.Background(), multiQ(1, "slack receiver"), redactedUser) require.NoError(t, err) require.Len(t, Receivers, 1) require.Equal(t, "slack receiver", Receivers[0].Name) @@ -74,7 +85,6 @@ func TestReceiverService_GetReceivers(t *testing.T) { func TestReceiverService_DecryptRedact(t *testing.T) { sqlStore := db.InitTestDB(t) secretsService := manager.SetupTestService(t, database.ProvideSecretsStore(sqlStore)) - ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()) getMethods := []string{"single", "multi"} @@ -129,7 +139,6 @@ func TestReceiverService_DecryptRedact(t *testing.T) { for _, method := range getMethods { t.Run(fmt.Sprintf("%s %s", tc.name, method), func(t *testing.T) { sut := createReceiverServiceSut(t, secretsService) - sut.ac = ac var res definitions.GettableApiReceiver var err error @@ -174,7 +183,7 @@ func createReceiverServiceSut(t *testing.T, encryptSvc secrets.Service) *Receive provisioningStore := fakes.NewFakeProvisioningStore() return &ReceiverService{ - actest.FakeAccessControl{}, + acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()), provisioningStore, store, encryptSvc, diff --git a/pkg/services/ngalert/provisioning/compat.go b/pkg/services/ngalert/provisioning/compat.go index 0502d921d8d..1bfeffa11e4 100644 --- a/pkg/services/ngalert/provisioning/compat.go +++ b/pkg/services/ngalert/provisioning/compat.go @@ -47,9 +47,13 @@ func PostableGrafanaReceiverToEmbeddedContactPoint(contactPoint *definitions.Pos } func GettableGrafanaReceiverToEmbeddedContactPoint(r *definitions.GettableGrafanaReceiver) (definitions.EmbeddedContactPoint, error) { - settingJson, err := simplejson.NewJson(r.Settings) - if err != nil { - return definitions.EmbeddedContactPoint{}, err + settingJson := simplejson.New() + if r.Settings != nil { + var err error + settingJson, err = simplejson.NewJson(r.Settings) + if err != nil { + return definitions.EmbeddedContactPoint{}, err + } } for k := range r.SecureFields { diff --git a/pkg/services/ngalert/provisioning/contactpoints_test.go b/pkg/services/ngalert/provisioning/contactpoints_test.go index b07dcc80237..44877c37198 100644 --- a/pkg/services/ngalert/provisioning/contactpoints_test.go +++ b/pkg/services/ngalert/provisioning/contactpoints_test.go @@ -16,7 +16,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" - "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/authz/zanzana" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" @@ -33,10 +32,17 @@ import ( func TestContactPointService(t *testing.T) { sqlStore := db.InitTestDB(t) secretsService := manager.SetupTestService(t, database.ProvideSecretsStore(sqlStore)) + + redactedUser := &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{ + 1: { + accesscontrol.ActionAlertingProvisioningRead: nil, + }, + }} + t.Run("service gets contact points from AM config", func(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) - cps, err := sut.GetContactPoints(context.Background(), cpsQuery(1), nil) + cps, err := sut.GetContactPoints(context.Background(), cpsQuery(1), redactedUser) require.NoError(t, err) require.Len(t, cps, 2) @@ -47,7 +53,7 @@ func TestContactPointService(t *testing.T) { t.Run("service filters contact points by name", func(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) - cps, err := sut.GetContactPoints(context.Background(), cpsQueryWithName(1, "slack receiver"), nil) + cps, err := sut.GetContactPoints(context.Background(), cpsQueryWithName(1, "slack receiver"), redactedUser) require.NoError(t, err) require.Len(t, cps, 1) @@ -57,7 +63,7 @@ func TestContactPointService(t *testing.T) { t.Run("service filters contact points by name, returns empty when no match", func(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) - cps, err := sut.GetContactPoints(context.Background(), cpsQueryWithName(1, "unknown"), nil) + cps, err := sut.GetContactPoints(context.Background(), cpsQueryWithName(1, "unknown"), redactedUser) require.NoError(t, err) require.Len(t, cps, 0) @@ -70,7 +76,7 @@ func TestContactPointService(t *testing.T) { _, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) require.NoError(t, err) - cps, err := sut.GetContactPoints(context.Background(), cpsQuery(1), nil) + cps, err := sut.GetContactPoints(context.Background(), cpsQuery(1), redactedUser) require.NoError(t, err) require.Len(t, cps, 3) require.Equal(t, "test-contact-point", cps[2].Name) @@ -86,7 +92,7 @@ func TestContactPointService(t *testing.T) { _, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) require.NoError(t, err) - cps, err := sut.GetContactPoints(context.Background(), cpsQueryWithName(1, newCp.Name), nil) + cps, err := sut.GetContactPoints(context.Background(), cpsQueryWithName(1, newCp.Name), redactedUser) require.NoError(t, err) require.Len(t, cps, 1) require.Equal(t, customUID, cps[0].UID) @@ -164,7 +170,7 @@ func TestContactPointService(t *testing.T) { t.Run("default provenance of contact points is none", func(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) - cps, err := sut.GetContactPoints(context.Background(), cpsQuery(1), nil) + cps, err := sut.GetContactPoints(context.Background(), cpsQuery(1), redactedUser) require.NoError(t, err) require.Equal(t, models.ProvenanceNone, models.Provenance(cps[0].Provenance)) @@ -222,7 +228,7 @@ func TestContactPointService(t *testing.T) { newCp, err := sut.CreateContactPoint(context.Background(), 1, newCp, test.from) require.NoError(t, err) - cps, err := sut.GetContactPoints(context.Background(), cpsQueryWithName(1, newCp.Name), nil) + cps, err := sut.GetContactPoints(context.Background(), cpsQueryWithName(1, newCp.Name), redactedUser) require.NoError(t, err) require.Equal(t, newCp.UID, cps[0].UID) require.Equal(t, test.from, models.Provenance(cps[0].Provenance)) @@ -231,7 +237,7 @@ func TestContactPointService(t *testing.T) { if test.errNil { require.NoError(t, err) - cps, err = sut.GetContactPoints(context.Background(), cpsQueryWithName(1, newCp.Name), nil) + cps, err = sut.GetContactPoints(context.Background(), cpsQueryWithName(1, newCp.Name), redactedUser) require.NoError(t, err) require.Equal(t, newCp.UID, cps[0].UID) require.Equal(t, test.to, models.Provenance(cps[0].Provenance)) @@ -260,22 +266,23 @@ func TestContactPointService(t *testing.T) { func TestContactPointServiceDecryptRedact(t *testing.T) { secretsService := manager.SetupTestService(t, database.ProvideSecretsStore(db.InitTestDB(t))) - receiverServiceWithAC := func(ecp *ContactPointService) *notifier.ReceiverService { - return notifier.NewReceiverService( - acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()), - // Get won't use the sut's config store, so we can use a different one here. - fakes.NewFakeAlertmanagerConfigStore(createEncryptedConfig(t, secretsService)), - ecp.provenanceStore, - ecp.encryptionService, - ecp.xact, - log.NewNopLogger(), - ) - } + + redactedUser := &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{ + 1: { + accesscontrol.ActionAlertingProvisioningRead: nil, + }, + }} + + decryptedUser := &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{ + 1: { + accesscontrol.ActionAlertingProvisioningReadSecrets: nil, + }, + }} t.Run("GetContactPoints gets redacted contact points by default", func(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) - cps, err := sut.GetContactPoints(context.Background(), cpsQuery(1), nil) + cps, err := sut.GetContactPoints(context.Background(), cpsQuery(1), redactedUser) require.NoError(t, err) require.Len(t, cps, 2) @@ -285,16 +292,14 @@ func TestContactPointServiceDecryptRedact(t *testing.T) { t.Run("GetContactPoints errors when Decrypt = true and user does not have permissions", func(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) - sut.receiverService = receiverServiceWithAC(sut) q := cpsQuery(1) q.Decrypt = true - _, err := sut.GetContactPoints(context.Background(), q, nil) + _, err := sut.GetContactPoints(context.Background(), q, redactedUser) require.ErrorIs(t, err, ErrPermissionDenied) }) t.Run("GetContactPoints errors when Decrypt = true and user is nil", func(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) - sut.receiverService = receiverServiceWithAC(sut) q := cpsQuery(1) q.Decrypt = true @@ -304,16 +309,11 @@ func TestContactPointServiceDecryptRedact(t *testing.T) { t.Run("GetContactPoints gets decrypted contact points when Decrypt = true and user has permissions", func(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) - sut.receiverService = receiverServiceWithAC(sut) expectedName := "slack receiver" q := cpsQueryWithName(1, expectedName) q.Decrypt = true - cps, err := sut.GetContactPoints(context.Background(), q, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{ - 1: { - accesscontrol.ActionAlertingProvisioningReadSecrets: nil, - }, - }}) + cps, err := sut.GetContactPoints(context.Background(), q, decryptedUser) require.NoError(t, err) require.Len(t, cps, 1) @@ -361,7 +361,7 @@ func createContactPointServiceSut(t *testing.T, secretService secrets.Service) * provisioningStore := fakes.NewFakeProvisioningStore() receiverService := notifier.NewReceiverService( - actest.FakeAccessControl{}, + acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()), store, provisioningStore, secretService, diff --git a/pkg/services/provisioning/alerting/contact_point_provisioner.go b/pkg/services/provisioning/alerting/contact_point_provisioner.go index 51faafd20e6..b0fca535d6a 100644 --- a/pkg/services/provisioning/alerting/contact_point_provisioner.go +++ b/pkg/services/provisioning/alerting/contact_point_provisioner.go @@ -37,7 +37,7 @@ func (c *defaultContactPointProvisioner) Provision(ctx context.Context, if _, exists := cpsCache[contactPointsConfig.OrgID]; !exists { cps, err := c.contactPointService.GetContactPoints(ctx, provisioning.ContactPointQuery{ OrgID: contactPointsConfig.OrgID, - }, nil) + }, provisionerUser(contactPointsConfig.OrgID)) if err != nil { return err }