IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Alerting: Fix contact point export 500 error and notifications/receivers missing settings (#90342)

Browse Source 
* Regression test

* Fix 500 error when exporting redacted receivers

* Fix tests to check permissions
This commit is contained in:
Matthew Jacobson 2024-07-12 11:42:22 -04:00 committed by GitHub
parent 5d8ca38b9b
commit b7767c79e7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 130 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/services/ngalert/api/api_provisioning.go
View File

@ -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, "")

37
pkg/services/ngalert/api/api_provisioning_test.go
View File

@ -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)

53
pkg/services/ngalert/notifier/receiver_svc.go
View File

@ -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 {

25
pkg/services/ngalert/notifier/receiver_svc_test.go
View File

@ -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,

10
pkg/services/ngalert/provisioning/compat.go
View File

@ -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 {

62
pkg/services/ngalert/provisioning/contactpoints_test.go
View File

@ -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,

2
pkg/services/provisioning/alerting/contact_point_provisioner.go
View File

@ -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
}