From 2cbd5334260cd939dd40d81ddc8b67b361074a36 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 8 Jul 2022 16:20:50 +0100 Subject: [PATCH 01/25] break identity client into an interface The interface has changed a little instead of allowing the client to parse out the authorization header, it takes the request object as is to extract the identity from it how the implementation decides. IdentityClient#authenticate is now deprecated, in favor of IdentityApi#getIdentity. I am leaving the IdentityClient in place deprecated so that plugins that use this can migrate away from it. Signed-off-by: Brian Fletcher --- .changeset/dry-tips-build.md | 5 + .changeset/thin-cows-watch.md | 7 + docs/permissions/plugin-authors/01-setup.md | 4 +- packages/backend/src/index.ts | 8 + packages/backend/src/plugins/permission.ts | 4 +- packages/backend/src/plugins/scaffolder.ts | 1 + packages/backend/src/types.ts | 2 + plugins/auth-node/api-report.md | 21 +- plugins/auth-node/package.json | 1 + .../src/DefaultIdentityClient.test.ts | 369 ++++++++++++++++++ .../auth-node/src/DefaultIdentityClient.ts | 168 ++++++++ plugins/auth-node/src/IdentityApi.ts | 35 ++ plugins/auth-node/src/IdentityClient.ts | 125 +----- plugins/auth-node/src/index.ts | 4 +- .../example-todo-list-backend/api-report.md | 4 +- .../src/service/router.test.ts | 4 +- .../src/service/router.ts | 12 +- .../src/service/standaloneServer.ts | 4 +- plugins/permission-backend/api-report.md | 4 +- plugins/permission-backend/package.json | 1 + .../src/service/router.test.ts | 24 +- .../permission-backend/src/service/router.ts | 10 +- plugins/scaffolder-backend/api-report.md | 3 + plugins/scaffolder-backend/package.json | 1 + .../src/service/router.test.ts | 90 ++++- .../scaffolder-backend/src/service/router.ts | 55 +-- 26 files changed, 753 insertions(+), 213 deletions(-) create mode 100644 .changeset/dry-tips-build.md create mode 100644 .changeset/thin-cows-watch.md create mode 100644 plugins/auth-node/src/DefaultIdentityClient.test.ts create mode 100644 plugins/auth-node/src/DefaultIdentityClient.ts create mode 100644 plugins/auth-node/src/IdentityApi.ts diff --git a/.changeset/dry-tips-build.md b/.changeset/dry-tips-build.md new file mode 100644 index 0000000000..ef3ec700d1 --- /dev/null +++ b/.changeset/dry-tips-build.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-node': minor +--- + +IdentityClient is now deprecated. Please migrate to IdentityApi and DefaultIdentityClient instead. The authenticate function on DefaultIdentityClient is also deprecated. Please use getIdentity instead. diff --git a/.changeset/thin-cows-watch.md b/.changeset/thin-cows-watch.md new file mode 100644 index 0000000000..12c96df268 --- /dev/null +++ b/.changeset/thin-cows-watch.md @@ -0,0 +1,7 @@ +--- +'@internal/plugin-todo-list-backend': patch +'@backstage/plugin-permission-backend': patch +'@backstage/plugin-scaffolder-backend': patch +--- + +Uptake the IdentityApi change to use getIdentiy instead of authenticate for retrieving the logged in users identity. diff --git a/docs/permissions/plugin-authors/01-setup.md b/docs/permissions/plugin-authors/01-setup.md index d08d1ee79c..18de410612 100644 --- a/docs/permissions/plugin-authors/01-setup.md +++ b/docs/permissions/plugin-authors/01-setup.md @@ -46,7 +46,7 @@ The source code is available here: Create a new `packages/backend/src/plugins/todolist.ts` with the following content: ```javascript - import { IdentityClient } from '@backstage/plugin-auth-node'; + import { DefaultIdentityClient } from '@backstage/plugin-auth-node'; import { createRouter } from '@internal/plugin-todo-list-backend'; import { Router } from 'express'; import { PluginEnvironment } from '../types'; @@ -57,7 +57,7 @@ The source code is available here: }: PluginEnvironment): Promise { return await createRouter({ logger, - identity: IdentityClient.create({ + identity: DefaultIdentityClient.create({ discovery, issuer: await discovery.getExternalBaseUrl('auth'), }), diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 1942c36ad1..00fc2d529e 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -59,6 +59,7 @@ import jenkins from './plugins/jenkins'; import permission from './plugins/permission'; import { PluginEnvironment } from './types'; import { ServerPermissionClient } from '@backstage/plugin-permission-node'; +import { DefaultIdentityClient } from '@backstage/plugin-auth-node'; function makeCreateEnv(config: Config) { const root = getRootLogger(); @@ -72,6 +73,11 @@ function makeCreateEnv(config: Config) { const databaseManager = DatabaseManager.fromConfig(config); const cacheManager = CacheManager.fromConfig(config); const taskScheduler = TaskScheduler.fromConfig(config); + const identity = DefaultIdentityClient.create({ + discovery, + algorithms: undefined, + issuer: undefined, + }); root.info(`Created UrlReader ${reader}`); @@ -80,6 +86,7 @@ function makeCreateEnv(config: Config) { const database = databaseManager.forPlugin(plugin); const cache = cacheManager.forPlugin(plugin); const scheduler = taskScheduler.forPlugin(plugin); + return { logger, cache, @@ -90,6 +97,7 @@ function makeCreateEnv(config: Config) { tokenManager, permissions, scheduler, + identity, }; }; } diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index e4c2e1d435..ab647b7f95 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { IdentityClient } from '@backstage/plugin-auth-node'; +import { DefaultIdentityClient } from '@backstage/plugin-auth-node'; import { createRouter } from '@backstage/plugin-permission-backend'; import { AuthorizeResult, @@ -40,7 +40,7 @@ export default async function createPlugin( logger: env.logger, discovery: env.discovery, policy: new AllowAllPermissionPolicy(), - identity: IdentityClient.create({ + identity: DefaultIdentityClient.create({ discovery: env.discovery, issuer: await env.discovery.getExternalBaseUrl('auth'), }), diff --git a/packages/backend/src/plugins/scaffolder.ts b/packages/backend/src/plugins/scaffolder.ts index 465616f433..19da3b3ae0 100644 --- a/packages/backend/src/plugins/scaffolder.ts +++ b/packages/backend/src/plugins/scaffolder.ts @@ -32,5 +32,6 @@ export default async function createPlugin( database: env.database, catalogClient: catalogClient, reader: env.reader, + identity: env.identity, }); } diff --git a/packages/backend/src/types.ts b/packages/backend/src/types.ts index 3e47b1a523..831eaebb66 100644 --- a/packages/backend/src/types.ts +++ b/packages/backend/src/types.ts @@ -28,6 +28,7 @@ import { PermissionAuthorizer, PermissionEvaluator, } from '@backstage/plugin-permission-common'; +import { IdentityApi } from '@backstage/plugin-auth-node'; export type PluginEnvironment = { logger: Logger; @@ -39,4 +40,5 @@ export type PluginEnvironment = { tokenManager: TokenManager; permissions: PermissionEvaluator | PermissionAuthorizer; scheduler: PluginTaskScheduler; + identity: IdentityApi; }; diff --git a/plugins/auth-node/api-report.md b/plugins/auth-node/api-report.md index 80c00b43fc..0f1c5bb934 100644 --- a/plugins/auth-node/api-report.md +++ b/plugins/auth-node/api-report.md @@ -4,6 +4,7 @@ ```ts import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { Request as Request_2 } from 'express'; // @public export interface BackstageIdentityResponse extends BackstageSignInResult { @@ -22,21 +23,39 @@ export type BackstageUserIdentity = { ownershipEntityRefs: string[]; }; +// @public +export class DefaultIdentityClient implements IdentityApi { + // @deprecated + authenticate(token: string | undefined): Promise; + static create(options: IdentityClientOptions): DefaultIdentityClient; + // (undocumented) + getIdentity(req: Request_2): Promise; +} + // @public export function getBearerTokenFromAuthorizationHeader( authorizationHeader: unknown, ): string | undefined; // @public +export interface IdentityApi { + getIdentity( + req: Request_2, + ): Promise; +} + +// @public @deprecated export class IdentityClient { + // @deprecated authenticate(token: string | undefined): Promise; + // (undocumented) static create(options: IdentityClientOptions): IdentityClient; } // @public export type IdentityClientOptions = { discovery: PluginEndpointDiscovery; - issuer: string; + issuer?: string; algorithms?: string[]; }; ``` diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index 011aade178..03fec84859 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -26,6 +26,7 @@ "@backstage/backend-common": "^0.14.1-next.1", "@backstage/config": "^1.0.1", "@backstage/errors": "^1.1.0-next.0", + "express": "^4.18.1", "jose": "^4.6.0", "node-fetch": "^2.6.7", "winston": "^3.2.1" diff --git a/plugins/auth-node/src/DefaultIdentityClient.test.ts b/plugins/auth-node/src/DefaultIdentityClient.test.ts new file mode 100644 index 0000000000..27aceaa780 --- /dev/null +++ b/plugins/auth-node/src/DefaultIdentityClient.test.ts @@ -0,0 +1,369 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { + decodeProtectedHeader, + exportJWK, + generateKeyPair, + SignJWT, +} from 'jose'; +import { cloneDeep } from 'lodash'; +import { rest } from 'msw'; +import { setupServer } from 'msw/node'; +import { v4 as uuid } from 'uuid'; + +import { DefaultIdentityClient } from './DefaultIdentityClient'; + +interface AnyJWK extends Record { + use: 'sig'; + alg: string; + kid: string; + kty: string; +} + +// Simplified copy of TokenFactory in @backstage/plugin-auth-backend +class FakeTokenFactory { + private readonly keys = new Array(); + + constructor( + private readonly options: { + issuer: string; + keyDurationSeconds: number; + }, + ) {} + + async issueToken(params: { + claims: { + sub: string; + ent?: string[]; + }; + }): Promise { + const pair = await generateKeyPair('ES256'); + const publicKey = await exportJWK(pair.publicKey); + const kid = uuid(); + publicKey.kid = kid; + this.keys.push(publicKey as AnyJWK); + + const iss = this.options.issuer; + const sub = params.claims.sub; + const ent = params.claims.ent; + const aud = 'backstage'; + const iat = Math.floor(Date.now() / 1000); + const exp = iat + this.options.keyDurationSeconds; + + return new SignJWT({ iss, sub, aud, iat, exp, ent, kid }) + .setProtectedHeader({ alg: 'ES256', ent: ent, kid: kid }) + .setIssuer(iss) + .setAudience(aud) + .setSubject(sub) + .setIssuedAt(iat) + .setExpirationTime(exp) + .sign(pair.privateKey); + } + + async listPublicKeys(): Promise<{ keys: AnyJWK[] }> { + return { keys: this.keys }; + } +} + +function jwtKid(jwt: string): string { + const header = decodeProtectedHeader(jwt); + return header.kid ?? ''; +} + +const server = setupServer(); +const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; +const discovery: PluginEndpointDiscovery = { + async getBaseUrl() { + return mockBaseUrl; + }, + async getExternalBaseUrl() { + return mockBaseUrl; + }, +}; + +describe('DefaultIdentityClient', () => { + let client: DefaultIdentityClient; + let factory: FakeTokenFactory; + const keyDurationSeconds = 5; + + beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); + afterAll(() => server.close()); + afterEach(() => server.resetHandlers()); + + beforeEach(() => { + client = DefaultIdentityClient.create({ discovery, issuer: mockBaseUrl }); + factory = new FakeTokenFactory({ + issuer: mockBaseUrl, + keyDurationSeconds, + }); + }); + + describe('identity client configuration', () => { + beforeEach(() => { + server.use( + rest.get( + `${mockBaseUrl}/.well-known/jwks.json`, + async (_, res, ctx) => { + const keys = await factory.listPublicKeys(); + return res(ctx.json(keys)); + }, + ), + ); + }); + + it('should defaults to ES256 when no algorithm is supplied', async () => { + const identityClient = DefaultIdentityClient.create({ + discovery, + issuer: mockBaseUrl, + }); + + const token = await factory.issueToken({ claims: { sub: 'foo' } }); + const response = await identityClient.authenticate(token); + + // expect that the authenticate is able to validate a token with ES256, which is the one set to FakeTokenFactory. + // This means that IdentityClient set ES256 by default. + expect(response).toEqual({ + token: token, + identity: { + type: 'user', + userEntityRef: 'foo', + ownershipEntityRefs: [], + }, + }); + }); + + it('should throw error on empty algorithms array', async () => { + const identityClient = DefaultIdentityClient.create({ + discovery, + issuer: mockBaseUrl, + algorithms: [''], + }); + + const token = await factory.issueToken({ claims: { sub: 'foo' } }); + return expect( + async () => await identityClient.authenticate(token), + ).rejects.toThrow(); + }); + + it('should throw error on empty algorithm string', async () => { + const identityClient = DefaultIdentityClient.create({ + discovery, + issuer: mockBaseUrl, + algorithms: [], + }); + + const token = await factory.issueToken({ claims: { sub: 'foo' } }); + return expect( + async () => await identityClient.authenticate(token), + ).rejects.toThrow(); + }); + }); + + describe('authenticate', () => { + beforeEach(() => { + server.use( + rest.get( + `${mockBaseUrl}/.well-known/jwks.json`, + async (_, res, ctx) => { + const keys = await factory.listPublicKeys(); + return res(ctx.json(keys)); + }, + ), + ); + }); + + it('should throw on undefined header', async () => { + return expect(async () => { + await client.authenticate(undefined); + }).rejects.toThrow(); + }); + + it('should accept fresh token', async () => { + const token = await factory.issueToken({ claims: { sub: 'foo' } }); + const response = await client.authenticate(token); + expect(response).toEqual({ + token: token, + identity: { + type: 'user', + userEntityRef: 'foo', + ownershipEntityRefs: [], + }, + }); + }); + + it('should decode claims correctly', async () => { + const token = await factory.issueToken({ + claims: { sub: 'foo', ent: ['entity1', 'entity2'] }, + }); + const response = await client.authenticate(token); + expect(response).toEqual({ + token: token, + identity: { + type: 'user', + userEntityRef: 'foo', + ownershipEntityRefs: ['entity1', 'entity2'], + }, + }); + }); + + it('should throw on incorrect issuer', async () => { + const hackerFactory = new FakeTokenFactory({ + issuer: 'hacker', + keyDurationSeconds, + }); + return expect(async () => { + const token = await hackerFactory.issueToken({ + claims: { sub: 'foo' }, + }); + await client.authenticate(token); + }).rejects.toThrow(); + }); + + it('should throw on expired token', async () => { + return expect(async () => { + const fixedTime = Date.now(); + jest + .spyOn(Date, 'now') + .mockImplementation(() => fixedTime - keyDurationSeconds * 1000 * 2); + const token = await factory.issueToken({ + claims: { sub: 'foo' }, + }); + jest.spyOn(Date, 'now').mockImplementation(() => fixedTime); + await client.authenticate(token); + }).rejects.toThrow(); + }); + + it('should throw on incorrect signing key', async () => { + const hackerFactory = new FakeTokenFactory({ + issuer: mockBaseUrl, + keyDurationSeconds, + }); + return expect(async () => { + const token = await hackerFactory.issueToken({ + claims: { sub: 'foo' }, + }); + await client.authenticate(token); + }).rejects.toThrow(); + }); + + it('should accept token from new key', async () => { + const fixedTime = Date.now(); + jest + .spyOn(Date, 'now') + .mockImplementation(() => fixedTime - keyDurationSeconds * 1000 * 2); + const token1 = await factory.issueToken({ claims: { sub: 'foo1' } }); + try { + // This throws as token has already expired + await client.authenticate(token1); + } catch (_err) { + // Ignore thrown error + } + // Move forward in time where the signing key has been rotated and the + // cooldown period to look up a new public key has elapsed. + jest + .spyOn(Date, 'now') + .mockImplementation( + () => fixedTime + 30 * keyDurationSeconds * 1000 + 2, + ); + const token = await factory.issueToken({ claims: { sub: 'foo' } }); + const response = await client.authenticate(token); + expect(response).toEqual({ + token: token, + identity: { + type: 'user', + userEntityRef: 'foo', + ownershipEntityRefs: [], + }, + }); + }); + + it('should not be fooled by the none algorithm', async () => { + return expect(async () => { + const token = await factory.issueToken({ claims: { sub: 'foo' } }); + const header = btoa( + JSON.stringify({ alg: 'none', kid: jwtKid(token) }), + ); + const payload = btoa( + JSON.stringify({ + iss: mockBaseUrl, + sub: 'foo', + aud: 'backstage', + iat: Date.now() / 1000, + exp: Date.now() / 1000 + 60000, + }), + ); + const fakeToken = `${header}.${payload}.`; + return await client.authenticate(fakeToken); + }).rejects.toThrow(); + }); + + it('should use an updated endpoint when the key is not found', async () => { + const updatedURL = 'http://backstage:9191/an-updated-base'; + const getBaseUrl = discovery.getBaseUrl; + const getExternalBaseUrl = discovery.getExternalBaseUrl; + // Generate a key and sign a token with it + await factory.issueToken({ claims: { sub: 'foo' } }); + // Only return the key from a single token + const singleKey = cloneDeep(await factory.listPublicKeys()); + server.use( + rest.get( + `${mockBaseUrl}/.well-known/jwks.json`, + async (_, res, ctx) => { + return res(ctx.json(singleKey)); + }, + ), + ); + // Update the discovery endpoint to point to a new URL + discovery.getBaseUrl = async () => { + return updatedURL; + }; + discovery.getExternalBaseUrl = async () => { + return updatedURL; + }; + let calledUpdatedEndpoint = false; + server.use( + rest.get(`${updatedURL}/.well-known/jwks.json`, async (_, res, ctx) => { + const keys = await factory.listPublicKeys(); + calledUpdatedEndpoint = true; + return res(ctx.json(keys)); + }), + ); + // Advance time + const future_11s = Date.now() + 11 * 1000; + const dateSpy = jest + .spyOn(Date, 'now') + .mockImplementation(() => future_11s); + // Issue a new token + const token = await factory.issueToken({ claims: { sub: 'foo2' } }); + const response = await client.authenticate(token); + // Verify that the endpoint was updated. + expect(calledUpdatedEndpoint).toBeTruthy(); + expect(response).toEqual({ + token: token, + identity: { + type: 'user', + userEntityRef: 'foo2', + ownershipEntityRefs: [], + }, + }); + // Restore the discovery endpoint and time + discovery.getBaseUrl = getBaseUrl; + discovery.getExternalBaseUrl = getExternalBaseUrl; + dateSpy.mockClear(); + }); + }); +}); diff --git a/plugins/auth-node/src/DefaultIdentityClient.ts b/plugins/auth-node/src/DefaultIdentityClient.ts new file mode 100644 index 0000000000..93fdb5542b --- /dev/null +++ b/plugins/auth-node/src/DefaultIdentityClient.ts @@ -0,0 +1,168 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { AuthenticationError } from '@backstage/errors'; +import { + createRemoteJWKSet, + decodeJwt, + decodeProtectedHeader, + FlattenedJWSInput, + JWSHeaderParameters, + jwtVerify, +} from 'jose'; +import { GetKeyFunction } from 'jose/dist/types/types'; + +import { BackstageIdentityResponse } from './types'; +import { getBearerTokenFromAuthorizationHeader, IdentityApi } from '.'; +import { Request } from 'express'; + +const CLOCK_MARGIN_S = 10; + +/** + * An identity client options object which allows extra configurations + * + * @experimental This is not a stable API yet + * @public + */ +export type IdentityClientOptions = { + discovery: PluginEndpointDiscovery; + issuer?: string; + + /** JWS "alg" (Algorithm) Header Parameter values. Defaults to an array containing just ES256. + * More info on supported algorithms: https://github.com/panva/jose */ + algorithms?: string[]; +}; + +/** + * An identity client to interact with auth-backend and authenticate Backstage + * tokens + * + * @experimental This is not a stable API yet + * @public + */ +export class DefaultIdentityClient implements IdentityApi { + private readonly discovery: PluginEndpointDiscovery; + private readonly issuer?: string; + private readonly algorithms?: string[]; + private keyStore?: GetKeyFunction; + private keyStoreUpdated: number = 0; + + /** + * Create a new {@link DefaultIdentityClient} instance. + */ + static create(options: IdentityClientOptions): DefaultIdentityClient { + return new DefaultIdentityClient(options); + } + + private constructor(options: IdentityClientOptions) { + this.discovery = options.discovery; + this.issuer = options.issuer; + this.algorithms = options.hasOwnProperty('algorithms') + ? options.algorithms + : ['ES256']; + } + + async getIdentity(req: Request) { + try { + return await this.authenticate( + getBearerTokenFromAuthorizationHeader(req.headers.authorization), + ); + } catch (e) { + return undefined; + } + } + + /** + * Verifies the given backstage identity token + * Returns a BackstageIdentity (user) matching the token. + * The method throws an error if verification fails. + * + * @deprecated You should start to use getIdentity instead of authenticate to retrieve the user + * identity. + */ + async authenticate( + token: string | undefined, + ): Promise { + // Extract token from header + if (!token) { + throw new AuthenticationError('No token specified'); + } + + // Verify token claims and signature + // Note: Claims must match those set by TokenFactory when issuing tokens + // Note: verify throws if verification fails + // Check if the keystore needs to be updated + await this.refreshKeyStore(token); + if (!this.keyStore) { + throw new AuthenticationError('No keystore exists'); + } + const decoded = await jwtVerify(token, this.keyStore, { + algorithms: this.algorithms, + audience: 'backstage', + issuer: this.issuer, + }); + // Verified, return the matching user as BackstageIdentity + // TODO: Settle internal user format/properties + if (!decoded.payload.sub) { + throw new AuthenticationError('No user sub found in token'); + } + + const user: BackstageIdentityResponse = { + token, + identity: { + type: 'user', + userEntityRef: decoded.payload.sub, + ownershipEntityRefs: decoded.payload.ent + ? (decoded.payload.ent as string[]) + : [], + }, + }; + return user; + } + + /** + * If the last keystore refresh is stale, update the keystore URL to the latest + */ + private async refreshKeyStore(rawJwtToken: string): Promise { + const payload = await decodeJwt(rawJwtToken); + const header = await decodeProtectedHeader(rawJwtToken); + + // Refresh public keys if needed + let keyStoreHasKey; + try { + if (this.keyStore) { + // Check if the key is present in the keystore + const [_, rawPayload, rawSignature] = rawJwtToken.split('.'); + keyStoreHasKey = await this.keyStore(header, { + payload: rawPayload, + signature: rawSignature, + }); + } + } catch (error) { + keyStoreHasKey = false; + } + // Refresh public key URL if needed + // Add a small margin in case clocks are out of sync + const issuedAfterLastRefresh = + payload?.iat && payload.iat > this.keyStoreUpdated - CLOCK_MARGIN_S; + if (!this.keyStore || (!keyStoreHasKey && issuedAfterLastRefresh)) { + const url = await this.discovery.getBaseUrl('auth'); + const endpoint = new URL(`${url}/.well-known/jwks.json`); + this.keyStore = createRemoteJWKSet(endpoint); + this.keyStoreUpdated = Date.now() / 1000; + } + } +} diff --git a/plugins/auth-node/src/IdentityApi.ts b/plugins/auth-node/src/IdentityApi.ts new file mode 100644 index 0000000000..2387d01f65 --- /dev/null +++ b/plugins/auth-node/src/IdentityApi.ts @@ -0,0 +1,35 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { Request } from 'express'; +import { BackstageIdentityResponse } from './types'; + +/** + * An identity client api to authenticate Backstage + * tokens + * + * @experimental This is not a stable API yet + * @public + */ +export interface IdentityApi { + /** + * Verifies the given backstage identity token + * Returns a BackstageIdentity (user) matching the token. + * The method throws an error if verification fails. + */ + getIdentity( + req: Request, + ): Promise; +} diff --git a/plugins/auth-node/src/IdentityClient.ts b/plugins/auth-node/src/IdentityClient.ts index 0773d22c26..8252a61f81 100644 --- a/plugins/auth-node/src/IdentityClient.ts +++ b/plugins/auth-node/src/IdentityClient.ts @@ -13,139 +13,42 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { PluginEndpointDiscovery } from '@backstage/backend-common'; -import { AuthenticationError } from '@backstage/errors'; + import { - createRemoteJWKSet, - decodeJwt, - decodeProtectedHeader, - FlattenedJWSInput, - JWSHeaderParameters, - jwtVerify, -} from 'jose'; -import { GetKeyFunction } from 'jose/dist/types/types'; - + DefaultIdentityClient, + IdentityClientOptions, +} from './DefaultIdentityClient'; import { BackstageIdentityResponse } from './types'; -const CLOCK_MARGIN_S = 10; - -/** - * An identity client options object which allows extra configurations - * - * @experimental This is not a stable API yet - * @public - */ -export type IdentityClientOptions = { - discovery: PluginEndpointDiscovery; - issuer: string; - - /** JWS "alg" (Algorithm) Header Parameter values. Defaults to an array containing just ES256. - * More info on supported algorithms: https://github.com/panva/jose */ - algorithms?: string[]; -}; - /** * An identity client to interact with auth-backend and authenticate Backstage * tokens * - * @experimental This is not a stable API yet * @public + * @experimental This is not a stable API yet + * @deprecated Please migrate to the DefaultIdentityClient. */ export class IdentityClient { - private readonly discovery: PluginEndpointDiscovery; - private readonly issuer: string; - private readonly algorithms: string[]; - private keyStore?: GetKeyFunction; - private keyStoreUpdated: number = 0; - - /** - * Create a new {@link IdentityClient} instance. - */ + private readonly defaultIdentityClient: DefaultIdentityClient; static create(options: IdentityClientOptions): IdentityClient { - return new IdentityClient(options); + return new IdentityClient(DefaultIdentityClient.create(options)); } - private constructor(options: IdentityClientOptions) { - this.discovery = options.discovery; - this.issuer = options.issuer; - this.algorithms = options.algorithms ?? ['ES256']; + private constructor(defaultIdentityClient: DefaultIdentityClient) { + this.defaultIdentityClient = defaultIdentityClient; } /** * Verifies the given backstage identity token * Returns a BackstageIdentity (user) matching the token. * The method throws an error if verification fails. + * + * @deprecated You should start to use IdentityApi#getIdentity instead of authenticate + * to retrieve the user identity. */ async authenticate( token: string | undefined, ): Promise { - // Extract token from header - if (!token) { - throw new AuthenticationError('No token specified'); - } - - // Verify token claims and signature - // Note: Claims must match those set by TokenFactory when issuing tokens - // Note: verify throws if verification fails - // Check if the keystore needs to be updated - await this.refreshKeyStore(token); - if (!this.keyStore) { - throw new AuthenticationError('No keystore exists'); - } - const decoded = await jwtVerify(token, this.keyStore, { - algorithms: this.algorithms, - audience: 'backstage', - issuer: this.issuer, - }); - // Verified, return the matching user as BackstageIdentity - // TODO: Settle internal user format/properties - if (!decoded.payload.sub) { - throw new AuthenticationError('No user sub found in token'); - } - - const user: BackstageIdentityResponse = { - token, - identity: { - type: 'user', - userEntityRef: decoded.payload.sub, - ownershipEntityRefs: decoded.payload.ent - ? (decoded.payload.ent as string[]) - : [], - }, - }; - return user; - } - - /** - * If the last keystore refresh is stale, update the keystore URL to the latest - */ - private async refreshKeyStore(rawJwtToken: string): Promise { - const payload = await decodeJwt(rawJwtToken); - const header = await decodeProtectedHeader(rawJwtToken); - - // Refresh public keys if needed - let keyStoreHasKey; - try { - if (this.keyStore) { - // Check if the key is present in the keystore - const [_, rawPayload, rawSignature] = rawJwtToken.split('.'); - keyStoreHasKey = await this.keyStore(header, { - payload: rawPayload, - signature: rawSignature, - }); - } - } catch (error) { - keyStoreHasKey = false; - } - // Refresh public key URL if needed - // Add a small margin in case clocks are out of sync - const issuedAfterLastRefresh = - payload?.iat && payload.iat > this.keyStoreUpdated - CLOCK_MARGIN_S; - if (!this.keyStore || (!keyStoreHasKey && issuedAfterLastRefresh)) { - const url = await this.discovery.getBaseUrl('auth'); - const endpoint = new URL(`${url}/.well-known/jwks.json`); - this.keyStore = createRemoteJWKSet(endpoint); - this.keyStoreUpdated = Date.now() / 1000; - } + return await this.defaultIdentityClient.authenticate(token); } } diff --git a/plugins/auth-node/src/index.ts b/plugins/auth-node/src/index.ts index cfb7275abd..a17678a038 100644 --- a/plugins/auth-node/src/index.ts +++ b/plugins/auth-node/src/index.ts @@ -21,8 +21,10 @@ */ export { getBearerTokenFromAuthorizationHeader } from './getBearerTokenFromAuthorizationHeader'; +export { DefaultIdentityClient } from './DefaultIdentityClient'; export { IdentityClient } from './IdentityClient'; -export type { IdentityClientOptions } from './IdentityClient'; +export type { IdentityApi } from './IdentityApi'; +export type { IdentityClientOptions } from './DefaultIdentityClient'; export type { BackstageIdentityResponse, BackstageSignInResult, diff --git a/plugins/example-todo-list-backend/api-report.md b/plugins/example-todo-list-backend/api-report.md index fa62ada8e7..c4f8665a70 100644 --- a/plugins/example-todo-list-backend/api-report.md +++ b/plugins/example-todo-list-backend/api-report.md @@ -4,7 +4,7 @@ ```ts import express from 'express'; -import { IdentityClient } from '@backstage/plugin-auth-node'; +import { IdentityApi } from '@backstage/plugin-auth-node'; import { Logger } from 'winston'; // @public @@ -13,7 +13,7 @@ export function createRouter(options: RouterOptions): Promise; // @public export interface RouterOptions { // (undocumented) - identity: IdentityClient; + identity: IdentityApi; // (undocumented) logger: Logger; } diff --git a/plugins/example-todo-list-backend/src/service/router.test.ts b/plugins/example-todo-list-backend/src/service/router.test.ts index e381aad143..3a1ce458f2 100644 --- a/plugins/example-todo-list-backend/src/service/router.test.ts +++ b/plugins/example-todo-list-backend/src/service/router.test.ts @@ -15,7 +15,7 @@ */ import { getVoidLogger } from '@backstage/backend-common'; -import { IdentityClient } from '@backstage/plugin-auth-node'; +import { DefaultIdentityClient } from '@backstage/plugin-auth-node'; import express from 'express'; import request from 'supertest'; @@ -27,7 +27,7 @@ describe('createRouter', () => { beforeAll(async () => { const router = await createRouter({ logger: getVoidLogger(), - identity: {} as IdentityClient, + identity: {} as DefaultIdentityClient, }); app = express().use(router); }); diff --git a/plugins/example-todo-list-backend/src/service/router.ts b/plugins/example-todo-list-backend/src/service/router.ts index cdaf1f82b4..58ce730d24 100644 --- a/plugins/example-todo-list-backend/src/service/router.ts +++ b/plugins/example-todo-list-backend/src/service/router.ts @@ -18,12 +18,9 @@ import { errorHandler } from '@backstage/backend-common'; import express from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; -import { - IdentityClient, - getBearerTokenFromAuthorizationHeader, -} from '@backstage/plugin-auth-node'; import { add, getAll, update } from './todos'; import { InputError } from '@backstage/errors'; +import { IdentityApi } from '@backstage/plugin-auth-node'; /** * Dependencies of the todo-list router @@ -32,7 +29,7 @@ import { InputError } from '@backstage/errors'; */ export interface RouterOptions { logger: Logger; - identity: IdentityClient; + identity: IdentityApi; } /** @@ -62,12 +59,9 @@ export async function createRouter( }); router.post('/todos', async (req, res) => { - const token = getBearerTokenFromAuthorizationHeader( - req.header('authorization'), - ); let author: string | undefined = undefined; - const user = token ? await identity.authenticate(token) : undefined; + const user = await identity.getIdentity(req); author = user?.identity.userEntityRef; if (!isTodoCreateRequest(req.body)) { diff --git a/plugins/example-todo-list-backend/src/service/standaloneServer.ts b/plugins/example-todo-list-backend/src/service/standaloneServer.ts index b0332e0931..eedf1230ca 100644 --- a/plugins/example-todo-list-backend/src/service/standaloneServer.ts +++ b/plugins/example-todo-list-backend/src/service/standaloneServer.ts @@ -19,7 +19,7 @@ import { loadBackendConfig, SingleHostDiscovery, } from '@backstage/backend-common'; -import { IdentityClient } from '@backstage/plugin-auth-node'; +import { DefaultIdentityClient } from '@backstage/plugin-auth-node'; import { Server } from 'http'; import { Logger } from 'winston'; import { createRouter } from './router'; @@ -39,7 +39,7 @@ export async function startStandaloneServer( const discovery = SingleHostDiscovery.fromConfig(config); const router = await createRouter({ logger, - identity: IdentityClient.create({ + identity: DefaultIdentityClient.create({ discovery, issuer: await discovery.getExternalBaseUrl('auth'), }), diff --git a/plugins/permission-backend/api-report.md b/plugins/permission-backend/api-report.md index 3c606e3cb9..e9cc4e6272 100644 --- a/plugins/permission-backend/api-report.md +++ b/plugins/permission-backend/api-report.md @@ -5,7 +5,7 @@ ```ts import { Config } from '@backstage/config'; import express from 'express'; -import { IdentityClient } from '@backstage/plugin-auth-node'; +import { IdentityApi } from '@backstage/plugin-auth-node'; import { Logger } from 'winston'; import { PermissionPolicy } from '@backstage/plugin-permission-node'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; @@ -20,7 +20,7 @@ export interface RouterOptions { // (undocumented) discovery: PluginEndpointDiscovery; // (undocumented) - identity: IdentityClient; + identity: IdentityApi; // (undocumented) logger: Logger; // (undocumented) diff --git a/plugins/permission-backend/package.json b/plugins/permission-backend/package.json index 4df852d255..cfd200407d 100644 --- a/plugins/permission-backend/package.json +++ b/plugins/permission-backend/package.json @@ -28,6 +28,7 @@ "@backstage/plugin-auth-node": "^0.2.3-next.1", "@backstage/plugin-permission-common": "^0.6.3-next.0", "@backstage/plugin-permission-node": "^0.6.3-next.1", + "@backstage/plugin-auth-node": "^0.0.1", "@types/express": "*", "dataloader": "^2.0.0", "express": "^4.17.1", diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index 6c6ad14af4..7668245679 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -17,7 +17,6 @@ import express from 'express'; import request from 'supertest'; import { getVoidLogger } from '@backstage/backend-common'; -import { IdentityClient } from '@backstage/plugin-auth-node'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { ApplyConditionsRequestEntry, @@ -71,17 +70,23 @@ describe('createRouter', () => { getExternalBaseUrl: jest.fn(), }, identity: { - authenticate: jest.fn(token => { + getIdentity: jest.fn(req => { + const token = req.headers.authorization?.replace(/^Bearer[ ]+/, ''); + if (!token) { - throw new Error('No token supplied!'); + return Promise.resolve(undefined); } return Promise.resolve({ - id: 'test-user', + identity: { + type: 'user', + userEntityRef: 'test-user', + ownershipEntityRefs: ['blah'], + }, token, }); }), - } as unknown as IdentityClient, + }, policy, }); @@ -184,7 +189,14 @@ describe('createRouter', () => { attributes: {}, }, }, - { id: 'test-user', token: 'test-token' }, + { + token: 'test-token', + identity: { + type: 'user', + userEntityRef: 'test-user', + ownershipEntityRefs: ['blah'], + }, + }, ); expect(response.body).toEqual({ items: [{ id: '123', result: AuthorizeResult.ALLOW }], diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index ea7fbcca24..217c135ac3 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -24,9 +24,8 @@ import { } from '@backstage/backend-common'; import { InputError } from '@backstage/errors'; import { - getBearerTokenFromAuthorizationHeader, BackstageIdentityResponse, - IdentityClient, + IdentityApi, } from '@backstage/plugin-auth-node'; import { AuthorizeResult, @@ -96,7 +95,7 @@ export interface RouterOptions { logger: Logger; discovery: PluginEndpointDiscovery; policy: PermissionPolicy; - identity: IdentityClient; + identity: IdentityApi; config: Config; } @@ -189,10 +188,7 @@ export async function createRouter( req: Request, res: Response, ) => { - const token = getBearerTokenFromAuthorizationHeader( - req.header('authorization'), - ); - const user = token ? await identity.authenticate(token) : undefined; + const user = await identity.getIdentity(req); const parseResult = evaluatePermissionRequestBatchSchema.safeParse( req.body, diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 2053f01e39..12d4746f42 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -13,6 +13,7 @@ import { createPullRequest } from 'octokit-plugin-create-pull-request'; import { Entity } from '@backstage/catalog-model'; import express from 'express'; import { GithubCredentialsProvider } from '@backstage/integration'; +import { IdentityApi } from '@backstage/plugin-auth-node'; import { JsonObject } from '@backstage/types'; import { JsonValue } from '@backstage/types'; import { Knex } from 'knex'; @@ -526,6 +527,8 @@ export interface RouterOptions { // (undocumented) database: PluginDatabaseManager; // (undocumented) + identity: IdentityApi; + // (undocumented) logger: Logger; // (undocumented) reader: UrlReader; diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 6f3aea1844..dfd264514e 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -42,6 +42,7 @@ "@backstage/integration": "^1.2.2-next.2", "@backstage/plugin-catalog-backend": "^1.2.1-next.2", "@backstage/plugin-scaffolder-common": "^1.1.2-next.0", + "@backstage/plugin-auth-node": "^0.0.1", "@backstage/types": "^1.0.0", "@gitbeaker/core": "^35.6.0", "@gitbeaker/node": "^35.1.0", diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index aea33f1158..72d494824b 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; + const mockAccess = jest.fn(); jest.doMock('fs-extra', () => ({ access: mockAccess, @@ -75,6 +77,16 @@ const mockUrlReader = UrlReaders.default({ describe('createRouter', () => { let app: express.Express; let taskBroker: TaskBroker; + const getIdentity = jest.fn(); + const rawPayload = Buffer.from( + JSON.stringify({ + sub: 'user:default/guest', + ent: ['group:default/guests'], + }), + 'utf8', + ).toString('base64'); + const mockToken = ['blob', rawPayload, 'blob'].join('.'); + const catalogClient = { getEntityByRef: jest.fn() } as unknown as CatalogApi; const mockTemplate: TemplateEntityV1beta3 = { @@ -123,6 +135,7 @@ describe('createRouter', () => { }; beforeEach(async () => { + getIdentity.mockReset(); const logger = getVoidLogger(); const databaseTaskStore = await DatabaseTaskStore.create({ database: await createDatabase().getClient(), @@ -134,6 +147,18 @@ describe('createRouter', () => { jest.spyOn(taskBroker, 'list'); jest.spyOn(taskBroker, 'event$'); + getIdentity.mockImplementation( + async (_req): Promise => { + return { + token: mockToken, + identity: { + type: 'user', + userEntityRef: 'user:default/guest', + ownershipEntityRefs: ['group:default/guests'], + }, + }; + }, + ); const router = await createRouter({ logger: getVoidLogger(), config: new ConfigReader({}), @@ -141,6 +166,9 @@ describe('createRouter', () => { catalogClient, reader: mockUrlReader, taskBroker, + identity: { + getIdentity, + }, }); app = express().use(router); @@ -214,8 +242,6 @@ describe('createRouter', () => { it('should call the broker with a correct spec', async () => { const broker = taskBroker.dispatch as jest.Mocked['dispatch']; - const mockToken = - 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; await request(app) .post('/v2/tasks') @@ -264,29 +290,51 @@ describe('createRouter', () => { ); }); - it('should not decorate a user when no backstage auth is passed', async () => { - const broker = taskBroker.dispatch as jest.Mocked['dispatch']; - - await request(app) - .post('/v2/tasks') - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', + describe('no auth is passed', () => { + beforeEach(async () => { + getIdentity.mockImplementation( + async (_req): Promise => { + return undefined; + }, + ); + const router = await createRouter({ + logger: getVoidLogger(), + config: new ConfigReader({}), + database: createDatabase(), + catalogClient, + reader: mockUrlReader, + taskBroker, + identity: { + getIdentity, }, }); + app = express().use(router); + }); + it('should not decorate a user when no backstage auth is passed', async () => { + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; - expect(broker).toHaveBeenCalledWith( - expect.objectContaining({ - createdBy: undefined, - spec: expect.objectContaining({ - user: { entity: undefined, ref: undefined }, + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + + expect(broker).toHaveBeenCalledWith( + expect.objectContaining({ + createdBy: undefined, + spec: expect.objectContaining({ + user: { entity: undefined, ref: undefined }, + }), }), - }), - ); + ); + }); }); }); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 2abb730250..74431b47f5 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -23,14 +23,13 @@ import { } from '@backstage/catalog-model'; import { Entity } from '@backstage/catalog-model'; import { Config, JsonObject } from '@backstage/config'; -import { InputError, NotFoundError, stringifyError } from '@backstage/errors'; +import { InputError, NotFoundError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; import { TemplateEntityV1beta3, TaskSpec, templateEntityV1beta3Validator, } from '@backstage/plugin-scaffolder-common'; -import { JsonValue } from '@backstage/types'; import express from 'express'; import Router from 'express-promise-router'; import { validate } from 'jsonschema'; @@ -48,6 +47,7 @@ import { import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; import { getEntityBaseUrl, getWorkingDirectory, findTemplate } from './helpers'; +import { IdentityApi } from '@backstage/plugin-auth-node'; /** * RouterOptions @@ -64,6 +64,7 @@ export interface RouterOptions { taskWorkers?: number; taskBroker?: TaskBroker; additionalTemplateFilters?: Record; + identity: IdentityApi; } function isSupportedTemplate(entity: TemplateEntityV1beta3) { @@ -89,6 +90,7 @@ export async function createRouter( actions, taskWorkers, additionalTemplateFilters, + identity, } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); @@ -146,7 +148,8 @@ export async function createRouter( '/v2/templates/:namespace/:kind/:name/parameter-schema', async (req, res) => { const { namespace, kind, name } = req.params; - const { token } = parseBearerToken(req.headers.authorization); + const userIdentity = await identity.getIdentity(req); + const token = userIdentity?.token; const template = await findTemplate({ catalogApi: catalogClient, entityRef: { kind, namespace, name }, @@ -185,9 +188,9 @@ export async function createRouter( const { kind, namespace, name } = parseEntityRef(templateRef, { defaultKind: 'template', }); - const { token, entityRef: userEntityRef } = parseBearerToken( - req.headers.authorization, - ); + const callerIdentity = await identity.getIdentity(req); + const token = callerIdentity?.token; + const userEntityRef = callerIdentity?.identity.userEntityRef; const userEntity = userEntityRef ? await catalogClient.getEntityByRef(userEntityRef, { token }) @@ -377,7 +380,7 @@ export async function createRouter( throw new InputError('Input template is not a template'); } - const { token } = parseBearerToken(req.headers.authorization); + const token = (await identity.getIdentity(req))?.token; for (const parameters of [template.spec.parameters ?? []].flat()) { const result = validate(body.values, parameters); @@ -427,41 +430,3 @@ export async function createRouter( return app; } - -function parseBearerToken(header?: string): { - token?: string; - entityRef?: string; -} { - if (!header) { - return {}; - } - - 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'); - } - - return { entityRef: sub, token }; - } catch (e) { - throw new InputError(`Invalid authorization header: ${stringifyError(e)}`); - } -} From a19afbba60ce5fa9436e0b07a3a2133bcd85d522 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 8 Jul 2022 17:08:56 +0100 Subject: [PATCH 02/25] fix auth-node versions Signed-off-by: Brian Fletcher --- plugins/permission-backend/package.json | 1 - plugins/scaffolder-backend/package.json | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/permission-backend/package.json b/plugins/permission-backend/package.json index cfd200407d..4df852d255 100644 --- a/plugins/permission-backend/package.json +++ b/plugins/permission-backend/package.json @@ -28,7 +28,6 @@ "@backstage/plugin-auth-node": "^0.2.3-next.1", "@backstage/plugin-permission-common": "^0.6.3-next.0", "@backstage/plugin-permission-node": "^0.6.3-next.1", - "@backstage/plugin-auth-node": "^0.0.1", "@types/express": "*", "dataloader": "^2.0.0", "express": "^4.17.1", diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 10ebd27af4..9c4c0f1da6 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -43,7 +43,7 @@ "@backstage/integration": "^1.2.2-next.2", "@backstage/plugin-catalog-backend": "^1.2.1-next.2", "@backstage/plugin-scaffolder-common": "^1.1.2-next.0", - "@backstage/plugin-auth-node": "^0.0.1", + "@backstage/plugin-auth-node": "^0.2.3-next.1", "@backstage/backend-plugin-api": "^0.0.0", "@backstage/plugin-catalog-node": "^0.0.0", "@backstage/types": "^1.0.0", From 49416194e8c7d63e6a83376f19a704bc6dd52812 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 8 Jul 2022 17:25:51 +0100 Subject: [PATCH 03/25] adds identity api to create-app backend Signed-off-by: Brian Fletcher --- .changeset/itchy-kiwis-warn.md | 5 +++++ .../default-app/packages/backend/package.json.hbs | 1 + .../templates/default-app/packages/backend/src/index.ts | 9 +++++++++ .../packages/backend/src/plugins/scaffolder.ts | 1 + .../templates/default-app/packages/backend/src/types.ts | 2 ++ 5 files changed, 18 insertions(+) create mode 100644 .changeset/itchy-kiwis-warn.md diff --git a/.changeset/itchy-kiwis-warn.md b/.changeset/itchy-kiwis-warn.md new file mode 100644 index 0000000000..d2af94d0ab --- /dev/null +++ b/.changeset/itchy-kiwis-warn.md @@ -0,0 +1,5 @@ +--- +'@backstage/create-app': patch +--- + +Adds IdentityApi configuration to create-app scaffolding templates. diff --git a/packages/create-app/templates/default-app/packages/backend/package.json.hbs b/packages/create-app/templates/default-app/packages/backend/package.json.hbs index 3bd070ee9b..2ee46723e6 100644 --- a/packages/create-app/templates/default-app/packages/backend/package.json.hbs +++ b/packages/create-app/templates/default-app/packages/backend/package.json.hbs @@ -24,6 +24,7 @@ "@backstage/config": "^{{version '@backstage/config'}}", "@backstage/plugin-app-backend": "^{{version '@backstage/plugin-app-backend'}}", "@backstage/plugin-auth-backend": "^{{version '@backstage/plugin-auth-backend'}}", + "@backstage/plugin-auth-node": "^{{version '@backstage/plugin-auth-node'}}", "@backstage/plugin-catalog-backend": "^{{version '@backstage/plugin-catalog-backend'}}", "@backstage/plugin-permission-common": "^{{version '@backstage/plugin-permission-common'}}", "@backstage/plugin-permission-node": "^{{version '@backstage/plugin-permission-node'}}", diff --git a/packages/create-app/templates/default-app/packages/backend/src/index.ts b/packages/create-app/templates/default-app/packages/backend/src/index.ts index 70bc66bcdd..40cfbe62f1 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/index.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/index.ts @@ -30,6 +30,7 @@ import techdocs from './plugins/techdocs'; import search from './plugins/search'; import { PluginEnvironment } from './types'; import { ServerPermissionClient } from '@backstage/plugin-permission-node'; +import {DefaultIdentityClient} from "../../../../../../../dist-types/plugins/auth-node/src"; function makeCreateEnv(config: Config) { const root = getRootLogger(); @@ -39,6 +40,13 @@ function makeCreateEnv(config: Config) { const databaseManager = DatabaseManager.fromConfig(config); const tokenManager = ServerTokenManager.noop(); const taskScheduler = TaskScheduler.fromConfig(config); + + const identity = DefaultIdentityClient.create({ + discovery, + issuer: undefined, + algorithms: undefined, + }); + const permissions = ServerPermissionClient.fromConfig(config, { discovery, tokenManager, @@ -61,6 +69,7 @@ function makeCreateEnv(config: Config) { tokenManager, scheduler, permissions, + identity, }; }; } diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts index 7ce5fcf31a..77e1e94a66 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts @@ -16,5 +16,6 @@ export default async function createPlugin( database: env.database, reader: env.reader, catalogClient, + identity: env.identity }); } diff --git a/packages/create-app/templates/default-app/packages/backend/src/types.ts b/packages/create-app/templates/default-app/packages/backend/src/types.ts index 8e0a86404b..9db9adadf0 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/types.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/types.ts @@ -9,6 +9,7 @@ import { } from '@backstage/backend-common'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; +import { IdentityApi } from "@backstage/plugin-auth-node"; export type PluginEnvironment = { logger: Logger; @@ -20,4 +21,5 @@ export type PluginEnvironment = { tokenManager: TokenManager; scheduler: PluginTaskScheduler; permissions: PermissionEvaluator; + identity: IdentityApi; }; From e3b9992182d79f5e77937a7384bb743d70aa867c Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 8 Jul 2022 17:27:40 +0100 Subject: [PATCH 04/25] fix identity client import Signed-off-by: Brian Fletcher --- .../templates/default-app/packages/backend/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/create-app/templates/default-app/packages/backend/src/index.ts b/packages/create-app/templates/default-app/packages/backend/src/index.ts index 40cfbe62f1..3d5b4b4f7c 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/index.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/index.ts @@ -30,7 +30,7 @@ import techdocs from './plugins/techdocs'; import search from './plugins/search'; import { PluginEnvironment } from './types'; import { ServerPermissionClient } from '@backstage/plugin-permission-node'; -import {DefaultIdentityClient} from "../../../../../../../dist-types/plugins/auth-node/src"; +import { DefaultIdentityClient } from "@backstage/plugin-auth-node"; function makeCreateEnv(config: Config) { const root = getRootLogger(); From 96c289c6a8ddc982fb87c568f4c11a01a06f6947 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 8 Jul 2022 17:36:13 +0100 Subject: [PATCH 05/25] add plugin auth node to create-app Signed-off-by: Brian Fletcher --- packages/create-app/src/lib/versions.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/create-app/src/lib/versions.ts b/packages/create-app/src/lib/versions.ts index 78b28b5811..b89b1d5463 100644 --- a/packages/create-app/src/lib/versions.ts +++ b/packages/create-app/src/lib/versions.ts @@ -49,6 +49,7 @@ import { version as theme } from '../../../theme/package.json'; import { version as pluginApiDocs } from '../../../../plugins/api-docs/package.json'; import { version as pluginAppBackend } from '../../../../plugins/app-backend/package.json'; import { version as pluginAuthBackend } from '../../../../plugins/auth-backend/package.json'; +import { version as pluginAuthNode } from '../../../../plugins/auth-node/package.json'; import { version as pluginCatalog } from '../../../../plugins/catalog/package.json'; import { version as pluginCatalogCommon } from '../../../../plugins/catalog-common/package.json'; import { version as pluginCatalogReact } from '../../../../plugins/catalog-react/package.json'; @@ -96,6 +97,7 @@ export const packageVersions = { '@backstage/plugin-api-docs': pluginApiDocs, '@backstage/plugin-app-backend': pluginAppBackend, '@backstage/plugin-auth-backend': pluginAuthBackend, + '@backstage/plugin-auth-node': pluginAuthNode, '@backstage/plugin-catalog': pluginCatalog, '@backstage/plugin-catalog-common': pluginCatalogCommon, '@backstage/plugin-catalog-react': pluginCatalogReact, From 4dabec83e774c34f619ef9c117a8fa1981a602d0 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 8 Jul 2022 21:17:15 +0100 Subject: [PATCH 06/25] fixes prettier issues Signed-off-by: Brian Fletcher --- .../templates/default-app/packages/backend/src/index.ts | 2 +- .../default-app/packages/backend/src/plugins/scaffolder.ts | 2 +- .../templates/default-app/packages/backend/src/types.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/create-app/templates/default-app/packages/backend/src/index.ts b/packages/create-app/templates/default-app/packages/backend/src/index.ts index 3d5b4b4f7c..492c05d556 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/index.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/index.ts @@ -30,7 +30,7 @@ import techdocs from './plugins/techdocs'; import search from './plugins/search'; import { PluginEnvironment } from './types'; import { ServerPermissionClient } from '@backstage/plugin-permission-node'; -import { DefaultIdentityClient } from "@backstage/plugin-auth-node"; +import { DefaultIdentityClient } from '@backstage/plugin-auth-node'; function makeCreateEnv(config: Config) { const root = getRootLogger(); diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts index 77e1e94a66..ef46f07870 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts @@ -16,6 +16,6 @@ export default async function createPlugin( database: env.database, reader: env.reader, catalogClient, - identity: env.identity + identity: env.identity, }); } diff --git a/packages/create-app/templates/default-app/packages/backend/src/types.ts b/packages/create-app/templates/default-app/packages/backend/src/types.ts index 9db9adadf0..9cd2c74be3 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/types.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/types.ts @@ -9,7 +9,7 @@ import { } from '@backstage/backend-common'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; -import { IdentityApi } from "@backstage/plugin-auth-node"; +import { IdentityApi } from '@backstage/plugin-auth-node'; export type PluginEnvironment = { logger: Logger; From 36832b084dee44c4746a477756e8d070985d9b3d Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 8 Jul 2022 21:34:03 +0100 Subject: [PATCH 07/25] add @types/express to auth-node Signed-off-by: Brian Fletcher --- plugins/auth-node/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index 03fec84859..cd06bd2035 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -33,6 +33,7 @@ }, "devDependencies": { "@backstage/cli": "^0.18.0-next.1", + "@types/express": "^4.17.6", "lodash": "^4.17.21", "msw": "^0.43.0", "uuid": "^8.0.0" From 2fd4fd0585dee9acec56693caaafa71926991206 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 8 Jul 2022 22:49:16 +0100 Subject: [PATCH 08/25] move types to deps Signed-off-by: Brian Fletcher --- plugins/auth-node/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index cd06bd2035..1e6c50866f 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -26,6 +26,7 @@ "@backstage/backend-common": "^0.14.1-next.1", "@backstage/config": "^1.0.1", "@backstage/errors": "^1.1.0-next.0", + "@types/express": "^4.17.6", "express": "^4.18.1", "jose": "^4.6.0", "node-fetch": "^2.6.7", @@ -33,7 +34,6 @@ }, "devDependencies": { "@backstage/cli": "^0.18.0-next.1", - "@types/express": "^4.17.6", "lodash": "^4.17.21", "msw": "^0.43.0", "uuid": "^8.0.0" From 59d673f8806478bac60256a72f9bc15460bbda05 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Mon, 25 Jul 2022 10:07:15 +0100 Subject: [PATCH 09/25] fix patch with recent changes Signed-off-by: Brian Fletcher --- .../src/service/router.test.ts | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index ddac22cfda..43dc6a8dd5 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -56,6 +56,7 @@ import { } from '@backstage/catalog-model'; import { createRouter, DatabaseTaskStore, TaskBroker } from '../index'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; +import { Logger } from 'winston'; function createDatabase(): PluginDatabaseManager { return DatabaseManager.fromConfig( @@ -79,6 +80,7 @@ describe('createRouter', () => { let app: express.Express; let loggerSpy: jest.SpyInstance; let taskBroker: TaskBroker; + let logger: Logger; const getIdentity = jest.fn(); const rawPayload = Buffer.from( JSON.stringify({ @@ -138,7 +140,7 @@ describe('createRouter', () => { beforeEach(async () => { getIdentity.mockReset(); - const logger = getVoidLogger(); + logger = getVoidLogger(); const databaseTaskStore = await DatabaseTaskStore.create({ database: await createDatabase().getClient(), }); @@ -301,7 +303,7 @@ describe('createRouter', () => { }, ); const router = await createRouter({ - logger: getVoidLogger(), + logger: logger, config: new ConfigReader({}), database: createDatabase(), catalogClient, @@ -313,6 +315,7 @@ describe('createRouter', () => { }); app = express().use(router); }); + it('should not decorate a user when no backstage auth is passed', async () => { const broker = taskBroker.dispatch as jest.Mocked['dispatch']; @@ -338,31 +341,28 @@ describe('createRouter', () => { }), ); }); - }); - it('should emit auditlog containing without user identifier when no backstage auth is passed', async () => { - await request(app) - .post('/v2/tasks') - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, - }); + it('should emit auditlog containing without user identifier', async () => { + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); - expect(loggerSpy).toHaveBeenCalledTimes(1); - expect(loggerSpy).toHaveBeenCalledWith( - 'Scaffolding task for template:default/create-react-app-template', - ); + expect(loggerSpy).toHaveBeenCalledTimes(1); + expect(loggerSpy).toHaveBeenCalledWith( + 'Scaffolding task for template:default/create-react-app-template', + ); + }); }); it('should emit auditlog containing user identifier when backstage auth is passed', async () => { - const mockToken = - 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; - await request(app) .post('/v2/tasks') .set('Authorization', `Bearer ${mockToken}`) From c240ba8f6548a9ff6f169b7961f25d636bdf6183 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Mon, 25 Jul 2022 12:06:34 +0100 Subject: [PATCH 10/25] fix minor typo Signed-off-by: Brian Fletcher --- .changeset/thin-cows-watch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/thin-cows-watch.md b/.changeset/thin-cows-watch.md index 12c96df268..24535109a3 100644 --- a/.changeset/thin-cows-watch.md +++ b/.changeset/thin-cows-watch.md @@ -4,4 +4,4 @@ '@backstage/plugin-scaffolder-backend': patch --- -Uptake the IdentityApi change to use getIdentiy instead of authenticate for retrieving the logged in users identity. +Uptake the IdentityApi change to use `getIdentity` instead of authenticate for retrieving the logged in users identity. From 4c104c80565fa0dd5ab452c577513ad05d2762a6 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Mon, 25 Jul 2022 12:29:27 +0100 Subject: [PATCH 11/25] add backticks to code references in changesets Signed-off-by: Brian Fletcher --- .changeset/dry-tips-build.md | 2 +- .changeset/itchy-kiwis-warn.md | 2 +- .changeset/thin-cows-watch.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.changeset/dry-tips-build.md b/.changeset/dry-tips-build.md index ef3ec700d1..9137acd62b 100644 --- a/.changeset/dry-tips-build.md +++ b/.changeset/dry-tips-build.md @@ -2,4 +2,4 @@ '@backstage/plugin-auth-node': minor --- -IdentityClient is now deprecated. Please migrate to IdentityApi and DefaultIdentityClient instead. The authenticate function on DefaultIdentityClient is also deprecated. Please use getIdentity instead. +`IdentityClient` is now deprecated. Please migrate to `IdentityApi` and `DefaultIdentityClient` instead. The authenticate function on `DefaultIdentityClient` is also deprecated. Please use `getIdentity` instead. diff --git a/.changeset/itchy-kiwis-warn.md b/.changeset/itchy-kiwis-warn.md index d2af94d0ab..e69aeecfa1 100644 --- a/.changeset/itchy-kiwis-warn.md +++ b/.changeset/itchy-kiwis-warn.md @@ -2,4 +2,4 @@ '@backstage/create-app': patch --- -Adds IdentityApi configuration to create-app scaffolding templates. +Adds `IdentityApi` configuration to `create-app` scaffolding templates. diff --git a/.changeset/thin-cows-watch.md b/.changeset/thin-cows-watch.md index 24535109a3..aedac54c8f 100644 --- a/.changeset/thin-cows-watch.md +++ b/.changeset/thin-cows-watch.md @@ -4,4 +4,4 @@ '@backstage/plugin-scaffolder-backend': patch --- -Uptake the IdentityApi change to use `getIdentity` instead of authenticate for retrieving the logged in users identity. +Uptake the `IdentityApi` change to use `getIdentity` instead of `authenticate` for retrieving the logged in users identity. From 7ab1fc429a92aee7bb3ee85aa05b353dbdaf35c8 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Thu, 28 Jul 2022 11:09:40 +0100 Subject: [PATCH 12/25] Fix auth node version Signed-off-by: Brian Fletcher --- plugins/scaffolder-backend/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index f9b1ccd544..28a0d36d80 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -43,7 +43,7 @@ "@backstage/integration": "^1.3.0-next.0", "@backstage/plugin-catalog-backend": "^1.3.1-next.0", "@backstage/plugin-scaffolder-common": "^1.1.2", - "@backstage/plugin-auth-node": "^0.2.3-next.1", + "@backstage/plugin-auth-node": "^0.2.4-next.0", "@backstage/backend-plugin-api": "^0.1.1-next.0", "@backstage/plugin-catalog-node": "^1.0.1-next.0", "@backstage/types": "^1.0.0", From 518e5ddcd1c27e34a5c7f63505e9b8184896c1b9 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Mon, 15 Aug 2022 14:34:26 +0100 Subject: [PATCH 13/25] addressing some code review comments Signed-off-by: Brian Fletcher --- .changeset/itchy-kiwis-warn.md | 47 +++++++++++++++++++ .changeset/thin-cows-watch.md | 1 - .../default-app/packages/backend/src/index.ts | 2 - plugins/auth-node/api-report.md | 7 +-- plugins/auth-node/package.json | 2 - .../auth-node/src/DefaultIdentityClient.ts | 3 +- plugins/auth-node/src/IdentityApi.ts | 5 +- .../scaffolder-backend/src/service/router.ts | 1 + 8 files changed, 52 insertions(+), 16 deletions(-) diff --git a/.changeset/itchy-kiwis-warn.md b/.changeset/itchy-kiwis-warn.md index e69aeecfa1..6f5289d074 100644 --- a/.changeset/itchy-kiwis-warn.md +++ b/.changeset/itchy-kiwis-warn.md @@ -3,3 +3,50 @@ --- Adds `IdentityApi` configuration to `create-app` scaffolding templates. + +To migrate to the new `IdentityApi`, edit the `packages/backend/src/index.ts` adding the following import: + +```typescript +import { DefaultIdentityClient } from '@backstage/plugin-auth-node'; +``` + +Use the factory function to create an `IdentityApi` in the `makeCreateEnv` function and return it from the +function as follows: + +```typescript +function makeCreateEnv(config: Config) { +... + const identity = DefaultIdentityClient.create({ + discovery, + }); +... + + return { + ..., + identity + } +} +``` + +Backend plugins can be upgraded to work with this new `IdentityApi`. + +Add `identity` to the `RouterOptions` type. + +```typescript +export interface RouterOptions { + ... + identity: IdentityApi; +} +``` + +Then you can use the `IdentityApi` from the plugin. + +```typescript +export async function createRouter( + options: RouterOptions, +): Promise { + const { identity } = options; + + const user = identity.getIdentity(req); + ... +``` diff --git a/.changeset/thin-cows-watch.md b/.changeset/thin-cows-watch.md index aedac54c8f..675a27769b 100644 --- a/.changeset/thin-cows-watch.md +++ b/.changeset/thin-cows-watch.md @@ -1,5 +1,4 @@ --- -'@internal/plugin-todo-list-backend': patch '@backstage/plugin-permission-backend': patch '@backstage/plugin-scaffolder-backend': patch --- diff --git a/packages/create-app/templates/default-app/packages/backend/src/index.ts b/packages/create-app/templates/default-app/packages/backend/src/index.ts index 492c05d556..7232a1afe6 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/index.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/index.ts @@ -43,8 +43,6 @@ function makeCreateEnv(config: Config) { const identity = DefaultIdentityClient.create({ discovery, - issuer: undefined, - algorithms: undefined, }); const permissions = ServerPermissionClient.fromConfig(config, { diff --git a/plugins/auth-node/api-report.md b/plugins/auth-node/api-report.md index 0f1c5bb934..902a3ad0b0 100644 --- a/plugins/auth-node/api-report.md +++ b/plugins/auth-node/api-report.md @@ -4,7 +4,6 @@ ```ts import { PluginEndpointDiscovery } from '@backstage/backend-common'; -import { Request as Request_2 } from 'express'; // @public export interface BackstageIdentityResponse extends BackstageSignInResult { @@ -29,7 +28,7 @@ export class DefaultIdentityClient implements IdentityApi { authenticate(token: string | undefined): Promise; static create(options: IdentityClientOptions): DefaultIdentityClient; // (undocumented) - getIdentity(req: Request_2): Promise; + getIdentity(req: Request): Promise; } // @public @@ -39,9 +38,7 @@ export function getBearerTokenFromAuthorizationHeader( // @public export interface IdentityApi { - getIdentity( - req: Request_2, - ): Promise; + getIdentity(req: Request): Promise; } // @public @deprecated diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index 38bb96d214..294bc08889 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -26,8 +26,6 @@ "@backstage/backend-common": "^0.15.0-next.0", "@backstage/config": "^1.0.1", "@backstage/errors": "^1.1.0", - "@types/express": "^4.17.6", - "express": "^4.18.1", "jose": "^4.6.0", "node-fetch": "^2.6.7", "winston": "^3.2.1" diff --git a/plugins/auth-node/src/DefaultIdentityClient.ts b/plugins/auth-node/src/DefaultIdentityClient.ts index 93fdb5542b..6d1394bd55 100644 --- a/plugins/auth-node/src/DefaultIdentityClient.ts +++ b/plugins/auth-node/src/DefaultIdentityClient.ts @@ -27,7 +27,6 @@ import { GetKeyFunction } from 'jose/dist/types/types'; import { BackstageIdentityResponse } from './types'; import { getBearerTokenFromAuthorizationHeader, IdentityApi } from '.'; -import { Request } from 'express'; const CLOCK_MARGIN_S = 10; @@ -78,7 +77,7 @@ export class DefaultIdentityClient implements IdentityApi { async getIdentity(req: Request) { try { return await this.authenticate( - getBearerTokenFromAuthorizationHeader(req.headers.authorization), + getBearerTokenFromAuthorizationHeader(req.headers.get('authorization')), ); } catch (e) { return undefined; diff --git a/plugins/auth-node/src/IdentityApi.ts b/plugins/auth-node/src/IdentityApi.ts index 2387d01f65..7aec553101 100644 --- a/plugins/auth-node/src/IdentityApi.ts +++ b/plugins/auth-node/src/IdentityApi.ts @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Request } from 'express'; import { BackstageIdentityResponse } from './types'; /** @@ -29,7 +28,5 @@ export interface IdentityApi { * Returns a BackstageIdentity (user) matching the token. * The method throws an error if verification fails. */ - getIdentity( - req: Request, - ): Promise; + getIdentity(req: Request): Promise; } diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index ebc9dfc69d..a20d3f6ae3 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -60,6 +60,7 @@ export interface RouterOptions { reader: UrlReader; database: PluginDatabaseManager; catalogClient: CatalogApi; + actions?: TemplateAction[]; taskWorkers?: number; taskBroker?: TaskBroker; From 90223b820bfdde38399b31b07e7162da4c382523 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Tue, 16 Aug 2022 11:58:59 +0100 Subject: [PATCH 14/25] adding express.Request back to interface I tried to use the node-fetch request, however the express handlers appear to be passing down express.Request so its seems to be the right choice. Signed-off-by: Brian Fletcher --- plugins/auth-node/api-report.md | 7 +++++-- plugins/auth-node/package.json | 1 + plugins/auth-node/src/DefaultIdentityClient.ts | 3 ++- plugins/auth-node/src/IdentityApi.ts | 5 ++++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/plugins/auth-node/api-report.md b/plugins/auth-node/api-report.md index 902a3ad0b0..0f1c5bb934 100644 --- a/plugins/auth-node/api-report.md +++ b/plugins/auth-node/api-report.md @@ -4,6 +4,7 @@ ```ts import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { Request as Request_2 } from 'express'; // @public export interface BackstageIdentityResponse extends BackstageSignInResult { @@ -28,7 +29,7 @@ export class DefaultIdentityClient implements IdentityApi { authenticate(token: string | undefined): Promise; static create(options: IdentityClientOptions): DefaultIdentityClient; // (undocumented) - getIdentity(req: Request): Promise; + getIdentity(req: Request_2): Promise; } // @public @@ -38,7 +39,9 @@ export function getBearerTokenFromAuthorizationHeader( // @public export interface IdentityApi { - getIdentity(req: Request): Promise; + getIdentity( + req: Request_2, + ): Promise; } // @public @deprecated diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index 294bc08889..2ac9a17813 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -26,6 +26,7 @@ "@backstage/backend-common": "^0.15.0-next.0", "@backstage/config": "^1.0.1", "@backstage/errors": "^1.1.0", + "express": "^4.17.13", "jose": "^4.6.0", "node-fetch": "^2.6.7", "winston": "^3.2.1" diff --git a/plugins/auth-node/src/DefaultIdentityClient.ts b/plugins/auth-node/src/DefaultIdentityClient.ts index 6d1394bd55..8c09157c5c 100644 --- a/plugins/auth-node/src/DefaultIdentityClient.ts +++ b/plugins/auth-node/src/DefaultIdentityClient.ts @@ -24,6 +24,7 @@ import { jwtVerify, } from 'jose'; import { GetKeyFunction } from 'jose/dist/types/types'; +import { Request } from 'express'; import { BackstageIdentityResponse } from './types'; import { getBearerTokenFromAuthorizationHeader, IdentityApi } from '.'; @@ -77,7 +78,7 @@ export class DefaultIdentityClient implements IdentityApi { async getIdentity(req: Request) { try { return await this.authenticate( - getBearerTokenFromAuthorizationHeader(req.headers.get('authorization')), + getBearerTokenFromAuthorizationHeader(req.headers.authorization), ); } catch (e) { return undefined; diff --git a/plugins/auth-node/src/IdentityApi.ts b/plugins/auth-node/src/IdentityApi.ts index 7aec553101..32a1040d18 100644 --- a/plugins/auth-node/src/IdentityApi.ts +++ b/plugins/auth-node/src/IdentityApi.ts @@ -14,6 +14,7 @@ * limitations under the License. */ import { BackstageIdentityResponse } from './types'; +import { Request } from 'express'; /** * An identity client api to authenticate Backstage @@ -28,5 +29,7 @@ export interface IdentityApi { * Returns a BackstageIdentity (user) matching the token. * The method throws an error if verification fails. */ - getIdentity(req: Request): Promise; + getIdentity( + req: Request, + ): Promise; } From 782acbafdd81e91c7b3701ddbc540adebb91e9a5 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Tue, 16 Aug 2022 13:01:06 +0100 Subject: [PATCH 15/25] fix express package Signed-off-by: Brian Fletcher --- plugins/auth-node/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index 2ac9a17813..4ae5c439f0 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -26,7 +26,7 @@ "@backstage/backend-common": "^0.15.0-next.0", "@backstage/config": "^1.0.1", "@backstage/errors": "^1.1.0", - "express": "^4.17.13", + "@types/express": "*", "jose": "^4.6.0", "node-fetch": "^2.6.7", "winston": "^3.2.1" From b16ebc2a1b8e750c6f981fe5538eb75531226ac6 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Tue, 16 Aug 2022 13:26:14 +0100 Subject: [PATCH 16/25] add express dep Signed-off-by: Brian Fletcher --- plugins/auth-node/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index 4ae5c439f0..f7e73418ae 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -26,6 +26,7 @@ "@backstage/backend-common": "^0.15.0-next.0", "@backstage/config": "^1.0.1", "@backstage/errors": "^1.1.0", + "express": "^4.17.1", "@types/express": "*", "jose": "^4.6.0", "node-fetch": "^2.6.7", From d102f14f9eb3d34414d12277b2b05853df5ff802 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Tue, 23 Aug 2022 15:49:24 +0100 Subject: [PATCH 17/25] fix some review comments Signed-off-by: Brian Fletcher --- .changeset/dry-tips-build.md | 2 +- .changeset/itchy-kiwis-warn.md | 5 +- packages/backend/src/index.ts | 2 - packages/backend/src/plugins/permission.ts | 6 +- .../default-app/packages/backend/src/index.ts | 1 - plugins/auth-node/api-report.md | 13 +- .../src/DefaultIdentityClient.test.ts | 48 + .../auth-node/src/DefaultIdentityClient.ts | 17 +- plugins/auth-node/src/IdentityApi.ts | 8 +- plugins/auth-node/src/index.ts | 1 + plugins/auth-node/src/types.ts | 11 + .../src/service/router.ts | 2 +- .../src/service/router.test.ts | 2 +- .../permission-backend/src/service/router.ts | 2 +- plugins/scaffolder-backend/api-report.md | 4 +- .../src/service/router.test.ts | 1571 ++++++++++++----- .../scaffolder-backend/src/service/router.ts | 105 +- 17 files changed, 1293 insertions(+), 507 deletions(-) diff --git a/.changeset/dry-tips-build.md b/.changeset/dry-tips-build.md index 9137acd62b..b800161fc8 100644 --- a/.changeset/dry-tips-build.md +++ b/.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. diff --git a/.changeset/itchy-kiwis-warn.md b/.changeset/itchy-kiwis-warn.md index 6f5289d074..28c4c875ff 100644 --- a/.changeset/itchy-kiwis-warn.md +++ b/.changeset/itchy-kiwis-warn.md @@ -47,6 +47,7 @@ export async function createRouter( ): Promise { const { identity } = options; - const user = identity.getIdentity(req); - ... + router.get('/user', async (req, res) => { + const user = await identity.getIdentity({ request: req }); + ... ``` diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 00fc2d529e..4fcbcab826 100644 --- a/packages/backend/src/index.ts +++ b/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}`); diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index ab647b7f95..0d3dcc588d 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/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, }); } diff --git a/packages/create-app/templates/default-app/packages/backend/src/index.ts b/packages/create-app/templates/default-app/packages/backend/src/index.ts index 44f4caf777..11422e00dd 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/index.ts +++ b/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, diff --git a/plugins/auth-node/api-report.md b/plugins/auth-node/api-report.md index 0f1c5bb934..c6d4b4e3f8 100644 --- a/plugins/auth-node/api-report.md +++ b/plugins/auth-node/api-report.md @@ -29,7 +29,11 @@ export class DefaultIdentityClient implements IdentityApi { authenticate(token: string | undefined): Promise; static create(options: IdentityClientOptions): DefaultIdentityClient; // (undocumented) - getIdentity(req: Request_2): Promise; + getIdentity({ + request, + }: IdentityApiGetIdentityRequest): Promise< + BackstageIdentityResponse | undefined + >; } // @public @@ -40,10 +44,15 @@ export function getBearerTokenFromAuthorizationHeader( // @public export interface IdentityApi { getIdentity( - req: Request_2, + options: IdentityApiGetIdentityRequest, ): Promise; } +// @public +export type IdentityApiGetIdentityRequest = { + request: Request_2; +}; + // @public @deprecated export class IdentityClient { // @deprecated diff --git a/plugins/auth-node/src/DefaultIdentityClient.test.ts b/plugins/auth-node/src/DefaultIdentityClient.test.ts index 27aceaa780..13d9c5a17b 100644 --- a/plugins/auth-node/src/DefaultIdentityClient.test.ts +++ b/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 { 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); + }); + }); }); diff --git a/plugins/auth-node/src/DefaultIdentityClient.ts b/plugins/auth-node/src/DefaultIdentityClient.ts index 8c09157c5c..167b0c2451 100644 --- a/plugins/auth-node/src/DefaultIdentityClient.ts +++ b/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), + ); } /** diff --git a/plugins/auth-node/src/IdentityApi.ts b/plugins/auth-node/src/IdentityApi.ts index 32a1040d18..86171fa3ab 100644 --- a/plugins/auth-node/src/IdentityApi.ts +++ b/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, + options: IdentityApiGetIdentityRequest, ): Promise; } diff --git a/plugins/auth-node/src/index.ts b/plugins/auth-node/src/index.ts index a17678a038..a70f667adf 100644 --- a/plugins/auth-node/src/index.ts +++ b/plugins/auth-node/src/index.ts @@ -29,4 +29,5 @@ export type { BackstageIdentityResponse, BackstageSignInResult, BackstageUserIdentity, + IdentityApiGetIdentityRequest, } from './types'; diff --git a/plugins/auth-node/src/types.ts b/plugins/auth-node/src/types.ts index 0d3e015beb..9aec98a372 100644 --- a/plugins/auth-node/src/types.ts +++ b/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; +}; + /** * Response object containing the {@link BackstageUserIdentity} and the token * from the authentication provider. diff --git a/plugins/example-todo-list-backend/src/service/router.ts b/plugins/example-todo-list-backend/src/service/router.ts index 58ce730d24..2973b6eab6 100644 --- a/plugins/example-todo-list-backend/src/service/router.ts +++ b/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)) { diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index 7668245679..7278835c5c 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/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) { diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 217c135ac3..684cdebefd 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -188,7 +188,7 @@ export async function createRouter( req: Request, res: Response, ) => { - const user = await identity.getIdentity(req); + const user = await identity.getIdentity({ request: req }); const parseResult = evaluatePermissionRequestBatchSchema.safeParse( req.body, diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index c49db66aa4..c692cee9aa 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/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) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 9e2c7b9265..1c26ee57fd 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -40,6 +40,10 @@ import { } from '@backstage/catalog-model'; import { createRouter, DatabaseTaskStore, TaskBroker } from '../index'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; +import { + IdentityApiGetIdentityRequest, + BackstageIdentityResponse, +} from '@backstage/plugin-auth-node'; const mockAccess = jest.fn(); @@ -74,6 +78,8 @@ const mockUrlReader = UrlReaders.default({ config: new ConfigReader({}), }); +const getIdentity = jest.fn(); + describe('createRouter', () => { let app: express.Express; let loggerSpy: jest.SpyInstance; @@ -125,372 +131,511 @@ describe('createRouter', () => { }, }; - beforeEach(async () => { - const logger = getVoidLogger(); - const databaseTaskStore = await DatabaseTaskStore.create({ - database: createDatabase(), - }); - taskBroker = new StorageTaskBroker(databaseTaskStore, logger); - - jest.spyOn(taskBroker, 'dispatch'); - jest.spyOn(taskBroker, 'get'); - jest.spyOn(taskBroker, 'list'); - jest.spyOn(taskBroker, 'event$'); - loggerSpy = jest.spyOn(logger, 'info'); - - const router = await createRouter({ - logger: logger, - config: new ConfigReader({}), - database: createDatabase(), - catalogClient, - reader: mockUrlReader, - taskBroker, - }); - app = express().use(router); - - jest - .spyOn(catalogClient, 'getEntityByRef') - .mockImplementation(async ref => { - const { kind } = parseEntityRef(ref); - - if (kind === 'template') { - return mockTemplate; - } - - if (kind === 'user') { - return mockUser; - } - throw new Error(`no mock found for kind: ${kind}`); + describe('not providing an identity api', () => { + beforeEach(async () => { + const logger = getVoidLogger(); + const databaseTaskStore = await DatabaseTaskStore.create({ + database: createDatabase(), }); - }); + taskBroker = new StorageTaskBroker(databaseTaskStore, logger); - afterEach(() => { - jest.resetAllMocks(); - }); + jest.spyOn(taskBroker, 'dispatch'); + jest.spyOn(taskBroker, 'get'); + jest.spyOn(taskBroker, 'list'); + jest.spyOn(taskBroker, 'event$'); + loggerSpy = jest.spyOn(logger, 'info'); - describe('GET /v2/actions', () => { - it('lists available actions', async () => { - const response = await request(app).get('/v2/actions').send(); - expect(response.status).toEqual(200); - expect(response.body[0].id).toBeDefined(); - expect(response.body.length).toBeGreaterThan(8); - }); - }); + const router = await createRouter({ + logger: logger, + config: new ConfigReader({}), + database: createDatabase(), + catalogClient, + reader: mockUrlReader, + taskBroker, + }); + app = express().use(router); - describe('POST /v2/tasks', () => { - it('rejects template values which do not match the template schema definition', async () => { - const response = await request(app) - .post('/v2/tasks') - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - storePath: 'https://github.com/backstage/backstage', - }, + jest + .spyOn(catalogClient, 'getEntityByRef') + .mockImplementation(async ref => { + const { kind } = parseEntityRef(ref); + + if (kind === 'template') { + return mockTemplate; + } + + if (kind === 'user') { + return mockUser; + } + throw new Error(`no mock found for kind: ${kind}`); }); - - expect(response.status).toEqual(400); }); - it('return the template id', async () => { - const broker = taskBroker.dispatch as jest.Mocked['dispatch']; - broker.mockResolvedValue({ - taskId: 'a-random-id', + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('GET /v2/actions', () => { + it('lists available actions', async () => { + const response = await request(app).get('/v2/actions').send(); + expect(response.status).toEqual(200); + expect(response.body[0].id).toBeDefined(); + expect(response.body.length).toBeGreaterThan(8); + }); + }); + + describe('POST /v2/tasks', () => { + it('rejects template values which do not match the template schema definition', async () => { + const response = await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + storePath: 'https://github.com/backstage/backstage', + }, + }); + + expect(response.status).toEqual(400); }); - const response = await request(app) - .post('/v2/tasks') - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, + it('return the template id', async () => { + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; + broker.mockResolvedValue({ + taskId: 'a-random-id', }); - expect(response.body.id).toBe('a-random-id'); - expect(response.status).toEqual(201); - }); - - it('should call the broker with a correct spec', async () => { - const broker = taskBroker.dispatch as jest.Mocked['dispatch']; - const mockToken = - 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; - - await request(app) - .post('/v2/tasks') - .set('Authorization', `Bearer ${mockToken}`) - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, - }); - expect(broker).toHaveBeenCalledWith( - expect.objectContaining({ - createdBy: 'user:default/guest', - secrets: { - backstageToken: mockToken, - }, - - spec: { - apiVersion: mockTemplate.apiVersion, - steps: mockTemplate.spec.steps.map((step, index) => ({ - ...step, - id: step.id ?? `step-${index + 1}`, - name: step.name ?? step.action, - })), - output: mockTemplate.spec.output ?? {}, - parameters: { + const response = await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { required: 'required-value', }, - user: { - entity: mockUser, - ref: 'user:default/guest', - }, - templateInfo: { - entityRef: stringifyEntityRef({ - kind: 'Template', - namespace: 'Default', - name: mockTemplate.metadata?.name, - }), - baseUrl: 'https://dev.azure.com', - entity: { - metadata: mockTemplate.metadata, - }, - }, - }, - }), - ); - }); + }); - it('should not throw when an invalid authorization header is passed', async () => { - const broker = taskBroker.dispatch as jest.Mocked['dispatch']; - const mockToken = 'blob.eyJzdWIiOiIiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; + expect(response.body.id).toBe('a-random-id'); + expect(response.status).toEqual(201); + }); - await request(app) - .post('/v2/tasks') - .set('Authorization', `Bearer ${mockToken}`) - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, - }); - expect(broker).toHaveBeenCalledWith( - expect.objectContaining({ - createdBy: undefined, - secrets: { - backstageToken: undefined, - }, + it('should call the broker with a correct spec', async () => { + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; + const mockToken = + 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; - spec: { - apiVersion: mockTemplate.apiVersion, - steps: mockTemplate.spec.steps.map((step, index) => ({ - ...step, - id: step.id ?? `step-${index + 1}`, - name: step.name ?? step.action, - })), - output: mockTemplate.spec.output ?? {}, - parameters: { + await request(app) + .post('/v2/tasks') + .set('Authorization', `Bearer ${mockToken}`) + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { required: 'required-value', }, - user: { - entity: undefined, - ref: undefined, + }); + expect(broker).toHaveBeenCalledWith( + expect.objectContaining({ + createdBy: 'user:default/guest', + secrets: { + backstageToken: mockToken, }, - templateInfo: { - entityRef: stringifyEntityRef({ - kind: 'Template', - namespace: 'Default', - name: mockTemplate.metadata?.name, - }), - baseUrl: 'https://dev.azure.com', - entity: { - metadata: mockTemplate.metadata, + + spec: { + apiVersion: mockTemplate.apiVersion, + steps: mockTemplate.spec.steps.map((step, index) => ({ + ...step, + id: step.id ?? `step-${index + 1}`, + name: step.name ?? step.action, + })), + output: mockTemplate.spec.output ?? {}, + parameters: { + required: 'required-value', + }, + user: { + entity: mockUser, + ref: 'user:default/guest', + }, + templateInfo: { + entityRef: stringifyEntityRef({ + kind: 'Template', + namespace: 'Default', + name: mockTemplate.metadata?.name, + }), + baseUrl: 'https://dev.azure.com', + entity: { + metadata: mockTemplate.metadata, + }, }, }, - }, - }), - ); + }), + ); + }); + + it('should not throw when an invalid authorization header is passed', async () => { + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; + const mockToken = 'blob.eyJzdWIiOiIiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; + + await request(app) + .post('/v2/tasks') + .set('Authorization', `Bearer ${mockToken}`) + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + expect(broker).toHaveBeenCalledWith( + expect.objectContaining({ + createdBy: undefined, + secrets: { + backstageToken: undefined, + }, + + spec: { + apiVersion: mockTemplate.apiVersion, + steps: mockTemplate.spec.steps.map((step, index) => ({ + ...step, + id: step.id ?? `step-${index + 1}`, + name: step.name ?? step.action, + })), + output: mockTemplate.spec.output ?? {}, + parameters: { + required: 'required-value', + }, + user: { + entity: undefined, + ref: undefined, + }, + templateInfo: { + entityRef: stringifyEntityRef({ + kind: 'Template', + namespace: 'Default', + name: mockTemplate.metadata?.name, + }), + baseUrl: 'https://dev.azure.com', + entity: { + metadata: mockTemplate.metadata, + }, + }, + }, + }), + ); + }); + + it('should not decorate a user when no backstage auth is passed', async () => { + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; + + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + + expect(broker).toHaveBeenCalledWith( + expect.objectContaining({ + createdBy: undefined, + spec: expect.objectContaining({ + user: { entity: undefined, ref: undefined }, + }), + }), + ); + }); + + it('should emit auditlog containing without user identifier when no backstage auth is passed', async () => { + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + + expect(loggerSpy).toHaveBeenCalledTimes(1); + expect(loggerSpy).toHaveBeenCalledWith( + 'Scaffolding task for template:default/create-react-app-template', + ); + }); + + it('should emit auditlog containing user identifier when backstage auth is passed', async () => { + const mockToken = + 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; + + await request(app) + .post('/v2/tasks') + .set('Authorization', `Bearer ${mockToken}`) + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + + expect(loggerSpy).toHaveBeenCalledTimes(1); + expect(loggerSpy).toHaveBeenCalledWith( + 'Scaffolding task for template:default/create-react-app-template created by user:default/guest', + ); + }); }); - it('should not decorate a user when no backstage auth is passed', async () => { - const broker = taskBroker.dispatch as jest.Mocked['dispatch']; - - await request(app) - .post('/v2/tasks') - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, + describe('GET /v2/tasks', () => { + it('return all tasks', async () => { + ( + taskBroker.list as jest.Mocked>['list'] + ).mockResolvedValue({ + tasks: [ + { + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + createdBy: '', + }, + ], }); - expect(broker).toHaveBeenCalledWith( - expect.objectContaining({ + const response = await request(app).get(`/v2/tasks`); + expect(taskBroker.list).toBeCalledWith({ createdBy: undefined, - spec: expect.objectContaining({ - user: { entity: undefined, ref: undefined }, - }), - }), - ); - }); + }); + expect(response.status).toEqual(200); + expect(response.body).toStrictEqual({ + tasks: [ + { + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + createdBy: '', + }, + ], + }); + }); - it('should emit auditlog containing without user identifier when no backstage auth is passed', async () => { - await request(app) - .post('/v2/tasks') - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, + it('return filtered tasks', async () => { + ( + taskBroker.list as jest.Mocked>['list'] + ).mockResolvedValue({ + tasks: [ + { + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + createdBy: 'user:default/foo', + }, + ], }); - expect(loggerSpy).toHaveBeenCalledTimes(1); - expect(loggerSpy).toHaveBeenCalledWith( - 'Scaffolding task for template:default/create-react-app-template', - ); - }); - - it('should emit auditlog containing user identifier when backstage auth is passed', async () => { - const mockToken = - 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; - - await request(app) - .post('/v2/tasks') - .set('Authorization', `Bearer ${mockToken}`) - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, + const response = await request(app).get( + `/v2/tasks?createdBy=user:default/foo`, + ); + expect(taskBroker.list).toBeCalledWith({ + createdBy: 'user:default/foo', }); - expect(loggerSpy).toHaveBeenCalledTimes(1); - expect(loggerSpy).toHaveBeenCalledWith( - 'Scaffolding task for template:default/create-react-app-template created by user:default/guest', - ); - }); - }); - - describe('GET /v2/tasks', () => { - it('return all tasks', async () => { - ( - taskBroker.list as jest.Mocked>['list'] - ).mockResolvedValue({ - tasks: [ - { - id: 'a-random-id', - spec: {} as any, - status: 'completed', - createdAt: '', - createdBy: '', - }, - ], - }); - - const response = await request(app).get(`/v2/tasks`); - expect(taskBroker.list).toBeCalledWith({ - createdBy: undefined, - }); - expect(response.status).toEqual(200); - expect(response.body).toStrictEqual({ - tasks: [ - { - id: 'a-random-id', - spec: {} as any, - status: 'completed', - createdAt: '', - createdBy: '', - }, - ], + expect(response.status).toEqual(200); + expect(response.body).toStrictEqual({ + tasks: [ + { + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + createdBy: 'user:default/foo', + }, + ], + }); }); }); - it('return filtered tasks', async () => { - ( - taskBroker.list as jest.Mocked>['list'] - ).mockResolvedValue({ - tasks: [ - { - id: 'a-random-id', - spec: {} as any, - status: 'completed', - createdAt: '', - createdBy: 'user:default/foo', - }, - ], - }); + describe('GET /v2/tasks/:taskId', () => { + it('does not divulge secrets', async () => { + (taskBroker.get as jest.Mocked['get']).mockResolvedValue({ + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + secrets: { backstageToken: 'secret' }, + createdBy: '', + }); - const response = await request(app).get( - `/v2/tasks?createdBy=user:default/foo`, - ); - expect(taskBroker.list).toBeCalledWith({ - createdBy: 'user:default/foo', - }); - - expect(response.status).toEqual(200); - expect(response.body).toStrictEqual({ - tasks: [ - { - id: 'a-random-id', - spec: {} as any, - status: 'completed', - createdAt: '', - createdBy: 'user:default/foo', - }, - ], + const response = await request(app).get(`/v2/tasks/a-random-id`); + expect(response.status).toEqual(200); + expect(response.body.status).toBe('completed'); + expect(response.body.secrets).toBeUndefined(); }); }); - }); - describe('GET /v2/tasks/:taskId', () => { - it('does not divulge secrets', async () => { - (taskBroker.get as jest.Mocked['get']).mockResolvedValue({ - id: 'a-random-id', - spec: {} as any, - status: 'completed', - createdAt: '', - secrets: { backstageToken: 'secret' }, - createdBy: '', + describe('GET /v2/tasks/:taskId/eventstream', () => { + it('should return log messages', async () => { + let subscriber: ZenObservable.SubscriptionObserver; + ( + taskBroker.event$ as jest.Mocked['event$'] + ).mockImplementation(({ taskId }) => { + return new ObservableImpl(observer => { + subscriber = observer; + setImmediate(() => { + observer.next({ + events: [ + { + id: 0, + taskId, + type: 'log', + createdAt: '', + body: { message: 'My log message' }, + }, + ], + }); + observer.next({ + events: [ + { + id: 1, + taskId, + type: 'completion', + createdAt: '', + body: { message: 'Finished!' }, + }, + ], + }); + }); + }); + // emit after this function returned + }); + + let statusCode: any = undefined; + let headers: any = {}; + const responseDataFn = jest.fn(); + + const req = request(app) + .get('/v2/tasks/a-random-id/eventstream') + .set('accept', 'text/event-stream') + .parse((res, _) => { + ({ statusCode, headers } = res as any); + + res.on('data', chunk => { + responseDataFn(chunk.toString()); + + // the server expects the client to abort the request + if (chunk.includes('completion')) { + req.abort(); + } + }); + }); + + // wait for the request to finish + await req.catch(() => { + // ignore 'aborted' error + }); + + expect(statusCode).toBe(200); + expect(headers['content-type']).toBe('text/event-stream'); + expect(responseDataFn).toBeCalledTimes(2); + expect(responseDataFn).toBeCalledWith(`event: log +data: {"id":0,"taskId":"a-random-id","type":"log","createdAt":"","body":{"message":"My log message"}} + +`); + expect(responseDataFn).toBeCalledWith(`event: completion +data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{"message":"Finished!"}} + +`); + + expect(taskBroker.event$).toBeCalledTimes(1); + expect(taskBroker.event$).toBeCalledWith({ taskId: 'a-random-id' }); + expect(subscriber!.closed).toBe(true); }); - const response = await request(app).get(`/v2/tasks/a-random-id`); - expect(response.status).toEqual(200); - expect(response.body.status).toBe('completed'); - expect(response.body.secrets).toBeUndefined(); - }); - }); + it('should return log messages with after query', async () => { + let subscriber: ZenObservable.SubscriptionObserver; + ( + taskBroker.event$ as jest.Mocked['event$'] + ).mockImplementation(({ taskId }) => { + return new ObservableImpl(observer => { + subscriber = observer; + setImmediate(() => { + observer.next({ + events: [ + { + id: 1, + taskId, + type: 'completion', + createdAt: '', + body: { message: 'Finished!' }, + }, + ], + }); + }); + }); + }); - describe('GET /v2/tasks/:taskId/eventstream', () => { - it('should return log messages', async () => { - let subscriber: ZenObservable.SubscriptionObserver; - ( - taskBroker.event$ as jest.Mocked['event$'] - ).mockImplementation(({ taskId }) => { - return new ObservableImpl(observer => { - subscriber = observer; - setImmediate(() => { + let statusCode: any = undefined; + let headers: any = {}; + + const req = request(app) + .get('/v2/tasks/a-random-id/eventstream') + .query({ after: 10 }) + .set('accept', 'text/event-stream') + .parse((res, _) => { + ({ statusCode, headers } = res as any); + + res.on('data', () => { + // close immediately + req.abort(); + }); + }); + + // wait for the request to finish + await req.catch(() => { + // ignore 'aborted' error + }); + + expect(statusCode).toBe(200); + expect(headers['content-type']).toBe('text/event-stream'); + + expect(taskBroker.event$).toBeCalledTimes(1); + expect(taskBroker.event$).toBeCalledWith({ + taskId: 'a-random-id', + after: 10, + }); + + expect(subscriber!.closed).toBe(true); + }); + }); + + describe('GET /v2/tasks/:taskId/events', () => { + it('should return log messages', async () => { + let subscriber: ZenObservable.SubscriptionObserver; + ( + taskBroker.event$ as jest.Mocked['event$'] + ).mockImplementation(({ taskId }) => { + return new ObservableImpl(observer => { + subscriber = observer; observer.next({ events: [ { @@ -500,10 +645,6 @@ describe('createRouter', () => { createdAt: '', body: { message: 'My log message' }, }, - ], - }); - observer.next({ - events: [ { id: 1, taskId, @@ -515,61 +656,614 @@ describe('createRouter', () => { }); }); }); - // emit after this function returned + + const response = await request(app).get('/v2/tasks/a-random-id/events'); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([ + { + id: 0, + taskId: 'a-random-id', + type: 'log', + createdAt: '', + body: { message: 'My log message' }, + }, + { + id: 1, + taskId: 'a-random-id', + type: 'completion', + createdAt: '', + body: { message: 'Finished!' }, + }, + ]); + + expect(taskBroker.event$).toBeCalledTimes(1); + expect(taskBroker.event$).toBeCalledWith({ taskId: 'a-random-id' }); + expect(subscriber!.closed).toBe(true); }); - let statusCode: any = undefined; - let headers: any = {}; - const responseDataFn = jest.fn(); - - const req = request(app) - .get('/v2/tasks/a-random-id/eventstream') - .set('accept', 'text/event-stream') - .parse((res, _) => { - ({ statusCode, headers } = res as any); - - res.on('data', chunk => { - responseDataFn(chunk.toString()); - - // the server expects the client to abort the request - if (chunk.includes('completion')) { - req.abort(); - } + it('should return log messages with after query', async () => { + let subscriber: ZenObservable.SubscriptionObserver; + ( + taskBroker.event$ as jest.Mocked['event$'] + ).mockImplementation(() => { + return new ObservableImpl(observer => { + subscriber = observer; + observer.next({ events: [] }); }); }); - // wait for the request to finish - await req.catch(() => { - // ignore 'aborted' error + const response = await request(app) + .get('/v2/tasks/a-random-id/events') + .query({ after: 10 }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([]); + + expect(taskBroker.event$).toBeCalledTimes(1); + expect(taskBroker.event$).toBeCalledWith({ + taskId: 'a-random-id', + after: 10, + }); + expect(subscriber!.closed).toBe(true); + }); + }); + }); + + describe('providing an identity api', () => { + beforeEach(async () => { + const logger = getVoidLogger(); + const databaseTaskStore = await DatabaseTaskStore.create({ + database: createDatabase(), + }); + taskBroker = new StorageTaskBroker(databaseTaskStore, logger); + + jest.spyOn(taskBroker, 'dispatch'); + jest.spyOn(taskBroker, 'get'); + jest.spyOn(taskBroker, 'list'); + jest.spyOn(taskBroker, 'event$'); + loggerSpy = jest.spyOn(logger, 'info'); + + getIdentity.mockImplementation( + async ({ + request: _request, + }: IdentityApiGetIdentityRequest): Promise< + BackstageIdentityResponse | undefined + > => { + return { + identity: { + userEntityRef: 'user:default/guest', + ownershipEntityRefs: [], + type: 'user', + }, + token: 'token', + }; + }, + ); + + const router = await createRouter({ + logger: logger, + config: new ConfigReader({}), + database: createDatabase(), + catalogClient, + reader: mockUrlReader, + taskBroker, + identity: { getIdentity }, + }); + app = express().use(router); + + jest + .spyOn(catalogClient, 'getEntityByRef') + .mockImplementation(async ref => { + const { kind } = parseEntityRef(ref); + + if (kind === 'template') { + return mockTemplate; + } + + if (kind === 'user') { + return mockUser; + } + throw new Error(`no mock found for kind: ${kind}`); + }); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('GET /v2/actions', () => { + it('lists available actions', async () => { + const response = await request(app).get('/v2/actions').send(); + expect(response.status).toEqual(200); + expect(response.body[0].id).toBeDefined(); + expect(response.body.length).toBeGreaterThan(8); + }); + }); + + describe('POST /v2/tasks', () => { + it('rejects template values which do not match the template schema definition', async () => { + const response = await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + storePath: 'https://github.com/backstage/backstage', + }, + }); + + expect(response.status).toEqual(400); }); - expect(statusCode).toBe(200); - expect(headers['content-type']).toBe('text/event-stream'); - expect(responseDataFn).toBeCalledTimes(2); - expect(responseDataFn).toBeCalledWith(`event: log + it('return the template id', async () => { + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; + broker.mockResolvedValue({ + taskId: 'a-random-id', + }); + + const response = await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + + expect(response.body.id).toBe('a-random-id'); + expect(response.status).toEqual(201); + }); + + it('should call the broker with a correct spec', async () => { + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; + + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + expect(broker).toHaveBeenCalledWith( + expect.objectContaining({ + createdBy: 'user:default/guest', + secrets: { + backstageToken: 'token', + }, + + spec: { + apiVersion: mockTemplate.apiVersion, + steps: mockTemplate.spec.steps.map((step, index) => ({ + ...step, + id: step.id ?? `step-${index + 1}`, + name: step.name ?? step.action, + })), + output: mockTemplate.spec.output ?? {}, + parameters: { + required: 'required-value', + }, + user: { + entity: mockUser, + ref: 'user:default/guest', + }, + templateInfo: { + entityRef: stringifyEntityRef({ + kind: 'Template', + namespace: 'Default', + name: mockTemplate.metadata?.name, + }), + baseUrl: 'https://dev.azure.com', + entity: { + metadata: mockTemplate.metadata, + }, + }, + }, + }), + ); + }); + + describe('when the identity api throws an error', () => { + beforeEach(() => { + getIdentity.mockImplementation( + async ({ + request: _request, + }: IdentityApiGetIdentityRequest): Promise< + BackstageIdentityResponse | undefined + > => { + throw new Error('whoops!'); + }, + ); + }); + it('should not throw', async () => { + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; + + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + expect(broker).toHaveBeenCalledWith( + expect.objectContaining({ + createdBy: undefined, + secrets: { + backstageToken: undefined, + }, + + spec: { + apiVersion: mockTemplate.apiVersion, + steps: mockTemplate.spec.steps.map((step, index) => ({ + ...step, + id: step.id ?? `step-${index + 1}`, + name: step.name ?? step.action, + })), + output: mockTemplate.spec.output ?? {}, + parameters: { + required: 'required-value', + }, + user: { + entity: undefined, + ref: undefined, + }, + templateInfo: { + entityRef: stringifyEntityRef({ + kind: 'Template', + namespace: 'Default', + name: mockTemplate.metadata?.name, + }), + baseUrl: 'https://dev.azure.com', + entity: { + metadata: mockTemplate.metadata, + }, + }, + }, + }), + ); + }); + }); + + describe('no auth is provided', () => { + beforeEach(() => { + getIdentity.mockImplementation( + async ({ + request: _request, + }: IdentityApiGetIdentityRequest): Promise< + BackstageIdentityResponse | undefined + > => { + return undefined; + }, + ); + }); + + it('should not decorate a user when no backstage auth is passed', async () => { + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; + + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + + expect(broker).toHaveBeenCalledWith( + expect.objectContaining({ + createdBy: undefined, + spec: expect.objectContaining({ + user: { entity: undefined, ref: undefined }, + }), + }), + ); + }); + + it('should emit auditlog containing without user identifier when no backstage auth is passed', async () => { + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + + expect(loggerSpy).toHaveBeenCalledTimes(1); + expect(loggerSpy).toHaveBeenCalledWith( + 'Scaffolding task for template:default/create-react-app-template', + ); + }); + }); + + it('should emit auditlog containing user identifier when backstage auth is passed', async () => { + const mockToken = + 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; + + await request(app) + .post('/v2/tasks') + .set('Authorization', `Bearer ${mockToken}`) + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + + expect(loggerSpy).toHaveBeenCalledTimes(1); + expect(loggerSpy).toHaveBeenCalledWith( + 'Scaffolding task for template:default/create-react-app-template created by user:default/guest', + ); + }); + }); + + describe('GET /v2/tasks', () => { + it('return all tasks', async () => { + ( + taskBroker.list as jest.Mocked>['list'] + ).mockResolvedValue({ + tasks: [ + { + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + createdBy: '', + }, + ], + }); + + const response = await request(app).get(`/v2/tasks`); + expect(taskBroker.list).toBeCalledWith({ + createdBy: undefined, + }); + expect(response.status).toEqual(200); + expect(response.body).toStrictEqual({ + tasks: [ + { + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + createdBy: '', + }, + ], + }); + }); + + it('return filtered tasks', async () => { + ( + taskBroker.list as jest.Mocked>['list'] + ).mockResolvedValue({ + tasks: [ + { + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + createdBy: 'user:default/foo', + }, + ], + }); + + const response = await request(app).get( + `/v2/tasks?createdBy=user:default/foo`, + ); + expect(taskBroker.list).toBeCalledWith({ + createdBy: 'user:default/foo', + }); + + expect(response.status).toEqual(200); + expect(response.body).toStrictEqual({ + tasks: [ + { + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + createdBy: 'user:default/foo', + }, + ], + }); + }); + }); + + describe('GET /v2/tasks/:taskId', () => { + it('does not divulge secrets', async () => { + (taskBroker.get as jest.Mocked['get']).mockResolvedValue({ + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + secrets: { backstageToken: 'secret' }, + createdBy: '', + }); + + const response = await request(app).get(`/v2/tasks/a-random-id`); + expect(response.status).toEqual(200); + expect(response.body.status).toBe('completed'); + expect(response.body.secrets).toBeUndefined(); + }); + }); + + describe('GET /v2/tasks/:taskId/eventstream', () => { + it('should return log messages', async () => { + let subscriber: ZenObservable.SubscriptionObserver; + ( + taskBroker.event$ as jest.Mocked['event$'] + ).mockImplementation(({ taskId }) => { + return new ObservableImpl(observer => { + subscriber = observer; + setImmediate(() => { + observer.next({ + events: [ + { + id: 0, + taskId, + type: 'log', + createdAt: '', + body: { message: 'My log message' }, + }, + ], + }); + observer.next({ + events: [ + { + id: 1, + taskId, + type: 'completion', + createdAt: '', + body: { message: 'Finished!' }, + }, + ], + }); + }); + }); + // emit after this function returned + }); + + let statusCode: any = undefined; + let headers: any = {}; + const responseDataFn = jest.fn(); + + const req = request(app) + .get('/v2/tasks/a-random-id/eventstream') + .set('accept', 'text/event-stream') + .parse((res, _) => { + ({ statusCode, headers } = res as any); + + res.on('data', chunk => { + responseDataFn(chunk.toString()); + + // the server expects the client to abort the request + if (chunk.includes('completion')) { + req.abort(); + } + }); + }); + + // wait for the request to finish + await req.catch(() => { + // ignore 'aborted' error + }); + + expect(statusCode).toBe(200); + expect(headers['content-type']).toBe('text/event-stream'); + expect(responseDataFn).toBeCalledTimes(2); + expect(responseDataFn).toBeCalledWith(`event: log data: {"id":0,"taskId":"a-random-id","type":"log","createdAt":"","body":{"message":"My log message"}} `); - expect(responseDataFn).toBeCalledWith(`event: completion + expect(responseDataFn).toBeCalledWith(`event: completion data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{"message":"Finished!"}} `); - expect(taskBroker.event$).toBeCalledTimes(1); - expect(taskBroker.event$).toBeCalledWith({ taskId: 'a-random-id' }); - expect(subscriber!.closed).toBe(true); + expect(taskBroker.event$).toBeCalledTimes(1); + expect(taskBroker.event$).toBeCalledWith({ taskId: 'a-random-id' }); + expect(subscriber!.closed).toBe(true); + }); + + it('should return log messages with after query', async () => { + let subscriber: ZenObservable.SubscriptionObserver; + ( + taskBroker.event$ as jest.Mocked['event$'] + ).mockImplementation(({ taskId }) => { + return new ObservableImpl(observer => { + subscriber = observer; + setImmediate(() => { + observer.next({ + events: [ + { + id: 1, + taskId, + type: 'completion', + createdAt: '', + body: { message: 'Finished!' }, + }, + ], + }); + }); + }); + }); + + let statusCode: any = undefined; + let headers: any = {}; + + const req = request(app) + .get('/v2/tasks/a-random-id/eventstream') + .query({ after: 10 }) + .set('accept', 'text/event-stream') + .parse((res, _) => { + ({ statusCode, headers } = res as any); + + res.on('data', () => { + // close immediately + req.abort(); + }); + }); + + // wait for the request to finish + await req.catch(() => { + // ignore 'aborted' error + }); + + expect(statusCode).toBe(200); + expect(headers['content-type']).toBe('text/event-stream'); + + expect(taskBroker.event$).toBeCalledTimes(1); + expect(taskBroker.event$).toBeCalledWith({ + taskId: 'a-random-id', + after: 10, + }); + + expect(subscriber!.closed).toBe(true); + }); }); - it('should return log messages with after query', async () => { - let subscriber: ZenObservable.SubscriptionObserver; - ( - taskBroker.event$ as jest.Mocked['event$'] - ).mockImplementation(({ taskId }) => { - return new ObservableImpl(observer => { - subscriber = observer; - setImmediate(() => { + describe('GET /v2/tasks/:taskId/events', () => { + it('should return log messages', async () => { + let subscriber: ZenObservable.SubscriptionObserver; + ( + taskBroker.event$ as jest.Mocked['event$'] + ).mockImplementation(({ taskId }) => { + return new ObservableImpl(observer => { + subscriber = observer; observer.next({ events: [ + { + id: 0, + taskId, + type: 'log', + createdAt: '', + body: { message: 'My log message' }, + }, { id: 1, taskId, @@ -581,120 +1275,57 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }); }); }); + + const response = await request(app).get('/v2/tasks/a-random-id/events'); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([ + { + id: 0, + taskId: 'a-random-id', + type: 'log', + createdAt: '', + body: { message: 'My log message' }, + }, + { + id: 1, + taskId: 'a-random-id', + type: 'completion', + createdAt: '', + body: { message: 'Finished!' }, + }, + ]); + + expect(taskBroker.event$).toBeCalledTimes(1); + expect(taskBroker.event$).toBeCalledWith({ taskId: 'a-random-id' }); + expect(subscriber!.closed).toBe(true); }); - let statusCode: any = undefined; - let headers: any = {}; - - const req = request(app) - .get('/v2/tasks/a-random-id/eventstream') - .query({ after: 10 }) - .set('accept', 'text/event-stream') - .parse((res, _) => { - ({ statusCode, headers } = res as any); - - res.on('data', () => { - // close immediately - req.abort(); + it('should return log messages with after query', async () => { + let subscriber: ZenObservable.SubscriptionObserver; + ( + taskBroker.event$ as jest.Mocked['event$'] + ).mockImplementation(() => { + return new ObservableImpl(observer => { + subscriber = observer; + observer.next({ events: [] }); }); }); - // wait for the request to finish - await req.catch(() => { - // ignore 'aborted' error - }); + const response = await request(app) + .get('/v2/tasks/a-random-id/events') + .query({ after: 10 }); - expect(statusCode).toBe(200); - expect(headers['content-type']).toBe('text/event-stream'); + expect(response.status).toEqual(200); + expect(response.body).toEqual([]); - expect(taskBroker.event$).toBeCalledTimes(1); - expect(taskBroker.event$).toBeCalledWith({ - taskId: 'a-random-id', - after: 10, - }); - - expect(subscriber!.closed).toBe(true); - }); - }); - - describe('GET /v2/tasks/:taskId/events', () => { - it('should return log messages', async () => { - let subscriber: ZenObservable.SubscriptionObserver; - ( - taskBroker.event$ as jest.Mocked['event$'] - ).mockImplementation(({ taskId }) => { - return new ObservableImpl(observer => { - subscriber = observer; - observer.next({ - events: [ - { - id: 0, - taskId, - type: 'log', - createdAt: '', - body: { message: 'My log message' }, - }, - { - id: 1, - taskId, - type: 'completion', - createdAt: '', - body: { message: 'Finished!' }, - }, - ], - }); - }); - }); - - const response = await request(app).get('/v2/tasks/a-random-id/events'); - - expect(response.status).toEqual(200); - expect(response.body).toEqual([ - { - id: 0, + expect(taskBroker.event$).toBeCalledTimes(1); + expect(taskBroker.event$).toBeCalledWith({ taskId: 'a-random-id', - type: 'log', - createdAt: '', - body: { message: 'My log message' }, - }, - { - id: 1, - taskId: 'a-random-id', - type: 'completion', - createdAt: '', - body: { message: 'Finished!' }, - }, - ]); - - expect(taskBroker.event$).toBeCalledTimes(1); - expect(taskBroker.event$).toBeCalledWith({ taskId: 'a-random-id' }); - expect(subscriber!.closed).toBe(true); - }); - - it('should return log messages with after query', async () => { - let subscriber: ZenObservable.SubscriptionObserver; - ( - taskBroker.event$ as jest.Mocked['event$'] - ).mockImplementation(() => { - return new ObservableImpl(observer => { - subscriber = observer; - observer.next({ events: [] }); + after: 10, }); + expect(subscriber!.closed).toBe(true); }); - - const response = await request(app) - .get('/v2/tasks/a-random-id/events') - .query({ after: 10 }); - - expect(response.status).toEqual(200); - expect(response.body).toEqual([]); - - expect(taskBroker.event$).toBeCalledTimes(1); - expect(taskBroker.event$).toBeCalledWith({ - taskId: 'a-random-id', - after: 10, - }); - expect(subscriber!.closed).toBe(true); }); }); }); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index a9ddfb6888..c26b6570aa 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/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; - 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); From 2b3b36170959f9b71d3b37534ea3278b3680d1f6 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Tue, 23 Aug 2022 17:22:27 +0100 Subject: [PATCH 18/25] fix api reports and report not needed mock token Signed-off-by: Brian Fletcher --- plugins/scaffolder-backend/api-report.md | 2 +- plugins/scaffolder-backend/src/service/router.test.ts | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index c692cee9aa..4115aa7645 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -420,7 +420,7 @@ export const createPublishGitlabMergeRequestAction: (options: { branchName: string; targetPath: string; token?: string | undefined; - commitAction?: 'update' | 'delete' | 'create' | undefined; + commitAction?: 'update' | 'create' | 'delete' | undefined; projectid?: string | undefined; removeSourceBranch?: boolean | undefined; assignee?: string | undefined; diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 1c26ee57fd..8042d4571f 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -1000,12 +1000,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }); it('should emit auditlog containing user identifier when backstage auth is passed', async () => { - const mockToken = - 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; - await request(app) .post('/v2/tasks') - .set('Authorization', `Bearer ${mockToken}`) .send({ templateRef: stringifyEntityRef({ kind: 'template', From f05f8d7e7d06ca84e9dc0e065f1a2359bfad16d2 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Thu, 25 Aug 2022 09:18:16 +0100 Subject: [PATCH 19/25] return error where identity api throws Signed-off-by: Brian Fletcher --- .../src/service/router.test.ts | 44 ++----------------- .../scaffolder-backend/src/service/router.ts | 33 +++----------- 2 files changed, 10 insertions(+), 67 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 8042d4571f..d820dd07c8 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -886,11 +886,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }, ); }); - it('should not throw', async () => { - const broker = - taskBroker.dispatch as jest.Mocked['dispatch']; - - await request(app) + it('return an error', async () => { + const response = await request(app) .post('/v2/tasks') .send({ templateRef: stringifyEntityRef({ @@ -901,42 +898,7 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ required: 'required-value', }, }); - expect(broker).toHaveBeenCalledWith( - expect.objectContaining({ - createdBy: undefined, - secrets: { - backstageToken: undefined, - }, - - spec: { - apiVersion: mockTemplate.apiVersion, - steps: mockTemplate.spec.steps.map((step, index) => ({ - ...step, - id: step.id ?? `step-${index + 1}`, - name: step.name ?? step.action, - })), - output: mockTemplate.spec.output ?? {}, - parameters: { - required: 'required-value', - }, - user: { - entity: undefined, - ref: undefined, - }, - templateInfo: { - entityRef: stringifyEntityRef({ - kind: 'Template', - namespace: 'Default', - name: mockTemplate.metadata?.name, - }), - baseUrl: 'https://dev.azure.com', - entity: { - metadata: mockTemplate.metadata, - }, - }, - }, - }), - ); + expect(response.status).not.toEqual(201); }); }); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index c26b6570aa..78e933b325 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -131,24 +131,6 @@ function buildDefaultIdentityClient({ }; } -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 @@ -228,10 +210,8 @@ export async function createRouter( async (req, res) => { const { namespace, kind, name } = req.params; - const userIdentity = await getIdentity({ + const userIdentity = await identity.getIdentity({ request: req, - logger, - identity, }); const token = userIdentity?.token; @@ -276,10 +256,8 @@ export async function createRouter( defaultKind: 'template', }); - const callerIdentity = await getIdentity({ + const callerIdentity = await identity.getIdentity({ request: req, - logger, - identity, }); const token = callerIdentity?.token; const userEntityRef = callerIdentity?.identity.userEntityRef; @@ -485,8 +463,11 @@ export async function createRouter( throw new InputError('Input template is not a template'); } - const token = (await getIdentity({ request: req, logger, identity })) - ?.token; + const token = ( + await identity.getIdentity({ + request: req, + }) + )?.token; for (const parameters of [template.spec.parameters ?? []].flat()) { const result = validate(body.values, parameters); From ed92f732a1957ab77880bffaf649739e29815923 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Thu, 25 Aug 2022 15:42:12 +0100 Subject: [PATCH 20/25] default id provider surface NotAllowedError Signed-off-by: Brian Fletcher --- plugins/auth-node/src/DefaultIdentityClient.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/plugins/auth-node/src/DefaultIdentityClient.ts b/plugins/auth-node/src/DefaultIdentityClient.ts index 167b0c2451..8f51bd7530 100644 --- a/plugins/auth-node/src/DefaultIdentityClient.ts +++ b/plugins/auth-node/src/DefaultIdentityClient.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import { PluginEndpointDiscovery } from '@backstage/backend-common'; -import { AuthenticationError } from '@backstage/errors'; +import { AuthenticationError, NotAllowedError } from '@backstage/errors'; import { createRemoteJWKSet, decodeJwt, @@ -81,9 +81,13 @@ export class DefaultIdentityClient implements IdentityApi { if (!request.headers.authorization) { return undefined; } - return await this.authenticate( - getBearerTokenFromAuthorizationHeader(request.headers.authorization), - ); + try { + return await this.authenticate( + getBearerTokenFromAuthorizationHeader(request.headers.authorization), + ); + } catch (e) { + throw new NotAllowedError('Failed to authenticate the provided token'); + } } /** From ec74f257943a7b75613e89bd02230a5c1f15969f Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Thu, 25 Aug 2022 15:54:07 +0100 Subject: [PATCH 21/25] fix test Signed-off-by: Brian Fletcher --- plugins/auth-node/src/DefaultIdentityClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-node/src/DefaultIdentityClient.ts b/plugins/auth-node/src/DefaultIdentityClient.ts index 8f51bd7530..a7946c77ff 100644 --- a/plugins/auth-node/src/DefaultIdentityClient.ts +++ b/plugins/auth-node/src/DefaultIdentityClient.ts @@ -86,7 +86,7 @@ export class DefaultIdentityClient implements IdentityApi { getBearerTokenFromAuthorizationHeader(request.headers.authorization), ); } catch (e) { - throw new NotAllowedError('Failed to authenticate the provided token'); + throw new NotAllowedError(e.message); } } From eda848d745212a1a547ef4a6bcc3ad22b50aa723 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Wed, 31 Aug 2022 11:33:37 +0100 Subject: [PATCH 22/25] fix merge Signed-off-by: Brian Fletcher --- .../src/service/router.test.ts | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 9bc327f25d..5e3c1c7278 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -1001,7 +1001,7 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }); const response = await request(app).get(`/v2/tasks`); - expect(taskBroker.list).toBeCalledWith({ + expect(taskBroker.list).toHaveBeenCalledWith({ createdBy: undefined, }); expect(response.status).toEqual(200); @@ -1036,7 +1036,7 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ const response = await request(app).get( `/v2/tasks?createdBy=user:default/foo`, ); - expect(taskBroker.list).toBeCalledWith({ + expect(taskBroker.list).toHaveBeenCalledWith({ createdBy: 'user:default/foo', }); @@ -1136,18 +1136,20 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ expect(statusCode).toBe(200); expect(headers['content-type']).toBe('text/event-stream'); - expect(responseDataFn).toBeCalledTimes(2); - expect(responseDataFn).toBeCalledWith(`event: log + expect(responseDataFn).toHaveBeenCalledTimes(2); + expect(responseDataFn).toHaveBeenCalledWith(`event: log data: {"id":0,"taskId":"a-random-id","type":"log","createdAt":"","body":{"message":"My log message"}} `); - expect(responseDataFn).toBeCalledWith(`event: completion + expect(responseDataFn).toHaveBeenCalledWith(`event: completion data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{"message":"Finished!"}} `); - expect(taskBroker.event$).toBeCalledTimes(1); - expect(taskBroker.event$).toBeCalledWith({ taskId: 'a-random-id' }); + expect(taskBroker.event$).toHaveBeenCalledTimes(1); + expect(taskBroker.event$).toHaveBeenCalledWith({ + taskId: 'a-random-id', + }); expect(subscriber!.closed).toBe(true); }); @@ -1198,8 +1200,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ expect(statusCode).toBe(200); expect(headers['content-type']).toBe('text/event-stream'); - expect(taskBroker.event$).toBeCalledTimes(1); - expect(taskBroker.event$).toBeCalledWith({ + expect(taskBroker.event$).toHaveBeenCalledTimes(1); + expect(taskBroker.event$).toHaveBeenCalledWith({ taskId: 'a-random-id', after: 10, }); @@ -1257,8 +1259,10 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }, ]); - expect(taskBroker.event$).toBeCalledTimes(1); - expect(taskBroker.event$).toBeCalledWith({ taskId: 'a-random-id' }); + expect(taskBroker.event$).toHaveBeenCalledTimes(1); + expect(taskBroker.event$).toHaveBeenCalledWith({ + taskId: 'a-random-id', + }); expect(subscriber!.closed).toBe(true); }); @@ -1280,8 +1284,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ expect(response.status).toEqual(200); expect(response.body).toEqual([]); - expect(taskBroker.event$).toBeCalledTimes(1); - expect(taskBroker.event$).toBeCalledWith({ + expect(taskBroker.event$).toHaveBeenCalledTimes(1); + expect(taskBroker.event$).toHaveBeenCalledWith({ taskId: 'a-random-id', after: 10, }); From 785177584a2e2f65d2fdb7e34730f3d7228b9d2d Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Thu, 1 Sep 2022 08:48:40 +0100 Subject: [PATCH 23/25] add explanation for the buildDefaultIdentityClient Signed-off-by: Brian Fletcher --- plugins/scaffolder-backend/src/service/router.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 78e933b325..040af546f1 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -75,6 +75,13 @@ 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({ logger, }: { From 861ab75dd05f8dec8dd2cd083d5f41b5ac274b0f Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Thu, 1 Sep 2022 10:22:18 +0100 Subject: [PATCH 24/25] fix version dep of auth-node Signed-off-by: Brian Fletcher --- plugins/scaffolder-backend/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 6e18cd462f..9b97c81181 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -41,7 +41,7 @@ "@backstage/config": "^1.0.1", "@backstage/errors": "^1.1.0", "@backstage/integration": "^1.3.1-next.0", - "@backstage/plugin-auth-node": "^0.2.5-next.0", + "@backstage/plugin-auth-node": "^0.2.5-next.1", "@backstage/plugin-catalog-backend": "^1.4.0-next.1", "@backstage/plugin-scaffolder-common": "^1.2.0-next.0", "@backstage/backend-plugin-api": "^0.1.2-next.0", From 46ed5154417563305eb34badf62eae3bd83b11ea Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Thu, 1 Sep 2022 10:58:14 +0100 Subject: [PATCH 25/25] fix yarn lock after merge Signed-off-by: Brian Fletcher --- plugins/auth-node/package.json | 2 +- yarn.lock | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index f512f961c6..360426d7bd 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -25,8 +25,8 @@ "@backstage/backend-common": "^0.15.1-next.1", "@backstage/config": "^1.0.1", "@backstage/errors": "^1.1.0", - "express": "^4.17.1", "@types/express": "*", + "express": "^4.17.1", "jose": "^4.6.0", "node-fetch": "^2.6.7", "winston": "^3.2.1" diff --git a/yarn.lock b/yarn.lock index 2445084328..6a2b6fc817 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4006,6 +4006,8 @@ __metadata: "@backstage/cli": ^0.19.0-next.1 "@backstage/config": ^1.0.1 "@backstage/errors": ^1.1.0 + "@types/express": "*" + express: ^4.17.1 jose: ^4.6.0 lodash: ^4.17.21 msw: ^0.46.0 @@ -6392,6 +6394,7 @@ __metadata: "@backstage/config": ^1.0.1 "@backstage/errors": ^1.1.0 "@backstage/integration": ^1.3.1-next.0 + "@backstage/plugin-auth-node": ^0.2.5-next.1 "@backstage/plugin-catalog-backend": ^1.4.0-next.1 "@backstage/plugin-catalog-node": ^1.0.2-next.0 "@backstage/plugin-scaffolder-common": ^1.2.0-next.0