From d5fd6aceca72d3ef0f7efb610ba6761b7a57e095 Mon Sep 17 00:00:00 2001 From: Matthew Jacobson Date: Thu, 29 Aug 2024 14:48:59 -0400 Subject: [PATCH] Alerting: Stop redacting receivers by default in receiver_svc (#92631) * Stop redacting receivers by default in receiver_svc [REDACTED] is only used in provisioning API since response doesn't include SecureFields. This is not necessary in k8s or notifications api, instead we do not include the encrypted settings in Settings at all, leaving it to SecureFields to specify when a secure field exists. * Capitalize logs messages --- .../ngalert/api/api_notifications_test.go | 2 +- pkg/services/ngalert/notifier/receiver_svc.go | 46 ++++++++++--------- .../ngalert/notifier/receiver_svc_test.go | 8 +++- .../ngalert/provisioning/contactpoints.go | 6 +++ 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/pkg/services/ngalert/api/api_notifications_test.go b/pkg/services/ngalert/api/api_notifications_test.go index c7f4bf542e4..b9ef77ecf42 100644 --- a/pkg/services/ngalert/api/api_notifications_test.go +++ b/pkg/services/ngalert/api/api_notifications_test.go @@ -326,7 +326,7 @@ func TestRouteGetReceiversResponses(t *testing.T) { }) t.Run("json body content is as expected", func(t *testing.T) { - expectedRedactedResponse := `{"name":"multiple integrations","grafana_managed_receiver_configs":[{"uid":"c2090fda-f824-4add-b545-5a4d5c2ef082","name":"multiple integrations","type":"prometheus-alertmanager","disableResolveMessage":true,"settings":{"basicAuthPassword":"[REDACTED]","basicAuthUser":"test","url":"http://localhost:9093"},"secureFields":{"basicAuthPassword":true}},{"uid":"c84539ec-f87e-4fc5-9a91-7a687d34bbd1","name":"multiple integrations","type":"discord","disableResolveMessage":false,"settings":{"avatar_url":"some avatar","url":"[REDACTED]","use_discord_username":true},"secureFields":{"url":true}}]}` + expectedRedactedResponse := `{"name":"multiple integrations","grafana_managed_receiver_configs":[{"uid":"c2090fda-f824-4add-b545-5a4d5c2ef082","name":"multiple integrations","type":"prometheus-alertmanager","disableResolveMessage":true,"settings":{"basicAuthUser":"test","url":"http://localhost:9093"},"secureFields":{"basicAuthPassword":true}},{"uid":"c84539ec-f87e-4fc5-9a91-7a687d34bbd1","name":"multiple integrations","type":"discord","disableResolveMessage":false,"settings":{"avatar_url":"some avatar","use_discord_username":true},"secureFields":{"url":true}}]}` expectedDecryptedResponse := `{"name":"multiple integrations","grafana_managed_receiver_configs":[{"uid":"c2090fda-f824-4add-b545-5a4d5c2ef082","name":"multiple integrations","type":"prometheus-alertmanager","disableResolveMessage":true,"settings":{"basicAuthPassword":"testpass","basicAuthUser":"test","url":"http://localhost:9093"},"secureFields":{"basicAuthPassword":true}},{"uid":"c84539ec-f87e-4fc5-9a91-7a687d34bbd1","name":"multiple integrations","type":"discord","disableResolveMessage":false,"settings":{"avatar_url":"some avatar","url":"some url","use_discord_username":true},"secureFields":{"url":true}}]}` t.Run("decrypt false", func(t *testing.T) { env := createTestEnv(t, testContactPointConfig) diff --git a/pkg/services/ngalert/notifier/receiver_svc.go b/pkg/services/ngalert/notifier/receiver_svc.go index edc07fa78ca..73a0ea3fd34 100644 --- a/pkg/services/ngalert/notifier/receiver_svc.go +++ b/pkg/services/ngalert/notifier/receiver_svc.go @@ -130,7 +130,17 @@ func (rs *ReceiverService) GetReceiver(ctx context.Context, q models.GetReceiver return nil, err } - rs.decryptOrRedactSecureSettings(ctx, rcv, q.Decrypt) + if q.Decrypt { + err := rcv.Decrypt(rs.decryptor(ctx)) + if err != nil { + rs.log.Warn("Failed to decrypt secure settings", "name", rcv.Name, "error", err) + } + } else { + err := rcv.Encrypt(rs.encryptor(ctx)) + if err != nil { + rs.log.Warn("Failed to encrypt secure settings", "name", rcv.Name, "error", err) + } + } return rcv, nil } @@ -167,8 +177,18 @@ func (rs *ReceiverService) GetReceivers(ctx context.Context, q models.GetReceive return nil, err } - for _, r := range filtered { - rs.decryptOrRedactSecureSettings(ctx, r, q.Decrypt) + for _, rcv := range filtered { + if q.Decrypt { + err := rcv.Decrypt(rs.decryptor(ctx)) + if err != nil { + rs.log.Warn("Failed to decrypt secure settings", "name", rcv.Name, "error", err) + } + } else { + err := rcv.Encrypt(rs.encryptor(ctx)) + if err != nil { + rs.log.Warn("Failed to encrypt secure settings", "name", rcv.Name, "error", err) + } + } } return limitOffset(filtered, q.Offset, q.Limit), nil @@ -260,7 +280,7 @@ func (rs *ReceiverService) DeleteReceiver(ctx context.Context, uid string, calle return err } } else { - rs.log.Debug("ignoring optimistic concurrency check because version was not provided", "receiver", existing.Name, "operation", "delete") + rs.log.Debug("Ignoring optimistic concurrency check because version was not provided", "receiver", existing.Name, "operation", "delete") } if err := rs.provenanceValidator(existing.Provenance, models.Provenance(callerProvenance)); err != nil { @@ -460,17 +480,6 @@ func (rs *ReceiverService) deleteProvenances(ctx context.Context, orgID int64, i return nil } -func (rs *ReceiverService) decryptOrRedactSecureSettings(ctx context.Context, recv *models.Receiver, decrypt bool) { - if decrypt { - err := recv.Decrypt(rs.decryptor(ctx)) - if err != nil { - rs.log.Warn("failed to decrypt secure settings", "name", recv.Name, "error", err) - } - } else { - recv.Redact(rs.redactor()) - } -} - // decryptor returns a models.DecryptFn that decrypts a secure setting. If decryption fails, the fallback value is used. func (rs *ReceiverService) decryptor(ctx context.Context) models.DecryptFn { return func(value string) (string, error) { @@ -486,13 +495,6 @@ func (rs *ReceiverService) decryptor(ctx context.Context) models.DecryptFn { } } -// redactor returns a models.RedactFn that redacts a secure setting. -func (rs *ReceiverService) redactor() models.RedactFn { - return func(value string) string { - return definitions.RedactedValue - } -} - // encryptor creates an encrypt function that delegates to secrets.Service and returns the base64 encoded result. func (rs *ReceiverService) encryptor(ctx context.Context) models.EncryptFn { return func(payload string) (string, error) { diff --git a/pkg/services/ngalert/notifier/receiver_svc_test.go b/pkg/services/ngalert/notifier/receiver_svc_test.go index b2ba2cca5c9..ce4bcdef792 100644 --- a/pkg/services/ngalert/notifier/receiver_svc_test.go +++ b/pkg/services/ngalert/notifier/receiver_svc_test.go @@ -179,8 +179,14 @@ func TestReceiverService_DecryptRedact(t *testing.T) { require.NoError(t, err) if tc.decrypt { require.Equal(t, "secure url", res.Integrations[0].Settings["url"]) + require.NotContains(t, res.Integrations[0].SecureSettings, "url") } else { - require.Equal(t, definitions.RedactedValue, res.Integrations[0].Settings["url"]) + require.NotContains(t, res.Integrations[0].Settings, "url") + + // Ensure the encrypted value exists and is not redacted or decrypted. + require.NotEmpty(t, res.Integrations[0].SecureSettings["url"]) + require.NotEqual(t, definitions.RedactedValue, res.Integrations[0].SecureSettings["url"]) + require.NotEqual(t, "secure url", res.Integrations[0].SecureSettings["url"]) } } }) diff --git a/pkg/services/ngalert/provisioning/contactpoints.go b/pkg/services/ngalert/provisioning/contactpoints.go index ba9026f5acc..d7e4ec26180 100644 --- a/pkg/services/ngalert/provisioning/contactpoints.go +++ b/pkg/services/ngalert/provisioning/contactpoints.go @@ -86,6 +86,12 @@ func (ecp *ContactPointService) GetContactPoints(ctx context.Context, q ContactP contactPoints := make([]apimodels.EmbeddedContactPoint, 0, len(res)) for _, recv := range res { for _, gr := range recv.Integrations { + if !q.Decrypt { + // Provisioning API redacts by default. + gr.Redact(func(value string) string { + return apimodels.RedactedValue + }) + } contactPoints = append(contactPoints, GrafanaIntegrationConfigToEmbeddedContactPoint(gr, recv.Provenance)) } }