fix some review comments

Signed-off-by: Brian Fletcher <brian@roadie.io>

.changeset/dry-tips-build.md

@@ -1,5 +1,5 @@
 ---
-'@backstage/plugin-auth-node': minor
+'@backstage/plugin-auth-node': patch
 ---
 
 `IdentityClient` is now deprecated. Please migrate to `IdentityApi` and `DefaultIdentityClient` instead. The authenticate function on `DefaultIdentityClient` is also deprecated. Please use `getIdentity` instead.

.changeset/itchy-kiwis-warn.md

@@ -47,6 +47,7 @@ export async function createRouter(
 ): Promise<express.Router> {
   const { identity } = options;
 
-  const user = identity.getIdentity(req);
-  ...
+  router.get('/user', async (req, res) => {
+    const user = await identity.getIdentity({ request: req });
+    ...
 ```

packages/backend/src/index.ts

@@ -75,8 +75,6 @@ function makeCreateEnv(config: Config) {
   const taskScheduler = TaskScheduler.fromConfig(config);
   const identity = DefaultIdentityClient.create({
     discovery,
-    algorithms: undefined,
-    issuer: undefined,
   });
 
   root.info(`Created UrlReader ${reader}`);

packages/backend/src/plugins/permission.ts

@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-import { DefaultIdentityClient } from '@backstage/plugin-auth-node';
 import { createRouter } from '@backstage/plugin-permission-backend';
 import {
   AuthorizeResult,
@@ -40,9 +39,6 @@ export default async function createPlugin(
     logger: env.logger,
     discovery: env.discovery,
     policy: new AllowAllPermissionPolicy(),
-    identity: DefaultIdentityClient.create({
-      discovery: env.discovery,
-      issuer: await env.discovery.getExternalBaseUrl('auth'),
-    }),
+    identity: env.identity,
   });
 }

packages/create-app/templates/default-app/packages/backend/src/index.ts

@@ -44,7 +44,6 @@ function makeCreateEnv(config: Config) {
   const identity = DefaultIdentityClient.create({
     discovery,
   });
-
   const permissions = ServerPermissionClient.fromConfig(config, {
     discovery,
     tokenManager,

plugins/auth-node/api-report.md

@@ -29,7 +29,11 @@ export class DefaultIdentityClient implements IdentityApi {
   authenticate(token: string | undefined): Promise<BackstageIdentityResponse>;
   static create(options: IdentityClientOptions): DefaultIdentityClient;
   // (undocumented)
-  getIdentity(req: Request_2): Promise<BackstageIdentityResponse | undefined>;
+  getIdentity({
+    request,
+  }: IdentityApiGetIdentityRequest): Promise<
+    BackstageIdentityResponse | undefined
+  >;
 }
 
 // @public
@@ -40,10 +44,15 @@ export function getBearerTokenFromAuthorizationHeader(
 // @public
 export interface IdentityApi {
   getIdentity(
-    req: Request_2<any>,
+    options: IdentityApiGetIdentityRequest,
   ): Promise<BackstageIdentityResponse | undefined>;
 }
 
+// @public
+export type IdentityApiGetIdentityRequest = {
+  request: Request_2<unknown>;
+};
+
 // @public @deprecated
 export class IdentityClient {
   // @deprecated

plugins/auth-node/src/DefaultIdentityClient.test.ts

@@ -26,6 +26,7 @@ import { setupServer } from 'msw/node';
 import { v4 as uuid } from 'uuid';
 
 import { DefaultIdentityClient } from './DefaultIdentityClient';
+import { IdentityApiGetIdentityRequest } from './types';
 
 interface AnyJWK extends Record<string, string> {
   use: 'sig';
@@ -366,4 +367,51 @@ describe('DefaultIdentityClient', () => {
       dateSpy.mockClear();
     });
   });
+
+  describe('getIdentity', () => {
+    beforeEach(() => {
+      server.use(
+        rest.get(
+          `${mockBaseUrl}/.well-known/jwks.json`,
+          async (_, res, ctx) => {
+            const keys = await factory.listPublicKeys();
+            return res(ctx.json(keys));
+          },
+        ),
+      );
+    });
+
+    it('returns the identity', async () => {
+      const token = await factory.issueToken({
+        claims: { sub: 'foo', ent: ['entity1', 'entity2'] },
+      });
+      const response = await client.getIdentity({
+        request: { headers: { authorization: `Bearer ${token}` } },
+      } as IdentityApiGetIdentityRequest);
+      expect(response).toEqual({
+        token: token,
+        identity: {
+          type: 'user',
+          userEntityRef: 'foo',
+          ownershipEntityRefs: ['entity1', 'entity2'],
+        },
+      });
+    });
+
+    it('given a corrupt the identity', async () => {
+      await expect(
+        client.getIdentity({
+          request: { headers: { authorization: `Bearer bad-token` } },
+        } as IdentityApiGetIdentityRequest),
+      ).rejects.toThrow('Invalid JWT');
+    });
+
+    it('given no authorization header', async () => {
+      expect(
+        await client.getIdentity({
+          request: { headers: {} },
+        } as IdentityApiGetIdentityRequest),
+      ).toEqual(undefined);
+    });
+  });
 });

plugins/auth-node/src/DefaultIdentityClient.ts

@@ -24,9 +24,11 @@ import {
   jwtVerify,
 } from 'jose';
 import { GetKeyFunction } from 'jose/dist/types/types';
-import { Request } from 'express';
 
-import { BackstageIdentityResponse } from './types';
+import {
+  BackstageIdentityResponse,
+  IdentityApiGetIdentityRequest,
+} from './types';
 import { getBearerTokenFromAuthorizationHeader, IdentityApi } from '.';
 
 const CLOCK_MARGIN_S = 10;
@@ -75,14 +77,13 @@ export class DefaultIdentityClient implements IdentityApi {
       : ['ES256'];
   }
 
-  async getIdentity(req: Request) {
-    try {
-      return await this.authenticate(
-        getBearerTokenFromAuthorizationHeader(req.headers.authorization),
-      );
-    } catch (e) {
+  async getIdentity({ request }: IdentityApiGetIdentityRequest) {
+    if (!request.headers.authorization) {
       return undefined;
     }
+    return await this.authenticate(
+      getBearerTokenFromAuthorizationHeader(request.headers.authorization),
+    );
   }
 
   /**

plugins/auth-node/src/IdentityApi.ts

@@ -13,8 +13,10 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { BackstageIdentityResponse } from './types';
-import { Request } from 'express';
+import {
+  BackstageIdentityResponse,
+  IdentityApiGetIdentityRequest,
+} from './types';
 
 /**
  * An identity client api to authenticate Backstage
@@ -30,6 +32,6 @@ export interface IdentityApi {
    * The method throws an error if verification fails.
    */
   getIdentity(
-    req: Request<any>,
+    options: IdentityApiGetIdentityRequest,
   ): Promise<BackstageIdentityResponse | undefined>;
 }

plugins/auth-node/src/index.ts

@@ -29,4 +29,5 @@ export type {
   BackstageIdentityResponse,
   BackstageSignInResult,
   BackstageUserIdentity,
+  IdentityApiGetIdentityRequest,
 } from './types';

plugins/auth-node/src/types.ts

@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+import { Request } from 'express';
+
 /**
  * A representation of a successful Backstage sign-in.
  *
@@ -29,6 +31,15 @@ export interface BackstageSignInResult {
   token: string;
 }
 
+/**
+ * Options to request the identity from a Backstage backend request
+ *
+ * @public
+ */
+export type IdentityApiGetIdentityRequest = {
+  request: Request<unknown>;
+};
+
 /**
  * Response object containing the {@link BackstageUserIdentity} and the token
  * from the authentication provider.

plugins/example-todo-list-backend/src/service/router.ts

@@ -61,7 +61,7 @@ export async function createRouter(
   router.post('/todos', async (req, res) => {
     let author: string | undefined = undefined;
 
-    const user = await identity.getIdentity(req);
+    const user = await identity.getIdentity({ request: req });
     author = user?.identity.userEntityRef;
 
     if (!isTodoCreateRequest(req.body)) {

plugins/permission-backend/src/service/router.test.ts

@@ -70,7 +70,7 @@ describe('createRouter', () => {
         getExternalBaseUrl: jest.fn(),
       },
       identity: {
-        getIdentity: jest.fn(req => {
+        getIdentity: jest.fn(({ request: req }) => {
           const token = req.headers.authorization?.replace(/^Bearer[ ]+/, '');
 
           if (!token) {

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

@@ -188,7 +188,7 @@ export async function createRouter(
       req: Request<EvaluatePermissionRequestBatch>,
       res: Response<EvaluatePermissionResponseBatch>,
     ) => {
-      const user = await identity.getIdentity(req);
+      const user = await identity.getIdentity({ request: req });
 
       const parseResult = evaluatePermissionRequestBatchSchema.safeParse(
         req.body,

plugins/scaffolder-backend/api-report.md

@@ -420,7 +420,7 @@ export const createPublishGitlabMergeRequestAction: (options: {
   branchName: string;
   targetPath: string;
   token?: string | undefined;
-  commitAction?: 'update' | 'create' | 'delete' | undefined;
+  commitAction?: 'update' | 'delete' | 'create' | undefined;
   projectid?: string | undefined;
   removeSourceBranch?: boolean | undefined;
   assignee?: string | undefined;
@@ -536,7 +536,7 @@ export interface RouterOptions {
   // (undocumented)
   database: PluginDatabaseManager;
   // (undocumented)
-  identity: IdentityApi;
+  identity?: IdentityApi;
   // (undocumented)
   logger: Logger;
   // (undocumented)

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

@@ -22,8 +22,8 @@ import {
   stringifyEntityRef,
   UserEntity,
 } from '@backstage/catalog-model';
-import { Config, JsonObject } from '@backstage/config';
-import { InputError, NotFoundError } from '@backstage/errors';
+import { Config, JsonObject, JsonValue } from '@backstage/config';
+import { InputError, NotFoundError, stringifyError } from '@backstage/errors';
 import { ScmIntegrations } from '@backstage/integration';
 import {
   TaskSpec,
@@ -47,7 +47,10 @@ import {
 import { createDryRunner } from '../scaffolder/dryrun';
 import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker';
 import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers';
-import { IdentityApi } from '@backstage/plugin-auth-node';
+import {
+  IdentityApi,
+  IdentityApiGetIdentityRequest,
+} from '@backstage/plugin-auth-node';
 
 /**
  * RouterOptions
@@ -65,13 +68,87 @@ export interface RouterOptions {
   taskWorkers?: number;
   taskBroker?: TaskBroker;
   additionalTemplateFilters?: Record<string, TemplateFilter>;
-  identity: IdentityApi;
+  identity?: IdentityApi;
 }
 
 function isSupportedTemplate(entity: TemplateEntityV1beta3) {
   return entity.apiVersion === 'scaffolder.backstage.io/v1beta3';
 }
 
+function buildDefaultIdentityClient({
+  logger,
+}: {
+  logger: Logger;
+}): IdentityApi {
+  return {
+    getIdentity: async ({ request }: IdentityApiGetIdentityRequest) => {
+      const header = request.headers.authorization;
+
+      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');
+        }
+
+        // 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 getIdentity = async ({
+  request,
+  identity,
+  logger,
+}: {
+  request: express.Request;
+  identity: IdentityApi;
+  logger: Logger;
+}) => {
+  let callerIdentity = undefined;
+  try {
+    callerIdentity = await identity.getIdentity({ request });
+  } catch (e: any) {
+    logger.debug(`identity could not be determined: ${e.message}`);
+  }
+  return callerIdentity;
+};
+
 /**
  * A method to create a router for the scaffolder backend plugin.
  * @public
@@ -91,10 +168,13 @@ export async function createRouter(
     actions,
     taskWorkers,
     additionalTemplateFilters,
-    identity,
   } = options;
 
   const logger = parentLogger.child({ plugin: 'scaffolder' });
+
+  const identity: IdentityApi =
+    options.identity || buildDefaultIdentityClient({ logger });
+
   const workingDirectory = await getWorkingDirectory(config, logger);
   const integrations = ScmIntegrations.fromConfig(config);
   let taskBroker: TaskBroker;
@@ -148,7 +228,11 @@ export async function createRouter(
       async (req, res) => {
         const { namespace, kind, name } = req.params;
 
-        const userIdentity = await identity.getIdentity(req);
+        const userIdentity = await getIdentity({
+          request: req,
+          logger,
+          identity,
+        });
         const token = userIdentity?.token;
 
         const template = await findTemplate({
@@ -192,7 +276,11 @@ export async function createRouter(
         defaultKind: 'template',
       });
 
-      const callerIdentity = await identity.getIdentity(req);
+      const callerIdentity = await getIdentity({
+        request: req,
+        logger,
+        identity,
+      });
       const token = callerIdentity?.token;
       const userEntityRef = callerIdentity?.identity.userEntityRef;
 
@@ -397,7 +485,8 @@ export async function createRouter(
         throw new InputError('Input template is not a template');
       }
 
-      const token = (await identity.getIdentity(req))?.token;
+      const token = (await getIdentity({ request: req, logger, identity }))
+        ?.token;
 
       for (const parameters of [template.spec.parameters ?? []].flat()) {
         const result = validate(body.values, parameters);