From a9791bcec036045580aad607833d3a47e0bccc35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 23 May 2024 14:32:45 +0200 Subject: [PATCH] fix for new config shape too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../external/ExternalTokenHandler.test.ts | 156 ++++++++++++++++++ .../auth/external/jwks.test.ts | 98 +++++++---- .../implementations/auth/external/jwks.ts | 50 ++++-- 3 files changed, 258 insertions(+), 46 deletions(-) diff --git a/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.test.ts b/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.test.ts index 1e82f76134..559107c59c 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.test.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.test.ts @@ -17,8 +17,72 @@ import { BackstagePrincipalAccessRestrictions } from '@backstage/backend-plugin-api'; import { ExternalTokenHandler } from './ExternalTokenHandler'; import { TokenHandler } from './types'; +import { + mockServices, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; +import { randomBytes } from 'crypto'; +import { SignJWT, exportJWK, generateKeyPair } from 'jose'; +import { DateTime } from 'luxon'; +import { v4 as uuid } from 'uuid'; +import { rest } from 'msw'; +import { setupServer } from 'msw/node'; + +// Simplified copy of TokenFactory in @backstage/plugin-auth-backend +interface AnyJWK extends Record { + use: 'sig'; + alg: string; + kid: string; + kty: string; +} +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('RS256'); + 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: 'RS256', 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 }; + } +} describe('ExternalTokenHandler', () => { + const server = setupServer(); + setupRequestMockHandlers(server); + it('skips over inner handlers that do not match, and applies plugin restrictions', async () => { const handler1: TokenHandler = { add: jest.fn(), @@ -52,4 +116,96 @@ describe('ExternalTokenHandler', () => { `"This token's access is restricted to plugin(s) 'plugin1'"`, ); }); + + it('successfully parses known methods', async () => { + const legacyKey = randomBytes(24); + + const factory = new FakeTokenFactory({ + issuer: 'blah', + keyDurationSeconds: 100, + }); + + server.use( + rest.get( + 'https://example.com/.well-known/jwks.json', + async (_, res, ctx) => { + const keys = await factory.listPublicKeys(); + return res(ctx.json(keys)); + }, + ), + ); + + const handler = ExternalTokenHandler.create({ + ownPluginId: 'catalog', + logger: mockServices.logger.mock(), + config: mockServices.rootConfig({ + data: { + backend: { + auth: { + externalAccess: [ + { + type: 'legacy', + options: { + secret: legacyKey.toString('base64'), + subject: 'legacy-subject', + }, + accessRestrictions: [ + { plugin: 'catalog', permission: 'catalog.entity.read' }, + ], + }, + { + type: 'static', + options: { + token: 'defdefdef', + subject: 'static-subject', + }, + accessRestrictions: [ + { plugin: 'catalog', permission: 'catalog.entity.read' }, + ], + }, + { + type: 'jwks', + options: { + url: 'https://example.com/.well-known/jwks.json', + algorithm: 'RS256', + issuer: 'blah', + audience: 'backstage', + subjectPrefix: 'custom-prefix', + }, + accessRestrictions: [ + { plugin: 'catalog', permission: 'catalog.entity.read' }, + ], + }, + ], + }, + }, + }, + }), + }); + + const legacyToken = await new SignJWT({ + sub: 'backstage-server', + exp: DateTime.now().plus({ minutes: 1 }).toUnixInteger(), + }) + .setProtectedHeader({ alg: 'HS256' }) + .sign(legacyKey); + + await expect(handler.verifyToken(legacyToken)).resolves.toEqual({ + subject: 'legacy-subject', + accessRestrictions: { permissionNames: ['catalog.entity.read'] }, + }); + + await expect(handler.verifyToken('defdefdef')).resolves.toEqual({ + subject: 'static-subject', + accessRestrictions: { permissionNames: ['catalog.entity.read'] }, + }); + + const jwksToken = await factory.issueToken({ + claims: { sub: 'jwks-subject' }, + }); + await expect(handler.verifyToken(jwksToken)).resolves.toEqual({ + subject: 'external:custom-prefix:jwks-subject', + accessRestrictions: { permissionNames: ['catalog.entity.read'] }, + }); + }); }); diff --git a/packages/backend-app-api/src/services/implementations/auth/external/jwks.test.ts b/packages/backend-app-api/src/services/implementations/auth/external/jwks.test.ts index 0466cdf034..56930e4480 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/jwks.test.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/jwks.test.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; import { ConfigReader } from '@backstage/config'; import { SignJWT, exportJWK, generateKeyPair } from 'jose'; @@ -21,13 +22,13 @@ import { setupServer } from 'msw/node'; import { v4 as uuid } from 'uuid'; import { JWKSHandler } from './jwks'; +// Simplified copy of TokenFactory in @backstage/plugin-auth-backend 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(); @@ -100,10 +101,12 @@ describe('JWKSHandler', () => { it('verifies token with valid entry', async () => { const validEntry = { - url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithm: 'RS256', - issuer: mockBaseUrl, - audience: 'backstage', + options: { + url: `${mockBaseUrl}/.well-known/jwks.json`, + algorithm: 'RS256', + issuer: mockBaseUrl, + audience: 'backstage', + }, }; const jwksHandler = new JWKSHandler(); @@ -120,17 +123,21 @@ describe('JWKSHandler', () => { it('skips invalid entry and continues verification', async () => { const invalidEntry = { - url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithm: 'RS256', - issuer: ['fakeIssuer'], - audience: ['fakeAud'], + options: { + url: `${mockBaseUrl}/.well-known/jwks.json`, + algorithm: 'RS256', + issuer: ['fakeIssuer'], + audience: ['fakeAud'], + }, }; const validEntry = { - url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithm: 'RS256', - issuer: ['multiple-issuers', mockBaseUrl], - audience: ['multiple-audiences', 'backstage'], + options: { + url: `${mockBaseUrl}/.well-known/jwks.json`, + algorithm: 'RS256', + issuer: ['multiple-issuers', mockBaseUrl], + audience: ['multiple-audiences', 'backstage'], + }, }; const jwksHandler = new JWKSHandler(); @@ -148,15 +155,19 @@ describe('JWKSHandler', () => { it('returns undefined if no valid entry found', async () => { const invalidEntry1 = { - url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithm: 'RS256', - issuer: 'wrong', + options: { + url: `${mockBaseUrl}/.well-known/jwks.json`, + algorithm: 'RS256', + issuer: 'wrong', + }, }; const invalidEntry2 = { - url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithm: ['HS256'], - audience: 'wrong', + options: { + url: `${mockBaseUrl}/.well-known/jwks.json`, + algorithm: ['HS256'], + audience: 'wrong', + }, }; const jwksHandler = new JWKSHandler(); @@ -178,17 +189,21 @@ describe('JWKSHandler', () => { expect(() => { jwksHandler.add( new ConfigReader({ - url: 'https://exampl e.com/jwks', + options: { + url: 'https://exampl e.com/jwks', + }, }), ); - }).toThrow('Invalid URL'); + }).toThrow('Illegal JWKS URL, must be a set of non-space characters'); expect(() => { jwksHandler.add( new ConfigReader({ - url: 'https://example.com/jwks\n', + options: { + url: 'https://example.com/jwks\n', + }, }), ); - }).toThrow('Illegal URL, must be a set of non-space characters'); + }).toThrow('Illegal JWKS URL, must be a set of non-space characters'); }); it('gracefully handles no added tokens', async () => { @@ -198,11 +213,13 @@ describe('JWKSHandler', () => { it('uses custom subject prefix if provided', async () => { const validEntry = { - url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithm: 'RS256', - issuer: mockBaseUrl, - audience: 'backstage', - subjectPrefix: 'custom-prefix', + options: { + url: `${mockBaseUrl}/.well-known/jwks.json`, + algorithm: 'RS256', + issuer: mockBaseUrl, + audience: 'backstage', + subjectPrefix: 'custom-prefix', + }, }; const jwksHandler = new JWKSHandler(); @@ -215,7 +232,30 @@ describe('JWKSHandler', () => { const result = await jwksHandler.verifyToken(token); expect(result).toEqual({ - subject: `external:${validEntry.subjectPrefix}:${mockSubject}`, + subject: `external:${validEntry.options.subjectPrefix}:${mockSubject}`, + }); + }); + + it('carries over access restrictions', async () => { + const jwksHandler = new JWKSHandler(); + jwksHandler.add( + new ConfigReader({ + options: { + url: `${mockBaseUrl}/.well-known/jwks.json`, + }, + accessRestrictions: [{ plugin: 'scaffolder', permission: 'do.it' }], + }), + ); + + const token = await factory.issueToken({ claims: { sub: mockSubject } }); + + await expect(jwksHandler.verifyToken(token)).resolves.toEqual({ + subject: `external:${mockSubject}`, + allAccessRestrictions: new Map( + Object.entries({ + scaffolder: { permissionNames: ['do.it'] }, + }), + ), }); }); }); diff --git a/packages/backend-app-api/src/services/implementations/auth/external/jwks.ts b/packages/backend-app-api/src/services/implementations/auth/external/jwks.ts index 8af44f4d54..d88dc62a47 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/jwks.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/jwks.ts @@ -16,8 +16,11 @@ import { jwtVerify, createRemoteJWKSet, JWTVerifyGetKey } from 'jose'; import { Config } from '@backstage/config'; -import { readStringOrStringArrayFromConfig } from './helpers'; -import { TokenHandler } from './types'; +import { + readAccessRestrictionsFromConfig, + readStringOrStringArrayFromConfig, +} from './helpers'; +import { AccessRestriptionsMap, TokenHandler } from './types'; /** * Handles `type: jwks` access. @@ -32,20 +35,30 @@ export class JWKSHandler implements TokenHandler { subjectPrefix?: string; url: URL; jwks: JWTVerifyGetKey; + allAccessRestrictions?: AccessRestriptionsMap; }> = []; - add(options: Config) { - const algorithms = readStringOrStringArrayFromConfig(options, 'algorithm'); - const issuers = readStringOrStringArrayFromConfig(options, 'issuer'); - const audiences = readStringOrStringArrayFromConfig(options, 'audience'); - const subjectPrefix = options.getOptionalString('subjectPrefix'); - const url = new URL(options.getString('url')); - const jwks = createRemoteJWKSet(url); - - if (!options.getString('url').match(/^\S+$/)) { - throw new Error('Illegal URL, must be a set of non-space characters'); + add(config: Config) { + if (!config.getString('options.url').match(/^\S+$/)) { + throw new Error( + 'Illegal JWKS URL, must be a set of non-space characters', + ); } + const algorithms = readStringOrStringArrayFromConfig( + config, + 'options.algorithm', + ); + const issuers = readStringOrStringArrayFromConfig(config, 'options.issuer'); + const audiences = readStringOrStringArrayFromConfig( + config, + 'options.audience', + ); + const subjectPrefix = config.getOptionalString('options.subjectPrefix'); + const url = new URL(config.getString('options.url')); + const jwks = createRemoteJWKSet(url); + const allAccessRestrictions = readAccessRestrictionsFromConfig(config); + this.#entries.push({ algorithms, audiences, @@ -53,6 +66,7 @@ export class JWKSHandler implements TokenHandler { jwks, subjectPrefix, url, + allAccessRestrictions, }); } @@ -68,11 +82,13 @@ export class JWKSHandler implements TokenHandler { }); if (sub) { - if (entry.subjectPrefix) { - return { subject: `external:${entry.subjectPrefix}:${sub}` }; - } - - return { subject: `external:${sub}` }; + const prefix = entry.subjectPrefix + ? `external:${entry.subjectPrefix}:` + : 'external:'; + return { + subject: `${prefix}${sub}`, + allAccessRestrictions: entry.allAccessRestrictions, + }; } } catch { continue;