diff --git a/.betterer.results b/.betterer.results index 05523a153df..61633c953c4 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1883,10 +1883,9 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "2"] ], "public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx:5381": [ - [0, 0, 0, "No untranslated strings. Wrap text with ", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Do not use any type assertions.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"] + [0, 0, 0, "Unexpected any. Specify a different type.", "0"], + [0, 0, 0, "Do not use any type assertions.", "1"], + [0, 0, 0, "Unexpected any. Specify a different type.", "2"] ], "public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"], diff --git a/go.mod b/go.mod index b2117a25bd0..a9d49da9f7b 100644 --- a/go.mod +++ b/go.mod @@ -74,7 +74,7 @@ require ( github.com/googleapis/gax-go/v2 v2.13.0 // @grafana/grafana-backend-group github.com/gorilla/mux v1.8.1 // @grafana/grafana-backend-group github.com/gorilla/websocket v1.5.0 // @grafana/grafana-app-platform-squad - github.com/grafana/alerting v0.0.0-20240822131459-9daa6239cc41 // @grafana/alerting-backend + github.com/grafana/alerting v0.0.0-20240829185616-8454ac21d7e5 // @grafana/alerting-backend github.com/grafana/authlib v0.0.0-20240906122029-0100695765b9 // @grafana/identity-access-team github.com/grafana/authlib/claims v0.0.0-20240903121118-16441568af1e // @grafana/identity-access-team github.com/grafana/codejen v0.0.3 // @grafana/dataviz-squad diff --git a/go.sum b/go.sum index d988a24a782..6f5312dbb39 100644 --- a/go.sum +++ b/go.sum @@ -2257,8 +2257,8 @@ github.com/gorilla/websocket v0.0.0-20170926233335-4201258b820c/go.mod h1:E7qHFY github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc= github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= -github.com/grafana/alerting v0.0.0-20240822131459-9daa6239cc41 h1:p+UsX43BoDH5YlG6xUd9xDS3M4sWouy8VJ+ODv5S6uE= -github.com/grafana/alerting v0.0.0-20240822131459-9daa6239cc41/go.mod h1:GMLi6d09Xqo96fCVUjNk//rcjP5NKEdjOzfWIffD5r4= +github.com/grafana/alerting v0.0.0-20240829185616-8454ac21d7e5 h1:gQprHfu5/GT/mpPuRm3QVL+7+0j1QsvKJuPIzQAsezM= +github.com/grafana/alerting v0.0.0-20240829185616-8454ac21d7e5/go.mod h1:GMLi6d09Xqo96fCVUjNk//rcjP5NKEdjOzfWIffD5r4= github.com/grafana/authlib v0.0.0-20240906122029-0100695765b9 h1:e+kFqd2sECBhbxOV1NoVxsudLygNQuu9bO+7FjNTkXo= github.com/grafana/authlib v0.0.0-20240906122029-0100695765b9/go.mod h1:PFzXbCrn0GIpN4KwT6NP1l5Z1CPLfmKHnYx8rZzQcyY= github.com/grafana/authlib/claims v0.0.0-20240903121118-16441568af1e h1:ng5SopWamGS0MHaCj2e5huWYxAfMeCrj1l/dbJnfiow= diff --git a/pkg/services/ngalert/api/tooling/definitions/contact_points.go b/pkg/services/ngalert/api/tooling/definitions/contact_points.go index 6fecda18fe5..dfcd878df6d 100644 --- a/pkg/services/ngalert/api/tooling/definitions/contact_points.go +++ b/pkg/services/ngalert/api/tooling/definitions/contact_points.go @@ -183,11 +183,11 @@ type SensugoIntegration struct { } type SigV4Config struct { - Region string `json:"region,omitempty" yaml:"region,omitempty" hcl:"region"` - AccessKey string `json:"access_key,omitempty" yaml:"access_key,omitempty" hcl:"access_key"` - SecretKey string `json:"secret_key,omitempty" yaml:"secret_key,omitempty" hcl:"secret_key"` - Profile string `json:"profile,omitempty" yaml:"profile,omitempty" hcl:"profile"` - RoleARN string `json:"role_arn,omitempty" yaml:"role_arn,omitempty" hcl:"role_arn"` + Region *string `json:"region,omitempty" yaml:"region,omitempty" hcl:"region"` + AccessKey *Secret `json:"access_key,omitempty" yaml:"access_key,omitempty" hcl:"access_key"` + SecretKey *Secret `json:"secret_key,omitempty" yaml:"secret_key,omitempty" hcl:"secret_key"` + Profile *string `json:"profile,omitempty" yaml:"profile,omitempty" hcl:"profile"` + RoleARN *string `json:"role_arn,omitempty" yaml:"role_arn,omitempty" hcl:"role_arn"` } type SnsIntegration struct { diff --git a/pkg/services/ngalert/models/fingerprint.go b/pkg/services/ngalert/models/fingerprint.go new file mode 100644 index 00000000000..5fb388df856 --- /dev/null +++ b/pkg/services/ngalert/models/fingerprint.go @@ -0,0 +1,55 @@ +package models + +import ( + "encoding/binary" + "fmt" + "hash" + "hash/fnv" + "math" + "unsafe" +) + +// fingerprint is a wrapper for hash.Hash64 that adds utility methods to simplify hash calculation of structs +type fingerprint struct { + h hash.Hash64 +} + +// creates a fingerprint that is backed by 64bit FNV-1a hash +func newFingerprint() fingerprint { + return fingerprint{h: fnv.New64a()} +} + +func (f fingerprint) String() string { + return fmt.Sprintf("%016x", f.h.Sum64()) +} + +func (f fingerprint) writeBytes(b []byte) { + _, _ = f.h.Write(b) + // add a byte sequence that cannot happen in UTF-8 strings. + _, _ = f.h.Write([]byte{255}) +} + +func (f fingerprint) writeString(s string) { + if len(s) == 0 { + f.writeBytes(nil) + return + } + // #nosec G103 + // avoid allocation when converting string to byte slice + f.writeBytes(unsafe.Slice(unsafe.StringData(s), len(s))) +} + +func (f fingerprint) writeFloat64(num float64) { + bits := math.Float64bits(num) + bytes := make([]byte, 8) + binary.LittleEndian.PutUint64(bytes, bits) + f.writeBytes(bytes) +} + +func (f fingerprint) writeBool(b bool) { + if b { + f.writeBytes([]byte{1}) + } else { + f.writeBytes([]byte{0}) + } +} diff --git a/pkg/services/ngalert/models/receivers.go b/pkg/services/ngalert/models/receivers.go index 239eec4bc4f..8bfeaafc475 100644 --- a/pkg/services/ngalert/models/receivers.go +++ b/pkg/services/ngalert/models/receivers.go @@ -2,15 +2,14 @@ package models import ( "context" - "encoding/binary" "encoding/json" "errors" "fmt" - "hash/fnv" "maps" "math" + "slices" "sort" - "unsafe" + "strings" alertingNotify "github.com/grafana/alerting/notify" @@ -148,9 +147,39 @@ type IntegrationConfig struct { // IntegrationField represents a field in an integration configuration. type IntegrationField struct { Name string + Fields map[string]IntegrationField Secure bool } +type IntegrationFieldPath []string + +func NewIntegrationFieldPath(path string) IntegrationFieldPath { + return strings.Split(path, ".") +} + +func (f IntegrationFieldPath) Head() string { + if len(f) > 0 { + return f[0] + } + return "" +} + +func (f IntegrationFieldPath) Tail() IntegrationFieldPath { + return f[1:] +} + +func (f IntegrationFieldPath) IsLeaf() bool { + return len(f) == 1 +} + +func (f IntegrationFieldPath) String() string { + return strings.Join(f, ".") +} + +func (f IntegrationFieldPath) Append(segment string) IntegrationFieldPath { + return append(f, segment) +} + // IntegrationConfigFromType returns an integration configuration for a given integration type. If the integration type is // not found an error is returned. func IntegrationConfigFromType(integrationType string) (IntegrationConfig, error) { @@ -160,23 +189,60 @@ func IntegrationConfigFromType(integrationType string) (IntegrationConfig, error } integrationConfig := IntegrationConfig{Type: config.Type, Fields: make(map[string]IntegrationField, len(config.Options))} + for _, option := range config.Options { - integrationConfig.Fields[option.PropertyName] = IntegrationField{ - Name: option.PropertyName, - Secure: option.Secure, - } + integrationConfig.Fields[option.PropertyName] = notifierOptionToIntegrationField(option) } return integrationConfig, nil } +func notifierOptionToIntegrationField(option channels_config.NotifierOption) IntegrationField { + f := IntegrationField{ + Name: option.PropertyName, + Secure: option.Secure, + Fields: make(map[string]IntegrationField, len(option.SubformOptions)), + } + for _, subformOption := range option.SubformOptions { + f.Fields[subformOption.PropertyName] = notifierOptionToIntegrationField(subformOption) + } + return f +} + // IsSecureField returns true if the field is both known and marked as secure in the integration configuration. -func (config *IntegrationConfig) IsSecureField(field string) bool { - if config.Fields != nil { - if f, ok := config.Fields[field]; ok { - return f.Secure +func (config *IntegrationConfig) IsSecureField(path IntegrationFieldPath) bool { + f, ok := config.GetField(path) + return ok && f.Secure +} + +func (config *IntegrationConfig) GetField(path IntegrationFieldPath) (IntegrationField, bool) { + for _, integrationField := range config.Fields { + if strings.EqualFold(integrationField.Name, path.Head()) { + if path.IsLeaf() { + return integrationField, true + } + return integrationField.GetField(path.Tail()) } } - return false + return IntegrationField{}, false +} + +func (config *IntegrationConfig) GetSecretFields() []IntegrationFieldPath { + return traverseFields(config.Fields, nil, func(i IntegrationField) bool { + return i.Secure + }) +} + +func traverseFields(flds map[string]IntegrationField, parentPath IntegrationFieldPath, predicate func(i IntegrationField) bool) []IntegrationFieldPath { + var result []IntegrationFieldPath + for key, field := range flds { + if predicate(field) { + result = append(result, parentPath.Append(key)) + } + if len(field.Fields) > 0 { + result = append(result, traverseFields(field.Fields, parentPath.Append(key), predicate)...) + } + } + return result } func (config *IntegrationConfig) Clone() IntegrationConfig { @@ -193,11 +259,28 @@ func (config *IntegrationConfig) Clone() IntegrationConfig { return clone } +func (field *IntegrationField) GetField(path IntegrationFieldPath) (IntegrationField, bool) { + for _, integrationField := range field.Fields { + if strings.EqualFold(integrationField.Name, path.Head()) { + if path.IsLeaf() { + return integrationField, true + } + return integrationField.GetField(path.Tail()) + } + } + return IntegrationField{}, false +} + func (field *IntegrationField) Clone() IntegrationField { - return IntegrationField{ + f := IntegrationField{ Name: field.Name, Secure: field.Secure, + Fields: make(map[string]IntegrationField, len(field.Fields)), } + for subName, sub := range field.Fields { + f.Fields[subName] = sub.Clone() + } + return f } func (integration *Integration) Clone() Integration { @@ -206,42 +289,133 @@ func (integration *Integration) Clone() Integration { Name: integration.Name, Config: integration.Config.Clone(), DisableResolveMessage: integration.DisableResolveMessage, - Settings: maps.Clone(integration.Settings), + Settings: cloneIntegrationSettings(integration.Settings), SecureSettings: maps.Clone(integration.SecureSettings), } } +// cloneIntegrationSettings implements a deep copy of settings map. +// It's not a generic purpose function because settings are limited to basic types, maps and slices. +func cloneIntegrationSettings(m map[string]any) map[string]any { + result := maps.Clone(m) // do a shallow copy of the map first + for k, v := range result { + if mp, ok := v.(map[string]any); ok { + result[k] = cloneIntegrationSettings(mp) + continue + } + if mp, ok := v.([]any); ok { + result[k] = cloneIntegrationSettingsSlice(mp) + continue + } + } + return result +} + +// cloneIntegrationSettingsSlice implements a deep copy of a []any in integration settings. +// It's not a generic purpose function because settings are limited to basic types, maps and slices. +func cloneIntegrationSettingsSlice(src []any) []any { + dst := slices.Clone(src) + for i, v := range dst { + if mp, ok := v.(map[string]any); ok { + dst[i] = cloneIntegrationSettings(mp) + continue + } + if mp, ok := v.([]any); ok { + dst[i] = cloneIntegrationSettingsSlice(mp) + continue + } + } + return dst +} + // Encrypt encrypts all fields in Settings that are marked as secure in the integration configuration. The encrypted values // are stored in SecureSettings and the original values are removed from Settings. // If a field is already in SecureSettings it is not encrypted again. func (integration *Integration) Encrypt(encryptFn EncryptFn) error { + secretFieldPaths := integration.Config.GetSecretFields() + if len(secretFieldPaths) == 0 { + return nil + } var errs []error - for key, val := range integration.Settings { - if isSecureField := integration.Config.IsSecureField(key); !isSecureField { + for _, path := range secretFieldPaths { + unencryptedSecureValue, ok, err := extractField(integration.Settings, path) + if err != nil { + errs = append(errs, fmt.Errorf("failed to extract secret field by path '%s': %w", path, err)) + } + if !ok { continue } - - delete(integration.Settings, key) - unencryptedSecureValue, isString := val.(string) - if !isString { + if _, exists := integration.SecureSettings[path.String()]; exists { continue } - - if _, exists := integration.SecureSettings[key]; exists { - continue - } - encrypted, err := encryptFn(unencryptedSecureValue) if err != nil { - errs = append(errs, fmt.Errorf("failed to encrypt secure setting '%s': %w", key, err)) + errs = append(errs, fmt.Errorf("failed to encrypt secure setting '%s': %w", path.String(), err)) } - - integration.SecureSettings[key] = encrypted + integration.SecureSettings[path.String()] = encrypted } - return errors.Join(errs...) } +func extractField(settings map[string]any, path IntegrationFieldPath) (string, bool, error) { + val, ok := settings[path.Head()] + if !ok { + return "", false, nil + } + if path.IsLeaf() { + secret, ok := val.(string) + if !ok { + return "", false, fmt.Errorf("expected string but got %T", val) + } + delete(settings, path.Head()) + return secret, true, nil + } + sub, ok := val.(map[string]any) + if !ok { + return "", false, fmt.Errorf("expected nested object but got %T", val) + } + return extractField(sub, path.Tail()) +} + +func getFieldValue(settings map[string]any, path IntegrationFieldPath) (any, bool) { + val, ok := settings[path.Head()] + if !ok { + return nil, false + } + if path.IsLeaf() { + return val, true + } + sub, ok := val.(map[string]any) + if !ok { + return nil, false + } + return getFieldValue(sub, path.Tail()) +} + +func setField(settings map[string]any, path IntegrationFieldPath, valueFn func(current any) any, skipIfNotExist bool) error { + if path.IsLeaf() { + current, ok := settings[path.Head()] + if skipIfNotExist && !ok { + return nil + } + settings[path.Head()] = valueFn(current) + return nil + } + val, ok := settings[path.Head()] + if !ok { + if skipIfNotExist { + return nil + } + val = map[string]any{} + settings[path.Head()] = val + } + sub, ok := val.(map[string]any) + if !ok { + return fmt.Errorf("expected nested object but got %T", val) + } + return setField(sub, path.Tail(), valueFn, skipIfNotExist) +} + // Decrypt decrypts all fields in SecureSettings and moves them to Settings. // The original values are removed from SecureSettings. func (integration *Integration) Decrypt(decryptFn DecryptFn) error { @@ -252,30 +426,35 @@ func (integration *Integration) Decrypt(decryptFn DecryptFn) error { errs = append(errs, fmt.Errorf("failed to decrypt secure setting '%s': %w", key, err)) } delete(integration.SecureSettings, key) - integration.Settings[key] = decrypted - } + path := NewIntegrationFieldPath(key) + err = setField(integration.Settings, path, func(current any) any { + return decrypted + }, false) + if err != nil { + errs = append(errs, fmt.Errorf("failed to set field '%s': %w", key, err)) + } + } return errors.Join(errs...) } // Redact redacts all fields in SecureSettings and moves them to Settings. // The original values are removed from SecureSettings. func (integration *Integration) Redact(redactFn RedactFn) { - for key, secureVal := range integration.SecureSettings { // TODO: Should we trust that the receiver is stored correctly or use known secure settings? - integration.Settings[key] = redactFn(secureVal) - delete(integration.SecureSettings, key) + for _, path := range integration.Config.GetSecretFields() { + _ = setField(integration.Settings, path, func(current any) any { + if s, ok := current.(string); ok && s != "" { + return redactFn(s) + } + return current + }, true) } - // We don't trust that the receiver is stored correctly, so we redact secure fields in the settings as well. - for key, val := range integration.Settings { - if val != "" && integration.Config.IsSecureField(key) { - s, isString := val.(string) - if !isString { - continue - } - integration.Settings[key] = redactFn(s) - delete(integration.SecureSettings, key) - } + for key, secureVal := range integration.SecureSettings { // TODO: Should we trust that the receiver is stored correctly or use known secure settings? + _ = setField(integration.Settings, NewIntegrationFieldPath(key), func(any) any { + return redactFn(secureVal) + }, false) + delete(integration.SecureSettings, key) } } @@ -304,11 +483,17 @@ func (integration *Integration) SecureFields() map[string]bool { secureFields[key] = true } } - // We mark secure fields in the settings as well. This is to ensure legacy behaviour for redacted secure settings. - for key, val := range integration.Settings { - if val != "" && integration.Config.IsSecureField(key) { - secureFields[key] = true + for _, path := range integration.Config.GetSecretFields() { + if secureFields[path.String()] { + continue + } + value, ok := getFieldValue(integration.Settings, path) + if !ok || value == nil { + continue + } + if v, _ := value.(string); v != "" { + secureFields[path.String()] = true } } @@ -369,41 +554,16 @@ func (r *Receiver) GetUID() string { } func (r *Receiver) Fingerprint() string { - sum := fnv.New64() - - writeBytes := func(b []byte) { - _, _ = sum.Write(b) - // add a byte sequence that cannot happen in UTF-8 strings. - _, _ = sum.Write([]byte{255}) - } - writeString := func(s string) { - if len(s) == 0 { - writeBytes(nil) - return - } - // #nosec G103 - // avoid allocation when converting string to byte slice - writeBytes(unsafe.Slice(unsafe.StringData(s), len(s))) - } - // this temp slice is used to convert ints to bytes. - tmp := make([]byte, 8) - writeInt := func(u int) { - binary.LittleEndian.PutUint64(tmp, uint64(u)) - writeBytes(tmp) - } + sum := newFingerprint() writeIntegration := func(in *Integration) { - writeString(in.UID) - writeString(in.Name) + sum.writeString(in.UID) + sum.writeString(in.Name) // Do not include fields in fingerprint as these are not part of the receiver definition. - writeString(in.Config.Type) + sum.writeString(in.Config.Type) - if in.DisableResolveMessage { - writeInt(1) - } else { - writeInt(0) - } + sum.writeBool(in.DisableResolveMessage) // allocate a slice that will be used for sorting keys, so we allocate it only once var keys []string @@ -426,49 +586,51 @@ func (r *Receiver) Fingerprint() string { sub := keys[:idx] sort.Strings(sub) for _, name := range sub { - writeString(name) - writeString(secureSettings[name]) + sum.writeString(name) + sum.writeString(secureSettings[name]) } } + writeSettings(sum, in.Settings) writeSecureSettings(in.SecureSettings) - - writeSettings := func(settings map[string]any) { - // maps do not guarantee predictable sequence of keys. - // Therefore, to make hash stable, we need to sort keys - if len(settings) == 0 { - return - } - idx := 0 - for k := range settings { - keys[idx] = k - idx++ - } - sub := keys[:idx] - sort.Strings(sub) - for _, name := range sub { - writeString(name) - - // TODO: Improve this. - v := settings[name] - bytes, err := json.Marshal(v) - if err != nil { - writeString(fmt.Sprintf("%+v", v)) - } else { - writeBytes(bytes) - } - } - } - writeSettings(in.Settings) } // fields that determine the rule state - writeString(r.UID) - writeString(r.Name) - writeString(string(r.Provenance)) + sum.writeString(r.UID) + sum.writeString(r.Name) + sum.writeString(string(r.Provenance)) for _, integration := range r.Integrations { writeIntegration(integration) } - return fmt.Sprintf("%016x", sum.Sum64()) + return sum.String() +} + +func writeSettings(f fingerprint, m map[string]any) { + if len(m) == 0 { + f.writeBytes(nil) + return + } + keysIter := maps.Keys(m) + keys := slices.Collect(keysIter) + sort.Strings(keys) + for _, key := range keys { + f.writeString(key) + switch v := m[key].(type) { + case string: + f.writeString(v) + case bool: + f.writeBool(v) + case float64: // unmarshalling to map[string]any represents all numbers as float64 + f.writeFloat64(v) + case map[string]any: + writeSettings(f, v) + default: + b, err := json.Marshal(v) + if err != nil { + f.writeString(fmt.Sprintf("%+v", v)) + } + f.writeBytes(b) + } + } } diff --git a/pkg/services/ngalert/models/receivers_test.go b/pkg/services/ngalert/models/receivers_test.go index 8f99166e2aa..a47387c58b5 100644 --- a/pkg/services/ngalert/models/receivers_test.go +++ b/pkg/services/ngalert/models/receivers_test.go @@ -6,6 +6,7 @@ import ( alertingNotify "github.com/grafana/alerting/notify" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/services/ngalert/notifier/channels_config" ) @@ -45,20 +46,19 @@ func TestReceiver_EncryptDecrypt(t *testing.T) { secrets, err := channels_config.GetSecretKeysForContactPointType(integrationType) assert.NoError(t, err) for _, key := range secrets { - if val, ok := encrypted.Settings[key]; ok { - if s, isString := val.(string); isString { - encryptedVal, err := encryptFn(s) - assert.NoError(t, err) - encrypted.SecureSettings[key] = encryptedVal - delete(encrypted.Settings, key) - } + val, ok, err := extractField(encrypted.Settings, NewIntegrationFieldPath(key)) + assert.NoError(t, err) + if ok { + encryptedVal, err := encryptFn(val) + assert.NoError(t, err) + encrypted.SecureSettings[key] = encryptedVal } } testIntegration := decrypedIntegration.Clone() err = testIntegration.Encrypt(encryptFn) assert.NoError(t, err) - assert.Equal(t, encrypted, testIntegration) + require.Equal(t, encrypted, testIntegration) err = testIntegration.Decrypt(decryptnFn) assert.NoError(t, err) @@ -80,12 +80,14 @@ func TestIntegration_Redact(t *testing.T) { secrets, err := channels_config.GetSecretKeysForContactPointType(integrationType) assert.NoError(t, err) for _, key := range secrets { - if val, ok := expected.Settings[key]; ok { - if s, isString := val.(string); isString && s != "" { - expected.Settings[key] = redactFn(s) + err := setField(expected.Settings, NewIntegrationFieldPath(key), func(current any) any { + if s, isString := current.(string); isString && s != "" { delete(expected.SecureSettings, key) + return redactFn(s) } - } + return current + }, true) + require.NoError(t, err) } validIntegration.Redact(redactFn) @@ -244,9 +246,9 @@ func TestIntegrationConfig(t *testing.T) { for field := range config.Fields { _, isSecret := allSecrets[field] - assert.Equal(t, isSecret, config.IsSecureField(field)) + assert.Equalf(t, isSecret, config.IsSecureField(NewIntegrationFieldPath(field)), "field '%s' is expected to be secret", field) } - assert.False(t, config.IsSecureField("__--**unknown_field**--__")) + assert.False(t, config.IsSecureField(IntegrationFieldPath{"__--**unknown_field**--__"})) }) } @@ -263,11 +265,13 @@ func TestIntegration_SecureFields(t *testing.T) { t.Run("contains SecureSettings", func(t *testing.T) { validIntegration := IntegrationGen(IntegrationMuts.WithValidConfig(integrationType))() expected := make(map[string]bool, len(validIntegration.SecureSettings)) - for field := range validIntegration.Config.Fields { - if validIntegration.Config.IsSecureField(field) { - expected[field] = true - validIntegration.SecureSettings[field] = "test" - delete(validIntegration.Settings, field) + for _, path := range validIntegration.Config.GetSecretFields() { + if validIntegration.Config.IsSecureField(path) { + expected[path.String()] = true + validIntegration.SecureSettings[path.String()] = "test" + _, _, err := extractField(validIntegration.Settings, path) + require.NoError(t, err) + continue } } assert.Equal(t, expected, validIntegration.SecureFields()) @@ -276,11 +280,13 @@ func TestIntegration_SecureFields(t *testing.T) { t.Run("contains secret Settings not in SecureSettings", func(t *testing.T) { validIntegration := IntegrationGen(IntegrationMuts.WithValidConfig(integrationType))() expected := make(map[string]bool, len(validIntegration.SecureSettings)) - for field := range validIntegration.Config.Fields { - if validIntegration.Config.IsSecureField(field) { - expected[field] = true - validIntegration.Settings[field] = "test" - delete(validIntegration.SecureSettings, field) + for _, path := range validIntegration.Config.GetSecretFields() { + if validIntegration.Config.IsSecureField(path) { + expected[path.String()] = true + assert.NoError(t, setField(validIntegration.Settings, path, func(current any) any { + return "test" + }, false)) + delete(validIntegration.SecureSettings, path.String()) } } assert.Equal(t, expected, validIntegration.SecureFields()) @@ -306,8 +312,13 @@ func TestReceiver_Fingerprint(t *testing.T) { ))() baseReceiver.Integrations[0].UID = "stable UID" baseReceiver.Integrations[0].DisableResolveMessage = true - baseReceiver.Integrations[0].SecureSettings = map[string]string{"test2": "test2"} - baseReceiver.Integrations[0].Settings["broken"] = broken{f1: "this"} // Add a broken type to ensure it is stable in the fingerprint. + baseReceiver.Integrations[0].SecureSettings = map[string]string{"test2": "test2", "test3": "test223", "test1": "rest22"} + baseReceiver.Integrations[0].Settings["broken"] = broken{f1: "this"} // Add a broken type to ensure it is stable in the fingerprint. + baseReceiver.Integrations[0].Settings["sub-map"] = map[string]any{ + "setting": "value", + "something": 123, + "data": []string{"test"}, + } // Add a broken type to ensure it is stable in the fingerprint. baseReceiver.Integrations[0].Config = IntegrationConfig{Type: baseReceiver.Integrations[0].Config.Type} // Remove all fields except Type. completelyDifferentReceiver := ReceiverGen(ReceiverMuts.WithName("test receiver2"), ReceiverMuts.WithIntegrations( @@ -320,7 +331,7 @@ func TestReceiver_Fingerprint(t *testing.T) { completelyDifferentReceiver.Integrations[0].Config = IntegrationConfig{Type: completelyDifferentReceiver.Integrations[0].Config.Type} // Remove all fields except Type. t.Run("stable across code changes", func(t *testing.T) { - expectedFingerprint := "ae141b582965f4f5" // If this is a valid fingerprint generation change, update the expected value. + expectedFingerprint := "a3402fdaba03030c" // If this is a valid fingerprint generation change, update the expected value. assert.Equal(t, expectedFingerprint, baseReceiver.Fingerprint()) }) t.Run("stable across clones", func(t *testing.T) { diff --git a/pkg/services/ngalert/notifier/channels_config/available_channels.go b/pkg/services/ngalert/notifier/channels_config/available_channels.go index b9a900b2df7..33a29ae2c0f 100644 --- a/pkg/services/ngalert/notifier/channels_config/available_channels.go +++ b/pkg/services/ngalert/notifier/channels_config/available_channels.go @@ -1484,9 +1484,8 @@ func GetAvailableNotifiers() []*NotifierPlugin { { // Since Grafana 11.1 Type: "sns", Name: "AWS SNS", - Description: "Sends notifications to Cisco Webex Teams", + Description: "Sends notifications to AWS Simple Notification Service", Heading: "Webex settings", - Info: "Notifications can be configured for any Cisco Webex Teams", Options: []NotifierOption{ { Label: "The Amazon SNS API URL", @@ -1602,18 +1601,30 @@ func GetSecretKeysForContactPointType(contactPointType string) ([]string, error) notifiers := GetAvailableNotifiers() for _, n := range notifiers { if strings.EqualFold(n.Type, contactPointType) { - var secureFields []string - for _, field := range n.Options { - if field.Secure { - secureFields = append(secureFields, field.PropertyName) - } - } - return secureFields, nil + return getSecretFields("", n.Options), nil } } return nil, fmt.Errorf("no secrets configured for type '%s'", contactPointType) } +func getSecretFields(parentPath string, options []NotifierOption) []string { + var secureFields []string + for _, field := range options { + name := field.PropertyName + if parentPath != "" { + name = parentPath + "." + name + } + if field.Secure { + secureFields = append(secureFields, name) + continue + } + if len(field.SubformOptions) > 0 { + secureFields = append(secureFields, getSecretFields(name, field.SubformOptions)...) + } + } + return secureFields +} + // ConfigForIntegrationType returns the config for the given integration type. Returns error is integration type is not known. func ConfigForIntegrationType(contactPointType string) (*NotifierPlugin, error) { notifiers := GetAvailableNotifiers() diff --git a/pkg/services/ngalert/notifier/channels_config/available_channels_test.go b/pkg/services/ngalert/notifier/channels_config/available_channels_test.go new file mode 100644 index 00000000000..f3290b3956a --- /dev/null +++ b/pkg/services/ngalert/notifier/channels_config/available_channels_test.go @@ -0,0 +1,89 @@ +package channels_config + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestGetSecretKeysForContactPointType(t *testing.T) { + testCases := []struct { + receiverType string + expectedSecretFields []string + }{ + {receiverType: "dingding", expectedSecretFields: []string{}}, + {receiverType: "kafka", expectedSecretFields: []string{"password"}}, + {receiverType: "email", expectedSecretFields: []string{}}, + {receiverType: "pagerduty", expectedSecretFields: []string{"integrationKey"}}, + {receiverType: "victorops", expectedSecretFields: []string{}}, + {receiverType: "oncall", expectedSecretFields: []string{"password", "authorization_credentials"}}, + {receiverType: "pushover", expectedSecretFields: []string{"apiToken", "userKey"}}, + {receiverType: "slack", expectedSecretFields: []string{"token", "url"}}, + {receiverType: "sensugo", expectedSecretFields: []string{"apikey"}}, + {receiverType: "teams", expectedSecretFields: []string{}}, + {receiverType: "telegram", expectedSecretFields: []string{"bottoken"}}, + {receiverType: "webhook", expectedSecretFields: []string{"password", "authorization_credentials"}}, + {receiverType: "wecom", expectedSecretFields: []string{"url", "secret"}}, + {receiverType: "prometheus-alertmanager", expectedSecretFields: []string{"basicAuthPassword"}}, + {receiverType: "discord", expectedSecretFields: []string{"url"}}, + {receiverType: "googlechat", expectedSecretFields: []string{}}, + {receiverType: "line", expectedSecretFields: []string{"token"}}, + {receiverType: "threema", expectedSecretFields: []string{"api_secret"}}, + {receiverType: "opsgenie", expectedSecretFields: []string{"apiKey"}}, + {receiverType: "webex", expectedSecretFields: []string{"bot_token"}}, + {receiverType: "sns", expectedSecretFields: []string{"sigv4.access_key", "sigv4.secret_key"}}, + } + for _, testCase := range testCases { + t.Run(testCase.receiverType, func(t *testing.T) { + got, err := GetSecretKeysForContactPointType(testCase.receiverType) + require.NoError(t, err) + t.Logf("got secret fields: %#v", got) + require.ElementsMatch(t, testCase.expectedSecretFields, got) + }) + } +} + +func Test_getSecretFields(t *testing.T) { + testCases := []struct { + name string + parentPath string + options []NotifierOption + expectedFields []string + }{ + { + name: "No secure fields", + parentPath: "", + options: []NotifierOption{ + {PropertyName: "field1", Secure: false, SubformOptions: nil}, + {PropertyName: "field2", Secure: false, SubformOptions: nil}, + }, + expectedFields: []string{}, + }, + { + name: "Single secure field", + parentPath: "", + options: []NotifierOption{ + {PropertyName: "field1", Secure: true, SubformOptions: nil}, + {PropertyName: "field2", Secure: false, SubformOptions: nil}, + }, + expectedFields: []string{"field1"}, + }, + { + name: "Secure field in subform", + parentPath: "parent", + options: []NotifierOption{ + {PropertyName: "field1", Secure: true, SubformOptions: nil}, + {PropertyName: "field2", Secure: false, SubformOptions: []NotifierOption{ + {PropertyName: "subfield1", Secure: true, SubformOptions: nil}, + }}, + }, + expectedFields: []string{"parent.field1", "parent.field2.subfield1"}, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := getSecretFields(tc.parentPath, tc.options) + require.ElementsMatch(t, got, tc.expectedFields) + }) + } +} diff --git a/pkg/services/ngalert/notifier/receiver_svc_test.go b/pkg/services/ngalert/notifier/receiver_svc_test.go index ce4bcdef792..8fabd53851c 100644 --- a/pkg/services/ngalert/notifier/receiver_svc_test.go +++ b/pkg/services/ngalert/notifier/receiver_svc_test.go @@ -246,7 +246,7 @@ func TestReceiverService_Delete(t *testing.T) { name: "delete receiver used by route fails", user: writer, deleteUID: legacy_storage.NameToUid("grafana-default-email"), - version: "1fd7897966a2adc5", // Correct version for grafana-default-email. + version: "cd95627c75892a39", // Correct version for grafana-default-email. expectedErr: makeReceiverInUseErr(true, nil), }, { @@ -764,7 +764,7 @@ func TestReceiverService_UpdateReceiverName(t *testing.T) { newReceiverName := "new-name" slackIntegration := models.IntegrationGen(models.IntegrationMuts.WithName(receiverName), models.IntegrationMuts.WithValidConfig("slack"))() baseReceiver := models.ReceiverGen(models.ReceiverMuts.WithName(receiverName), models.ReceiverMuts.WithIntegrations(slackIntegration))() - baseReceiver.Version = "1fd7897966a2adc5" // Correct version for grafana-default-email. + baseReceiver.Version = "cd95627c75892a39" // Correct version for grafana-default-email. baseReceiver.Name = newReceiverName // Done here instead of in a mutator so we keep the same uid. store := sut.ruleNotificationsStore.(*fakeConfigStore) diff --git a/pkg/services/ngalert/provisioning/compat.go b/pkg/services/ngalert/provisioning/compat.go index 714452ddcc6..13d0bba88a9 100644 --- a/pkg/services/ngalert/provisioning/compat.go +++ b/pkg/services/ngalert/provisioning/compat.go @@ -1,6 +1,8 @@ package provisioning import ( + "strings" + alertingNotify "github.com/grafana/alerting/notify" "github.com/grafana/grafana/pkg/components/simplejson" @@ -41,7 +43,7 @@ func PostableGrafanaReceiverToEmbeddedContactPoint(contactPoint *definitions.Pos if decryptedValue == "" { continue } - embeddedContactPoint.Settings.Set(k, decryptedValue) + embeddedContactPoint.Settings.SetPath(strings.Split(k, "."), decryptedValue) } return embeddedContactPoint, nil } diff --git a/pkg/services/ngalert/provisioning/compat_test.go b/pkg/services/ngalert/provisioning/compat_test.go new file mode 100644 index 00000000000..f5921410122 --- /dev/null +++ b/pkg/services/ngalert/provisioning/compat_test.go @@ -0,0 +1,125 @@ +package provisioning + +import ( + "math/rand" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/exp/maps" + + "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" + "github.com/grafana/grafana/pkg/services/ngalert/models" +) + +func TestPostableGrafanaReceiverToEmbeddedContactPoint(t *testing.T) { + expectedProvenance := models.KnownProvenances[rand.Intn(len(models.KnownProvenances))] + tests := []struct { + name string + input definitions.PostableGrafanaReceiver + expected definitions.EmbeddedContactPoint + }{ + { + name: "should create expected object", + input: definitions.PostableGrafanaReceiver{ + UID: "test-uid", + Name: "test-name", + Type: "test-type", + DisableResolveMessage: true, + Settings: definitions.RawMessage(`{ "name": "test" }`), + }, + expected: definitions.EmbeddedContactPoint{ + UID: "test-uid", + Name: "test-name", + Type: "test-type", + Settings: simplejson.NewFromAny(map[string]any{ + "name": "test", + }), + DisableResolveMessage: true, + Provenance: string(expectedProvenance), + }, + }, + { + name: "should merge decrypted secrets into settings", + input: definitions.PostableGrafanaReceiver{ + Settings: definitions.RawMessage(`{ "name": "test" }`), + SecureSettings: map[string]string{ + "secret": "data", + }, + }, + expected: definitions.EmbeddedContactPoint{ + Settings: simplejson.NewFromAny(map[string]any{ + "name": "test", + "secret": "data", + }), + Provenance: string(expectedProvenance), + }, + }, + { + name: "should override existing settings with decrypted secrets into settings", + input: definitions.PostableGrafanaReceiver{ + Settings: definitions.RawMessage(`{ "name": "test" }`), + SecureSettings: map[string]string{ + "name": "secret-data", + }, + }, + expected: definitions.EmbeddedContactPoint{ + Settings: simplejson.NewFromAny(map[string]any{ + "name": "secret-data", + }), + Provenance: string(expectedProvenance), + }, + }, + { + name: "should support nested secrets", + input: definitions.PostableGrafanaReceiver{ + Settings: definitions.RawMessage(`{ "name": "test" }`), + SecureSettings: map[string]string{ + "secret.sub-secret": "data", + }, + }, + expected: definitions.EmbeddedContactPoint{ + Settings: simplejson.NewFromAny(map[string]any{ + "name": "test", + "secret": map[string]any{ + "sub-secret": "data", + }, + }), + Provenance: string(expectedProvenance), + }, + }, + { + name: "should amend to nested structs", + input: definitions.PostableGrafanaReceiver{ + Settings: definitions.RawMessage(`{ "name": "test", "secret": { "data": "test"} }`), + SecureSettings: map[string]string{ + "secret.sub-secret": "secret-data", + }, + }, + expected: definitions.EmbeddedContactPoint{ + Settings: simplejson.NewFromAny(map[string]any{ + "name": "test", + "secret": map[string]any{ + "data": "test", + "sub-secret": "secret-data", + }, + }), + Provenance: string(expectedProvenance), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var decrypted []string + decrypt := func(s string) string { + decrypted = append(decrypted, s) + return s + } + embeddedContactPoint, err := PostableGrafanaReceiverToEmbeddedContactPoint(&tt.input, expectedProvenance, decrypt) + require.NoError(t, err) + assert.Equal(t, tt.expected, embeddedContactPoint) + assert.ElementsMatch(t, maps.Values(tt.input.SecureSettings), decrypted) + }) + } +} diff --git a/pkg/services/ngalert/provisioning/contactpoints.go b/pkg/services/ngalert/provisioning/contactpoints.go index d7e4ec26180..97a6fb10ab1 100644 --- a/pkg/services/ngalert/provisioning/contactpoints.go +++ b/pkg/services/ngalert/provisioning/contactpoints.go @@ -490,37 +490,62 @@ func RemoveSecretsForContactPoint(e *apimodels.EmbeddedContactPoint) (map[string return nil, err } for _, secretKey := range secretKeys { - foundSecretKey, secretValue, err := getCaseInsensitive(e.Settings, secretKey) + secretValue, err := extractCaseInsensitive(e.Settings, secretKey) if err != nil { return nil, err } - e.Settings.Del(foundSecretKey) + if secretValue == "" { + continue + } s[secretKey] = secretValue } return s, nil } -// getCaseInsensitive returns the value of the specified key, preferring an exact match but accepting a case-insensitive match. +// extractCaseInsensitive returns the value of the specified key, preferring an exact match but accepting a case-insensitive match. // If no key matches, the second return value is an empty string. -func getCaseInsensitive(jsonObj *simplejson.Json, key string) (string, string, error) { - // Check for an exact key match first. - if value, ok := jsonObj.CheckGet(key); ok { - return key, value.MustString(), nil +func extractCaseInsensitive(jsonObj *simplejson.Json, key string) (string, error) { + if key == "" { + return "", nil } - - // If no exact match is found, look for a case-insensitive match. - settingsMap, err := jsonObj.Map() - if err != nil { - return "", "", err - } - - for k, v := range settingsMap { - if strings.EqualFold(k, key) { - return k, v.(string), nil + path := strings.Split(key, ".") + getNodeCaseInsensitive := func(n *simplejson.Json, field string) (string, *simplejson.Json, error) { + // Check for an exact key match first. + if value, ok := n.CheckGet(field); ok { + return field, value, nil } + + // If no exact match is found, look for a case-insensitive match. + settingsMap, err := n.Map() + if err != nil { + return "", nil, err + } + + for k := range settingsMap { + if strings.EqualFold(k, field) { + return k, n.GetPath(k), nil + } + } + return "", nil, nil } - return key, "", nil + node := jsonObj + for idx, segment := range path { + _, value, err := getNodeCaseInsensitive(node, segment) + if err != nil { + return "", err + } + if value == nil { + return "", nil + } + if idx == len(path)-1 { + resultValue := value.MustString() + node.Del(segment) + return resultValue, nil + } + node = value + } + return "", nil } // convertRecSvcErr converts errors from notifier.ReceiverService to errors expected from ContactPointService. diff --git a/pkg/services/ngalert/provisioning/contactpoints_test.go b/pkg/services/ngalert/provisioning/contactpoints_test.go index 4f71bddaf11..34d057f8eab 100644 --- a/pkg/services/ngalert/provisioning/contactpoints_test.go +++ b/pkg/services/ngalert/provisioning/contactpoints_test.go @@ -4,12 +4,15 @@ import ( "context" "encoding/json" "fmt" + "slices" "strings" "testing" + "github.com/grafana/alerting/notify" "github.com/prometheus/alertmanager/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/exp/maps" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/db" @@ -22,6 +25,7 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/notifier" + "github.com/grafana/grafana/pkg/services/ngalert/notifier/channels_config" "github.com/grafana/grafana/pkg/services/ngalert/notifier/legacy_storage" "github.com/grafana/grafana/pkg/services/ngalert/tests/fakes" "github.com/grafana/grafana/pkg/services/secrets" @@ -376,6 +380,60 @@ func TestContactPointServiceDecryptRedact(t *testing.T) { }) } +func TestRemoveSecretsForContactPoint(t *testing.T) { + overrides := map[string]func(settings map[string]any){ + "webhook": func(settings map[string]any) { // add additional field to the settings because valid config does not allow it to be specified along with password + settings["authorization_credentials"] = "test-authz-creds" + }, + } + + configs := notify.AllKnownConfigsForTesting + keys := maps.Keys(configs) + slices.Sort(keys) + for _, integrationType := range keys { + cfg := configs[integrationType] + var settings map[string]any + require.NoError(t, json.Unmarshal([]byte(cfg.Config), &settings)) + if f, ok := overrides[integrationType]; ok { + f(settings) + } + settingsRaw, err := json.Marshal(settings) + require.NoError(t, err) + + expectedFields, err := channels_config.GetSecretKeysForContactPointType(integrationType) + require.NoError(t, err) + + t.Run(integrationType, func(t *testing.T) { + cp := definitions.EmbeddedContactPoint{ + Name: "integration-" + integrationType, + Type: integrationType, + Settings: simplejson.MustJson(settingsRaw), + } + secureFields, err := RemoveSecretsForContactPoint(&cp) + require.NoError(t, err) + + FIELDS_ASSERT: + for _, field := range expectedFields { + assert.Contains(t, secureFields, field) + path := strings.Split(field, ".") + var expectedValue any = settings + for _, segment := range path { + v, ok := expectedValue.(map[string]any) + if !ok { + assert.Fail(t, fmt.Sprintf("cannot get expected value for field '%s'", field)) + continue FIELDS_ASSERT + } + expectedValue = v[segment] + } + assert.EqualValues(t, secureFields[field], expectedValue) + v, err := cp.Settings.GetPath(path...).Value() + assert.NoError(t, err) + assert.Nilf(t, v, "field %s is expected to be removed from the settings", field) + } + }) + } +} + func createContactPointServiceSut(t *testing.T, secretService secrets.Service) *ContactPointService { // Encrypt secure settings. cfg := createEncryptedConfig(t, secretService) 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 eb57f14c55c..1c99524f434 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; -import { useFormContext, FieldError, FieldErrors, DeepMap } from 'react-hook-form'; +import { DeepMap, FieldError, FieldErrors, useFormContext } from 'react-hook-form'; -import { Button, Field, Input } from '@grafana/ui'; +import { Field, SecretInput } from '@grafana/ui'; import { NotificationChannelOption, NotificationChannelSecureFields } from 'app/types'; import { ChannelValues, ReceiverFormValues } from '../../../types/receiver-form'; @@ -33,6 +33,7 @@ export function ChannelOptions({ }: Props): JSX.Element { const { watch } = useFormContext>(); const currentFormValues = watch(); // react hook form types ARE LYING! + return ( <> {selectedChannelOptions.map((option: NotificationChannelOption, index: number) => { @@ -50,18 +51,8 @@ export function ChannelOptions({ if (secureFields && secureFields[option.propertyName]) { return ( - - onResetSecureField(option.propertyName)} fill="text" type="button" size="sm"> - Clear - - ) - } - /> + + onResetSecureField(option.propertyName)} isConfigured /> ); } @@ -74,6 +65,8 @@ export function ChannelOptions({ return ( ({ const onResetSecureField = (key: string) => { if (_secureFields[key]) { - const updatedSecureFields = { ...secureFields }; + const updatedSecureFields = { ..._secureFields }; delete updatedSecureFields[key]; setSecureFields(updatedSecureFields); setValue(`${pathPrefix}.secureFields`, updatedSecureFields); diff --git a/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx b/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx index 0d1e6b74f15..a856376bddb 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx @@ -93,6 +93,17 @@ export function ReceiverForm({ const { fields, append, remove } = useControlledFieldArray({ name: 'items', formAPI, softDelete: true }); const submitCallback = async (values: ReceiverFormValues) => { + values.items.forEach((item) => { + if (item.secureFields) { + // omit secure fields with boolean value as BE expects not touched fields to be omitted: https://github.com/grafana/grafana/pull/71307 + Object.keys(item.secureFields).forEach((key) => { + if (item.secureFields[key] === true || item.secureFields[key] === false) { + delete item.secureFields[key]; + } + }); + } + }); + try { await onSubmit({ ...values, 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 3a3a4729d48..e46e5b7fc80 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 @@ -4,8 +4,8 @@ import { FC, useEffect } from 'react'; import { Controller, DeepMap, FieldError, useFormContext } from 'react-hook-form'; import { GrafanaTheme2 } from '@grafana/data'; -import { Checkbox, Field, Input, RadioButtonList, Select, TextArea, useStyles2 } from '@grafana/ui'; -import { NotificationChannelOption } from 'app/types'; +import { Checkbox, Field, Input, RadioButtonList, SecretInput, Select, TextArea, useStyles2 } from '@grafana/ui'; +import { NotificationChannelOption, NotificationChannelSecureFields } from 'app/types'; import { KeyValueMapInput } from './KeyValueMapInput'; import { StringArrayInput } from './StringArrayInput'; @@ -16,16 +16,21 @@ import { WrapWithTemplateSelection } from './TemplateSelector'; interface Props { defaultValue: any; option: NotificationChannelOption; + // this is defined if the option is rendered inside a subform + parentOption?: NotificationChannelOption; invalid?: boolean; pathPrefix: string; pathSuffix?: string; error?: FieldError | DeepMap; readOnly?: boolean; customValidator?: (value: string) => boolean | string | Promise; + onResetSecureField?: (propertyName: string) => void; + secureFields?: NotificationChannelSecureFields; } export const OptionField: FC = ({ option, + parentOption, invalid, pathPrefix, pathSuffix = '', @@ -33,13 +38,16 @@ export const OptionField: FC = ({ defaultValue, readOnly = false, customValidator, + onResetSecureField, + secureFields = {}, }) => { const optionPath = `${pathPrefix}${pathSuffix}`; - const isSecure = pathSuffix === 'secureSettings.'; if (option.element === 'subform') { return ( = ({ pathPrefix={optionPath} readOnly={readOnly} pathIndex={pathPrefix} + parentOption={parentOption} customValidator={customValidator} - isSecure={isSecure} + onResetSecureField={onResetSecureField} + secureFields={secureFields} /> ); }; -const OptionInput: FC = ({ +const OptionInput: FC = ({ option, invalid, id, @@ -90,18 +100,17 @@ const OptionInput: FC { const styles = useStyles2(getStyles); const { control, register, unregister, getValues, setValue } = useFormContext(); - const name = `${pathPrefix}${option.propertyName}`; - useEffect(() => { - // Remove the value of secure fields so it doesn't show the incorrect value when clearing the field - if (isSecure) { - setValue(name, null); - } - }, [isSecure, name, setValue]); + const name = `${pathPrefix}${option.propertyName}`; + const nestedKey = parentOption ? `${parentOption.propertyName}.${option.propertyName}` : option.propertyName; + + const isEncryptedInput = secureFields?.[nestedKey]; // workaround for https://github.com/react-hook-form/react-hook-form/issues/4993#issuecomment-829012506 useEffect( @@ -138,22 +147,26 @@ const OptionInput: FC - - option.validationRule ? validateOption(v, option.validationRule, option.required) : true, - customValidator: (v) => (customValidator ? customValidator(v) : true), - }, - setValueAs: option.setValueAs, - })} - placeholder={option.placeholder} - /> + {isEncryptedInput ? ( + onResetSecureField?.(nestedKey)} isConfigured /> + ) : ( + + option.validationRule ? validateOption(v, option.validationRule, option.required) : true, + customValidator: (v) => (customValidator ? customValidator(v) : true), + }, + setValueAs: option.setValueAs, + })} + placeholder={option.placeholder} + /> + )} ); diff --git a/public/app/features/alerting/unified/components/receivers/form/fields/SubformField.tsx b/public/app/features/alerting/unified/components/receivers/form/fields/SubformField.tsx index 3fe2ae27d8a..4ccadfde32a 100644 --- a/public/app/features/alerting/unified/components/receivers/form/fields/SubformField.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/fields/SubformField.tsx @@ -2,7 +2,7 @@ import { useState } from 'react'; import { FieldError, DeepMap, useFormContext } from 'react-hook-form'; import { Button, useStyles2 } from '@grafana/ui'; -import { NotificationChannelOption } from 'app/types'; +import { NotificationChannelOption, NotificationChannelSecureFields } from 'app/types'; import { ActionIcon } from '../../../rules/ActionIcon'; @@ -15,9 +15,19 @@ interface Props { pathPrefix: string; errors?: DeepMap; readOnly?: boolean; + secureFields?: NotificationChannelSecureFields; + onResetSecureField?: (propertyName: string) => void; } -export const SubformField = ({ option, pathPrefix, errors, defaultValue, readOnly = false }: Props) => { +export const SubformField = ({ + option, + pathPrefix, + errors, + defaultValue, + readOnly = false, + secureFields = {}, + onResetSecureField, +}: Props) => { const styles = useStyles2(getReceiverFormFieldStyles); const name = `${pathPrefix}${option.propertyName}`; const { watch } = useFormContext(); @@ -45,7 +55,10 @@ export const SubformField = ({ option, pathPrefix, errors, defaultValue, readOnl return ( { - if (option.secure && values.secureSettings[option.propertyName]) { - delete values.settings[option.propertyName]; - values.secureFields[option.propertyName] = true; - } - if (option.secure && values.settings[option.propertyName]) { - values.secureSettings[option.propertyName] = values.settings[option.propertyName]; - delete values.settings[option.propertyName]; - } - }); - return values; } +/** + * Recursively find all keys that should be marked a secure fields, using JSONpath for nested fields. + */ +export function getSecureFieldNames(notifier: NotifierDTO): string[] { + // eg. ['foo', 'bar.baz'] + const secureFieldPaths: string[] = []; + + // we'll pass in the prefix for each iteration so we can track the JSON path + function findSecureOptions(options: NotificationChannelOption[], prefix?: string) { + for (const option of options) { + const key = prefix ? `${prefix}.${option.propertyName}` : option.propertyName; + + // if the field is a subform, recurse + if (option.subformOptions) { + findSecureOptions(option.subformOptions, key); + continue; + } + + if (option.secure) { + secureFieldPaths.push(key); + continue; + } + } + } + + findSecureOptions(notifier.options); + + return secureFieldPaths; +} + export function formChannelValuesToGrafanaChannelConfig( values: GrafanaChannelValues, defaults: GrafanaChannelValues, @@ -245,13 +263,21 @@ export function formChannelValuesToGrafanaChannelConfig( }; // find all secure field definitions - const secureFieldNames: string[] = - notifier?.options.filter((option) => option.secure).map((option) => option.propertyName) ?? []; + const secureFieldNames = notifier ? getSecureFieldNames(notifier) : []; // we make sure all fields that are marked as "secure" will be moved to "SecureSettings" instead of "settings" - const shouldBeSecure = pick(channel.settings, secureFieldNames); + const secureSettings = reduce( + secureFieldNames, + (acc: Record = {}, key) => { + // the value for secure settings can come from either the "settings" (accidental) or "secureFields" if editing an existing receiver + acc[key] = get(channel.settings, key) ?? get(values.secureFields, key); + return acc; + }, + {} + ); + channel.secureSettings = { - ...shouldBeSecure, + ...secureSettings, ...channel.secureSettings, }; @@ -291,7 +317,7 @@ export function omitEmptyValues(obj: T): T { // Will remove empty ('', null, undefined) object properties unless they were previously defined. // existing is a map of property names that were previously defined. export function omitEmptyUnlessExisting(settings = {}, existing = {}): Record { - return omitBy(settings, (value, key) => isUnacceptableValue(value) && !(key in existing)); + return omitBy(settings, (value, key) => isUnacceptableValue(value) && !has(existing, key)); } export function omitTemporaryIdentifiers(object: Readonly): T {