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

[release-12.3.1] Alerting: Fix contact points issue (#115427)

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

Co-authored-by: Sonia Aguilar <soniaaguilarpeiron@gmail.com>
This commit is contained in:
Yuri Tseretyan
2025-12-16 18:49:10 +00:00
committed by GitHub
co-authored by Sonia Aguilar
parent 9f42e3426a
commit 28a284f40b
35 changed files with 1017 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": "",
@@ -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",
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)