From 0016b574860ea7aa3ed52bfb9df4ebc7c3a9d0e1 Mon Sep 17 00:00:00 2001 From: Matthew Jacobson Date: Thu, 12 Jun 2025 17:00:09 -0400 Subject: [PATCH] Alerting: Add OAuth2 Support for Webhook Receiver (#106302) * Add to available channels * Export * Fix bug in deeply nested secrets BE: Slice re-use bug when traversing deeply. FE: Only at most one level of nesting was being taken into account when determining secureFields keys. This change adds a new field on NotificationChannelOption: secureFieldKey. This is populated on API GET via transform. This change gives us the option to hardcode secureFieldKey in the backend and no longer calculate the key via settings topology. * Update grafana/alerting to 3e20fda3b872 * Prettier * Linting * Fix IntegrationConfig test to catch secure field mismatch --- go.mod | 2 +- go.sum | 4 +- .../api/tooling/definitions/contact_points.go | 36 +++ pkg/services/ngalert/models/receivers.go | 13 +- pkg/services/ngalert/models/receivers_test.go | 9 +- .../channels_config/available_channels.go | 203 +++++++++++--- .../available_channels_test.go | 13 +- .../alerting/unified/api/alertmanagerApi.ts | 21 +- .../receivers/form/ChannelOptions.tsx | 10 +- .../receivers/form/ChannelSubForm.tsx | 29 +- .../form/GrafanaReceiverForm.test.tsx | 139 +++++++++- .../GrafanaReceiverForm.test.tsx.snap | 26 +- .../receivers/form/fields/OptionField.tsx | 15 +- .../receivers/form/fields/SubformField.tsx | 6 +- .../alerting/unified/mockGrafanaNotifiers.ts | 252 ++++++++++++++++++ public/app/types/alerting.ts | 1 + 16 files changed, 697 insertions(+), 82 deletions(-) diff --git a/go.mod b/go.mod index 6f85332903a..b5d8de4cb23 100644 --- a/go.mod +++ b/go.mod @@ -77,7 +77,7 @@ require ( github.com/googleapis/gax-go/v2 v2.14.1 // @grafana/grafana-backend-group github.com/gorilla/mux v1.8.1 // @grafana/grafana-backend-group github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // @grafana/grafana-app-platform-squad - github.com/grafana/alerting v0.0.0-20250605155607-02235095d018 // @grafana/alerting-backend + github.com/grafana/alerting v0.0.0-20250610043455-3e20fda3b872 // @grafana/alerting-backend github.com/grafana/authlib v0.0.0-20250515162837-2f4a8263eabb // @grafana/identity-access-team github.com/grafana/authlib/types v0.0.0-20250325095148-d6da9c164a7d // @grafana/identity-access-team github.com/grafana/dataplane/examples v0.0.1 // @grafana/observability-metrics diff --git a/go.sum b/go.sum index e8c24ae49bc..2d4348b27c0 100644 --- a/go.sum +++ b/go.sum @@ -1569,8 +1569,8 @@ github.com/gorilla/sessions v1.2.1 h1:DHd3rPN5lE3Ts3D8rKkQ8x/0kqfeNmBAaiSi+o7Fsg github.com/gorilla/sessions v1.2.1/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM= github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 h1:JeSE6pjso5THxAzdVpqr6/geYxZytqFMBCOtn/ujyeo= github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674/go.mod h1:r4w70xmWCQKmi1ONH4KIaBptdivuRPyosB9RmPlGEwA= -github.com/grafana/alerting v0.0.0-20250605155607-02235095d018 h1:sGEBflMw3DUsfV4SCl1at+QRwGi5h29iiAuk8ZZn5Mw= -github.com/grafana/alerting v0.0.0-20250605155607-02235095d018/go.mod h1:e2nDocz4yTgLYGJFMrlL3DTjdMTEYPABorKc191b8/w= +github.com/grafana/alerting v0.0.0-20250610043455-3e20fda3b872 h1:bhUBPxMHtQB+AJaPcASElgwC7WFjWQvEiD3FoMEAv0E= +github.com/grafana/alerting v0.0.0-20250610043455-3e20fda3b872/go.mod h1:VANfU4LMnOR4Gv4hkCN9fGyQ/8Lf2DKOvzG70vI2IS4= github.com/grafana/authlib v0.0.0-20250515162837-2f4a8263eabb h1:oTl2j6/4miQUYmXANp2pBuYCWA5f8NVYFfCWpczpFso= github.com/grafana/authlib v0.0.0-20250515162837-2f4a8263eabb/go.mod h1:PBtQaXwkFu4BAt2aXsR7w8p8NVpdjV5aJYhqRDei9Us= github.com/grafana/authlib/types v0.0.0-20250325095148-d6da9c164a7d h1:34E6btDAhdDOiSEyrMaYaHwnJpM8w9QKzVQZIBzLNmM= diff --git a/pkg/services/ngalert/api/tooling/definitions/contact_points.go b/pkg/services/ngalert/api/tooling/definitions/contact_points.go index 9acbe144e88..7f98ad65c92 100644 --- a/pkg/services/ngalert/api/tooling/definitions/contact_points.go +++ b/pkg/services/ngalert/api/tooling/definitions/contact_points.go @@ -324,6 +324,7 @@ type WebhookIntegration struct { Message *string `json:"message,omitempty" yaml:"message,omitempty" hcl:"message"` TLSConfig *TLSConfig `json:"tlsConfig,omitempty" yaml:"tlsConfig,omitempty" hcl:"tlsConfig,block"` HMACConfig *HMACConfig `json:"hmacConfig,omitempty" yaml:"hmacConfig,omitempty" hcl:"hmacConfig,block"` + HTTPConfig *HTTPClientConfig `json:"http_config,omitempty" yaml:"http_config,omitempty" hcl:"http_config,block"` Payload *CustomPayload `json:"payload,omitempty" yaml:"payload,omitempty" hcl:"payload,block"` } @@ -343,6 +344,41 @@ type HMACConfig struct { TimestampHeader string `yaml:"timestampHeader,omitempty" json:"timestampHeader,omitempty" hcl:"timestamp_header"` } +// HTTPClientConfig holds common configurations for notifier HTTP clients. +type HTTPClientConfig struct { + OAuth2Config *OAuth2Config `json:"oauth2,omitempty" yaml:"oauth2,omitempty" hcl:"oauth2,block"` +} + +type ProxyConfig struct { + // ProxyURL is the HTTP proxy server to use to connect to the targets. + ProxyURL *string `yaml:"proxy_url,omitempty" json:"proxy_url,omitempty" hcl:"proxy_url"` + // NoProxy contains addresses that should not use a proxy. + NoProxy *string `yaml:"no_proxy,omitempty" json:"no_proxy,omitempty" hcl:"no_proxy"` + // ProxyFromEnvironment uses environment HTTP_PROXY, HTTPS_PROXY and NO_PROXY to determine proxies. + ProxyFromEnvironment *bool `yaml:"proxy_from_environment,omitempty" json:"proxy_from_environment,omitempty" hcl:"proxy_from_environment"` + // ProxyConnectHeader optionally specifies headers to send to proxies during CONNECT requests. + ProxyConnectHeader *map[string]string `yaml:"proxy_connect_header,omitempty" json:"proxy_connect_header,omitempty" hcl:"proxy_connect_header"` +} + +type OAuth2Config struct { + // ClientID is the OAuth2 client ID. + ClientID string `json:"client_id" yaml:"client_id" hcl:"client_id"` + // ClientSecret is the OAuth2 client secret. + ClientSecret *Secret `json:"client_secret" yaml:"client_secret" hcl:"client_secret"` + // TokenURL is the URL to get the OAuth2 token. + TokenURL string `json:"token_url" yaml:"token_url" hcl:"token_url"` + + // Scopes is the optional list of OAuth2 scopes. + Scopes *[]string `json:"scopes,omitempty" yaml:"scopes,omitempty" hcl:"scopes"` + // EndpointParams is the optional map of additional parameters to include in the token request. + EndpointParams *map[string]string `json:"endpoint_params,omitempty" yaml:"endpoint_params,omitempty" hcl:"endpoint_params"` + // TLSConfig is the optional TLS configuration to use for the OAuth2 token request. + TLSConfig *TLSConfig `json:"tls_config,omitempty" yaml:"tls_config,omitempty" hcl:"tls_config,block"` + + // ProxyConfig is the optional proxy configuration to use for the OAuth2 token request. + ProxyConfig *ProxyConfig `json:"proxy_config,omitempty" yaml:"proxy_config,omitempty" hcl:"proxy_config,block"` +} + type WecomIntegration struct { DisableResolveMessage *bool `json:"-" yaml:"-" hcl:"disable_resolve_message"` diff --git a/pkg/services/ngalert/models/receivers.go b/pkg/services/ngalert/models/receivers.go index 60d95aaad04..5e1a16485ee 100644 --- a/pkg/services/ngalert/models/receivers.go +++ b/pkg/services/ngalert/models/receivers.go @@ -191,8 +191,12 @@ func (f IntegrationFieldPath) String() string { return strings.Join(f, ".") } -func (f IntegrationFieldPath) Append(segment string) IntegrationFieldPath { - return append(f, segment) +func (f IntegrationFieldPath) With(segment string) IntegrationFieldPath { + // Copy the existing path to avoid modifying the original slice. + newPath := make(IntegrationFieldPath, len(f)+1) + copy(newPath, f) + newPath[len(newPath)-1] = segment + return newPath } // IntegrationConfigFromType returns an integration configuration for a given integration type. If the integration type is @@ -250,11 +254,12 @@ func (config *IntegrationConfig) GetSecretFields() []IntegrationFieldPath { func traverseFields(flds map[string]IntegrationField, parentPath IntegrationFieldPath, predicate func(i IntegrationField) bool) []IntegrationFieldPath { var result []IntegrationFieldPath for key, field := range flds { + path := parentPath.With(key) if predicate(field) { - result = append(result, parentPath.Append(key)) + result = append(result, path) } if len(field.Fields) > 0 { - result = append(result, traverseFields(field.Fields, parentPath.Append(key), predicate)...) + result = append(result, traverseFields(field.Fields, path, predicate)...) } } return result diff --git a/pkg/services/ngalert/models/receivers_test.go b/pkg/services/ngalert/models/receivers_test.go index d83f346f0ae..9ca835e7fe9 100644 --- a/pkg/services/ngalert/models/receivers_test.go +++ b/pkg/services/ngalert/models/receivers_test.go @@ -244,11 +244,14 @@ func TestIntegrationConfig(t *testing.T) { allSecrets[key] = struct{}{} } - for field := range config.Fields { - _, isSecret := allSecrets[field] - assert.Equalf(t, isSecret, config.IsSecureField(NewIntegrationFieldPath(field)), "field '%s' is expected to be secret", field) + secretFields := config.GetSecretFields() + for _, path := range secretFields { + _, isSecret := allSecrets[path.String()] + assert.Equalf(t, isSecret, config.IsSecureField(path), "field '%s' is expected to be secret", path) + delete(allSecrets, path.String()) } assert.False(t, config.IsSecureField(IntegrationFieldPath{"__--**unknown_field**--__"})) + assert.Empty(t, allSecrets, "mismatched secret fields for integration type %s: %v", integrationType, allSecrets) }) } diff --git a/pkg/services/ngalert/notifier/channels_config/available_channels.go b/pkg/services/ngalert/notifier/channels_config/available_channels.go index 5b70d7f58fa..b1df07f320a 100644 --- a/pkg/services/ngalert/notifier/channels_config/available_channels.go +++ b/pkg/services/ngalert/notifier/channels_config/available_channels.go @@ -113,6 +113,163 @@ func GetAvailableNotifiers() []*NotifierPlugin { }, } + tlsSubformOptions := func() []NotifierOption { + return []NotifierOption{ + { + Label: "Disable certificate verification", + Element: ElementTypeCheckbox, + Description: "Do not verify the server's certificate chain and host name.", + PropertyName: "insecureSkipVerify", + Required: false, + }, + { + Label: "CA Certificate", + Element: ElementTypeTextArea, + Description: "Certificate in PEM format to use when verifying the server's certificate chain.", + InputType: InputTypeText, + PropertyName: "caCertificate", + Required: false, + Secure: true, + }, + { + Label: "Client Certificate", + Element: ElementTypeTextArea, + Description: "Client certificate in PEM format to use when connecting to the server.", + InputType: InputTypeText, + PropertyName: "clientCertificate", + Required: false, + Secure: true, + }, + { + Label: "Client Key", + Element: ElementTypeTextArea, + Description: "Client key in PEM format to use when connecting to the server.", + InputType: InputTypeText, + PropertyName: "clientKey", + Required: false, + Secure: true, + }, + } + } + + proxyOption := func() NotifierOption { + return NotifierOption{ // New in 12.1. + Label: "Proxy Config", + PropertyName: "proxy_config", + Description: "Optional proxy configuration.", + Element: ElementTypeSubform, + SubformOptions: []NotifierOption{ + { + Label: "Proxy URL", + PropertyName: "proxy_url", + Description: "HTTP proxy server to use to connect to the targets.", + Element: ElementTypeInput, + InputType: InputTypeText, + Placeholder: "https://proxy.example.com", + Required: false, + Secure: false, + }, + { + Label: "Proxy from environment", + PropertyName: "proxy_from_environment", + Description: "Use environment HTTP_PROXY, HTTPS_PROXY and NO_PROXY to determine proxies.", + Element: ElementTypeCheckbox, + Required: false, + Secure: false, + }, + { + Label: "No Proxy", + PropertyName: "no_proxy", + Description: "Comma-separated list of addresses that should not use a proxy.", + Element: ElementTypeInput, + InputType: InputTypeText, + Placeholder: "example.com,1.2.3.4", + Required: false, + Secure: false, + }, + { + Label: "Proxy Connect Header", + PropertyName: "proxy_connect_header", + Description: "Optional headers to send to proxies during CONNECT requests.", + Element: ElementTypeKeyValueMap, + InputType: InputTypeText, + Required: false, + Secure: false, + }, + }, + } + } + + commonHttpClientOption := func() NotifierOption { + return NotifierOption{ // New in 12.1. + Label: "HTTP Config", + PropertyName: "http_config", + Description: "Common HTTP client options.", + Element: ElementTypeSubform, + SubformOptions: []NotifierOption{ + { // New in 12.1. + Label: "OAuth2", + PropertyName: "oauth2", + Description: "OAuth2 configuration options", + Element: ElementTypeSubform, + SubformOptions: []NotifierOption{ + { + Label: "Token URL", + PropertyName: "token_url", + Element: ElementTypeInput, + Description: "URL for the access token endpoint.", + InputType: InputTypeText, + Required: true, + Secure: false, + }, + { + Label: "Client ID", + PropertyName: "client_id", + Element: ElementTypeInput, + Description: "Client ID to use when authenticating.", + InputType: InputTypeText, + Required: true, + Secure: false, + }, + { + Label: "Client Secret", + PropertyName: "client_secret", + Element: ElementTypeInput, + Description: "Client secret to use when authenticating.", + InputType: InputTypeText, + Required: true, + Secure: true, + }, + { + Label: "Scopes", + PropertyName: "scopes", + Element: ElementStringArray, + Description: "Optional scopes to request when obtaining an access token.", + Required: false, + Secure: false, + }, + { + Label: "Endpoint Parameters", + PropertyName: "endpoint_params", + Element: ElementTypeKeyValueMap, + Description: "Optional parameters to append to the access token request.", + Required: false, + Secure: false, + }, + { + Label: "TLS", + PropertyName: "tls_config", + Description: "Optional TLS configuration options for OAuth2 requests.", + Element: ElementTypeSubform, + SubformOptions: tlsSubformOptions(), + }, + proxyOption(), + }, + }, + }, + } + } + return []*NotifierPlugin{ { Type: "dingding", @@ -1006,46 +1163,11 @@ func GetAvailableNotifiers() []*NotifierPlugin { }, { - Label: "TLS", - PropertyName: "tlsConfig", - Description: "TLS configuration options", - Element: ElementTypeSubform, - SubformOptions: []NotifierOption{ - { - Label: "Disable certificate verification", - Element: ElementTypeCheckbox, - Description: "Do not verify the server's certificate chain and host name.", - PropertyName: "insecureSkipVerify", - Required: false, - }, - { - Label: "CA Certificate", - Element: ElementTypeTextArea, - Description: "Certificate in PEM format to use when verifying the server's certificate chain.", - InputType: InputTypeText, - PropertyName: "caCertificate", - Required: false, - Secure: true, - }, - { - Label: "Client Certificate", - Element: ElementTypeTextArea, - Description: "Client certificate in PEM format to use when connecting to the server.", - InputType: InputTypeText, - PropertyName: "clientCertificate", - Required: false, - Secure: true, - }, - { - Label: "Client Key", - Element: ElementTypeTextArea, - Description: "Client key in PEM format to use when connecting to the server.", - InputType: InputTypeText, - PropertyName: "clientKey", - Required: false, - Secure: true, - }, - }, + Label: "TLS", + PropertyName: "tlsConfig", + Description: "TLS configuration options", + Element: ElementTypeSubform, + SubformOptions: tlsSubformOptions(), }, { Label: "HMAC Signature", @@ -1083,6 +1205,7 @@ func GetAvailableNotifiers() []*NotifierPlugin { }, }, }, + commonHttpClientOption(), // New in 12.1. }, }, { diff --git a/pkg/services/ngalert/notifier/channels_config/available_channels_test.go b/pkg/services/ngalert/notifier/channels_config/available_channels_test.go index 0fe4330faf6..ed7564db17d 100644 --- a/pkg/services/ngalert/notifier/channels_config/available_channels_test.go +++ b/pkg/services/ngalert/notifier/channels_config/available_channels_test.go @@ -22,7 +22,18 @@ func TestGetSecretKeysForContactPointType(t *testing.T) { {receiverType: "sensugo", expectedSecretFields: []string{"apikey"}}, {receiverType: "teams", expectedSecretFields: []string{}}, {receiverType: "telegram", expectedSecretFields: []string{"bottoken"}}, - {receiverType: "webhook", expectedSecretFields: []string{"password", "authorization_credentials", "tlsConfig.caCertificate", "tlsConfig.clientCertificate", "tlsConfig.clientKey", "hmacConfig.secret"}}, + {receiverType: "webhook", expectedSecretFields: []string{ + "password", + "authorization_credentials", + "tlsConfig.caCertificate", + "tlsConfig.clientCertificate", + "tlsConfig.clientKey", + "hmacConfig.secret", + "http_config.oauth2.client_secret", + "http_config.oauth2.tls_config.caCertificate", + "http_config.oauth2.tls_config.clientCertificate", + "http_config.oauth2.tls_config.clientKey", + }}, {receiverType: "wecom", expectedSecretFields: []string{"url", "secret"}}, {receiverType: "prometheus-alertmanager", expectedSecretFields: []string{"basicAuthPassword"}}, {receiverType: "discord", expectedSecretFields: []string{"url"}}, diff --git a/public/app/features/alerting/unified/api/alertmanagerApi.ts b/public/app/features/alerting/unified/api/alertmanagerApi.ts index 5f5bb7728c3..0d579e37e53 100644 --- a/public/app/features/alerting/unified/api/alertmanagerApi.ts +++ b/public/app/features/alerting/unified/api/alertmanagerApi.ts @@ -14,7 +14,7 @@ import { GrafanaAlertingConfiguration, Matcher, } from '../../../../plugins/datasource/alertmanager/types'; -import { NotifierDTO } from '../../../../types'; +import { NotificationChannelOption, NotifierDTO } from '../../../../types'; import { withPerformanceLogging } from '../Analytics'; import { matcherToMatcherField } from '../utils/alertmanager'; import { @@ -106,6 +106,25 @@ export const alertmanagerApi = alertingApi.injectEndpoints({ grafanaNotifiers: build.query({ query: () => ({ url: '/api/alert-notifiers' }), + transformResponse: (response: NotifierDTO[]) => { + const populateSecureFieldKey = ( + option: NotificationChannelOption, + prefix: string + ): NotificationChannelOption => ({ + ...option, + secureFieldKey: option.secure && !option.secureFieldKey ? `${prefix}${option.propertyName}` : undefined, + subformOptions: option.subformOptions?.map((suboption) => + populateSecureFieldKey(suboption, `${prefix}${option.propertyName}.`) + ), + }); + + return response.map((notifier) => ({ + ...notifier, + options: notifier.options.map((option) => { + return populateSecureFieldKey(option, ''); + }), + })); + }, }), // this endpoint requires administrator privileges diff --git a/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx b/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx index 0648a964d4a..e402c4f8875 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx @@ -18,7 +18,7 @@ export interface Props { selectedChannelOptions: NotificationChannelOption[]; onResetSecureField: (key: string) => void; - onDeleteSubform?: (propertyName: string) => void; + onDeleteSubform?: (settingsPath: string, option: NotificationChannelOption) => void; errors?: FieldErrors; /** * The path for the integration in the array of integrations. @@ -66,7 +66,7 @@ export function ChannelOptions({ return null; } - if (secureFields && secureFields[option.propertyName]) { + if (secureFields && secureFields[option.secureFieldKey ?? option.propertyName]) { return ( ({ > onResetSecureField(option.propertyName)} + onReset={() => onResetSecureField(option.secureFieldKey ?? option.propertyName)} isConfigured /> @@ -85,7 +85,7 @@ export function ChannelOptions({ const error: FieldError | DeepMap | undefined = ( (option.secure ? errors?.secureFields : errors?.settings) as DeepMap | undefined - )?.[option.propertyName]; + )?.[option.secureFieldKey ?? option.propertyName]; const defaultValue = defaultValues?.settings?.[option.propertyName]; @@ -122,6 +122,7 @@ const determineRequired = ( return option.required ? 'Required' : false; } + // TODO: This doesn't work with nested secureFields. const dependentOn = Boolean(settings[option.dependsOn]) || Boolean(secureFields[option.dependsOn]); if (dependentOn) { @@ -140,5 +141,6 @@ const determineReadOnly = ( return false; } + // TODO: This doesn't work with nested secureFields. return Boolean(settings[option.dependsOn]) || Boolean(secureFields[option.dependsOn]); }; diff --git a/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx b/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx index e06d26bc4ed..7e47f08b5a2 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx @@ -2,11 +2,12 @@ import { css } from '@emotion/css'; import { sortBy } from 'lodash'; import * as React from 'react'; import { useEffect, useMemo } from 'react'; -import { Controller, FieldErrors, useFormContext, useWatch } from 'react-hook-form'; +import { Controller, FieldErrors, useFormContext } from 'react-hook-form'; import { GrafanaTheme2, SelectableValue } from '@grafana/data'; import { Trans, t } from '@grafana/i18n'; import { Alert, Button, Field, Select, Stack, Text, useStyles2 } from '@grafana/ui'; +import { NotificationChannelOption } from 'app/types'; import { useUnifiedAlertingSelector } from '../../../hooks/useUnifiedAlertingSelector'; import { @@ -99,9 +100,6 @@ export function ChannelSubForm({ return () => subscription.unsubscribe(); }, [selectedType, initialValues, setValue, settingsFieldPath, typeFieldPath, watch]); - // const [_secureFields, setSecureFields] = useState>(secureFields ?? {}); - const formSecureFields = useWatch({ control, name: `${channelFieldPath}.secureFields` }); - const onResetSecureField = (key: string) => { // formSecureFields might not be up to date if this function is called multiple times in a row const currentSecureFields = getValues(`${channelFieldPath}.secureFields`); @@ -110,12 +108,29 @@ export function ChannelSubForm({ } }; - const onDeleteSubform = (propertyName: string) => { - const relatedSecureFields = Object.keys(formSecureFields).filter((key) => key.startsWith(propertyName)); + const findSecureFieldsRecursively = (options: NotificationChannelOption[]): string[] => { + const secureFields: string[] = []; + options?.forEach((option) => { + if (option.secure && option.secureFieldKey) { + secureFields.push(option.secureFieldKey); + } + if (option.subformOptions) { + secureFields.push(...findSecureFieldsRecursively(option.subformOptions)); + } + }); + return secureFields; + }; + + const onDeleteSubform = (settingsPath: string, option: NotificationChannelOption) => { + // Get all subform options with secure=true recursively. + const relatedSecureFields = findSecureFieldsRecursively(option.subformOptions ?? []); relatedSecureFields.forEach((key) => { onResetSecureField(key); }); - setValue(`${channelFieldPath}.settings.${propertyName}`, undefined); + const fieldPath = settingsPath.startsWith(`${channelFieldPath}.settings.`) + ? settingsPath.slice(`${channelFieldPath}.settings.`.length) + : settingsPath; + setValue(`${settingsFieldPath}.${fieldPath}`, undefined); }; const typeOptions = useMemo( diff --git a/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx b/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx index f8962f5c174..937c64f4afb 100644 --- a/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx @@ -65,12 +65,26 @@ const ui = { webhook: { url: byRole('textbox', { name: /^URL/ }), tlsConfig: { - header: byRole('heading', { name: /TLS/ }), - caCertificate: byRole('textbox', { name: /^CA certificate/ }), - clientCert: byRole('textbox', { name: /^Client certificate/ }), - clientKey: byRole('textbox', { name: /^Client key/ }), + container: byTestId('items.0.settings.tlsConfig.container'), + caCertificate: byRole('textbox', { name: /^CA Certificate/ }), + clientCert: byRole('textbox', { name: /^Client Certificate/ }), + clientKey: byRole('textbox', { name: /^Client Key/ }), deleteButton: byTestId('items.0.settings.tlsConfig.delete-button'), }, + httpConfig: { + container: byTestId('items.0.settings.http_config.container'), + oauth2: { + container: byTestId('items.0.settings.http_config.oauth2.container'), + clientSecret: byRole('textbox', { name: /^Client Secret/ }), + tls_config: { + container: byTestId('items.0.settings.http_config.oauth2.tls_config.container'), + caCertificate: byRole('textbox', { name: /^CA Certificate/ }), + clientCert: byRole('textbox', { name: /^Client Certificate/ }), + clientKey: byRole('textbox', { name: /^Client Key/ }), + deleteButton: byTestId('items.0.settings.http_config.oauth2.tls_config.delete-button'), + }, + }, + }, optionalSettings: byRole('button', { name: /optional webhook settings/i }), }, }; @@ -416,6 +430,81 @@ describe('GrafanaReceiverForm', () => { }); describe('Webhook contact point', () => { + it('should mark secure fields as configured when values exist', async () => { + const contactPointName = 'webhook-test'; + const contactPoint = alertingFactory.alertmanager.grafana.contactPoint + .withIntegrations((integrationFactory) => [ + integrationFactory + .webhook() + .params({ + settings: { + url: 'http://example.com', + tlsConfig: { + insecureSkipVerify: false, + }, + http_config: { + oauth2: { + client_id: 'client-id', + token_url: 'http://example.com/oauth2/token', + scopes: ['scope1', 'scope2'], + endpoint_params: { + param1: 'value1', + param2: 'value2', + }, + tls_config: { + insecureSkipVerify: false, + }, + proxy_config: { + proxy_url: 'http://example.com/proxy', + no_proxy: 'example.com', + proxy_from_environment: true, + proxy_connect_header: { + 'X-Custom-Header': 'custom-value', + }, + }, + }, + }, + }, + secureFields: { + 'tlsConfig.caCertificate': true, + 'tlsConfig.clientCertificate': true, + 'tlsConfig.clientKey': true, + 'http_config.oauth2.client_secret': true, + 'http_config.oauth2.tls_config.caCertificate': true, + 'http_config.oauth2.tls_config.clientCertificate': true, + 'http_config.oauth2.tls_config.clientKey': true, + }, + }) + .build(), + ]) + .build({ id: 'webhook-id', name: contactPointName, metadata: { name: contactPointName } }); + + const { user } = renderWithProvider(); + + await waitFor(() => expect(ui.loadingIndicator.query()).not.toBeInTheDocument()); + await waitFor(() => expect(ui.webhook.optionalSettings.query()).toBeInTheDocument()); + await user.click(ui.webhook.optionalSettings.get()); + + const tlsContainer = await ui.webhook.tlsConfig.container.find(); + const caCertField = ui.webhook.tlsConfig.caCertificate.get(tlsContainer); + const clientCertField = ui.webhook.tlsConfig.clientCert.get(tlsContainer); + const clientKeyField = ui.webhook.tlsConfig.clientKey.get(tlsContainer); + expect(caCertField).toHaveValue('configured'); + expect(clientCertField).toHaveValue('configured'); + expect(clientKeyField).toHaveValue('configured'); + + // Deeply nested secure fields. + const oauth2Container = await ui.webhook.httpConfig.oauth2.container.find(); + const clientSecretField = ui.webhook.httpConfig.oauth2.clientSecret.get(oauth2Container); + const oauthCaCertField = ui.webhook.httpConfig.oauth2.tls_config.caCertificate.get(oauth2Container); + const oauthClientCertField = ui.webhook.httpConfig.oauth2.tls_config.clientCert.get(oauth2Container); + const oauthClientKeyField = ui.webhook.httpConfig.oauth2.tls_config.clientKey.get(oauth2Container); + expect(clientSecretField).toHaveValue('configured'); + expect(oauthCaCertField).toHaveValue('configured'); + expect(oauthClientCertField).toHaveValue('configured'); + expect(oauthClientKeyField).toHaveValue('configured'); + }); + it('should properly remove TLS config when deleted', async () => { const contactPointName = 'webhook-test'; const contactPoint = alertingFactory.alertmanager.grafana.contactPoint @@ -431,6 +520,35 @@ describe('GrafanaReceiverForm', () => { clientKey: 'client-key', insecureSkipVerify: false, }, + http_config: { + oauth2: { + client_id: 'client-id', + token_url: 'http://example.com/oauth2/token', + scopes: ['scope1', 'scope2'], + endpoint_params: { + param1: 'value1', + param2: 'value2', + }, + tls_config: { + // This tls config has existing values via secureFields, delete should remove this correctly as well. + insecureSkipVerify: false, + }, + proxy_config: { + proxy_url: 'http://example.com/proxy', + no_proxy: 'example.com', + proxy_from_environment: true, + proxy_connect_header: { + 'X-Custom-Header': 'custom-value', + }, + }, + }, + }, + }, + secureFields: { + 'http_config.oauth2.client_secret': true, + 'http_config.oauth2.tls_config.caCertificate': true, + 'http_config.oauth2.tls_config.clientCertificate': true, + 'http_config.oauth2.tls_config.clientKey': true, }, }) .build(), @@ -448,9 +566,14 @@ describe('GrafanaReceiverForm', () => { // Find and click the delete button next to TLS config await user.click(ui.webhook.optionalSettings.get()); - expect(await ui.webhook.tlsConfig.header.find(undefined)).toBeInTheDocument(); + // Delete new tlsConfig values. + expect(await ui.webhook.tlsConfig.container.find()).toBeInTheDocument(); await user.click(await ui.webhook.tlsConfig.deleteButton.find()); + // Delete existing oauth2 values. + expect(await ui.webhook.httpConfig.oauth2.tls_config.container.find()).toBeInTheDocument(); + await user.click(await ui.webhook.httpConfig.oauth2.tls_config.deleteButton.find()); + await user.click(ui.saveButton.get()); const requests = await capture; @@ -467,6 +590,12 @@ describe('GrafanaReceiverForm', () => { expect(integrationPayload.secureFields).not.toHaveProperty('tlsConfig.clientCert'); expect(integrationPayload.secureFields).not.toHaveProperty('tlsConfig.clientKey'); + // Verify that OAuth2 TLS config is not present in the settings + expect(integrationPayload.settings).not.toHaveProperty('http_config.oauth2.tls_config'); + expect(integrationPayload.secureFields).not.toHaveProperty('http_config.oauth2.tls_config.caCertificate'); + expect(integrationPayload.secureFields).not.toHaveProperty('http_config.oauth2.tls_config.clientCert'); + expect(integrationPayload.secureFields).not.toHaveProperty('http_config.oauth2.tls_config.clientKey'); + expect(postRequestBody).toMatchSnapshot(); }); }); diff --git a/public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap b/public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap index dd2fba64876..84de2030021 100644 --- a/public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap +++ b/public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap @@ -41,8 +41,32 @@ exports[`GrafanaReceiverForm Webhook contact point should properly remove TLS co { "disableResolveMessage": false, "name": "webhook-test", - "secureFields": {}, + "secureFields": { + "http_config.oauth2.client_secret": true, + }, "settings": { + "http_config": { + "oauth2": { + "client_id": "client-id", + "endpoint_params": { + "param1": "value1", + "param2": "value2", + }, + "proxy_config": { + "no_proxy": "example.com", + "proxy_connect_header": { + "X-Custom-Header": "custom-value", + }, + "proxy_from_environment": true, + "proxy_url": "http://example.com/proxy", + }, + "scopes": [ + "scope1", + "scope2", + ], + "token_url": "http://example.com/oauth2/token", + }, + }, "url": "http://example.com", }, "type": "webhook", diff --git a/public/app/features/alerting/unified/components/receivers/form/fields/OptionField.tsx b/public/app/features/alerting/unified/components/receivers/form/fields/OptionField.tsx index 15a3c62f46e..62db9294dc3 100644 --- a/public/app/features/alerting/unified/components/receivers/form/fields/OptionField.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/fields/OptionField.tsx @@ -26,21 +26,18 @@ interface Props { defaultValue: any; option: NotificationChannelOption; getOptionMeta?: (option: NotificationChannelOption) => OptionMeta; - // this is defined if the option is rendered inside a subform - parentOption?: NotificationChannelOption; invalid?: boolean; pathPrefix: string; error?: FieldError | DeepMap; readOnly?: boolean; customValidator?: (value: string) => boolean | string | Promise; onResetSecureField?: (propertyName: string) => void; - onDeleteSubform?: (propertyName: string) => void; + onDeleteSubform?: (settingsPath: string, option: NotificationChannelOption) => void; secureFields: NotificationChannelSecureFields; } export const OptionField: FC = ({ option, - parentOption, invalid, pathPrefix, error, @@ -94,7 +91,6 @@ export const OptionField: FC = ({ invalid={invalid} pathPrefix={pathPrefix} readOnly={readOnly} - parentOption={parentOption} customValidator={customValidator} onResetSecureField={onResetSecureField} secureFields={secureFields} @@ -113,7 +109,6 @@ const OptionInput: FC = ({ customValidator, onResetSecureField, secureFields = {}, - parentOption, getOptionMeta, }) => { const styles = useStyles2(getStyles); @@ -122,9 +117,9 @@ const OptionInput: FC = ({ const optionMeta = getOptionMeta?.(option); const name = `${pathPrefix}${option.propertyName}`; - const nestedKey = parentOption ? `${parentOption.propertyName}.${option.propertyName}` : option.propertyName; - const isEncryptedInput = secureFields?.[nestedKey]; + const secureFieldKey = option.secure && option.secureFieldKey ? option.secureFieldKey : ''; + const isEncryptedInput = secureFieldKey && secureFields?.[secureFieldKey]; // workaround for https://github.com/react-hook-form/react-hook-form/issues/4993#issuecomment-829012506 useEffect( @@ -162,7 +157,7 @@ const OptionInput: FC = ({ onSelectTemplate={onSelectTemplate} > {isEncryptedInput ? ( - onResetSecureField?.(nestedKey)} isConfigured /> + onResetSecureField?.(secureFieldKey)} isConfigured /> ) : ( = ({ onSelectTemplate={onSelectTemplate} > {isEncryptedInput ? ( - onResetSecureField?.(nestedKey)} isConfigured /> + onResetSecureField?.(secureFieldKey)} isConfigured /> ) : (