fix: address PR feedback for actions permissions

Signed-off-by: benjdlambert <ben@blam.sh>

.changeset/actions-permissions-defaults.md

@@ -2,4 +2,4 @@
 '@backstage/backend-defaults': patch
 ---
 
-Added permissions integration to the actions registry. Actions registered with a `permission` field are now checked against the permissions framework when listing and invoking. Denied actions are filtered from list results, and invoking a denied action returns a `404 Not Found` as if the action does not exist.
+Added permissions integration to the actions registry. Actions registered with a `permission` field are now checked against the permissions framework when listing and invoking. Denied actions are filtered from list results, and invoking a denied action returns a `404 Not Found` as if the action does not exist. Permissions are automatically registered with the `PermissionsRegistryService` so they appear in the permission policy system.

docs/backend-system/core-services/actions-registry.md

@@ -162,7 +162,9 @@ export const myPlugin = createBackendPlugin({
 
 Actions can optionally declare a `permission` to control visibility and access through the Backstage permissions framework. When a permission is set, the action is only visible in listings and accessible by users who are authorized.
 
-Actions that are denied by the permission policy are filtered from `list()` results and return a `404 Not Found` on `invoke()`, as if they don't exist.
+When accessed via the Actions Service or the `/.backstage/actions/v1/...` HTTP endpoints, actions that are denied by the permission policy are filtered from list results and return a `404 Not Found` on invocation, as if they don't exist.
+
+Permissions declared on actions are automatically registered with the `PermissionsRegistryService` so they appear in the permission policy system.
 
 ### Adding a Permission to an Action
 
@@ -170,8 +172,8 @@ Actions that are denied by the permission policy are filtered from `list()` resu
 import { createPermission } from '@backstage/plugin-permission-common';
 
 // Define a permission for your action
-const deleteEntityPermission = createPermission({
-  name: 'catalog.entity.delete',
+const myDeletePermission = createPermission({
+  name: 'my-plugin.actions.deleteEntity',
   attributes: { action: 'delete' },
 });
 
@@ -179,7 +181,7 @@ actionsRegistry.register({
   name: 'delete-entity',
   title: 'Delete Entity',
   description: 'Removes an entity from the catalog',
-  permission: deleteEntityPermission,
+  permission: myDeletePermission,
   schema: {
     input: z => z.object({ entityRef: z.string() }),
     output: z => z.object({ deleted: z.boolean() }),

packages/backend-defaults/src/alpha/entrypoints/actionsRegistry/DefaultActionsRegistryService.ts

@@ -19,6 +19,7 @@ import {
   BackstageCredentials,
   HttpAuthService,
   LoggerService,
+  PermissionsRegistryService,
   PermissionsService,
   PluginMetadataService,
 } from '@backstage/backend-plugin-api';
@@ -44,6 +45,7 @@ export class DefaultActionsRegistryService implements ActionsRegistryService {
   private readonly auth: AuthService;
   private readonly metadata: PluginMetadataService;
   private readonly permissions: PermissionsService;
+  private readonly permissionsRegistry: PermissionsRegistryService;
 
   private constructor(
     logger: LoggerService,
@@ -51,12 +53,14 @@ export class DefaultActionsRegistryService implements ActionsRegistryService {
     auth: AuthService,
     metadata: PluginMetadataService,
     permissions: PermissionsService,
+    permissionsRegistry: PermissionsRegistryService,
   ) {
     this.logger = logger;
     this.httpAuth = httpAuth;
     this.auth = auth;
     this.metadata = metadata;
     this.permissions = permissions;
+    this.permissionsRegistry = permissionsRegistry;
   }
 
   static create({
@@ -65,12 +69,14 @@ export class DefaultActionsRegistryService implements ActionsRegistryService {
     auth,
     metadata,
     permissions,
+    permissionsRegistry,
   }: {
     httpAuth: HttpAuthService;
     logger: LoggerService;
     auth: AuthService;
     metadata: PluginMetadataService;
     permissions: PermissionsService;
+    permissionsRegistry: PermissionsRegistryService;
   }): DefaultActionsRegistryService {
     return new DefaultActionsRegistryService(
       logger,
@@ -78,6 +84,7 @@ export class DefaultActionsRegistryService implements ActionsRegistryService {
       auth,
       metadata,
       permissions,
+      permissionsRegistry,
     );
   }
 
@@ -97,7 +104,9 @@ export class DefaultActionsRegistryService implements ActionsRegistryService {
       return res.json({
         actions: allowedActions.map(([id, action]) => ({
           id,
-          ...action,
+          name: action.name,
+          title: action.title,
+          description: action.description,
           attributes: {
             // Inspired by the @modelcontextprotocol/sdk defaults for the hints.
             // https://github.com/modelcontextprotocol/typescript-sdk/blob/dd69efa1de8646bb6b195ff8d5f52e13739f4550/src/types.ts#L777-L812
@@ -195,6 +204,10 @@ export class DefaultActionsRegistryService implements ActionsRegistryService {
       throw new Error(`Action with id "${id}" is already registered`);
     }
 
+    if (options.permission) {
+      this.permissionsRegistry.addPermissions([options.permission]);
+    }
+
     this.actions.set(id, options);
   }
 

packages/backend-defaults/src/alpha/entrypoints/actionsRegistry/actionsRegistryServiceFactory.test.ts

@@ -824,5 +824,49 @@ describe('actionsRegistryServiceFactory', () => {
         expect.objectContaining({ credentials: expect.anything() }),
       );
     });
+
+    it('should register the permission with the permissions registry', async () => {
+      const permissionsRegistryMock = mockServices.permissionsRegistry.mock();
+
+      const pluginSubject = createBackendPlugin({
+        pluginId: 'my-plugin',
+        register(reg) {
+          reg.registerInit({
+            deps: {
+              actionsRegistry: actionsRegistryServiceRef,
+            },
+            async init({ actionsRegistry }) {
+              actionsRegistry.register({
+                name: 'protected-action',
+                title: 'Protected Action',
+                description: 'Permission required',
+                permission: testPermission,
+                schema: {
+                  input: z => z.object({}),
+                  output: z => z.object({}),
+                },
+                action: async () => ({ output: {} }),
+              });
+            },
+          });
+        },
+      });
+
+      await startTestBackend({
+        features: [
+          pluginSubject,
+          actionsRegistryServiceFactory,
+          httpRouterServiceFactory,
+          mockServices.httpAuth.factory({
+            defaultCredentials: mockCredentials.service('user:default/mock'),
+          }),
+          permissionsRegistryMock.factory,
+        ],
+      });
+
+      expect(permissionsRegistryMock.addPermissions).toHaveBeenCalledWith([
+        testPermission,
+      ]);
+    });
   });
 });

packages/backend-defaults/src/alpha/entrypoints/actionsRegistry/actionsRegistryServiceFactory.ts

@@ -33,14 +33,24 @@ export const actionsRegistryServiceFactory = createServiceFactory({
     logger: coreServices.logger,
     auth: coreServices.auth,
     permissions: coreServices.permissions,
+    permissionsRegistry: coreServices.permissionsRegistry,
   },
-  factory: ({ metadata, httpRouter, httpAuth, logger, auth, permissions }) => {
+  factory: ({
+    metadata,
+    httpRouter,
+    httpAuth,
+    logger,
+    auth,
+    permissions,
+    permissionsRegistry,
+  }) => {
     const actionsRegistryService = DefaultActionsRegistryService.create({
       httpAuth,
       logger,
       auth,
       metadata,
       permissions,
+      permissionsRegistry,
     });
 
     httpRouter.use(actionsRegistryService.createRouter());