diff --git a/eslint-suppressions.json b/eslint-suppressions.json index a3292e1a102..06c6cabc1cc 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1606,22 +1606,6 @@ "count": 1 } }, - "public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx": { - "@typescript-eslint/consistent-type-assertions": { - "count": 1 - }, - "@typescript-eslint/no-explicit-any": { - "count": 2 - }, - "no-restricted-syntax": { - "count": 1 - } - }, - "public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx": { - "no-restricted-syntax": { - "count": 1 - } - }, "public/app/features/alerting/unified/components/receivers/form/CloudCommonChannelSettings.tsx": { "no-restricted-syntax": { "count": 1 @@ -1632,17 +1616,6 @@ "count": 1 } }, - "public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx": { - "@typescript-eslint/consistent-type-assertions": { - "count": 2 - }, - "@typescript-eslint/no-explicit-any": { - "count": 1 - }, - "no-restricted-syntax": { - "count": 1 - } - }, "public/app/features/alerting/unified/components/receivers/form/fields/OptionField.tsx": { "@typescript-eslint/consistent-type-assertions": { "count": 1 diff --git a/pkg/api/alerting.go b/pkg/api/alerting.go index 27abb6ebbfc..8fb2f366bf2 100644 --- a/pkg/api/alerting.go +++ b/pkg/api/alerting.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/api/response" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" + "github.com/grafana/grafana/pkg/services/ngalert/models" ) func (hs *HTTPServer) GetAlertNotifiers() func(*contextmodel.ReqContext) response.Response { @@ -23,13 +24,13 @@ func (hs *HTTPServer) GetAlertNotifiers() func(*contextmodel.ReqContext) respons } type NotifierPlugin struct { - Type string `json:"type"` - TypeAlias string `json:"typeAlias,omitempty"` - Name string `json:"name"` - Heading string `json:"heading"` - Description string `json:"description"` - Info string `json:"info"` - Options []schema.Field `json:"options"` + Type string `json:"type"` + TypeAlias string `json:"typeAlias,omitempty"` + Name string `json:"name"` + Heading string `json:"heading"` + Description string `json:"description"` + Info string `json:"info"` + Options []Field `json:"options"` } result := make([]*NotifierPlugin, 0, len(v2)) @@ -44,9 +45,56 @@ func (hs *HTTPServer) GetAlertNotifiers() func(*contextmodel.ReqContext) respons Description: s.Description, Heading: s.Heading, Info: s.Info, - Options: v1.Options, + Options: schemaFieldsToFields(s.Type, nil, v1.Options), }) } return response.JSON(http.StatusOK, result) } } + +type Field struct { + Element schema.ElementType `json:"element"` + InputType schema.InputType `json:"inputType"` + Label string `json:"label"` + Description string `json:"description"` + Placeholder string `json:"placeholder"` + PropertyName string `json:"propertyName"` + SelectOptions []schema.SelectOption `json:"selectOptions"` + ShowWhen schema.ShowWhen `json:"showWhen"` + Required bool `json:"required"` + Protected bool `json:"protected,omitempty"` + ValidationRule string `json:"validationRule"` + Secure bool `json:"secure"` + DependsOn string `json:"dependsOn"` + SubformOptions []Field `json:"subformOptions"` +} + +func schemaFieldsToFields(iType schema.IntegrationType, parent schema.IntegrationFieldPath, fields []schema.Field) []Field { + if fields == nil { + return nil + } + result := make([]Field, 0, len(fields)) + for _, f := range fields { + result = append(result, schemaFieldToField(iType, parent, f)) + } + return result +} + +func schemaFieldToField(iType schema.IntegrationType, parent schema.IntegrationFieldPath, f schema.Field) Field { + return Field{ + Element: f.Element, + InputType: f.InputType, + Label: f.Label, + Description: f.Description, + Placeholder: f.Placeholder, + PropertyName: f.PropertyName, + SelectOptions: f.SelectOptions, + ShowWhen: f.ShowWhen, + Required: f.Required, + ValidationRule: f.ValidationRule, + Secure: f.Secure, + DependsOn: f.DependsOn, + SubformOptions: schemaFieldsToFields(iType, append(parent, f.PropertyName), f.SubformOptions), + Protected: models.IsProtectedField(iType, append(parent, f.PropertyName)), + } +} diff --git a/pkg/registry/apps/alerting/notifications/receiver/conversions.go b/pkg/registry/apps/alerting/notifications/receiver/conversions.go index b0d79121912..993c63a46fd 100644 --- a/pkg/registry/apps/alerting/notifications/receiver/conversions.go +++ b/pkg/registry/apps/alerting/notifications/receiver/conversions.go @@ -110,10 +110,11 @@ func convertToK8sResource( } var permissionMapper = map[ngmodels.ReceiverPermission]string{ - ngmodels.ReceiverPermissionReadSecret: "canReadSecrets", - ngmodels.ReceiverPermissionAdmin: "canAdmin", - ngmodels.ReceiverPermissionWrite: "canWrite", - ngmodels.ReceiverPermissionDelete: "canDelete", + ngmodels.ReceiverPermissionReadSecret: "canReadSecrets", + ngmodels.ReceiverPermissionAdmin: "canAdmin", + ngmodels.ReceiverPermissionWrite: "canWrite", + ngmodels.ReceiverPermissionDelete: "canDelete", + ngmodels.ReceiverPermissionModifyProtected: "canModifyProtected", } func convertToDomainModel(receiver *model.Receiver) (*ngmodels.Receiver, map[string][]string, error) { diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index 3f93ce6ad82..d7fdd0f8f9b 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -451,6 +451,7 @@ const ( ActionAlertingReceiversReadSecrets = "alert.notifications.receivers.secrets:read" ActionAlertingReceiversCreate = "alert.notifications.receivers:create" ActionAlertingReceiversUpdate = "alert.notifications.receivers:write" + ActionAlertingReceiversUpdateProtected = "alert.notifications.receivers.protected:write" ActionAlertingReceiversDelete = "alert.notifications.receivers:delete" ActionAlertingReceiversTest = "alert.notifications.receivers:test" ActionAlertingReceiversPermissionsRead = "receivers.permissions:read" diff --git a/pkg/services/accesscontrol/ossaccesscontrol/receivers.go b/pkg/services/accesscontrol/ossaccesscontrol/receivers.go index 49ca4a0b3e8..9dd4def1db3 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/receivers.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/receivers.go @@ -23,7 +23,7 @@ import ( var ReceiversViewActions = []string{accesscontrol.ActionAlertingReceiversRead} var ReceiversEditActions = append(ReceiversViewActions, []string{accesscontrol.ActionAlertingReceiversUpdate, accesscontrol.ActionAlertingReceiversDelete}...) -var ReceiversAdminActions = append(ReceiversEditActions, []string{accesscontrol.ActionAlertingReceiversReadSecrets, accesscontrol.ActionAlertingReceiversPermissionsRead, accesscontrol.ActionAlertingReceiversPermissionsWrite}...) +var ReceiversAdminActions = append(ReceiversEditActions, []string{accesscontrol.ActionAlertingReceiversReadSecrets, accesscontrol.ActionAlertingReceiversPermissionsRead, accesscontrol.ActionAlertingReceiversPermissionsWrite, accesscontrol.ActionAlertingReceiversUpdateProtected}...) // defaultPermissions returns the default permissions for a newly created receiver. func defaultPermissions() []accesscontrol.SetResourcePermissionCommand { diff --git a/pkg/services/ngalert/accesscontrol.go b/pkg/services/ngalert/accesscontrol.go index e36c123c621..a4280ced09c 100644 --- a/pkg/services/ngalert/accesscontrol.go +++ b/pkg/services/ngalert/accesscontrol.go @@ -288,12 +288,13 @@ var ( Role: accesscontrol.RoleDTO{ Name: accesscontrol.FixedRolePrefix + "alerting:admin", DisplayName: "Full admin access", - Description: "Full write access in Grafana and all external providers, including their permissions and secrets", + Description: "Full write access in Grafana and all external providers, including their permissions, protected fields and secrets", Group: models.AlertRolesGroup, Permissions: accesscontrol.ConcatPermissions(alertingWriterRole.Role.Permissions, []accesscontrol.Permission{ {Action: accesscontrol.ActionAlertingReceiversPermissionsRead, Scope: models.ScopeReceiversAll}, {Action: accesscontrol.ActionAlertingReceiversPermissionsWrite, Scope: models.ScopeReceiversAll}, {Action: accesscontrol.ActionAlertingReceiversReadSecrets, Scope: models.ScopeReceiversAll}, + {Action: accesscontrol.ActionAlertingReceiversUpdateProtected, Scope: models.ScopeReceiversAll}, }), }, Grants: []string{string(org.RoleAdmin)}, diff --git a/pkg/services/ngalert/accesscontrol/receivers.go b/pkg/services/ngalert/accesscontrol/receivers.go index eb2904854bf..f12a9114912 100644 --- a/pkg/services/ngalert/accesscontrol/receivers.go +++ b/pkg/services/ngalert/accesscontrol/receivers.go @@ -95,6 +95,26 @@ var ( ) } + // Asserts pre-conditions for access to modify protected fields of receivers. If this evaluates to false, the user cannot modify protected fields of any receivers. + updateReceiversProtectedPreConditionsEval = ac.EvalAll( + updateReceiversPreConditionsEval, + ac.EvalPermission(ac.ActionAlertingReceiversUpdateProtected), // Action for receivers. UID scope. + ) + + // Asserts access to modify protected fields of a specific receiver. + updateReceiverProtectedEval = func(uid string) ac.Evaluator { + return ac.EvalAll( + updateReceiverEval(uid), + ac.EvalPermission(ac.ActionAlertingReceiversUpdateProtected, models.ScopeReceiversProvider.GetResourceScopeUID(uid)), + ) + } + + // Asserts access to modify protected fields of all receivers. + updateAllReceiverProtectedEval = ac.EvalAll( + updateAllReceiversEval, + ac.EvalPermission(ac.ActionAlertingReceiversUpdateProtected, models.ScopeReceiversAll), + ) + // Delete // Asserts pre-conditions for delete access to receivers. If this evaluates to false, the user cannot delete any receivers. @@ -141,12 +161,13 @@ var ( ) type ReceiverAccess[T models.Identified] struct { - read actionAccess[T] - readDecrypted actionAccess[T] - create actionAccess[T] - update actionAccess[T] - delete actionAccess[T] - permissions actionAccess[T] + read actionAccess[T] + readDecrypted actionAccess[T] + create actionAccess[T] + update actionAccess[T] + updateProtected actionAccess[T] + delete actionAccess[T] + permissions actionAccess[T] } // NewReceiverAccess creates a new ReceiverAccess service. If includeProvisioningActions is true, the service will include @@ -201,6 +222,18 @@ func NewReceiverAccess[T models.Identified](a ac.AccessControl, includeProvision }, authorizeAll: updateAllReceiversEval, }, + updateProtected: actionAccess[T]{ + genericService: genericService{ + ac: a, + }, + resource: "receiver", + action: "update protected fields of", // this produces message "user is not authorized to update protected fields of X receiver" + authorizeSome: updateReceiversProtectedPreConditionsEval, + authorizeOne: func(receiver models.Identified) ac.Evaluator { + return updateReceiverProtectedEval(receiver.GetUID()) + }, + authorizeAll: updateAllReceiverProtectedEval, + }, delete: actionAccess[T]{ genericService: genericService{ ac: a, @@ -311,6 +344,14 @@ func (s ReceiverAccess[T]) AuthorizeUpdate(ctx context.Context, user identity.Re return s.update.Authorize(ctx, user, receiver) } +func (s ReceiverAccess[T]) HasUpdateProtected(ctx context.Context, user identity.Requester, receiver T) (bool, error) { + return s.updateProtected.Has(ctx, user, receiver) +} + +func (s ReceiverAccess[T]) AuthorizeUpdateProtected(ctx context.Context, user identity.Requester, receiver T) error { + return s.updateProtected.Authorize(ctx, user, receiver) +} + // Global // AuthorizeCreate checks if user has access to create receivers. Returns an error if user does not have access. @@ -380,6 +421,12 @@ func (s ReceiverAccess[T]) Access(ctx context.Context, user identity.Requester, basePerms.Set(models.ReceiverPermissionDelete, true) // Has access to all receivers. } + if err := s.updateProtected.AuthorizePreConditions(ctx, user); err != nil { + basePerms.Set(models.ReceiverPermissionModifyProtected, false) + } else if err := s.updateProtected.AuthorizeAll(ctx, user); err == nil { + basePerms.Set(models.ReceiverPermissionModifyProtected, true) + } + if basePerms.AllSet() { // Shortcut for the case when all permissions are known based on preconditions. result := make(map[string]models.ReceiverPermissionSet, len(receivers)) @@ -412,6 +459,11 @@ func (s ReceiverAccess[T]) Access(ctx context.Context, user identity.Requester, permSet.Set(models.ReceiverPermissionDelete, err == nil) } + if _, ok := permSet.Has(models.ReceiverPermissionModifyProtected); !ok { + err := s.updateProtected.authorize(ctx, user, rcv) + permSet.Set(models.ReceiverPermissionModifyProtected, err == nil) + } + result[rcv.GetUID()] = permSet } return result, nil diff --git a/pkg/services/ngalert/accesscontrol/receivers_test.go b/pkg/services/ngalert/accesscontrol/receivers_test.go index 7be1e9498e2..316fa3a98a6 100644 --- a/pkg/services/ngalert/accesscontrol/receivers_test.go +++ b/pkg/services/ngalert/accesscontrol/receivers_test.go @@ -204,6 +204,33 @@ func TestReceiverAccess(t *testing.T) { recv3.UID: permissions(), }, }, + { + name: "update protected cannot update receivers", + user: newEmptyUser( + ac.Permission{Action: ac.ActionAlertingReceiversRead, Scope: models.ScopeReceiversAll}, + ac.Permission{Action: ac.ActionAlertingReceiversUpdateProtected, Scope: models.ScopeReceiversAll}, + ), + expected: map[string]models.ReceiverPermissionSet{ + recv1.UID: permissions(), + recv2.UID: permissions(), + recv3.UID: permissions(), + }, + }, + { + name: "update protected receivers", + user: newEmptyUser( + ac.Permission{Action: ac.ActionAlertingReceiversRead, Scope: models.ScopeReceiversAll}, + ac.Permission{Action: ac.ActionAlertingReceiversUpdateProtected, Scope: models.ScopeReceiversProvider.GetResourceScopeUID(recv1.UID)}, + ac.Permission{Action: ac.ActionAlertingReceiversUpdate, Scope: models.ScopeReceiversProvider.GetResourceScopeUID(recv1.UID)}, + ac.Permission{Action: ac.ActionAlertingReceiversUpdate, Scope: models.ScopeReceiversProvider.GetResourceScopeUID(recv2.UID)}, + ac.Permission{Action: ac.ActionAlertingReceiversUpdateProtected, Scope: models.ScopeReceiversProvider.GetResourceScopeUID(recv3.UID)}, + ), + expected: map[string]models.ReceiverPermissionSet{ + recv1.UID: permissions(models.ReceiverPermissionWrite, models.ReceiverPermissionModifyProtected), + recv2.UID: permissions(models.ReceiverPermissionWrite), + recv3.UID: permissions(), + }, + }, // Receiver delete. { name: "global receiver delete should have delete but no write", diff --git a/pkg/services/ngalert/api/api_alertmanager.go b/pkg/services/ngalert/api/api_alertmanager.go index 90dcb26fc74..f8b7216bcdb 100644 --- a/pkg/services/ngalert/api/api_alertmanager.go +++ b/pkg/services/ngalert/api/api_alertmanager.go @@ -10,8 +10,10 @@ import ( "time" alertingNotify "github.com/grafana/alerting/notify" + "github.com/grafana/alerting/receivers/schema" "github.com/grafana/grafana/pkg/api/response" + "github.com/grafana/grafana/pkg/apimachinery/errutil" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" @@ -32,6 +34,7 @@ const ( type receiversAuthz interface { FilterRead(ctx context.Context, user identity.Requester, receivers ...ReceiverStatus) ([]ReceiverStatus, error) + AuthorizeUpdateProtected(context.Context, identity.Requester, ReceiverStatus) error } type AlertmanagerSrv struct { @@ -210,11 +213,16 @@ func (srv AlertmanagerSrv) RouteGetReceivers(c *contextmodel.ReqContext) respons } func (srv AlertmanagerSrv) RoutePostTestReceivers(c *contextmodel.ReqContext, body apimodels.TestReceiversConfigBodyParams) response.Response { - if err := srv.crypto.ProcessSecureSettings(c.Req.Context(), c.GetOrgID(), body.Receivers); err != nil { + if err := srv.crypto.ProcessSecureSettings(c.Req.Context(), c.GetOrgID(), body.Receivers, func(receiverName string, paths []schema.IntegrationFieldPath) error { + return srv.receiverAuthz.AuthorizeUpdateProtected(c.Req.Context(), c.SignedInUser, ReceiverStatus{Name: receiverName}) + }); err != nil { var unknownReceiverError UnknownReceiverError if errors.As(err, &unknownReceiverError) { return ErrResp(http.StatusBadRequest, err, "") } + if errors.As(err, &errutil.Error{}) { + return response.Err(err) + } return ErrResp(http.StatusInternalServerError, err, "failed to post process Alertmanager configuration") } diff --git a/pkg/services/ngalert/models/permissions.go b/pkg/services/ngalert/models/permissions.go index a34e184fc99..ce52755eac8 100644 --- a/pkg/services/ngalert/models/permissions.go +++ b/pkg/services/ngalert/models/permissions.go @@ -9,10 +9,11 @@ import ( type ReceiverPermission string const ( - ReceiverPermissionReadSecret ReceiverPermission = "secrets" - ReceiverPermissionAdmin ReceiverPermission = "admin" - ReceiverPermissionWrite ReceiverPermission = "write" - ReceiverPermissionDelete ReceiverPermission = "delete" + ReceiverPermissionReadSecret ReceiverPermission = "secrets" + ReceiverPermissionAdmin ReceiverPermission = "admin" + ReceiverPermissionWrite ReceiverPermission = "write" + ReceiverPermissionDelete ReceiverPermission = "delete" + ReceiverPermissionModifyProtected ReceiverPermission = "modify-protected" ) // ReceiverPermissions returns all possible silence permissions. @@ -22,6 +23,7 @@ func ReceiverPermissions() []ReceiverPermission { ReceiverPermissionAdmin, ReceiverPermissionWrite, ReceiverPermissionDelete, + ReceiverPermissionModifyProtected, } } diff --git a/pkg/services/ngalert/models/receivers_diff.go b/pkg/services/ngalert/models/receivers_diff.go new file mode 100644 index 00000000000..bfe9328542f --- /dev/null +++ b/pkg/services/ngalert/models/receivers_diff.go @@ -0,0 +1,230 @@ +package models + +import ( + "fmt" + "reflect" + "strings" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/grafana/alerting/receivers/schema" + + "github.com/grafana/grafana/pkg/util/cmputil" +) + +type IntegrationDiffReport struct { + cmputil.DiffReport +} + +// expandPaths recursively collects all sub-paths for keys in the provided map value +func (r IntegrationDiffReport) expandPaths(basePath schema.IntegrationFieldPath, mapVal reflect.Value) []schema.IntegrationFieldPath { + result := make([]schema.IntegrationFieldPath, 0) + iter := mapVal.MapRange() + for iter.Next() { + keyStr := fmt.Sprintf("%v", iter.Key()) // Assume string keys + p := basePath.With(keyStr) + // Recurse if the sub-value is another map + if m, ok := r.getMap(iter.Value()); ok { + result = append(result, r.expandPaths(p, m)...) + continue + } + result = append(result, p) + } + return result +} + +func (r IntegrationDiffReport) getMap(v reflect.Value) (reflect.Value, bool) { + if v.Kind() == reflect.Map { + return v, true + } + if v.Kind() == reflect.Ptr || v.Kind() == reflect.Interface { + return r.getMap(v.Elem()) + } + return reflect.Value{}, false +} + +func (r IntegrationDiffReport) needExpand(diff cmputil.Diff) (reflect.Value, bool) { + ml, lok := r.getMap(diff.Left) + mr, rok := r.getMap(diff.Right) + if lok == rok { + return reflect.Value{}, false + } + if lok { + return ml, true + } + return mr, true +} + +func (r IntegrationDiffReport) GetSettingsPaths() []schema.IntegrationFieldPath { + diffs := r.GetDiffsForField("Settings") + paths := make([]schema.IntegrationFieldPath, 0, len(diffs)) + for _, diff := range diffs { + // diff.Path has format like Settings[url] or Settings[sub-form][field] + p := diff.Path + var path schema.IntegrationFieldPath + for { + start := strings.Index(p, "[") + if start == -1 { + break + } + p = p[start+1:] + end := strings.Index(p, "]") + if end == -1 { + break + } + fieldName := p[:end] + p = p[end+1:] + path = append(path, fieldName) + } + if m, ok := r.needExpand(diff); ok { + paths = append(paths, r.expandPaths(path, m)...) + continue + } + if len(path) > 0 { + paths = append(paths, path) + } + } + return paths +} + +func (r IntegrationDiffReport) GetSecureSettingsPaths() []schema.IntegrationFieldPath { + diffs := r.GetDiffsForField("SecureSettings") + paths := make([]schema.IntegrationFieldPath, 0, len(diffs)) + for _, diff := range diffs { + if diff.Path == "SecureSettings" { + if m, ok := r.needExpand(diff); ok { + paths = append(paths, r.expandPaths(nil, m)...) + } + continue + } + // diff.Path has format like SecureSettings[field.sub-field.sub] + p := schema.ParseIntegrationPath(diff.Path[len("SecureSettings[") : len(diff.Path)-1]) + paths = append(paths, p) + } + return paths +} + +func (integration *Integration) Diff(incoming Integration) IntegrationDiffReport { + var reporter cmputil.DiffReporter + var settingsCmp = cmpopts.AcyclicTransformer("settingsMap", func(in map[string]any) map[string]any { + if in == nil { + return map[string]any{} + } + return in + }) + var secureCmp = cmpopts.AcyclicTransformer("secureMap", func(in map[string]string) map[string]string { + if in == nil { + return map[string]string{} + } + return in + }) + schemaCmp := cmp.Comparer(func(a, b schema.IntegrationSchemaVersion) bool { + isAZero := reflect.ValueOf(a).IsZero() + isBZero := reflect.ValueOf(b).IsZero() + if isAZero && isBZero { + return true + } + if isAZero || isBZero { + return false + } + return a.Type() == b.Type() && a.Version == b.Version + }) + var cur Integration + if integration != nil { + cur = *integration + } + cmp.Equal(cur, incoming, cmp.Reporter(&reporter), settingsCmp, secureCmp, schemaCmp) + return IntegrationDiffReport{DiffReport: reporter.Diffs} +} + +// HasReceiversDifferentProtectedFields returns true if the receiver has any protected fields that are different from the incoming receiver. +func HasReceiversDifferentProtectedFields(existing, incoming *Receiver) map[string][]schema.IntegrationFieldPath { + existingIntegrations := make(map[string]*Integration, len(existing.Integrations)) + for _, integration := range existing.Integrations { + existingIntegrations[integration.UID] = integration + } + + var result = make(map[string][]schema.IntegrationFieldPath) + for _, in := range incoming.Integrations { + if in.UID == "" { + continue + } + ex, ok := existingIntegrations[in.UID] + if !ok { + continue + } + paths := HasIntegrationsDifferentProtectedFields(ex, in) + if len(paths) > 0 { + result[in.UID] = paths + } + } + return result +} + +// HasIntegrationsDifferentProtectedFields returns list of paths to protected fields that are different between two integrations. +func HasIntegrationsDifferentProtectedFields(existing, incoming *Integration) []schema.IntegrationFieldPath { + diff := existing.Diff(*incoming) + // The incoming receiver always has both secret and non-secret fields in Settings. + // So, if it's specified and happens to be sensitive, we consider it changed + var result []schema.IntegrationFieldPath + settingsDiff := diff.GetSettingsPaths() + for _, path := range settingsDiff { + if IsProtectedField(incoming.Config.Type(), path) { + result = append(result, path) + } + } + return result +} + +// IsProtectedField returns true if the field at the given path is existing protected one. +// This includes: +// 1. URL fields marked as secure in the schema (e.g., webhook URLs with credentials) +// 2. URL fields NOT marked as secure but could contain credentials (e.g., API endpoints) +func IsProtectedField(integrationType schema.IntegrationType, path schema.IntegrationFieldPath) bool { + str := strings.ToLower(string(integrationType)) + pathStr := path.String() + + switch str { + case "prometheus-alertmanager": + return pathStr == "url" + case "dingding": + return pathStr == "url" // marked as secure + case "discord": + return pathStr == "url" // marked as secure (webhook URL) + case "googlechat": + return pathStr == "url" // marked as secure + case "jira": + return pathStr == "api_url" + case "kafka": + return pathStr == "kafkaRestProxy" + case "line": + return false + case "mqtt": + return pathStr == "brokerUrl" + case "oncall": + return pathStr == "url" + case "opsgenie": + return pathStr == "apiUrl" + case "pagerduty": + return pathStr == "url" + case "sensugo": + return pathStr == "url" + case "slack": + return pathStr == "url" || pathStr == "endpointUrl" + case "teams": + return pathStr == "url" + case "victorops": + return pathStr == "url" // marked as secure + case "webex": + return pathStr == "api_url" + case "webhook": + return pathStr == "url" || + pathStr == "http_config.oauth2.token_url" || + pathStr == "http_config.oauth2.proxy_config.proxy_url" + case "wecom": + return pathStr == "url" || // marked as secure + pathStr == "endpointUrl" + default: + return false + } +} diff --git a/pkg/services/ngalert/models/receivers_diff_test.go b/pkg/services/ngalert/models/receivers_diff_test.go new file mode 100644 index 00000000000..dffeaee1dc2 --- /dev/null +++ b/pkg/services/ngalert/models/receivers_diff_test.go @@ -0,0 +1,262 @@ +package models + +import ( + "slices" + "testing" + + alertingNotify "github.com/grafana/alerting/notify" + "github.com/grafana/alerting/receivers/schema" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIntegrationDiff(t *testing.T) { + s, _ := alertingNotify.GetSchemaVersionForIntegration("webhook", schema.V1) + a := Integration{ + UID: "test-uid", + Name: "test-name", + Config: s, + DisableResolveMessage: false, + Settings: map[string]any{ + "url": "http://localhost", + "name": 123, + "flag": true, + "child": map[string]any{ + "sub-form-field": "test", + }, + }, + SecureSettings: map[string]string{ + "password": "12345", + "token": "token-12345", + }, + } + + t.Run("no diff if equal", func(t *testing.T) { + result := a.Diff(a) + assert.Empty(t, result) + }) + + t.Run("should deep compare settings", func(t *testing.T) { + b := a + b.Settings = map[string]any{ + "url": "http://localhost:123", + "flag": false, + "child": map[string]any{ + "sub-form-field": "test123", + "sub-child": map[string]any{ + "test": "test", + }, + }, + } + + result := a.Diff(b) + assert.ElementsMatch(t, + []string{"Settings[url]", "Settings[name]", "Settings[flag]", "Settings[child][sub-form-field]", "Settings[child][sub-child]"}, + result.Paths()) + }) + + t.Run("should shallow compare schemas", func(t *testing.T) { + b := a + b.Config, _ = alertingNotify.GetSchemaVersionForIntegration("slack", schema.V1) + result := a.Diff(b) + assert.ElementsMatch(t, + []string{"Config"}, + result.Paths()) + }) + + t.Run("should compare with zero objects", func(t *testing.T) { + result := a.Diff(Integration{}) + assert.ElementsMatch(t, + []string{ + "UID", + "Name", + "Config", + "Settings[child]", + "Settings[flag]", + "Settings[name]", + "Settings[url]", + "SecureSettings[password]", + "SecureSettings[token]", + }, + result.Paths()) + }) +} + +func TestIntegrationDiffReport_GetSettingsPaths(t *testing.T) { + a := Integration{ + UID: "test-uid", + Name: "test-name", + Config: schema.IntegrationSchemaVersion{}, + DisableResolveMessage: false, + Settings: map[string]any{ + "url": "http://localhost", + "child": map[string]any{ + "field": "test", + "sub-child": map[string]any{ + "test": "test", + }, + }, + }, + } + + testCases := []struct { + name string + left map[string]any + right map[string]any + paths []string + }{ + { + name: "empty", + left: map[string]any{}, + right: map[string]any{}, + }, + { + name: "left is empty", + left: map[string]any{}, + right: map[string]any{ + "field": "test", + }, + paths: []string{"field"}, + }, + { + name: "right is empty", + left: map[string]any{ + "field": "test", + }, + right: map[string]any{}, + paths: []string{"field"}, + }, + { + name: "expands nested", + left: map[string]any{ + "field": map[string]any{ + "sub-field": map[string]any{ + "test": "test", + }, + }, + }, + right: map[string]any{ + "another": map[string]any{ + "sub-field": map[string]any{ + "test": "test", + }, + }, + }, + paths: []string{ + "field.sub-field.test", + "another.sub-field.test", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + b := a + b.Settings = tc.right + a.Settings = tc.left + diff := a.Diff(b) + + actual := diff.GetSettingsPaths() + actualStrings := make([]string, 0, len(actual)) + for _, f := range actual { + actualStrings = append(actualStrings, f.String()) + } + assert.ElementsMatch(t, tc.paths, actualStrings) + }) + } +} + +func TestHasDifferentProtectedFields(t *testing.T) { + m := IntegrationMuts + + testCase := []struct { + name string + existing Integration + incoming Integration + expected map[string][]string + }{ + { + name: "different UID do not match", + existing: IntegrationGen(m.WithUID("existing"), m.WithValidConfig("webhook"))(), + incoming: IntegrationGen( + m.WithValidConfig("webhook"), + m.AddSetting("url", "http://some-other-url"), + m.WithUID("incoming"), + )(), + expected: nil, + }, + { + name: "find url protected", + existing: IntegrationGen(m.WithUID("1"), m.WithValidConfig("webhook"))(), + incoming: IntegrationGen( + m.WithValidConfig("webhook"), + m.AddSetting("url", "http://some-other-url"), + m.AddSetting("http_config", map[string]any{ + "oauth2": map[string]any{ + "proxy_config": map[string]any{ + "proxy_url": "http://some-other-url-proxy", + }, + "token_url": "http://some-other-url-token", + }, + }), + m.WithUID("1"), + )(), + expected: map[string][]string{ + "1": { + "http_config.oauth2.proxy_config.proxy_url", + "http_config.oauth2.token_url", + "url", + }, + }, + }, + { + name: "secure and protected", // simulate the situation when protected secured field is in secure settings but the incoming one has it in settings + existing: IntegrationGen( + m.WithUID("1"), + m.WithValidConfig("discord"), + m.RemoveSetting("url"), + m.WithSecureSettings(map[string]string{ + "url": "", + }))(), + incoming: IntegrationGen( + m.WithValidConfig("discord"), + m.AddSetting("url", "http://some-other-url"), + m.WithSecureSettings(nil), + m.WithUID("1"), + )(), + expected: map[string][]string{ + "1": { + "url", + }, + }, + }, + } + + for _, tc := range testCase { + t.Run(tc.name, func(t *testing.T) { + existing := &Receiver{ + Integrations: []*Integration{ + &tc.existing, + }, + } + incoming := &Receiver{ + Integrations: []*Integration{ + &tc.incoming, + }, + } + actual := HasReceiversDifferentProtectedFields(existing, incoming) + if len(tc.expected) == 0 { + require.Empty(t, actual) + return + } + actualStrings := make(map[string][]string, len(actual)) + for uid, paths := range actual { + for _, path := range paths { + actualStrings[uid] = append(actualStrings[uid], path.String()) + } + slices.Sort(actualStrings[uid]) + } + assert.EqualValues(t, tc.expected, actualStrings) + }) + } +} diff --git a/pkg/services/ngalert/models/testing.go b/pkg/services/ngalert/models/testing.go index 321f13199d7..6950a148fca 100644 --- a/pkg/services/ngalert/models/testing.go +++ b/pkg/services/ngalert/models/testing.go @@ -1378,6 +1378,12 @@ func (n IntegrationMutators) AddSecureSetting(key, val string) Mutator[Integrati } } +func (n IntegrationMutators) RemoveSetting(key string) Mutator[Integration] { + return func(c *Integration) { + delete(c.Settings, key) + } +} + func randomMapKey[K comparable, V any](m map[K]V) (K, V) { randIdx := rand.Intn(len(m)) i := 0 diff --git a/pkg/services/ngalert/notifier/alertmanager_config.go b/pkg/services/ngalert/notifier/alertmanager_config.go index 87d573124e7..7e755903791 100644 --- a/pkg/services/ngalert/notifier/alertmanager_config.go +++ b/pkg/services/ngalert/notifier/alertmanager_config.go @@ -333,7 +333,7 @@ func (moa *MultiOrgAlertmanager) SaveAndApplyAlertmanagerConfiguration(ctx conte config.ExtraConfigs = extraConfigs } - if err := moa.Crypto.ProcessSecureSettings(ctx, org, config.AlertmanagerConfig.Receivers); err != nil { + if err := moa.Crypto.ProcessSecureSettings(ctx, org, config.AlertmanagerConfig.Receivers, nil); err != nil { return fmt.Errorf("failed to post process Alertmanager configuration: %w", err) } diff --git a/pkg/services/ngalert/notifier/crypto.go b/pkg/services/ngalert/notifier/crypto.go index ab5d5f03096..246cd03d64d 100644 --- a/pkg/services/ngalert/notifier/crypto.go +++ b/pkg/services/ngalert/notifier/crypto.go @@ -29,16 +29,18 @@ const ( cryptoPrefix = "crypto_" ) +type AuthorizeProtectedFn func(uid string, paths []schema.IntegrationFieldPath) error + // Crypto allows decryption of Alertmanager Configuration and encryption of arbitrary payloads. type Crypto interface { - LoadSecureSettings(ctx context.Context, orgId int64, receivers []*definitions.PostableApiReceiver) error + LoadSecureSettings(ctx context.Context, orgId int64, receivers []*definitions.PostableApiReceiver, fn AuthorizeProtectedFn) error Encrypt(ctx context.Context, payload []byte, opt secrets.EncryptionOptions) ([]byte, error) Decrypt(ctx context.Context, payload []byte) ([]byte, error) EncryptExtraConfigs(ctx context.Context, config *definitions.PostableUserConfig) error DecryptExtraConfigs(ctx context.Context, config *definitions.PostableUserConfig) error getDecryptedSecret(r *definitions.PostableGrafanaReceiver, key string) (string, error) - ProcessSecureSettings(ctx context.Context, orgId int64, recvs []*definitions.PostableApiReceiver) error + ProcessSecureSettings(ctx context.Context, orgId int64, recvs []*definitions.PostableApiReceiver, fn AuthorizeProtectedFn) error } // alertmanagerCrypto implements decryption of Alertmanager configuration and encryption of arbitrary payloads based on Grafana's encryptions. @@ -57,7 +59,7 @@ func NewCrypto(secrets secrets.Service, configs configurationStore, log log.Logg } // ProcessSecureSettings encrypts new secure settings and loads existing secure settings from the database. -func (c *alertmanagerCrypto) ProcessSecureSettings(ctx context.Context, orgId int64, recvs []*definitions.PostableApiReceiver) error { +func (c *alertmanagerCrypto) ProcessSecureSettings(ctx context.Context, orgId int64, recvs []*definitions.PostableApiReceiver, authorizeProtected AuthorizeProtectedFn) error { // First, we encrypt the new or updated secure settings. Then, we load the existing secure settings from the database // and add back any that weren't updated. // We perform these steps in this order to ensure the hash of the secure settings remains stable when no secure @@ -68,7 +70,7 @@ func (c *alertmanagerCrypto) ProcessSecureSettings(ctx context.Context, orgId in return fmt.Errorf("failed to encrypt receivers: %w", err) } - if err := c.LoadSecureSettings(ctx, orgId, recvs); err != nil { + if err := c.LoadSecureSettings(ctx, orgId, recvs, authorizeProtected); err != nil { return err } @@ -167,7 +169,7 @@ func encryptReceiverConfigs(c []*definitions.PostableApiReceiver, encrypt defini } // LoadSecureSettings adds the corresponding unencrypted secrets stored to the list of input receivers. -func (c *alertmanagerCrypto) LoadSecureSettings(ctx context.Context, orgId int64, receivers []*definitions.PostableApiReceiver) error { +func (c *alertmanagerCrypto) LoadSecureSettings(ctx context.Context, orgId int64, receivers []*definitions.PostableApiReceiver, authorizeProtected AuthorizeProtectedFn) error { // Get the last known working configuration. amConfig, err := c.configs.GetLatestAlertmanagerConfiguration(ctx, orgId) if err != nil { @@ -176,10 +178,10 @@ func (c *alertmanagerCrypto) LoadSecureSettings(ctx context.Context, orgId int64 return fmt.Errorf("failed to get latest configuration: %w", err) } } - + var currentConfig *definitions.PostableUserConfig currentReceiverMap := make(map[string]*definitions.PostableGrafanaReceiver) if amConfig != nil { - currentConfig, err := Load([]byte(amConfig.AlertmanagerConfiguration)) + currentConfig, err = Load([]byte(amConfig.AlertmanagerConfiguration)) // If the current config is un-loadable, treat it as if it never existed. Providing a new, valid config should be able to "fix" this state. if err != nil { c.log.Warn("Last known alertmanager configuration was invalid. Overwriting...") @@ -209,6 +211,33 @@ func (c *alertmanagerCrypto) LoadSecureSettings(ctx context.Context, orgId int64 return UnknownReceiverError{UID: gr.UID} } + if authorizeProtected != nil { + incoming, errIn := legacy_storage.PostableGrafanaReceiverToIntegration(gr) + existing, errEx := legacy_storage.PostableGrafanaReceiverToIntegration(cgmr) + var secure []schema.IntegrationFieldPath + authz := true + if errIn == nil && errEx == nil { + secure = models.HasIntegrationsDifferentProtectedFields(existing, incoming) + authz = len(secure) > 0 + } + // if conversion failed, consider there are changes and authorize + if authz && currentConfig != nil { + var receiverName string + NAME: + for _, rcv := range currentConfig.AlertmanagerConfig.Receivers { + for _, intg := range rcv.GrafanaManagedReceivers { + if intg.UID == cgmr.UID { + receiverName = rcv.Name + break NAME + } + } + } + if err := authorizeProtected(receiverName, secure); err != nil { + return err + } + } + } + // Frontend sends only the secure settings that have to be updated // Therefore we have to copy from the last configuration only those secure settings not included in the request for key, encryptedValue := range cgmr.SecureSettings { diff --git a/pkg/services/ngalert/notifier/receiver_svc.go b/pkg/services/ngalert/notifier/receiver_svc.go index d25f735fcd7..71659f462df 100644 --- a/pkg/services/ngalert/notifier/receiver_svc.go +++ b/pkg/services/ngalert/notifier/receiver_svc.go @@ -79,6 +79,9 @@ type receiverAccessControlService interface { AuthorizeUpdate(context.Context, identity.Requester, *models.Receiver) error AuthorizeDeleteByUID(context.Context, identity.Requester, string) error + HasUpdateProtected(context.Context, identity.Requester, *models.Receiver) (bool, error) + AuthorizeUpdateProtected(context.Context, identity.Requester, *models.Receiver) error + Access(ctx context.Context, user identity.Requester, receivers ...*models.Receiver) (map[string]models.ReceiverPermissionSet, error) } @@ -448,6 +451,18 @@ func (rs *ReceiverService) UpdateReceiver(ctx context.Context, r *models.Receive return nil, err } + // if user does not have permissions to update protected, check the diff and return error if there is a change in protected fields + canUpdateProtected, _ := rs.authz.HasUpdateProtected(ctx, user, r) + if !canUpdateProtected { + diff := models.HasReceiversDifferentProtectedFields(existing, r) + if len(diff) > 0 { + err = rs.authz.AuthorizeUpdateProtected(ctx, user, r) + if err != nil { + return nil, makeProtectedFieldsAuthzError(err, diff) + } + } + } + // We need to perform two important steps to process settings on an updated integration: // 1. Encrypt new or updated secret fields as they will arrive in plain text. // 2. For updates, callers do not re-send unchanged secure settings and instead mark them in SecureFields. We need diff --git a/pkg/services/ngalert/notifier/receiver_svc_err.go b/pkg/services/ngalert/notifier/receiver_svc_err.go new file mode 100644 index 00000000000..06b27602f75 --- /dev/null +++ b/pkg/services/ngalert/notifier/receiver_svc_err.go @@ -0,0 +1,30 @@ +package notifier + +import ( + "errors" + "slices" + + "github.com/grafana/alerting/receivers/schema" + + "github.com/grafana/grafana/pkg/apimachinery/errutil" +) + +func makeProtectedFieldsAuthzError(err error, diff map[string][]schema.IntegrationFieldPath) error { + var authzErr errutil.Error + if !errors.As(err, &authzErr) { + return err + } + if authzErr.PublicPayload == nil { + authzErr.PublicPayload = map[string]interface{}{} + } + fields := make(map[string][]string, len(diff)) + for field, paths := range diff { + fields[field] = make([]string, len(paths)) + for i, path := range paths { + fields[field][i] = path.String() + } + slices.Sort(fields[field]) + } + authzErr.PublicPayload["changed_protected_fields"] = fields + return authzErr +} diff --git a/pkg/services/ngalert/notifier/receiver_svc_test.go b/pkg/services/ngalert/notifier/receiver_svc_test.go index 221f3e805d0..d5e70c85e10 100644 --- a/pkg/services/ngalert/notifier/receiver_svc_test.go +++ b/pkg/services/ngalert/notifier/receiver_svc_test.go @@ -556,8 +556,9 @@ func TestReceiverService_Update(t *testing.T) { writer := &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{ 1: { - accesscontrol.ActionAlertingNotificationsWrite: nil, - accesscontrol.ActionAlertingNotificationsRead: nil, + accesscontrol.ActionAlertingNotificationsWrite: nil, + accesscontrol.ActionAlertingNotificationsRead: nil, + accesscontrol.ActionAlertingReceiversUpdateProtected: {models.ScopeReceiversAll}, }, }} decryptUser := &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{ @@ -1157,7 +1158,7 @@ func TestReceiverServiceAC_Update(t *testing.T) { }, }} - slackIntegration := models.IntegrationGen(models.IntegrationMuts.WithName("test receiver"), models.IntegrationMuts.WithValidConfig("slack")) + slackIntegration := models.IntegrationGen(models.IntegrationMuts.WithName("test receiver"), models.IntegrationMuts.WithValidConfig("webhook")) emailIntegration := models.IntegrationGen(models.IntegrationMuts.WithName("test receiver"), models.IntegrationMuts.WithValidConfig("email")) recv1 := models.ReceiverGen(models.ReceiverMuts.WithName("receiver1"), models.ReceiverMuts.WithIntegrations(slackIntegration(), emailIntegration()))() recv2 := models.ReceiverGen(models.ReceiverMuts.WithName("receiver2"), models.ReceiverMuts.WithIntegrations(slackIntegration(), emailIntegration()))() @@ -1169,8 +1170,8 @@ func TestReceiverServiceAC_Update(t *testing.T) { name string permissions map[string][]string existing []models.Receiver - - hasAccess []models.Receiver + incoming []models.Receiver + hasAccess []models.Receiver }{ { name: "not authorized without permissions", @@ -1258,6 +1259,43 @@ func TestReceiverServiceAC_Update(t *testing.T) { existing: allReceivers(), hasAccess: []models.Receiver{recv1, recv3}, }, + { + name: "protected fields modified without permission", + permissions: map[string][]string{ + accesscontrol.ActionAlertingReceiversUpdate: {models.ScopeReceiversAll}, + accesscontrol.ActionAlertingReceiversRead: {models.ScopeReceiversAll}, + }, + existing: []models.Receiver{ + recv1, + }, + incoming: []models.Receiver{ + func() models.Receiver { + f := recv1.Clone() + f.Integrations[0].Settings["url"] = "https://example.com/new" + return f + }(), + }, + hasAccess: nil, + }, + { + name: "protected fields modified with permission", + permissions: map[string][]string{ + accesscontrol.ActionAlertingReceiversUpdate: {models.ScopeReceiversAll}, + accesscontrol.ActionAlertingReceiversRead: {models.ScopeReceiversAll}, + accesscontrol.ActionAlertingReceiversUpdateProtected: {models.ScopeReceiversAll}, + }, + existing: []models.Receiver{ + recv1, + }, + incoming: []models.Receiver{ + func() models.Receiver { + f := recv1.Clone() + f.Integrations[0].Settings["url"] = "https://example.com/new" + return f + }(), + }, + hasAccess: []models.Receiver{recv1}, + }, } for _, tc := range testCases { @@ -1283,7 +1321,11 @@ func TestReceiverServiceAC_Update(t *testing.T) { } return false } - for _, recv := range allReceivers() { + incoming := allReceivers() + if tc.incoming != nil { + incoming = tc.incoming + } + for _, recv := range incoming { clone := recv.Clone() clone.Version = versions[recv.UID] response, err := sut.UpdateReceiver(context.Background(), &clone, nil, orgId, usr) diff --git a/pkg/services/sqlstore/migrations/accesscontrol/alerting.go b/pkg/services/sqlstore/migrations/accesscontrol/alerting.go index f9942fac80e..deddf59eb12 100644 --- a/pkg/services/sqlstore/migrations/accesscontrol/alerting.go +++ b/pkg/services/sqlstore/migrations/accesscontrol/alerting.go @@ -116,3 +116,49 @@ func (m *receiverCreateScopeMigration) Exec(sess *xorm.Session, mg *migrator.Mig func AddReceiverCreateScopeMigration(mg *migrator.Migrator) { mg.AddMigration("remove scope from alert.notifications.receivers:create", &receiverCreateScopeMigration{}) } + +type receiverProtectedFieldsEditor struct { + migrator.MigrationBase +} + +var _ migrator.CodeMigration = new(alertingMigrator) + +func (m *receiverProtectedFieldsEditor) SQL(migrator.Dialect) string { + return "code migration" +} + +func (m *receiverProtectedFieldsEditor) Exec(sess *xorm.Session, mg *migrator.Migrator) error { + sql := `SELECT * + FROM permission AS P + WHERE action = 'alert.notifications.receivers.secrets:read' + AND EXISTS(SELECT 1 FROM role AS R WHERE R.id = P.role_id AND R.name LIKE 'managed:%') + AND NOT EXISTS(SELECT 1 + FROM permission AS P2 + WHERE P2.role_id = P.role_id + AND P2.action = 'alert.notifications.receivers.protected:write' AND P2.scope = P.scope + )` + var results []accesscontrol.Permission + if err := sess.SQL(sql).Find(&results); err != nil { + return fmt.Errorf("failed to query permissions: %w", err) + } + + permissionsToCreate := make([]accesscontrol.Permission, 0, len(results)) + rolesAffected := make(map[int64][]string, 0) + for _, result := range results { + result.ID = 0 + result.Action = "alert.notifications.receivers.protected:write" + result.Created = time.Now() + result.Updated = time.Now() + permissionsToCreate = append(permissionsToCreate, result) + rolesAffected[result.RoleID] = append(rolesAffected[result.RoleID], result.Identifier) + } + _, err := sess.InsertMulti(&permissionsToCreate) + for id, ids := range rolesAffected { + mg.Logger.Debug("Added permission 'alert.notifications.receivers.protected:write' to managed role", "roleID", id, "identifiers", ids) + } + return err +} + +func AddReceiverProtectedFieldsEditor(mg *migrator.Migrator) { + mg.AddMigration("add 'alert.notifications.receivers.protected:write' to receiver admins", &receiverProtectedFieldsEditor{}) +} diff --git a/pkg/services/sqlstore/migrations/migrations.go b/pkg/services/sqlstore/migrations/migrations.go index 04bcbc98f31..78ed892831b 100644 --- a/pkg/services/sqlstore/migrations/migrations.go +++ b/pkg/services/sqlstore/migrations/migrations.go @@ -156,4 +156,6 @@ func (oss *OSSMigrations) AddMigration(mg *Migrator) { ualert.DropTitleUniqueIndexMigration(mg) ualert.AddStateFiredAtColumn(mg) + + accesscontrol.AddReceiverProtectedFieldsEditor(mg) } diff --git a/pkg/tests/api/alerting/test-data/alert-notifiers-v1-snapshot.json b/pkg/tests/api/alerting/test-data/alert-notifiers-v1-snapshot.json index 59324e97edd..e102d40887a 100644 --- a/pkg/tests/api/alerting/test-data/alert-notifiers-v1-snapshot.json +++ b/pkg/tests/api/alerting/test-data/alert-notifiers-v1-snapshot.json @@ -82,6 +82,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": true, "dependsOn": "", @@ -208,6 +209,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": true, "dependsOn": "", @@ -352,6 +354,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": true, "dependsOn": "", @@ -415,6 +418,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -712,6 +716,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -874,6 +879,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -1158,6 +1164,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -1356,6 +1363,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -1757,6 +1765,7 @@ "is": "" }, "required": false, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -1784,6 +1793,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -2282,6 +2292,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -2574,6 +2585,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": true, "dependsOn": "token", @@ -2592,6 +2604,7 @@ "is": "" }, "required": false, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -2917,6 +2930,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -3267,6 +3281,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": true, "dependsOn": "", @@ -3357,6 +3372,7 @@ "is": "" }, "required": false, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -3438,6 +3454,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -3880,6 +3897,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -4078,6 +4096,7 @@ "is": "" }, "required": false, + "protected": true, "validationRule": "", "secure": false, "dependsOn": "", @@ -4165,6 +4184,7 @@ "is": "" }, "required": true, + "protected": true, "validationRule": "", "secure": true, "dependsOn": "secret", diff --git a/pkg/tests/apis/alerting/notifications/receivers/receiver_test.go b/pkg/tests/apis/alerting/notifications/receivers/receiver_test.go index 0f7b476ecad..5497c8f1685 100644 --- a/pkg/tests/apis/alerting/notifications/receivers/receiver_test.go +++ b/pkg/tests/apis/alerting/notifications/receivers/receiver_test.go @@ -143,7 +143,7 @@ func TestIntegrationResourcePermissions(t *testing.T) { adminClient := test_common.NewReceiverClient(t, admin) writeACMetadata := []string{"canWrite", "canDelete"} - allACMetadata := []string{"canWrite", "canDelete", "canReadSecrets", "canAdmin"} + allACMetadata := []string{"canWrite", "canDelete", "canReadSecrets", "canAdmin", "canModifyProtected"} mustID := func(user apis.User) int64 { id, err := user.Identity.GetInternalID() @@ -404,13 +404,14 @@ func TestIntegrationAccessControl(t *testing.T) { org1 := helper.Org1 type testCase struct { - user apis.User - canRead bool - canUpdate bool - canCreate bool - canDelete bool - canReadSecrets bool - canAdmin bool + user apis.User + canRead bool + canUpdate bool + canUpdateProtected bool + canCreate bool + canDelete bool + canReadSecrets bool + canAdmin bool } // region users unauthorized := helper.CreateUser("unauthorized", "Org1", org.RoleNone, []resourcepermissions.SetResourcePermissionCommand{}) @@ -473,20 +474,22 @@ func TestIntegrationAccessControl(t *testing.T) { testCases := []testCase{ { - user: unauthorized, - canRead: false, - canUpdate: false, - canCreate: false, - canDelete: false, + user: unauthorized, + canRead: false, + canUpdate: false, + canUpdateProtected: false, + canCreate: false, + canDelete: false, }, { - user: org1.Admin, - canRead: true, - canCreate: true, - canUpdate: true, - canDelete: true, - canAdmin: true, - canReadSecrets: true, + user: org1.Admin, + canRead: true, + canCreate: true, + canUpdate: true, + canUpdateProtected: true, + canDelete: true, + canAdmin: true, + canReadSecrets: true, }, { user: org1.Editor, @@ -535,22 +538,24 @@ func TestIntegrationAccessControl(t *testing.T) { canDelete: true, }, { - user: adminLikeUser, - canRead: true, - canCreate: true, - canUpdate: true, - canDelete: true, - canAdmin: true, - canReadSecrets: true, + user: adminLikeUser, + canRead: true, + canCreate: true, + canUpdate: true, + canUpdateProtected: true, + canDelete: true, + canAdmin: true, + canReadSecrets: true, }, { - user: adminLikeUserLongName, - canRead: true, - canCreate: true, - canUpdate: true, - canDelete: true, - canAdmin: true, - canReadSecrets: true, + user: adminLikeUserLongName, + canRead: true, + canCreate: true, + canUpdate: true, + canUpdateProtected: true, + canDelete: true, + canAdmin: true, + canReadSecrets: true, }, } @@ -609,6 +614,9 @@ func TestIntegrationAccessControl(t *testing.T) { if tc.canUpdate { expectedWithMetadata.SetAccessControl("canWrite") } + if tc.canUpdateProtected { + expectedWithMetadata.SetAccessControl("canModifyProtected") + } if tc.canDelete { expectedWithMetadata.SetAccessControl("canDelete") } @@ -672,6 +680,32 @@ func TestIntegrationAccessControl(t *testing.T) { require.Truef(t, errors.IsNotFound(err), "Should get NotFound error but got: %s", err) }) }) + + updatedExpected = expected.Copy().(*v0alpha1.Receiver) + updatedExpected.Spec.Integrations = []v0alpha1.ReceiverIntegration{ + createIntegration(t, "webhook"), + } + + expected, err = adminClient.Update(ctx, updatedExpected, v1.UpdateOptions{}) + require.NoErrorf(t, err, "Payload %s", string(d)) + require.NotNil(t, expected) + + updatedProtected := expected.Copy().(*v0alpha1.Receiver) + updatedProtected.Spec.Integrations[0].Settings["url"] = "http://localhost:8080/webhook" + + if tc.canUpdateProtected { + t.Run("should be able to update protected fields of the receiver", func(t *testing.T) { + updated, err := client.Update(ctx, updatedProtected, v1.UpdateOptions{}) + require.NoErrorf(t, err, "Payload %s", string(d)) + require.NotNil(t, updated) + expected = updated + }) + } else { + t.Run("should be forbidden to edit protected fields of the receiver", func(t *testing.T) { + _, err := client.Update(ctx, updatedProtected, v1.UpdateOptions{}) + require.Truef(t, errors.IsForbidden(err), "should get Forbidden error but got %s", err) + }) + } } else { t.Run("should be forbidden to update receiver", func(t *testing.T) { _, err := client.Update(ctx, updatedExpected, v1.UpdateOptions{}) @@ -684,6 +718,7 @@ func TestIntegrationAccessControl(t *testing.T) { require.Truef(t, errors.IsForbidden(err), "should get Forbidden error but got %s", err) }) }) + require.Falsef(t, tc.canUpdateProtected, "Invalid combination of assertions. CanUpdateProtected should be false") } deleteOptions := v1.DeleteOptions{Preconditions: &v1.Preconditions{ResourceVersion: util.Pointer(expected.ResourceVersion)}} @@ -1291,6 +1326,7 @@ func TestIntegrationCRUD(t *testing.T) { receiver.SetAccessControl("canDelete") receiver.SetAccessControl("canReadSecrets") receiver.SetAccessControl("canAdmin") + receiver.SetAccessControl("canModifyProtected") receiver.SetInUse(0, nil) receiver.SetCanUse(true) diff --git a/public/app/features/alerting/unified/components/receivers/NewReceiverView.test.tsx b/public/app/features/alerting/unified/components/receivers/NewReceiverView.test.tsx index e1a1aa5aeba..ad6ee94d73e 100644 --- a/public/app/features/alerting/unified/components/receivers/NewReceiverView.test.tsx +++ b/public/app/features/alerting/unified/components/receivers/NewReceiverView.test.tsx @@ -1,6 +1,6 @@ import { Route, Routes } from 'react-router-dom-v5-compat'; import { render, screen } from 'test/test-utils'; -import { byLabelText, byPlaceholderText, byRole, byTestId } from 'testing-library-selector'; +import { byPlaceholderText, byRole, byTestId } from 'testing-library-selector'; import { captureRequests } from 'app/features/alerting/unified/mocks/server/events'; import { AccessControlAction } from 'app/types/accessControl'; @@ -122,7 +122,7 @@ const ui = { inputs: { name: byPlaceholderText('Name'), email: { - addresses: byLabelText(/Addresses/), + addresses: byRole('textbox', { name: /^Addresses/ }), }, }, }; 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 fbdfdbbf493..f4f350dcc6b 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx @@ -29,8 +29,8 @@ export interface Props { * This is used to access the settings and secure fields for the integration in a type-safe way. */ integrationPrefix: `items.${number}`; + canEditProtectedFields: boolean; readOnly?: boolean; - customValidators?: Record['customValidator']>; } @@ -43,6 +43,7 @@ export function ChannelOptions({ integrationPrefix, readOnly = false, customValidators = {}, + canEditProtectedFields, }: Props): JSX.Element { const { watch } = useFormContext>(); @@ -53,7 +54,7 @@ export function ChannelOptions({ const getOptionMeta = (option: NotificationChannelOption): OptionMeta => ({ required: determineRequired(option, settings, secureFields), - readOnly: determineReadOnly(option, settings, secureFields), + readOnly: determineReadOnly(option, settings, secureFields, canEditProtectedFields), }); return ( @@ -77,6 +78,7 @@ export function ChannelOptions({ label={option.label} description={option.description} htmlFor={`${settingsPath}${option.propertyName}`} + noMargin > ({ ); } - const error: FieldError | DeepMap | undefined = ( - (option.secure ? errors?.secureFields : errors?.settings) as DeepMap | undefined - )?.[option.secureFieldKey ?? option.propertyName]; + const errorSource = option.secure ? errors?.secureFields : errors?.settings; + const propertyKey = option.secureFieldKey ?? option.propertyName; + const error = // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + (errorSource as Record, FieldError>> | undefined)?.[ + propertyKey + ]; const defaultValue = defaultValues?.settings?.[option.propertyName]; @@ -139,8 +144,14 @@ const determineRequired = ( const determineReadOnly = ( option: NotificationChannelOption, settings: Record, - secureFields: NotificationChannelSecureFields + secureFields: NotificationChannelSecureFields, + canEditProtectedFields: boolean ) => { + if (option.protected && !canEditProtectedFields) { + return true; + } + + // Handle fields with dependencies (e.g., field B depends on field A being set) if (!option.dependsOn) { return false; } diff --git a/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.test.tsx b/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.test.tsx index 4fda03e9d53..00a933acec3 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.test.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.test.tsx @@ -89,6 +89,7 @@ describe('ChannelSubForm', () => { commonSettingsComponent={GrafanaCommonChannelSettings} isEditable={true} isTestable={false} + canEditProtectedFields={true} /> 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 0202e7c2e52..fafcd858efa 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx @@ -35,6 +35,7 @@ interface Props { onDelete?: () => void; isEditable?: boolean; isTestable?: boolean; + canEditProtectedFields: boolean; customValidators?: React.ComponentProps['customValidators']; } @@ -52,6 +53,7 @@ export function ChannelSubForm({ commonSettingsComponent: CommonSettingsComponent, isEditable = true, isTestable, + canEditProtectedFields, customValidators = {}, }: Props): JSX.Element { const styles = useStyles2(getStyles); @@ -210,6 +212,7 @@ export function ChannelSubForm({ label={t('alerting.channel-sub-form.label-integration', 'Integration')} htmlFor={contactPointTypeInputId} data-testid={`${pathPrefix}type`} + noMargin > ({ onDeleteSubform={onDeleteSubform} integrationPrefix={channelFieldPath} readOnly={!isEditable} + canEditProtectedFields={canEditProtectedFields} customValidators={customValidators} /> {!!(mandatoryOptions.length && optionalOptions.length) && ( @@ -301,6 +305,7 @@ export function ChannelSubForm({ errors={errors} integrationPrefix={channelFieldPath} readOnly={!isEditable} + canEditProtectedFields={canEditProtectedFields} customValidators={customValidators} /> diff --git a/public/app/features/alerting/unified/components/receivers/form/CloudReceiverForm.tsx b/public/app/features/alerting/unified/components/receivers/form/CloudReceiverForm.tsx index 6512dcf86b3..ea490821452 100644 --- a/public/app/features/alerting/unified/components/receivers/form/CloudReceiverForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/CloudReceiverForm.tsx @@ -91,6 +91,7 @@ export const CloudReceiverForm = ({ contactPoint, alertManagerSourceName, readOn alertManagerSourceName={alertManagerSourceName} defaultItem={defaultChannelValues} commonSettingsComponent={CloudCommonChannelSettings} + canEditProtectedFields={true} /> ); diff --git a/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.tsx b/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.tsx index de17ab6c7c0..df68ab185dc 100644 --- a/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.tsx @@ -9,7 +9,7 @@ import { } from 'app/features/alerting/unified/components/contact-points/useContactPoints'; import { showManageContactPointPermissions } from 'app/features/alerting/unified/components/contact-points/utils'; import { GRAFANA_RULES_SOURCE_NAME } from 'app/features/alerting/unified/utils/datasource'; -import { canEditEntity } from 'app/features/alerting/unified/utils/k8s/utils'; +import { canEditEntity, canModifyProtectedEntity } from 'app/features/alerting/unified/utils/k8s/utils'; import { GrafanaManagedContactPoint, GrafanaManagedReceiverConfig, @@ -124,8 +124,10 @@ export const GrafanaReceiverForm = ({ contactPoint, readOnly = false, editMode } // If there is no contact point it means we're creating a new one, so scoped permissions doesn't exist yet const hasScopedEditPermissions = contactPoint ? canEditEntity(contactPoint) : true; + const hasScopedEditProtectedPermissions = contactPoint ? canModifyProtectedEntity(contactPoint) : true; const isEditable = !readOnly && hasScopedEditPermissions && !contactPoint?.provisioned; const isTestable = !readOnly; + const canEditProtectedFields = editMode ? hasScopedEditProtectedPermissions : true; if (isLoadingNotifiers || isLoadingOnCallIntegration) { return ( @@ -178,6 +180,7 @@ export const GrafanaReceiverForm = ({ contactPoint, readOnly = false, editMode } canManagePermissions={ editMode && contactPoint && showManageContactPointPermissions(GRAFANA_RULES_SOURCE_NAME, contactPoint) } + canEditProtectedFields={canEditProtectedFields} /> {testReceivers && ( { showDefaultRouteWarning?: boolean; contactPointId?: string; canManagePermissions?: boolean; + canEditProtectedFields: boolean; } export function ReceiverForm({ @@ -58,6 +59,7 @@ export function ReceiverForm({ showDefaultRouteWarning, contactPointId, canManagePermissions, + canEditProtectedFields, }: Props) { const notifyApp = useAppNotification(); const styles = useStyles2(getStyles); @@ -66,15 +68,16 @@ export function ReceiverForm({ // normalize deprecated and new config values const normalizedConfig = normalizeFormValues(initialValues); - const defaultValues = normalizedConfig ?? { + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + const defaultValues = (normalizedConfig ?? { name: '', items: [ { ...defaultItem, __id: String(Math.random()), - } as any, + }, ], - }; + }) as ReceiverFormValues; const formAPI = useForm>({ // making a copy here beacuse react-hook-form will mutate these, and break if the object is frozen. for real. @@ -148,6 +151,7 @@ export function ReceiverForm({ invalid={!!errors.name} error={errors.name && errors.name.message} required + noMargin > ({ onDelete={() => remove(index)} pathPrefix={pathPrefix} notifiers={notifiers} - errors={errors?.items?.[index] as FieldErrors} + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + errors={errors?.items?.[index] as FieldErrors | undefined} commonSettingsComponent={commonSettingsComponent} isEditable={isEditable} isTestable={isTestable} + canEditProtectedFields={canEditProtectedFields} customValidators={customValidators ? customValidators[field.type] : undefined} /> ); 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 d965fafb40d..f037a127b64 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 @@ -3,15 +3,19 @@ import { FC } from 'react'; import { Controller, DeepMap, FieldError, useFormContext } from 'react-hook-form'; import { GrafanaTheme2 } from '@grafana/data'; +import { t } from '@grafana/i18n'; import { Checkbox, Field, + Icon, Input, RadioButtonList, SecretInput, SecretTextArea, Select, + Stack, TextArea, + Tooltip, useStyles2, } from '@grafana/ui'; import { @@ -64,6 +68,7 @@ export const OptionField: FC = ({ errors={error} pathPrefix={pathPrefix} onDelete={onDeleteSubform} + getOptionMeta={getOptionMeta} /> ); } @@ -76,13 +81,34 @@ export const OptionField: FC = ({ option={option} pathPrefix={pathPrefix} errors={error as Array> | undefined} + getOptionMeta={getOptionMeta} /> ); } + const shouldShowProtectedIndicator = option.protected && getOptionMeta?.(option).readOnly; + + const labelText = option.element !== 'checkbox' && option.element !== 'radio' ? option.label : undefined; + + const label = shouldShowProtectedIndicator ? ( + + + + + {labelText} + + ) : ( + labelText + ); + return ( > | null; defaultValue?: SelectableValue; showWhen: { field: string; is: string | boolean }; diff --git a/public/app/features/alerting/unified/utils/k8s/constants.ts b/public/app/features/alerting/unified/utils/k8s/constants.ts index 1eb3c371d59..cf297261733 100644 --- a/public/app/features/alerting/unified/utils/k8s/constants.ts +++ b/public/app/features/alerting/unified/utils/k8s/constants.ts @@ -21,6 +21,8 @@ export enum K8sAnnotations { AccessAdmin = 'grafana.com/access/canAdmin', /** Annotation key that indicates that the calling user is able to delete this entity */ AccessDelete = 'grafana.com/access/canDelete', + /** Annotation key that indicates that the calling user is able to modify protected fields of this entity */ + AccessModifyProtected = 'grafana.com/access/canModifyProtected', } /** diff --git a/public/app/features/alerting/unified/utils/k8s/utils.ts b/public/app/features/alerting/unified/utils/k8s/utils.ts index 8aecb9dd46b..48ec5685a69 100644 --- a/public/app/features/alerting/unified/utils/k8s/utils.ts +++ b/public/app/features/alerting/unified/utils/k8s/utils.ts @@ -42,6 +42,9 @@ export const canAdminEntity = (k8sEntity: EntityToCheck) => export const canDeleteEntity = (k8sEntity: EntityToCheck) => getAnnotation(k8sEntity, K8sAnnotations.AccessDelete) === 'true'; +export const canModifyProtectedEntity = (k8sEntity: EntityToCheck) => + getAnnotation(k8sEntity, K8sAnnotations.AccessModifyProtected) === 'true'; + /** * Escape \ and = characters for field selectors. * The Kubernetes API Machinery will decode those automatically. diff --git a/public/app/types/accessControl.ts b/public/app/types/accessControl.ts index abbf8969b05..ddf332ccb8c 100644 --- a/public/app/types/accessControl.ts +++ b/public/app/types/accessControl.ts @@ -141,6 +141,7 @@ export enum AccessControlAction { AlertingReceiversCreate = 'alert.notifications.receivers:create', AlertingReceiversWrite = 'alert.notifications.receivers:write', AlertingReceiversRead = 'alert.notifications.receivers:read', + AlertingReceiversUpdateProtected = 'alert.notifications.receivers.protected:write', // Alerting routes actions AlertingRoutesRead = 'alert.notifications.routes:read', diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 50ddc6b897b..37d65c9b11c 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -2227,6 +2227,13 @@ "receiver-metadata-badge": { "aria-label-open-external-link": "Open external link" }, + "receivers": { + "protected": { + "field": { + "description": "This field is protected and can only be edited by users with elevated permissions" + } + } + }, "receivers-section": { "button-more": "More", "new-menu": {