refactor: apply review suggestions

Signed-off-by: Camila Belo <camilaibs@gmail.com>

.changeset/fluffy-donuts-end.md

@@ -2,5 +2,4 @@
 '@backstage/plugin-scaffolder-backend': minor
 ---
 
-Migrate plugin to use the new auth services.
-**BREAKING**: Add a required service discovery to the router options and remove the optional identity from it. Also change the permissions type to be `PermissionsService`.
+Migrate plugin to use the new auth services, add an optional service discovery to the router options and change the permissions type to be `PermissionsService`.

.changeset/fresh-parrots-learn.md

@@ -2,4 +2,4 @@
 '@backstage/plugin-scaffolder-node': patch
 ---
 
-Update task secrets and context types to contain the new auth initiator credentials.
+Update the task context type to contain the new auth initiator credentials.

.changeset/shaggy-pears-dream.md

@@ -1,5 +0,0 @@
----
-'@backstage/create-app': patch
----
-
-Update scaffolder template to pass the discovery service to the create router and remove the identity option.

.changeset/sixty-balloons-lie.md

@@ -2,4 +2,4 @@
 '@backstage/plugin-scaffolder-node-test-utils': patch
 ---
 
-Add an initiator credentials getter to the default mocked context.
+Add an initiator credentials getter to the default mock context.

packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts

@@ -16,7 +16,7 @@ export default async function createPlugin(
     database: env.database,
     reader: env.reader,
     catalogClient,
-    discovery: env.discovery,
+    identity: env.identity,
     permissions: env.permissions,
   });
 }

plugins/scaffolder-backend/api-report.md

@@ -23,6 +23,7 @@ import * as github from '@backstage/plugin-scaffolder-backend-module-github';
 import * as gitlab from '@backstage/plugin-scaffolder-backend-module-gitlab';
 import { HttpAuthService } from '@backstage/backend-plugin-api';
 import { HumanDuration } from '@backstage/types';
+import { IdentityApi } from '@backstage/plugin-auth-node';
 import { JsonObject } from '@backstage/types';
 import { JsonValue } from '@backstage/types';
 import { Knex } from 'knex';
@@ -468,10 +469,12 @@ export interface RouterOptions {
   // (undocumented)
   database: PluginDatabaseManager;
   // (undocumented)
-  discovery: DiscoveryService;
+  discovery?: DiscoveryService;
   // (undocumented)
   httpAuth?: HttpAuthService;
   // (undocumented)
+  identity?: IdentityApi;
+  // (undocumented)
   lifecycle?: LifecycleService;
   // (undocumented)
   logger: Logger;
@@ -525,7 +528,7 @@ export type TaskEventType = TaskEventType_2;
 export class TaskManager implements TaskContext_2 {
   // (undocumented)
   get cancelSignal(): AbortSignal;
-  // (undocumented)gs
+  // (undocumented)
   complete(result: TaskCompletionState_2, metadata?: JsonObject): Promise<void>;
   // (undocumented)
   static create(
@@ -533,6 +536,7 @@ export class TaskManager implements TaskContext_2 {
     storage: TaskStore,
     abortSignal: AbortSignal,
     logger: Logger,
+    auth?: AuthService,
   ): TaskManager;
   // (undocumented)
   get createdBy(): string | undefined;
@@ -541,6 +545,8 @@ export class TaskManager implements TaskContext_2 {
   // (undocumented)
   emitLog(message: string, logMetadata?: JsonObject): Promise<void>;
   // (undocumented)
+  getInitiatorCredentials(): Promise<BackstageCredentials>;
+  // (undocumented)
   getTaskState?(): Promise<
     | {
         state?: JsonObject;
@@ -548,12 +554,8 @@ export class TaskManager implements TaskContext_2 {
     | undefined
   >;
   // (undocumented)
-  getInitiatorCredentials(): Promise<BackstageCredentials>;
-  // (undocumented)
   getWorkspaceName(): Promise<string>;
   // (undocumented)
-  isDryRun?: boolean | undefined;
-  // (undocumented)
   get secrets(): TaskSecrets_2 | undefined;
   // (undocumented)
   get spec(): TaskSpecV1beta3;

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

@@ -30,7 +30,10 @@ import {
 } from '@backstage/plugin-scaffolder-node';
 import { TaskStore } from './types';
 import { readDuration } from './helper';
-import { BackstageCredentials } from '@backstage/backend-plugin-api';
+import {
+  AuthService,
+  BackstageCredentials,
+} from '@backstage/backend-plugin-api';
 
 type TaskState = {
   checkpoints: {
@@ -60,8 +63,9 @@ export class TaskManager implements TaskContext {
     storage: TaskStore,
     abortSignal: AbortSignal,
     logger: Logger,
+    auth?: AuthService,
   ) {
-    const agent = new TaskManager(task, storage, abortSignal, logger);
+    const agent = new TaskManager(task, storage, abortSignal, logger, auth);
     agent.startTimeout();
     return agent;
   }
@@ -72,8 +76,8 @@ export class TaskManager implements TaskContext {
     private readonly storage: TaskStore,
     private readonly signal: AbortSignal,
     private readonly logger: Logger,
+    private readonly auth?: AuthService,
   ) {}
-  isDryRun?: boolean | undefined;
 
   get spec() {
     return this.task.spec;
@@ -174,8 +178,14 @@ export class TaskManager implements TaskContext {
     }, 1000);
   }
 
-  getInitiatorCredentials(): Promise<BackstageCredentials> {
-    return JSON.parse(this.task.secrets!.initiatorCredentials!);
+  async getInitiatorCredentials(): Promise<BackstageCredentials> {
+    if (this.task.secrets && '__initiatorCredentials' in this.task.secrets) {
+      return JSON.parse(this.task.secrets.__initiatorCredentials);
+    }
+    if (!this.auth) {
+      throw new Error('Unable to access credentials in ....');
+    }
+    return this.auth.getNoneCredentials();
   }
 }
 
@@ -220,6 +230,7 @@ export class StorageTaskBroker implements TaskBroker {
     private readonly storage: TaskStore,
     private readonly logger: Logger,
     private readonly config?: Config,
+    private readonly auth?: AuthService,
   ) {}
 
   async list(options?: {
@@ -305,6 +316,7 @@ export class StorageTaskBroker implements TaskBroker {
           this.storage,
           abortController.signal,
           this.logger,
+          this.auth,
         );
       }
 

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

@@ -15,6 +15,7 @@
  */
 
 import {
+  HostDiscovery,
   PluginDatabaseManager,
   UrlReader,
   createLegacyAuthAdapters,
@@ -29,9 +30,9 @@ import {
   UserEntity,
 } from '@backstage/catalog-model';
 import { Config, readDurationFromConfig } from '@backstage/config';
-import { InputError, NotFoundError } from '@backstage/errors';
+import { InputError, NotFoundError, stringifyError } from '@backstage/errors';
 import { ScmIntegrations } from '@backstage/integration';
-import { HumanDuration, JsonObject } from '@backstage/types';
+import { HumanDuration, JsonObject, JsonValue } from '@backstage/types';
 import {
   TaskSpec,
   TemplateEntityV1beta3,
@@ -83,6 +84,10 @@ import {
   LifecycleService,
   PermissionsService,
 } from '@backstage/backend-plugin-api';
+import {
+  IdentityApi,
+  IdentityApiGetIdentityRequest,
+} from '@backstage/plugin-auth-node';
 
 /**
  *
@@ -153,13 +158,78 @@ export interface RouterOptions {
   >;
   auth?: AuthService;
   httpAuth?: HttpAuthService;
-  discovery: DiscoveryService;
+  identity?: IdentityApi;
+  discovery?: DiscoveryService;
 }
 
 function isSupportedTemplate(entity: TemplateEntityV1beta3) {
   return entity.apiVersion === 'scaffolder.backstage.io/v1beta3';
 }
 
+/*
+ * @deprecated This function remains as the DefaultIdentityClient behaves slightly differently to the pre-existing
+ * scaffolder behaviour. Specifically if the token fails to parse, the DefaultIdentityClient will raise an error.
+ * The scaffolder did not raise an error in this case. As such we chose to allow it to behave as it did previously
+ * until someone explicitly passes an IdentityApi. When we have reasonable confidence that most backstage deployments
+ * are using the IdentityApi, we can remove this function.
+ */
+function buildDefaultIdentityClient(options: RouterOptions): IdentityApi {
+  return {
+    getIdentity: async ({ request }: IdentityApiGetIdentityRequest) => {
+      const header = request.headers.authorization;
+      const { logger } = options;
+
+      if (!header) {
+        return undefined;
+      }
+
+      try {
+        const token = header.match(/^Bearer\s(\S+\.\S+\.\S+)$/i)?.[1];
+        if (!token) {
+          throw new TypeError('Expected Bearer with JWT');
+        }
+
+        const [_header, rawPayload, _signature] = token.split('.');
+        const payload: JsonValue = JSON.parse(
+          Buffer.from(rawPayload, 'base64').toString(),
+        );
+
+        if (
+          typeof payload !== 'object' ||
+          payload === null ||
+          Array.isArray(payload)
+        ) {
+          throw new TypeError('Malformed JWT payload');
+        }
+
+        const sub = payload.sub;
+        if (typeof sub !== 'string') {
+          throw new TypeError('Expected string sub claim');
+        }
+
+        if (sub === 'backstage-server') {
+          return undefined;
+        }
+
+        // Check that it's a valid ref, otherwise this will throw.
+        parseEntityRef(sub);
+
+        return {
+          identity: {
+            userEntityRef: sub,
+            ownershipEntityRefs: [],
+            type: 'user',
+          },
+          token,
+        };
+      } catch (e) {
+        logger.error(`Invalid authorization header: ${stringifyError(e)}`);
+        return undefined;
+      }
+    },
+  };
+}
+
 const readDuration = (
   config: Config,
   key: string,
@@ -178,8 +248,6 @@ const readDuration = (
 export async function createRouter(
   options: RouterOptions,
 ): Promise<express.Router> {
-  const { auth, httpAuth } = createLegacyAuthAdapters(options);
-
   const router = Router();
   // Be generous in upload size to support a wide range of templates in dry-run mode.
   router.use(express.json({ limit: '10MB' }));
@@ -197,7 +265,16 @@ export async function createRouter(
     additionalTemplateGlobals,
     permissions,
     permissionRules,
+    discovery = HostDiscovery.fromConfig(config),
+    identity = buildDefaultIdentityClient(options),
   } = options;
+
+  const { auth, httpAuth } = createLegacyAuthAdapters({
+    ...options,
+    identity,
+    discovery,
+  });
+
   const concurrentTasksLimit =
     options.concurrentTasksLimit ??
     options.config.getOptionalNumber('scaffolder.concurrentTasksLimit');
@@ -210,7 +287,7 @@ export async function createRouter(
   let taskBroker: TaskBroker;
   if (!options.taskBroker) {
     const databaseTaskStore = await DatabaseTaskStore.create({ database });
-    taskBroker = new StorageTaskBroker(databaseTaskStore, logger, config);
+    taskBroker = new StorageTaskBroker(databaseTaskStore, logger, config, auth);
 
     if (scheduler && databaseTaskStore.listStaleTasks) {
       await scheduler.scheduleTask({
@@ -624,7 +701,6 @@ export async function createRouter(
         secrets: {
           ...body.secrets,
           ...(token && { backstageToken: token }),
-          initiatorCredentials: JSON.stringify(credentials),
         },
         credentials,
       });

plugins/scaffolder-node/api-report.md

@@ -40,6 +40,7 @@ export type ActionContext<
     value: TActionOutput[keyof TActionOutput],
   ): void;
   createTemporaryDirectory(): Promise<string>;
+  getInitiatorCredentials(): Promise<BackstageCredentials>;
   templateInfo?: TemplateInfo;
   isDryRun?: boolean;
   user?: {
@@ -48,7 +49,6 @@ export type ActionContext<
   };
   signal?: AbortSignal;
   each?: JsonObject;
-  getInitiatorCredentials(): Promise<BackstageCredentials>;
 };
 
 // @public (undocumented)
@@ -351,6 +351,8 @@ export interface TaskContext {
   // (undocumented)
   emitLog(message: string, logMetadata?: JsonObject): Promise<void>;
   // (undocumented)
+  getInitiatorCredentials(): Promise<BackstageCredentials>;
+  // (undocumented)
   getTaskState?(): Promise<
     | {
         state?: JsonObject;
@@ -358,8 +360,6 @@ export interface TaskContext {
     | undefined
   >;
   // (undocumented)
-  getInitiatorCredentials(): Promise<BackstageCredentials>;
-  // (undocumented)
   getWorkspaceName(): Promise<string>;
   // (undocumented)
   isDryRun?: boolean;
@@ -389,7 +389,6 @@ export type TaskEventType = 'completion' | 'log' | 'cancelled' | 'recovered';
 // @public
 export type TaskSecrets = Record<string, string> & {
   backstageToken?: string;
-  initiatorCredentials?: string;
 };
 
 // @public

plugins/scaffolder-node/src/actions/types.ts

@@ -50,6 +50,11 @@ export type ActionContext<
    */
   createTemporaryDirectory(): Promise<string>;
 
+  /**
+   * Get the credentials for the current request
+   */
+  getInitiatorCredentials(): Promise<BackstageCredentials>;
+
   templateInfo?: TemplateInfo;
 
   /**
@@ -81,11 +86,6 @@ export type ActionContext<
    * Optional value of each invocation
    */
   each?: JsonObject;
-
-  /**
-   * Get the credentials for the current request
-   */
-  getInitiatorCredentials(): Promise<BackstageCredentials>;
 };
 
 /** @public */

plugins/scaffolder-node/src/tasks/types.ts

@@ -24,11 +24,7 @@ import { JsonObject, JsonValue, Observable } from '@backstage/types';
  * @public
  */
 export type TaskSecrets = Record<string, string> & {
-  /**
-   * @deprecated use "initiatorCredentials" instead
-   */
   backstageToken?: string;
-  initiatorCredentials?: string;
 };
 
 /**