Alerting: Fix RBAC actions for notification policies (#49185)

* squash actions "alert.notifications:update", "alert.notifications:create", "alert.notifications:delete" to "alert.notifications:write"
* add migration
* update UI to use the write action
* update docs
* changelog

docs/sources/enterprise/access-control/custom-role-actions-scopes.md

@@ -30,10 +30,8 @@ The following list contains role-based access control actions.
 | `alert.instances:update`             | n/a                                                                                     | Update and expire silences in the current organization.                                                                                                                                          |
 | `alert.notifications.external:read`  | `datasources:*`<br>`datasources:uid:*`                                                  | Read templates, contact points, notification policies, and mute timings in data sources that support alerting.                                                                                   |
 | `alert.notifications.external:write` | `datasources:*`<br>`datasources:uid:*`                                                  | Manage templates, contact points, notification policies, and mute timings in data sources that support alerting.                                                                                 |
-| `alert.notifications:create`         | n/a                                                                                     | Create templates, contact points, notification policies, and mute timings in the current organization.                                                                                           |
+| `alert.notifications:write`          | n/a                                                                                     | Manage templates, contact points, notification policies, and mute timings in the current organization.                                                                                           |
-| `alert.notifications:delete`         | n/a                                                                                     | Delete templates, contact points, notification policies, and mute timings in the current organization.                                                                                           |
 | `alert.notifications:read`           | n/a                                                                                     | Read all templates, contact points, notification policies, and mute timings in the current organization.                                                                                         |
-| `alert.notifications:update`         | n/a                                                                                     | Update templates, contact points, notification policies, and mute timings in the current organization.                                                                                           |
 | `alert.rules.external:read`          | `datasources:*`<br>`datasources:uid:*`                                                  | Read alert rules in data sources that support alerting (Prometheus, Mimir, and Loki)                                                                                                             |
 | `alert.rules.external:write`         | `datasources:*`<br>`datasources:uid:*`                                                  | Create, update, and delete alert rules in data sources that support alerting (Mimir and Loki).                                                                                                   |
 | `alert.rules:create`                 | `folders:*`<br>`folders:uid:*`                                                          | Create Grafana alert rules in a folder. Combine this permission with `folders:read` in a scope that includes the folder and `datasources:query` in the scope of data sources the user can query. |

docs/sources/enterprise/access-control/rbac-fixed-basic-role-definitions.md

@@ -28,7 +28,7 @@ The following tables list permissions associated with basic and fixed roles.
 | -------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | `fixed:alerting.instances:editor`      | All permissions from `fixed:alerting.instances:reader` and<br> `alert.instances:create`<br>`alert.instances:update` for organization scope <br> `alert.instances.external:write` for scope `datasources:*`                                                               | Create, update and expire all silences in the organization produced by Grafana, Mimir, and Loki.[\*](#alerting-roles)                                                                                                                                                                 |
 | `fixed:alerting.instances:reader`      | `alert.instances:read` for organization scope <br> `alert.instances.external:read` for scope `datasources:*`                                                                                                                                                             | Read all alerts and silences in the organization produced by Grafana Alerts and Mimir and Loki alerts and silences.[\*](#alerting-roles)                                                                                                                                              |
-| `fixed:alerting.notifications:editor`  | All permissions from `fixed:alerting.notifications:reader` and<br>`alert.notifications:create`<br>`alert.notifications:update`<br>`alert.notifications:delete` for organization scope<br>`alert.notifications.external:read` for scope `datasources:*`                   | Create, update, and delete contact points, templates, mute timings and notification policies for Grafana and external Alertmanager.[\*](#alerting-roles)                                                                                                                              |
+| `fixed:alerting.notifications:editor`  | All permissions from `fixed:alerting.notifications:reader` and<br>`alert.notifications:write`for organization scope<br>`alert.notifications.external:read` for scope `datasources:*`                                                                                     | Create, update, and delete contact points, templates, mute timings and notification policies for Grafana and external Alertmanager.[\*](#alerting-roles)                                                                                                                              |
 | `fixed:alerting.notifications:reader`  | `alert.notifications:read` for organization scope<br>`alert.notifications.external:read` for scope `datasources:*`                                                                                                                                                       | Read all Grafana and Alertmanager contact points, templates, and notification policies.[\*](#alerting-roles)                                                                                                                                                                          |
 | `fixed:alerting.rules:editor`          | All permissions from `fixed:alerting.rules:reader` and <br> `alert.rule:create` <br> `alert.rule:update` <br> `alert.rule:delete` for scope `folders:*` <br> `alert.rules.external:write` for scope `datasources:*`                                                      | Create, update, and delete all\* Grafana, Mimir, and Loki alert rules.[\*](#alerting-roles)                                                                                                                                                                                           |
 | `fixed:alerting.rules:reader`          | `alert.rule:read` for scope `folders:*` <br> `alert.rules.external:read` for scope `datasources:*`                                                                                                                                                                       | Read all\* Grafana, Mimir, and Loki alert rules.[\*](#alerting-roles)                                                                                                                                                                                                                 |

pkg/services/accesscontrol/models.go

@@ -372,10 +372,8 @@ const (
 	ActionAlertingInstanceRead   = "alert.instances:read"
 
 	// Alerting Notification policies actions
-	ActionAlertingNotificationsCreate = "alert.notifications:create"
 	ActionAlertingNotificationsRead  = "alert.notifications:read"
-	ActionAlertingNotificationsUpdate = "alert.notifications:update"
+	ActionAlertingNotificationsWrite = "alert.notifications:write"
-	ActionAlertingNotificationsDelete = "alert.notifications:delete"
 
 	// External alerting rule actions. We can only narrow it down to writes or reads, as we don't control the atomicity in the external system.
 	ActionAlertingRuleExternalWrite = "alert.rules.external:write"

pkg/services/ngalert/CHANGELOG.md

@@ -56,6 +56,7 @@ Scopes must have an order to ensure consistency and ease of search, this helps u
 - [FEATURE] Indicate whether alert rule is provisioned when GETting the rule #48458
 - [BUGFIX] Migration: ignore alerts that do not belong to any existing organization\dashboard #49192
 - [BUGFIX] Allow anonymous access to alerts #49203 
+- [BUGFIX] RBAC: replace create\update\delete actions for notification policies by alert.notifications:write #49185
 
 ## 8.5.3
 

pkg/services/ngalert/accesscontrol.go

@@ -125,16 +125,10 @@ var (
 			DisplayName: "Notifications Editor",
 			Description: "Can add, update, and delete contact points and notification policies in Grafana and external providers",
 			Group:       AlertRolesGroup,
-			Version:     1,
+			Version:     2,
 			Permissions: accesscontrol.ConcatPermissions(notificationsReaderRole.Role.Permissions, []accesscontrol.Permission{
 				{
-					Action: accesscontrol.ActionAlertingNotificationsCreate,
+					Action: accesscontrol.ActionAlertingNotificationsWrite,
-				},
-				{
-					Action: accesscontrol.ActionAlertingNotificationsUpdate,
-				},
-				{
-					Action: accesscontrol.ActionAlertingNotificationsDelete,
 				},
 				{
 					Action: accesscontrol.ActionAlertingNotificationsExternalWrite,
@@ -162,7 +156,7 @@ var (
 			DisplayName: "Full access",
 			Description: "Can add,update and delete alert rules, instances, silences, contact points, and notification policies in Grafana and all external providers",
 			Group:       AlertRolesGroup,
-			Version:     2,
+			Version:     3,
 			Permissions: accesscontrol.ConcatPermissions(rulesEditorRole.Role.Permissions, instancesEditorRole.Role.Permissions, notificationsEditorRole.Role.Permissions),
 		},
 		Grants: []string{string(models.ROLE_EDITOR), string(models.ROLE_ADMIN)},

pkg/services/ngalert/api/authorization.go

@@ -145,7 +145,7 @@ func (api *API) authorize(method, path string) web.Handler {
 
 	// Grafana Paths
 	case http.MethodDelete + "/api/alertmanager/grafana/config/api/v1/alerts": // reset alertmanager config to the default
-		eval = ac.EvalPermission(ac.ActionAlertingNotificationsDelete)
+		eval = ac.EvalPermission(ac.ActionAlertingNotificationsWrite)
 	case http.MethodGet + "/api/alertmanager/grafana/config/api/v1/alerts":
 		fallback = middleware.ReqEditorRole
 		eval = ac.EvalPermission(ac.ActionAlertingNotificationsRead)
@@ -153,14 +153,14 @@ func (api *API) authorize(method, path string) web.Handler {
 		eval = ac.EvalPermission(ac.ActionAlertingNotificationsRead)
 	case http.MethodPost + "/api/alertmanager/grafana/config/api/v1/alerts":
 		// additional authorization is done in the request handler
-		eval = ac.EvalAny(ac.EvalPermission(ac.ActionAlertingNotificationsUpdate), ac.EvalPermission(ac.ActionAlertingNotificationsCreate), ac.EvalPermission(ac.ActionAlertingNotificationsDelete))
+		eval = ac.EvalAny(ac.EvalPermission(ac.ActionAlertingNotificationsWrite))
 	case http.MethodPost + "/api/alertmanager/grafana/config/api/v1/receivers/test":
 		fallback = middleware.ReqEditorRole
 		eval = ac.EvalPermission(ac.ActionAlertingNotificationsRead)
 
 	// External Alertmanager Paths
 	case http.MethodDelete + "/api/alertmanager/{DatasourceUID}/config/api/v1/alerts":
-		eval = ac.EvalPermission(ac.ActionAlertingNotificationsDelete, datasources.ScopeProvider.GetResourceScopeUID(ac.Parameter(":DatasourceUID")))
+		eval = ac.EvalPermission(ac.ActionAlertingNotificationsExternalWrite, datasources.ScopeProvider.GetResourceScopeUID(ac.Parameter(":DatasourceUID")))
 	case http.MethodGet + "/api/alertmanager/{DatasourceUID}/api/v2/status":
 		eval = ac.EvalPermission(ac.ActionAlertingNotificationsExternalRead, datasources.ScopeProvider.GetResourceScopeUID(ac.Parameter(":DatasourceUID")))
 	case http.MethodGet + "/api/alertmanager/{DatasourceUID}/config/api/v1/alerts":

pkg/services/sqlstore/migrations/accesscontrol/alerting.go

@@ -0,0 +1,87 @@
+package accesscontrol
+
+import (
+	"fmt"
+	"time"
+
+	"xorm.io/xorm"
+
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
+)
+
+func AddAlertingPermissionsMigrator(mg *migrator.Migrator) {
+	mg.AddMigration("alerting notification permissions", &alertingMigrator{})
+}
+
+type alertingMigrator struct {
+	sess     *xorm.Session
+	migrator *migrator.Migrator
+	migrator.MigrationBase
+}
+
+var _ migrator.CodeMigration = new(alertingMigrator)
+
+func (m *alertingMigrator) SQL(migrator.Dialect) string {
+	return "code migration"
+}
+
+func (m *alertingMigrator) Exec(sess *xorm.Session, migrator *migrator.Migrator) error {
+	m.sess = sess
+	m.migrator = migrator
+	return m.migrateNotificationActions()
+}
+
+func (m *alertingMigrator) migrateNotificationActions() error {
+	var results []accesscontrol.Permission
+	err := m.sess.Table(&accesscontrol.Permission{}).In("action", "alert.notifications:update", "alert.notifications:create", "alert.notifications:delete", accesscontrol.ActionAlertingNotificationsWrite).Find(&results)
+	if err != nil {
+		return fmt.Errorf("failed to query permission table: %w", err)
+	}
+	groupByRoleID := make(map[int64]bool)
+	toDelete := make([]interface{}, 0, len(results))
+	for _, result := range results {
+		if result.Action == accesscontrol.ActionAlertingNotificationsWrite {
+			groupByRoleID[result.RoleID] = false
+			continue // do not delete this permission
+		}
+		if _, ok := groupByRoleID[result.RoleID]; !ok {
+			groupByRoleID[result.RoleID] = true
+		}
+		toDelete = append(toDelete, result.ID)
+	}
+
+	toAdd := make([]accesscontrol.Permission, 0, len(groupByRoleID))
+
+	now := time.Now()
+	for roleID, add := range groupByRoleID {
+		if !add {
+			m.migrator.Logger.Info(fmt.Sprintf("skip adding action %s to role ID %d because it is already there", accesscontrol.ActionAlertingNotificationsWrite, roleID))
+			continue
+		}
+		toAdd = append(toAdd, accesscontrol.Permission{
+			RoleID:  roleID,
+			Action:  accesscontrol.ActionAlertingNotificationsWrite,
+			Scope:   "",
+			Created: now,
+			Updated: now,
+		})
+	}
+
+	if len(toAdd) > 0 {
+		added, err := m.sess.Table(&accesscontrol.Permission{}).InsertMulti(toAdd)
+		if err != nil {
+			return fmt.Errorf("failed to insert new permissions:%w", err)
+		}
+		m.migrator.Logger.Debug(fmt.Sprintf("updated %d of %d roles with new permission %s", added, len(toAdd), accesscontrol.ActionAlertingNotificationsWrite))
+	}
+
+	if len(toDelete) > 0 {
+		_, err = m.sess.Table(&accesscontrol.Permission{}).In("id", toDelete...).Delete(accesscontrol.Permission{})
+		if err != nil {
+			return fmt.Errorf("failed to delete deprecated permissions [alert.notifications:update, alert.notifications:create, alert.notifications:delete]:%w", err)
+		}
+	}
+
+	return nil
+}

pkg/services/sqlstore/migrations/migrations.go

@@ -80,6 +80,7 @@ func (*OSSMigrations) AddMigration(mg *Migrator) {
 		if mg.Cfg.RBACEnabled {
 			accesscontrol.AddTeamMembershipMigrations(mg)
 			accesscontrol.AddDashboardPermissionsMigrator(mg)
+			accesscontrol.AddAlertingPermissionsMigrator(mg)
 		}
 	}
 	addQueryHistoryStarMigrations(mg)

public/app/features/alerting/routes.tsx

@@ -113,7 +113,7 @@ const unifiedRoutes: RouteDescriptor[] = [
   {
     path: '/alerting/routes/mute-timing/new',
     roles: evaluateAccess(
-      [AccessControlAction.AlertingNotificationsCreate, AccessControlAction.AlertingNotificationsExternalWrite],
+      [AccessControlAction.AlertingNotificationsWrite, AccessControlAction.AlertingNotificationsExternalWrite],
       ['Editor', 'Admin']
     ),
     component: SafeDynamicImport(
@@ -123,7 +123,7 @@ const unifiedRoutes: RouteDescriptor[] = [
   {
     path: '/alerting/routes/mute-timing/edit',
     roles: evaluateAccess(
-      [AccessControlAction.AlertingNotificationsUpdate, AccessControlAction.AlertingNotificationsExternalWrite],
+      [AccessControlAction.AlertingNotificationsWrite, AccessControlAction.AlertingNotificationsExternalWrite],
       ['Editor', 'Admin']
     ),
     component: SafeDynamicImport(
@@ -173,7 +173,7 @@ const unifiedRoutes: RouteDescriptor[] = [
   {
     path: '/alerting/notifications/templates/new',
     roles: evaluateAccess(
-      [AccessControlAction.AlertingNotificationsCreate, AccessControlAction.AlertingNotificationsExternalWrite],
+      [AccessControlAction.AlertingNotificationsWrite, AccessControlAction.AlertingNotificationsExternalWrite],
       ['Editor', 'Admin']
     ),
     component: SafeDynamicImport(
@@ -183,7 +183,7 @@ const unifiedRoutes: RouteDescriptor[] = [
   {
     path: '/alerting/notifications/templates/:id/edit',
     roles: evaluateAccess(
-      [AccessControlAction.AlertingNotificationsUpdate, AccessControlAction.AlertingNotificationsExternalWrite],
+      [AccessControlAction.AlertingNotificationsWrite, AccessControlAction.AlertingNotificationsExternalWrite],
       ['Editor', 'Admin']
     ),
     component: SafeDynamicImport(
@@ -193,7 +193,7 @@ const unifiedRoutes: RouteDescriptor[] = [
   {
     path: '/alerting/notifications/receivers/new',
     roles: evaluateAccess(
-      [AccessControlAction.AlertingNotificationsCreate, AccessControlAction.AlertingNotificationsExternalWrite],
+      [AccessControlAction.AlertingNotificationsWrite, AccessControlAction.AlertingNotificationsExternalWrite],
       ['Editor', 'Admin']
     ),
     component: SafeDynamicImport(
@@ -203,7 +203,7 @@ const unifiedRoutes: RouteDescriptor[] = [
   {
     path: '/alerting/notifications/receivers/:id/edit',
     roles: evaluateAccess(
-      [AccessControlAction.AlertingNotificationsUpdate, AccessControlAction.AlertingNotificationsExternalWrite],
+      [AccessControlAction.AlertingNotificationsWrite, AccessControlAction.AlertingNotificationsExternalWrite],
       ['Editor', 'Admin']
     ),
     component: SafeDynamicImport(
@@ -213,7 +213,7 @@ const unifiedRoutes: RouteDescriptor[] = [
   {
     path: '/alerting/notifications/global-config',
     roles: evaluateAccess(
-      [AccessControlAction.AlertingNotificationsUpdate, AccessControlAction.AlertingNotificationsExternalWrite],
+      [AccessControlAction.AlertingNotificationsWrite, AccessControlAction.AlertingNotificationsExternalWrite],
       ['Editor', 'Admin']
     ),
     component: SafeDynamicImport(

public/app/features/alerting/unified/Receivers.test.tsx

@@ -137,9 +137,7 @@ describe('Receivers', () => {
     mocks.contextSrv.hasPermission.mockImplementation((action) => {
       const permissions = [
         AccessControlAction.AlertingNotificationsRead,
-        AccessControlAction.AlertingNotificationsCreate,
+        AccessControlAction.AlertingNotificationsWrite,
-        AccessControlAction.AlertingNotificationsUpdate,
-        AccessControlAction.AlertingNotificationsDelete,
         AccessControlAction.AlertingNotificationsExternalRead,
         AccessControlAction.AlertingNotificationsExternalWrite,
       ];

public/app/features/alerting/unified/utils/access-control.ts

@@ -34,15 +34,15 @@ export const notificationsPermissions = {
     external: AccessControlAction.AlertingNotificationsExternalRead,
   },
   create: {
-    grafana: AccessControlAction.AlertingNotificationsCreate,
+    grafana: AccessControlAction.AlertingNotificationsWrite,
     external: AccessControlAction.AlertingNotificationsExternalWrite,
   },
   update: {
-    grafana: AccessControlAction.AlertingNotificationsUpdate,
+    grafana: AccessControlAction.AlertingNotificationsWrite,
     external: AccessControlAction.AlertingNotificationsExternalWrite,
   },
   delete: {
-    grafana: AccessControlAction.AlertingNotificationsDelete,
+    grafana: AccessControlAction.AlertingNotificationsWrite,
     external: AccessControlAction.AlertingNotificationsExternalWrite,
   },
 };

public/app/types/accessControl.ts

@@ -92,10 +92,8 @@ export enum AccessControlAction {
   AlertingInstanceRead = 'alert.instances:read',
 
   // Alerting Notification policies
-  AlertingNotificationsCreate = 'alert.notifications:create',
   AlertingNotificationsRead = 'alert.notifications:read',
-  AlertingNotificationsUpdate = 'alert.notifications:update',
+  AlertingNotificationsWrite = 'alert.notifications:write',
-  AlertingNotificationsDelete = 'alert.notifications:delete',
 
   // External alerting rule actions.
   AlertingRuleExternalWrite = 'alert.rules.external:write',