public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Alerting: Protected fields for Contact points (#115442)

Browse Source 
* Alerting: Protect sensitive fields of contact points from
 unauthorized modification

- Introduce a new permission alert.notifications.receivers.protected:write. The permission is granted to contact point administrators.
- Introduce field Protected to NotifierOption
- Introduce DiffReport for models.Integrations with focus on Settings. The diff report is extended with methods that return all keys that are different between two settings.
- Add new annotation 'grafana.com/access/CanModifyProtected' to Receiver model
- Update receiver service to enforce the permission and return status 403 if unauthorized user modifies protected field
- Update receiver testing API to enforce permission and return status 403 if unauthorized user modifies protected field.
- Update UI to disable protected fields if user cannot modify them
This commit is contained in:
Yuri Tseretyan
2025-12-16 15:56:02 -05:00
committed by GitHub
parent 30fb1c032a
commit f2c30cbbd1
37 changed files with 1482 additions and 114 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/tests/api/alerting/test-data/alert-notifiers-v1-snapshot.json
+20
View File
@@ -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": "",
@@ -451,6 +454,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -748,6 +752,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -910,6 +915,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -1194,6 +1200,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -1392,6 +1399,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -1793,6 +1801,7 @@
"is": ""
},
"required": false,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -1820,6 +1829,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -2318,6 +2328,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -2610,6 +2621,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": true,
"dependsOn": "token",
@@ -2628,6 +2640,7 @@
"is": ""
},
"required": false,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -2953,6 +2966,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -3303,6 +3317,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": true,
"dependsOn": "",
@@ -3393,6 +3408,7 @@
"is": ""
},
"required": false,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -3474,6 +3490,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -3916,6 +3933,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -4114,6 +4132,7 @@
"is": ""
},
"required": false,
"protected": true,
"validationRule": "",
"secure": false,
"dependsOn": "",
@@ -4201,6 +4220,7 @@
"is": ""
},
"required": true,
"protected": true,
"validationRule": "",
"secure": true,
"dependsOn": "secret",
pkg/tests/apis/alerting/notifications/receivers/receiver_test.go
+70 -34
View File
@@ -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)
pkg/tests/apis/alerting/notifications/receivers/test-data/imported-expected-snapshot.json
+16
View File
@@ -8,6 +8,7 @@
"annotations": {
"grafana.com/access/canAdmin": "true",
"grafana.com/access/canDelete": "true",
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/access/canWrite": "true",
"grafana.com/canUse": "true",
@@ -40,6 +41,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -61,6 +63,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -105,6 +108,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -153,6 +157,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -211,6 +216,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -256,6 +262,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -317,6 +324,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "1",
@@ -388,6 +396,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -441,6 +450,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -525,6 +535,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -579,6 +590,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -625,6 +637,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -674,6 +687,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",
@@ -722,6 +736,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "1",
@@ -767,6 +782,7 @@
"kind": "Receiver",
"metadata": {
"annotations": {
"grafana.com/access/canModifyProtected": "true",
"grafana.com/access/canReadSecrets": "true",
"grafana.com/canUse": "false",
"grafana.com/inUse/routes": "0",