fix(scaffolder): use initiator credentials for getting action

Signed-off-by: Hellgren Heikki <heikki.hellgren@op.fi>

plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.test.ts

@@ -23,17 +23,14 @@ import { actionsRegistryServiceMock } from '@backstage/backend-test-utils/alpha'
 describe('DefaultTemplateActionRegistry', () => {
   let registry: DefaultTemplateActionRegistry;
   let mockActionsService: ReturnType<typeof actionsRegistryServiceMock>;
-  let mockAuthService: ReturnType<typeof mockServices.auth>;
   let mockLogger: ReturnType<typeof mockServices.logger.mock>;
 
   beforeEach(() => {
     mockActionsService = actionsRegistryServiceMock();
-    mockAuthService = mockServices.auth();
     mockLogger = mockServices.logger.mock();
 
     registry = new DefaultTemplateActionRegistry(
       mockActionsService,
-      mockAuthService,
       mockLogger,
     );
   });
@@ -80,7 +77,9 @@ describe('DefaultTemplateActionRegistry', () => {
       });
 
       registry.register(action);
-      const result = await registry.get('test-action');
+      const result = await registry.get('test-action', {
+        credentials: mockCredentials.user(),
+      });
 
       expect(result).toBe(action);
     });
@@ -101,7 +100,9 @@ describe('DefaultTemplateActionRegistry', () => {
         action: async () => ({ output: {} }),
       });
 
-      const result = await registry.get('test:service-action');
+      const result = await registry.get('test:service-action', {
+        credentials: mockCredentials.user(),
+      });
 
       expect(result.id).toBe('test:service-action');
       expect(result.description).toBe('Service action');
@@ -109,28 +110,26 @@ describe('DefaultTemplateActionRegistry', () => {
     });
 
     it('should throw NotFoundError when action is not found', async () => {
-      await expect(registry.get('non-existent-action')).rejects.toThrow(
-        NotFoundError,
-      );
-      await expect(registry.get('non-existent-action')).rejects.toThrow(
+      await expect(
+        registry.get('non-existent-action', {
+          credentials: mockCredentials.user(),
+        }),
+      ).rejects.toThrow(NotFoundError);
+      await expect(
+        registry.get('non-existent-action', {
+          credentials: mockCredentials.user(),
+        }),
+      ).rejects.toThrow(
         "Template action with ID 'non-existent-action' is not registered",
       );
     });
-
-    it('should use own service credentials when getting action', async () => {
-      const spy = jest.spyOn(mockActionsService, 'list');
-
-      await expect(registry.get('non-existent')).rejects.toThrow();
-
-      expect(spy).toHaveBeenCalledWith({
-        credentials: await mockAuthService.getOwnServiceCredentials(),
-      });
-    });
   });
 
   describe('list', () => {
     it('should return empty map when no actions are registered', async () => {
-      const result = await registry.list();
+      const result = await registry.list({
+        credentials: mockCredentials.user(),
+      });
 
       expect(result).toBeInstanceOf(Map);
       expect(result.size).toBe(0);
@@ -152,7 +151,9 @@ describe('DefaultTemplateActionRegistry', () => {
       registry.register(action1);
       registry.register(action2);
 
-      const result = await registry.list();
+      const result = await registry.list({
+        credentials: mockCredentials.user(),
+      });
 
       expect(result.size).toBe(2);
       expect(result.get('action-1')).toBe(action1);
@@ -175,7 +176,9 @@ describe('DefaultTemplateActionRegistry', () => {
         action: async () => ({ output: {} }),
       });
 
-      const result = await registry.list();
+      const result = await registry.list({
+        credentials: mockCredentials.user(),
+      });
 
       expect(result.size).toBe(1);
       const action = result.get('test:service-action');
@@ -204,7 +207,9 @@ describe('DefaultTemplateActionRegistry', () => {
       });
 
       registry.register(localAction);
-      const result = await registry.list();
+      const result = await registry.list({
+        credentials: mockCredentials.user(),
+      });
 
       expect(result.get('test:same-id')).toBe(localAction);
       expect(mockLogger.warn).toHaveBeenCalledWith(
@@ -212,25 +217,6 @@ describe('DefaultTemplateActionRegistry', () => {
       );
     });
 
-    it('should use provided credentials when listing actions', async () => {
-      const credentials = mockCredentials.user();
-      const spy = jest.spyOn(mockActionsService, 'list');
-
-      await registry.list({ credentials });
-
-      expect(spy).toHaveBeenCalledWith({ credentials });
-    });
-
-    it('should use own service credentials when no credentials provided', async () => {
-      const spy = jest.spyOn(mockActionsService, 'list');
-
-      await registry.list();
-
-      expect(spy).toHaveBeenCalledWith({
-        credentials: await mockAuthService.getOwnServiceCredentials(),
-      });
-    });
-
     it('should set supportsDryRun to false for destructive actions', async () => {
       mockActionsService.register({
         name: 'destructive-action',
@@ -247,7 +233,9 @@ describe('DefaultTemplateActionRegistry', () => {
         action: async () => ({ output: {} }),
       });
 
-      const result = await registry.list();
+      const result = await registry.list({
+        credentials: mockCredentials.user(),
+      });
       const action = result.get('test:destructive-action');
 
       expect(action!.supportsDryRun).toBe(false);
@@ -269,7 +257,9 @@ describe('DefaultTemplateActionRegistry', () => {
         action: async () => ({ output: {} }),
       });
 
-      const result = await registry.list();
+      const result = await registry.list({
+        credentials: mockCredentials.user(),
+      });
       const action = result.get('test:non-readonly-action');
 
       expect(action!.supportsDryRun).toBe(false);

plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts

@@ -18,7 +18,6 @@ import { ConflictError, NotFoundError } from '@backstage/errors';
 import { TemplateAction } from '@backstage/plugin-scaffolder-node';
 import { ActionsService } from '@backstage/backend-plugin-api/alpha';
 import {
-  AuthService,
   BackstageCredentials,
   LoggerService,
 } from '@backstage/backend-plugin-api';
@@ -31,9 +30,12 @@ import { JsonObject } from '@backstage/types';
  */
 export interface TemplateActionRegistry {
   register(action: TemplateAction<any, any, any>): void;
-  get(actionId: string): Promise<TemplateAction<any, any, any>>;
-  list(options?: {
-    credentials?: BackstageCredentials;
+  get(
+    actionId: string,
+    options: { credentials: BackstageCredentials },
+  ): Promise<TemplateAction<any, any, any>>;
+  list(options: {
+    credentials: BackstageCredentials;
   }): Promise<Map<string, TemplateAction<any, any, any>>>;
 }
 
@@ -45,7 +47,6 @@ export class DefaultTemplateActionRegistry implements TemplateActionRegistry {
 
   constructor(
     private readonly actionsRegistry: ActionsService,
-    private readonly auth: AuthService,
     private readonly logger: LoggerService,
   ) {}
 
@@ -59,8 +60,11 @@ export class DefaultTemplateActionRegistry implements TemplateActionRegistry {
     this.actions.set(action.id, action);
   }
 
-  async get(actionId: string): Promise<TemplateAction<any, any, any>> {
-    const action = (await this.list()).get(actionId);
+  async get(
+    actionId: string,
+    options: { credentials: BackstageCredentials },
+  ): Promise<TemplateAction<any, any, any>> {
+    const action = (await this.list(options)).get(actionId);
     if (!action) {
       throw new NotFoundError(
         `Template action with ID '${actionId}' is not registered. See https://backstage.io/docs/features/software-templates/builtin-actions/ on how to add a new action module.`,
@@ -69,14 +73,13 @@ export class DefaultTemplateActionRegistry implements TemplateActionRegistry {
     return action;
   }
 
-  async list(options?: {
-    credentials?: BackstageCredentials;
+  async list(options: {
+    credentials: BackstageCredentials;
   }): Promise<Map<string, TemplateAction<any, any, any>>> {
     const ret = new Map(this.actions);
 
     const { actions } = await this.actionsRegistry.list({
-      credentials:
-        options?.credentials ?? (await this.auth.getOwnServiceCredentials()),
+      credentials: options.credentials,
     });
 
     for (const action of actions) {

plugins/scaffolder-backend/src/scaffolder/dryrun/DecoratedActionsRegistry.ts

@@ -31,9 +31,12 @@ export class DecoratedActionsRegistry implements TemplateActionRegistry {
     }
   }
 
-  async get(actionId: string): Promise<TemplateAction> {
+  async get(
+    actionId: string,
+    options: { credentials: BackstageCredentials },
+  ): Promise<TemplateAction> {
     try {
-      return await this.innerRegistry.get(actionId);
+      return await this.innerRegistry.get(actionId, options);
     } catch (e) {
       if (!this.innerActions.has(actionId)) {
         throw e;
@@ -42,8 +45,8 @@ export class DecoratedActionsRegistry implements TemplateActionRegistry {
     }
   }
 
-  async list(options?: {
-    credentials?: BackstageCredentials;
+  async list(options: {
+    credentials: BackstageCredentials;
   }): Promise<Map<string, TemplateAction<any, any, any>>> {
     const inner = await this.innerRegistry.list(options);
     return new Map<string, TemplateAction<any, any, any>>([

plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts

@@ -110,7 +110,6 @@ describe('NunjucksWorkflowRunner', () => {
 
     actionRegistry = new DefaultTemplateActionRegistry(
       actionsRegistryServiceMock(),
-      mockServices.auth(),
       mockServices.logger.mock(),
     );
     fakeActionHandler = jest.fn();

plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts

@@ -33,7 +33,7 @@ import {
   SecureTemplater,
   SecureTemplateRenderer,
 } from '../../lib/templating/SecureTemplater';
-import { TemplateActionRegistry } from '../actions/TemplateActionRegistry.ts';
+import { TemplateActionRegistry } from '../actions/TemplateActionRegistry';
 import { generateExampleOutput, isTruthy } from './helper';
 import { TaskTrackType, WorkflowResponse, WorkflowRunner } from './types';
 
@@ -245,7 +245,9 @@ export class NunjucksWorkflowRunner implements WorkflowRunner {
         return;
       }
       const action: TemplateAction<JsonObject> =
-        await this.options.actionRegistry.get(step.action);
+        await this.options.actionRegistry.get(step.action, {
+          credentials: await task.getInitiatorCredentials(),
+        });
       const { taskLogger } = createStepLogger({
         task,
         step,

plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts

@@ -25,7 +25,7 @@ import {
   TemplateGlobal,
 } from '@backstage/plugin-scaffolder-node';
 import PQueue from 'p-queue';
-import { TemplateActionRegistry } from '../actions/TemplateActionRegistry.ts';
+import { TemplateActionRegistry } from '../actions/TemplateActionRegistry';
 import { NunjucksWorkflowRunner } from './NunjucksWorkflowRunner';
 import { WorkflowRunner } from './types';
 import { setTimeout } from 'timers/promises';

plugins/scaffolder-backend/src/service/router.ts

@@ -271,7 +271,6 @@ export async function createRouter(
 
   const actionRegistry = new DefaultTemplateActionRegistry(
     actionsRegistry,
-    auth,
     logger,
   );