From 95396a44f9b6ae7c12f2886751a4ec1049993e3b Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Sat, 6 Sep 2025 19:03:36 +0200 Subject: [PATCH] feat: simplify external token handler API - Remove accessRestrictions from individual handlers (now managed by framework) - Change API to evaluate configs individually for better performance Signed-off-by: Juan Pablo Garcia Ripa --- .changeset/thirty-rules-press.md | 21 ++ docs/auth/service-to-service-auth.md | 100 +++++- packages/backend-defaults/report-auth.api.md | 2 +- .../entrypoints/auth/DefaultAuthService.ts | 4 +- .../auth/authServiceFactory.test.ts | 73 ++-- .../entrypoints/auth/authServiceFactory.ts | 6 +- .../external/ExternalAuthTokenHandler.test.ts | 174 ++++------ .../auth/external/ExternalAuthTokenHandler.ts | 133 +++----- .../src/entrypoints/auth/external/helpers.ts | 8 +- .../entrypoints/auth/external/jwks.test.ts | 131 ++------ .../src/entrypoints/auth/external/jwks.ts | 109 +++--- .../entrypoints/auth/external/legacy.test.ts | 314 ++++++++---------- .../src/entrypoints/auth/external/legacy.ts | 174 ++++------ .../entrypoints/auth/external/static.test.ts | 196 ++++++----- .../src/entrypoints/auth/external/static.ts | 103 ++++-- .../src/entrypoints/auth/external/types.ts | 42 ++- .../src/entrypoints/auth/index.ts | 6 +- 17 files changed, 728 insertions(+), 868 deletions(-) diff --git a/.changeset/thirty-rules-press.md b/.changeset/thirty-rules-press.md index d9968317cb..760780eb84 100644 --- a/.changeset/thirty-rules-press.md +++ b/.changeset/thirty-rules-press.md @@ -3,3 +3,24 @@ --- Add a new `externalTokenHandlersServiceRef` to allow custom external token validations + +BREAKING CHANGE: The `backend.auth.keys` config has been removed. Please migrate to the new `backend.auth.externalAccess` config as described in the documentation: https://backstage.io/docs/auth/service-to-service-auth + +**Migration Example:** + +```yaml +# ❌ Old format (no longer supported) +backend: + auth: + keys: + - secret: your-secret-key + +# ✅ New format +backend: + auth: + externalAccess: + - type: static + options: + token: your-secret-key + subject: external:backstage-plugin # this is the current default for old keys +``` diff --git a/docs/auth/service-to-service-auth.md b/docs/auth/service-to-service-auth.md index 322edbe6b6..4ba094068e 100644 --- a/docs/auth/service-to-service-auth.md +++ b/docs/auth/service-to-service-auth.md @@ -255,10 +255,16 @@ backend: subject: legacy-scaffolder ``` -The old style keys config is also supported as an alternative, but please -consider using the new style above instead: +:::warning +The old style `backend.auth.keys` config is **no longer supported** and has been removed. +If you are still using this configuration, you must migrate to the new `backend.auth.externalAccess` format above. -```yaml title="in e.g. app-config.production.yaml" +You'll see an error like `"Unknown key 'backend.auth.keys'"` during Backstage startup if you haven't migrated yet. +::: + +To migrate from the old config format: + +```yaml title="❌ Old format (no longer supported)" backend: auth: keys: @@ -266,6 +272,22 @@ backend: - secret: my-secret-key-scaffolder ``` +Convert to: + +```yaml title="✅ New format" +backend: + auth: + externalAccess: + - type: legacy + options: + secret: my-secret-key-catalog + subject: external:backstage-plugin + - type: legacy + options: + secret: my-secret-key-scaffolder + subject: external:backstage-plugin +``` + The secrets must be any base64-encoded random data, but for security reasons should be sufficiently long so as not to be easy to guess by brute force. You can for example generate them on the command line: @@ -444,11 +466,17 @@ const decoratedPluginTokenHandler = createServiceFactory({ }); ``` -### Custom ExternalTokenHandler +### Adding custom ExternalTokenHandler The `externalTokenTypeHandlersRef` can be used to add custom external token handlers to the default implementation. -The returned object from the factory function must have a `type` property which is used to identify the handler. The `factory` method is called with an array of `Config` objects, for the given type. The factory method can return a single handler or an array of handlers. The handlers are then used to handle the token for the given type. +Your service factory must return an object with a `type` property that matches the token type in your configuration (e.g., 'custom', 'api-key'). When Backstage encounters tokens of this type, it calls your `initialize` method with all the configuration entries that match this type. Your factory can return either a single token handler or an array of handlers to process and validate these tokens. + +:::note Note + +During token verification, all the token handlers are tested. Consider this when adding many token handlers, as it may impact performance. + +::: For example, if we want to add a custom external token handler for the `custom` type: @@ -474,20 +502,72 @@ And we can implement the custom token handler like this: ```ts import { - TokenHandler, + ExternalTokenHandler, externalTokenTypeHandlersRef, + createExternalTokenHandler, } from '@backstage/backend-defaults/auth'; -import { Config } from '@backstage/config'; import { createServiceFactory } from '@backstage/backend-plugin-api'; const customExternalTokenHandlers = createServiceFactory({ service: externalTokenTypeHandlersRef, deps: {}, async factory() { - return { + return createExternalTokenHandler({ type: 'custom', - factory: (configs: Config[]) => new CustomTokenHandler(configs), // can return a simple handler or an array of handlers - }; + initialize({ options }) { + // Initialize your handler context from config + const customOptions = options.getString('customOptions'); + return { customOptions }; + }, + async verifyToken(token, context) { + // Your custom token validation logic here + // Return undefined if token is invalid + // Return { subject: 'your-subject' } if token is valid + + if (token === 'valid-token') { + return { subject: `custom:${context.customOptions}` }; + } + return undefined; + }, + }); + }, +}); +``` + +The `createExternalTokenHandler` helper simplifies creating external token handlers with the new API: + +- **`type`**: A string identifier for your token handler type that matches the config +- **`initialize`**: Called once for each config entry of this type, receives the config options and returns a context object that will be passed to `verifyToken` +- **`verifyToken`**: Called for each token verification with the token and context, returns the subject if valid or `undefined` if not + +```ts +// Example of a more complex handler with external API call +const apiTokenHandler = createExternalTokenHandler({ + type: 'api-validation', + initialize({ options }) { + const apiBaseUrl = options.getString('apiBaseUrl'); + const apiKey = options.getString('apiKey'); + return { apiBaseUrl, apiKey }; + }, + async verifyToken(token, { apiBaseUrl, apiKey }) { + try { + const response = await fetch(`${apiBaseUrl}/validate-token`, { + method: 'POST', + headers: { + Authorization: `Bearer ${apiKey}`, + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ token }), + }); + + if (response.ok) { + const { userId } = await response.json(); + return { subject: `api:${userId}` }; + } + } catch (error) { + // Log error but don't throw - return undefined for invalid tokens + } + return undefined; }, }); ``` diff --git a/packages/backend-defaults/report-auth.api.md b/packages/backend-defaults/report-auth.api.md index 2a80a59d2d..176bdd16de 100644 --- a/packages/backend-defaults/report-auth.api.md +++ b/packages/backend-defaults/report-auth.api.md @@ -60,7 +60,7 @@ export const pluginTokenHandlerDecoratorServiceRef: ServiceRef< >; // @public -export interface TokenHandler { +export interface ExternalTokenHandler { // (undocumented) verifyToken(token: string): Promise< | { diff --git a/packages/backend-defaults/src/entrypoints/auth/DefaultAuthService.ts b/packages/backend-defaults/src/entrypoints/auth/DefaultAuthService.ts index d8cb60e8b5..c2c4a064a3 100644 --- a/packages/backend-defaults/src/entrypoints/auth/DefaultAuthService.ts +++ b/packages/backend-defaults/src/entrypoints/auth/DefaultAuthService.ts @@ -25,7 +25,7 @@ import { import { AuthenticationError } from '@backstage/errors'; import { JsonObject } from '@backstage/types'; import { decodeJwt } from 'jose'; -import { ExternalTokenHandler } from './external/ExternalTokenHandler'; +import { ExternalAuthTokenManager } from './external/export class ExternalAuthTokenManager {'; import { createCredentialsWithNonePrincipal, createCredentialsWithServicePrincipal, @@ -41,7 +41,7 @@ export class DefaultAuthService implements AuthService { constructor( private readonly userTokenHandler: UserTokenHandler, private readonly pluginTokenHandler: PluginTokenHandler, - private readonly externalTokenHandler: ExternalTokenHandler, + private readonly externalTokenHandler: ExternalAuthTokenManager, private readonly pluginId: string, private readonly disableDefaultAuthPolicy: boolean, private readonly pluginKeySource: PluginKeySource, diff --git a/packages/backend-defaults/src/entrypoints/auth/authServiceFactory.test.ts b/packages/backend-defaults/src/entrypoints/auth/authServiceFactory.test.ts index b808973016..b6d5beefa2 100644 --- a/packages/backend-defaults/src/entrypoints/auth/authServiceFactory.test.ts +++ b/packages/backend-defaults/src/entrypoints/auth/authServiceFactory.test.ts @@ -30,9 +30,9 @@ import { setupServer } from 'msw/node'; import { toInternalBackstageCredentials } from './helpers'; import { PluginTokenHandler } from './plugin/PluginTokenHandler'; import { createServiceFactory } from '@backstage/backend-plugin-api'; -import { AccessRestrictionsMap, TokenHandler } from './external/types'; -import { Config } from '@backstage/config'; -import { externalTokenTypeHandlersRef } from './external/ExternalTokenHandler'; + +import { externalTokenTypeHandlersRef } from './external/ExternalAuthTokenHandler'; +import { createExternalTokenHandler } from './external/helpers'; const server = setupServer(); @@ -44,7 +44,7 @@ const mockDeps = [ backend: { baseUrl: 'http://localhost', auth: { - keys: [{ secret: 'abc' }], + // keys: [{ secret: 'abc' }], externalAccess: [ { type: 'static', @@ -460,7 +460,6 @@ describe('authServiceFactory', () => { describe('add custom ExternalTokenHandler', () => { it('should allow custom logic to be injected into the plugin token handler', async () => { const customLogic = jest.fn(); - const customAddEntry = jest.fn(); const customConfig = jest.fn(); const deps = [ discoveryServiceFactory, @@ -469,7 +468,7 @@ describe('authServiceFactory', () => { backend: { baseUrl: 'http://localhost', auth: { - keys: [{ secret: 'abc' }], + // keys: [{ secret: 'abc' }w], externalAccess: [ { type: 'static', @@ -502,37 +501,30 @@ describe('authServiceFactory', () => { }), ]; - class CustomHandler implements TokenHandler { - constructor(options: Config[]) { - for (const option of options) { - customAddEntry(option); - } - } - async verifyToken(token: string): Promise< - | { - subject: string; - allAccessRestrictions?: AccessRestrictionsMap; - } - | undefined - > { + const customExternalTokenHandler = createExternalTokenHandler<{ + [`custom-config`]: string; + foo: string; + }>({ + type: 'custom', + initialize({ options }) { + const customConfigValue = options.getString('custom-config'); + const foo = options.getString('foo'); + customConfig(customConfigValue); + return { [`custom-config`]: customConfigValue, foo }; + }, + async verifyToken(token, ctx) { customLogic(token); return { - subject: 'foo', + subject: ctx.foo, }; - } - } + }, + }); const customPluginTokenHandler = createServiceFactory({ service: externalTokenTypeHandlersRef, deps: {}, async factory() { - return { - type: 'custom', - factory: (configs: Config[]) => { - customConfig(configs); - return new CustomHandler(configs); - }, - }; + return customExternalTokenHandler; }, }); const tester = ServiceFactoryTester.from(authServiceFactory, { @@ -541,29 +533,8 @@ describe('authServiceFactory', () => { const searchAuth = await tester.getSubject('search'); await searchAuth.authenticate('custom-token'); - expect(customAddEntry).toHaveBeenCalledWith( - expect.objectContaining({ - data: expect.objectContaining({ - options: expect.objectContaining({ - [`custom-config`]: 'custom-config', - foo: 'bar', - }), - }), - }), - ); expect(customLogic).toHaveBeenCalledWith('custom-token'); - expect(customConfig).toHaveBeenCalledWith( - expect.arrayContaining([ - expect.objectContaining({ - data: expect.objectContaining({ - options: expect.objectContaining({ - [`custom-config`]: 'custom-config', - foo: 'bar', - }), - }), - }), - ]), - ); + expect(customConfig).toHaveBeenCalledWith('custom-config'); }); }); }); diff --git a/packages/backend-defaults/src/entrypoints/auth/authServiceFactory.ts b/packages/backend-defaults/src/entrypoints/auth/authServiceFactory.ts index 43a3c9da50..34fc3b51ef 100644 --- a/packages/backend-defaults/src/entrypoints/auth/authServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/auth/authServiceFactory.ts @@ -21,9 +21,9 @@ import { } from '@backstage/backend-plugin-api'; import { DefaultAuthService } from './DefaultAuthService'; import { - ExternalTokenHandler, + ExternalAuthTokenManager, externalTokenTypeHandlersRef, -} from './external/ExternalTokenHandler'; +} from './external/ExternalAuthTokenHandler'; import { DefaultPluginTokenHandler, PluginTokenHandler, @@ -107,7 +107,7 @@ export const authServiceFactory = createServiceFactory({ }), ); - const externalTokens = ExternalTokenHandler.create({ + const externalTokens = ExternalAuthTokenManager.create({ ownPluginId: plugin.getId(), config, logger, diff --git a/packages/backend-defaults/src/entrypoints/auth/external/ExternalAuthTokenHandler.test.ts b/packages/backend-defaults/src/entrypoints/auth/external/ExternalAuthTokenHandler.test.ts index 3348d84e93..d97875d228 100644 --- a/packages/backend-defaults/src/entrypoints/auth/external/ExternalAuthTokenHandler.test.ts +++ b/packages/backend-defaults/src/entrypoints/auth/external/ExternalAuthTokenHandler.test.ts @@ -15,8 +15,9 @@ */ import { BackstagePrincipalAccessRestrictions } from '@backstage/backend-plugin-api'; -import { ExternalTokenHandler } from './ExternalTokenHandler'; -import { TokenHandler } from './types'; +import { ExternalAuthTokenManager } from './ExternalAuthTokenHandler'; +import { createExternalTokenHandler } from './helpers'; +import { AccessRestrictionsMap, ExternalTokenHandler } from './types'; import { mockServices, registerMswTestHooks, @@ -84,25 +85,51 @@ describe('ExternalTokenHandler', () => { registerMswTestHooks(server); it('skips over inner handlers that do not match, and applies plugin restrictions', async () => { - const handler1: TokenHandler = { + const handler1: ExternalTokenHandler = createExternalTokenHandler({ + type: 'type1', + initialize: jest.fn().mockResolvedValue(undefined), verifyToken: jest.fn().mockResolvedValue(undefined), - }; + }); - const handler2: TokenHandler = { - verifyToken: jest.fn().mockResolvedValue({ - subject: 'sub', - allAccessRestrictions: new Map( - Object.entries({ - plugin1: { - permissionNames: ['do.it'], - } satisfies BackstagePrincipalAccessRestrictions, - }), - ), + const handler2: ExternalTokenHandler = + createExternalTokenHandler({ + type: 'type2', + initialize: jest.fn().mockResolvedValue(undefined), + verifyToken: jest.fn().mockResolvedValue({ + subject: 'sub', + }), + }); + + const accessRestrictions: AccessRestrictionsMap = new Map( + Object.entries({ + plugin1: { + permissionNames: ['do.it'], + } satisfies BackstagePrincipalAccessRestrictions, }), - }; + ); - const plugin1 = new ExternalTokenHandler('plugin1', [handler1, handler2]); - const plugin2 = new ExternalTokenHandler('plugin2', [handler1, handler2]); + const plugin1 = new ExternalAuthTokenManager('plugin1', [ + { + context: undefined, + handler: handler1, + }, + { + context: undefined, + handler: handler2, + allAccessRestrictions: accessRestrictions, + }, + ]); + const plugin2 = new ExternalAuthTokenManager('plugin2', [ + { + context: undefined, + handler: handler1, + }, + { + context: undefined, + handler: handler2, + allAccessRestrictions: accessRestrictions, + }, + ]); await expect(plugin1.verifyToken('token')).resolves.toEqual({ subject: 'sub', @@ -133,7 +160,7 @@ describe('ExternalTokenHandler', () => { ), ); - const handler = ExternalTokenHandler.create({ + const handler = ExternalAuthTokenManager.create({ ownPluginId: 'catalog', logger: mockServices.logger.mock(), config: mockServices.rootConfig({ @@ -222,12 +249,10 @@ describe('ExternalTokenHandler', () => { ), ); - const customHandler: TokenHandler = { - verifyToken: jest.fn(), - }; - const customHandlerFactory = jest.fn(() => customHandler); + const verifyMock = jest.fn().mockResolvedValue({}); + const initializeMock = jest.fn().mockReturnValue({ context: 'a' }); - const handler = ExternalTokenHandler.create({ + const handler = ExternalAuthTokenManager.create({ ownPluginId: 'catalog', logger: mockServices.logger.mock(), config: mockServices.rootConfig({ @@ -252,30 +277,23 @@ describe('ExternalTokenHandler', () => { }, }), externalTokenHandlers: [ - { + createExternalTokenHandler({ type: 'internal-custom', - factory: customHandlerFactory, - }, + initialize: initializeMock, + verifyToken: verifyMock, + }), ], }); - expect(customHandlerFactory).toHaveBeenCalledWith( - expect.arrayContaining([ - expect.objectContaining({ - data: { - type: 'internal-custom', - options: { - issuer: 'my-company', - subject: 'internal-subject', - audience: 'backstage', - }, - accessRestrictions: [ - { plugin: 'catalog', permission: 'catalog.entity.read' }, - ], - }, - }), - ]), - ); + expect(initializeMock).toHaveBeenCalledWith({ + options: expect.objectContaining({ + data: { + issuer: 'my-company', + subject: 'internal-subject', + audience: 'backstage', + }, + }), + }); const customToken = await factory.issueToken({ claims: { sub: 'internal-subject' }, @@ -283,11 +301,11 @@ describe('ExternalTokenHandler', () => { await handler.verifyToken(customToken); - expect(customHandler.verifyToken).toHaveBeenCalled(); + expect(verifyMock).toHaveBeenCalledWith(customToken, { context: 'a' }); }); it('should fail if config contains types not declared', async () => { const createHandler = () => - ExternalTokenHandler.create({ + ExternalAuthTokenManager.create({ ownPluginId: 'catalog', logger: mockServices.logger.mock(), config: mockServices.rootConfig({ @@ -314,13 +332,13 @@ describe('ExternalTokenHandler', () => { }); expect(createHandler).toThrowErrorMatchingInlineSnapshot( - `"Unknown type(s) 'internal-custom' in backend.auth.externalAccess, expected one of 'static', 'legacy', 'jwks'"`, + `"Unknown type 'internal-custom' in backend.auth.externalAccess, expected one of 'static', 'legacy', 'jwks'"`, ); }); it('should show valid custom types in errors', async () => { const createHandler = () => - ExternalTokenHandler.create({ + ExternalAuthTokenManager.create({ ownPluginId: 'catalog', logger: mockServices.logger.mock(), config: mockServices.rootConfig({ @@ -345,68 +363,18 @@ describe('ExternalTokenHandler', () => { }, }), externalTokenHandlers: [ - { + createExternalTokenHandler({ type: 'internal-custom', - factory: () => ({ - verifyToken: jest.fn(), + initialize: jest.fn().mockResolvedValue(undefined), + verifyToken: jest.fn().mockResolvedValue({ + subject: 'sub', }), - }, + }), ], }); expect(createHandler).toThrowErrorMatchingInlineSnapshot( - `"Unknown type(s) 'internal-custom-invalid' in backend.auth.externalAccess, expected one of 'static', 'legacy', 'jwks', 'internal-custom'"`, - ); - }); - it('should show all invalid config types', async () => { - const createHandler = () => - ExternalTokenHandler.create({ - ownPluginId: 'catalog', - logger: mockServices.logger.mock(), - config: mockServices.rootConfig({ - data: { - backend: { - auth: { - externalAccess: [ - { - type: 'internal-custom-invalid', - options: { - issuer: 'my-company', - subject: 'internal-subject', - audience: 'backstage', - }, - accessRestrictions: [ - { plugin: 'catalog', permission: 'catalog.entity.read' }, - ], - }, - { - type: 'internal-custom-invalid-2', - options: { - issuer: 'my-company', - subject: 'internal-subject', - audience: 'backstage', - }, - accessRestrictions: [ - { plugin: 'catalog', permission: 'catalog.entity.read' }, - ], - }, - ], - }, - }, - }, - }), - externalTokenHandlers: [ - { - type: 'internal-custom', - factory: () => ({ - verifyToken: jest.fn(), - }), - }, - ], - }); - - expect(createHandler).toThrowErrorMatchingInlineSnapshot( - `"Unknown type(s) 'internal-custom-invalid, internal-custom-invalid-2' in backend.auth.externalAccess, expected one of 'static', 'legacy', 'jwks', 'internal-custom'"`, + `"Unknown type 'internal-custom-invalid' in backend.auth.externalAccess, expected one of 'static', 'legacy', 'jwks', 'internal-custom'"`, ); }); }); diff --git a/packages/backend-defaults/src/entrypoints/auth/external/ExternalAuthTokenHandler.ts b/packages/backend-defaults/src/entrypoints/auth/external/ExternalAuthTokenHandler.ts index c3fab70026..bf57ad42f4 100644 --- a/packages/backend-defaults/src/entrypoints/auth/external/ExternalAuthTokenHandler.ts +++ b/packages/backend-defaults/src/entrypoints/auth/external/ExternalAuthTokenHandler.ts @@ -21,85 +21,82 @@ import { RootConfigService, } from '@backstage/backend-plugin-api'; import { NotAllowedError } from '@backstage/errors'; -import { LegacyConfigWrapper, LegacyTokenHandler } from './legacy'; -import { StaticTokenHandler } from './static'; -import { JWKSHandler } from './jwks'; -import { TokenHandler } from './types'; -import { Config } from '@backstage/config'; -import { groupBy } from 'lodash'; -import { TokenTypeHandler } from './types'; +import { legacyTokenHandler } from './legacy'; +import { staticTokenHandler } from './static'; +import { jwksTokenHandler } from './jwks'; +import { AccessRestrictionsMap, ExternalTokenHandler } from './types'; +import { readAccessRestrictionsFromConfig } from './helpers'; const NEW_CONFIG_KEY = 'backend.auth.externalAccess'; const OLD_CONFIG_KEY = 'backend.auth.keys'; let loggedDeprecationWarning = false; -type LegacyTokenTypeHandler = { - type: 'legacy'; - /** - * A factory function that takes all token configuration for a given type - * and returns a TokenHandler or an array of TokenHandlers. - */ - factory: ( - configs: ( - | import('@backstage/config').Config - | { legacy: true; config: import('@backstage/config').Config } - )[], - ) => TokenHandler | TokenHandler[]; -}; - /** * @public * This service is used to decorate the default plugin token handler with custom logic. */ -export const externalTokenTypeHandlersRef = createServiceRef({ +export const externalTokenTypeHandlersRef = createServiceRef< + ExternalTokenHandler +>({ id: 'core.auth.externalTokenHandlers', multiton: true, // defaultFactory // :pepe-think: seems like is not possible to use defaultFactory with multiton }); -const defaultHandlers: (TokenTypeHandler | LegacyTokenTypeHandler)[] = [ - { - type: 'static', - factory: configs => new StaticTokenHandler(configs), - }, - { - type: 'legacy', - factory: (configs: (Config | { legacy: true; config: Config })[]) => - new LegacyTokenHandler(configs), - }, - { - type: 'jwks', - factory: configs => new JWKSHandler(configs), - }, +const defaultHandlers: ExternalTokenHandler[] = [ + staticTokenHandler, + legacyTokenHandler, + jwksTokenHandler, ]; +type ContextMapEntry = { + context: T; + handler: ExternalTokenHandler; + allAccessRestrictions?: AccessRestrictionsMap; +}; /** * Handles all types of external caller token types (i.e. not Backstage user * tokens, nor Backstage backend plugin tokens). * * @internal */ -export class ExternalTokenHandler { +export class ExternalAuthTokenManager { static create(options: { ownPluginId: string; config: RootConfigService; logger: LoggerService; - externalTokenHandlers?: TokenTypeHandler[]; - }): ExternalTokenHandler { + externalTokenHandlers?: ExternalTokenHandler[]; + }): ExternalAuthTokenManager { const { ownPluginId, config, - logger, externalTokenHandlers: customHandlers, } = options; const handlersTypes = [...defaultHandlers, ...(customHandlers ?? [])]; const handlerConfigs = config.getOptionalConfigArray(NEW_CONFIG_KEY) ?? []; - const handlerConfigByType: Record & { - legacy?: (Config | LegacyConfigWrapper)[]; - } = groupBy(handlerConfigs, (handlerConfig: Config) => - handlerConfig.getString('type'), + const contexts: ContextMapEntry[] = handlerConfigs.map( + handlerConfig => { + const type = handlerConfig.getString('type'); + + const handler = handlersTypes.find(h => h.type === type); + if (!handler) { + const valid = handlersTypes.map(h => `'${h.type}'`).join(', '); + throw new Error( + `Unknown type '${type}' in ${NEW_CONFIG_KEY}, expected one of ${valid}`, + ); + } + return { + context: handler.initialize({ + options: handlerConfig.getConfig('options'), + }), + handler, + allAccessRestrictions: handlerConfig + ? readAccessRestrictionsFromConfig(handlerConfig) + : undefined, + }; + }, ); // Load the old keys too @@ -107,45 +104,22 @@ export class ExternalTokenHandler { if (legacyConfigs.length && !loggedDeprecationWarning) { loggedDeprecationWarning = true; - logger.warn( - `DEPRECATION WARNING: The ${OLD_CONFIG_KEY} config has been replaced by ${NEW_CONFIG_KEY}, see https://backstage.io/docs/auth/service-to-service-auth`, - ); - } - for (const handlerConfig of legacyConfigs) { - handlerConfigByType.legacy ??= []; - handlerConfigByType.legacy.push({ legacy: true, config: handlerConfig }); - } - - const invalidTypes = Object.keys(handlerConfigByType).filter( - type => !handlersTypes.some(handler => handler.type === type), - ); - - if (invalidTypes.length > 0) { - const valid = handlersTypes - .map(handler => `'${handler.type}'`) - .join(', '); + // :pepe-think: this message was here for more than a year, and replacing this simplifies things a lot throw new Error( - `Unknown type(s) '${invalidTypes.join( - ', ', - )}' in ${NEW_CONFIG_KEY}, expected one of ${valid}`, + `The ${OLD_CONFIG_KEY} config has been replaced by ${NEW_CONFIG_KEY}, see https://backstage.io/docs/auth/service-to-service-auth`, ); } - const handlers = handlersTypes.flatMap(handler => { - const configs = handlerConfigByType[handler.type] ?? []; - const handlerInstances = handler.factory(configs); - if (Array.isArray(handlerInstances)) { - return handlerInstances; - } - return [handlerInstances]; - }); - - return new ExternalTokenHandler(ownPluginId, handlers); + return new ExternalAuthTokenManager(ownPluginId, contexts); } constructor( private readonly ownPluginId: string, - private readonly handlers: TokenHandler[], + private readonly contexts: { + context: unknown; + handler: ExternalTokenHandler; + allAccessRestrictions?: AccessRestrictionsMap; + }[], ) {} async verifyToken(token: string): Promise< @@ -155,10 +129,9 @@ export class ExternalTokenHandler { } | undefined > { - for (const handler of this.handlers) { - const result = await handler.verifyToken(token); + for (const { handler, allAccessRestrictions, context } of this.contexts) { + const result = await handler.verifyToken(token, context); if (result) { - const { allAccessRestrictions, ...rest } = result; if (allAccessRestrictions) { const accessRestrictions = allAccessRestrictions.get( this.ownPluginId, @@ -173,12 +146,12 @@ export class ExternalTokenHandler { } return { - ...rest, + ...result, accessRestrictions, }; } - return rest; + return result; } } diff --git a/packages/backend-defaults/src/entrypoints/auth/external/helpers.ts b/packages/backend-defaults/src/entrypoints/auth/external/helpers.ts index 4a01117e28..296ec2e72b 100644 --- a/packages/backend-defaults/src/entrypoints/auth/external/helpers.ts +++ b/packages/backend-defaults/src/entrypoints/auth/external/helpers.ts @@ -15,7 +15,7 @@ */ import { Config } from '@backstage/config'; -import { AccessRestrictionsMap } from './types'; +import { AccessRestrictionsMap, ExternalTokenHandler } from './types'; /** * Parses and returns the `accessRestrictions` configuration from an @@ -147,3 +147,9 @@ function readPermissionAttributes(externalAccessEntryConfig: Config) { return Object.keys(result).length ? result : undefined; } + +export function createExternalTokenHandler( + handler: ExternalTokenHandler, +): ExternalTokenHandler { + return handler; +} diff --git a/packages/backend-defaults/src/entrypoints/auth/external/jwks.test.ts b/packages/backend-defaults/src/entrypoints/auth/external/jwks.test.ts index eefbce5093..778e5090bf 100644 --- a/packages/backend-defaults/src/entrypoints/auth/external/jwks.test.ts +++ b/packages/backend-defaults/src/entrypoints/auth/external/jwks.test.ts @@ -20,7 +20,7 @@ import { SignJWT, exportJWK, generateKeyPair } from 'jose'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { v4 as uuid } from 'uuid'; -import { JWKSHandler } from './jwks'; +import { jwksTokenHandler } from './jwks'; // Simplified copy of TokenFactory in @backstage/plugin-auth-backend interface AnyJWK extends Record { @@ -101,108 +101,45 @@ describe('JWKSHandler', () => { it('verifies token with valid entry', async () => { const validEntry = { - options: { - url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithm: 'RS256', - issuer: mockBaseUrl, - audience: 'backstage', - }, + url: `${mockBaseUrl}/.well-known/jwks.json`, + algorithm: 'RS256', + issuer: mockBaseUrl, + audience: 'backstage', }; - const jwksHandler = new JWKSHandler([new ConfigReader(validEntry)]); + const context = jwksTokenHandler.initialize({ + options: new ConfigReader(validEntry), + }); const token = await factory.issueToken({ claims: { sub: mockSubject }, }); - const result = await jwksHandler.verifyToken(token); + const result = await jwksTokenHandler.verifyToken(token, context); expect(result).toEqual({ subject: `external:${mockSubject}` }); }); - it('skips invalid entry and continues verification', async () => { - const invalidEntry = { - options: { - url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithm: 'RS256', - issuer: ['fakeIssuer'], - audience: ['fakeAud'], - }, - }; - - const validEntry = { - options: { - url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithm: 'RS256', - issuer: ['multiple-issuers', mockBaseUrl], - audience: ['multiple-audiences', 'backstage'], - }, - }; - const jwksHandler = new JWKSHandler([ - new ConfigReader(invalidEntry), - new ConfigReader(validEntry), - ]); - - const token = await factory.issueToken({ - claims: { sub: mockSubject }, - }); - - const result = await jwksHandler.verifyToken(token); - - expect(result).toEqual({ subject: `external:${mockSubject}` }); - }); - - it('returns undefined if no valid entry found', async () => { - const invalidEntry1 = { - options: { - url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithm: 'RS256', - issuer: 'wrong', - }, - }; - - const invalidEntry2 = { - options: { - url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithm: ['HS256'], - audience: 'wrong', - }, - }; - const jwksHandler = new JWKSHandler([ - new ConfigReader(invalidEntry1), - new ConfigReader(invalidEntry2), - ]); - - const token = await factory.issueToken({ - claims: { sub: mockSubject }, - }); - - const result = await jwksHandler.verifyToken(token); - - expect(result).toBeUndefined(); - }); - it('rejects bad config', () => { expect(() => { - return new JWKSHandler([ - new ConfigReader({ - options: { url: 'https://exampl e.com/jwks' }, + return jwksTokenHandler.initialize({ + options: new ConfigReader({ + url: 'https://exampl e.com/jwks', }), - ]); + }); }).toThrow('Illegal JWKS URL, must be a set of non-space characters'); expect(() => { - return new JWKSHandler([ - new ConfigReader({ - options: { - url: 'https://example.com/jwks\n', - }, + return jwksTokenHandler.initialize({ + options: new ConfigReader({ + url: 'https://example.com/jwks\n', }), - ]); + }); }).toThrow('Illegal JWKS URL, must be a set of non-space characters'); }); it('gracefully handles no added tokens', async () => { - const handler = new JWKSHandler([]); - await expect(handler.verifyToken('ghi')).resolves.toBeUndefined(); + await expect( + jwksTokenHandler.verifyToken('ghi', {} as any), + ).resolves.toBeUndefined(); }); it('uses custom subject prefix if provided', async () => { @@ -215,38 +152,18 @@ describe('JWKSHandler', () => { subjectPrefix: 'custom-prefix', }, }; - const jwksHandler = new JWKSHandler([new ConfigReader(validEntry)]); + const context = jwksTokenHandler.initialize({ + options: new ConfigReader(validEntry.options), + }); const token = await factory.issueToken({ claims: { sub: mockSubject }, }); - const result = await jwksHandler.verifyToken(token); + const result = await jwksTokenHandler.verifyToken(token, context); expect(result).toEqual({ subject: `external:${validEntry.options.subjectPrefix}:${mockSubject}`, }); }); - - it('carries over access restrictions', async () => { - const jwksHandler = new JWKSHandler([ - 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-defaults/src/entrypoints/auth/external/jwks.ts b/packages/backend-defaults/src/entrypoints/auth/external/jwks.ts index 5f7fcb4127..892fcf2181 100644 --- a/packages/backend-defaults/src/entrypoints/auth/external/jwks.ts +++ b/packages/backend-defaults/src/entrypoints/auth/external/jwks.ts @@ -15,56 +15,40 @@ */ import { jwtVerify, createRemoteJWKSet, JWTVerifyGetKey } from 'jose'; -import { Config } from '@backstage/config'; import { + createExternalTokenHandler, readAccessRestrictionsFromConfig, readStringOrStringArrayFromConfig, } from './helpers'; -import { AccessRestrictionsMap, TokenHandler } from './types'; +import { AccessRestrictionsMap } from './types'; -/** - * Handles `type: jwks` access. - * - * @internal - */ -export class JWKSHandler implements TokenHandler { - #entries: Array<{ - algorithms?: string[]; - audiences?: string[]; - issuers?: string[]; - subjectPrefix?: string; - url: URL; - jwks: JWTVerifyGetKey; - allAccessRestrictions?: AccessRestrictionsMap; - }> = []; +type JWKSTokenContext = { + algorithms?: string[]; + audiences?: string[]; + issuers?: string[]; + subjectPrefix?: string; + url: URL; + jwks: JWTVerifyGetKey; + allAccessRestrictions?: AccessRestrictionsMap; +}; - constructor(configs: Config[]) { - for (const config of configs) { - this.add(config); - } - } - private add(config: Config) { - if (!config.getString('options.url').match(/^\S+$/)) { +export const jwksTokenHandler = createExternalTokenHandler({ + type: 'jwks', + initialize({ options }): JWKSTokenContext { + if (!options.getString('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 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); - const allAccessRestrictions = readAccessRestrictionsFromConfig(config); - - this.#entries.push({ + const allAccessRestrictions = readAccessRestrictionsFromConfig(options); + return { algorithms, audiences, issuers, @@ -72,34 +56,31 @@ export class JWKSHandler implements TokenHandler { subjectPrefix, url, allAccessRestrictions, - }); - return this; - } + }; + }, - async verifyToken(token: string) { - for (const entry of this.#entries) { - try { - const { - payload: { sub }, - } = await jwtVerify(token, entry.jwks, { - algorithms: entry.algorithms, - issuer: entry.issuers, - audience: entry.audiences, - }); + async verifyToken(token: string, context: JWKSTokenContext) { + try { + const { + payload: { sub }, + } = await jwtVerify(token, context.jwks, { + algorithms: context.algorithms, + issuer: context.issuers, + audience: context.audiences, + }); - if (sub) { - const prefix = entry.subjectPrefix - ? `external:${entry.subjectPrefix}:` - : 'external:'; - return { - subject: `${prefix}${sub}`, - allAccessRestrictions: entry.allAccessRestrictions, - }; - } - } catch { - continue; + if (sub) { + const prefix = context.subjectPrefix + ? `external:${context.subjectPrefix}:` + : 'external:'; + return { + subject: `${prefix}${sub}`, + allAccessRestrictions: context.allAccessRestrictions, + }; } + } catch { + return undefined; } return undefined; - } -} + }, +}); diff --git a/packages/backend-defaults/src/entrypoints/auth/external/legacy.test.ts b/packages/backend-defaults/src/entrypoints/auth/external/legacy.test.ts index 4892d0a21a..43160b46da 100644 --- a/packages/backend-defaults/src/entrypoints/auth/external/legacy.test.ts +++ b/packages/backend-defaults/src/entrypoints/auth/external/legacy.test.ts @@ -18,49 +18,27 @@ import { ConfigReader } from '@backstage/config'; import { randomBytes } from 'crypto'; import { SignJWT, importJWK } from 'jose'; import { DateTime } from 'luxon'; -import { LegacyTokenHandler } from './legacy'; +import { legacyTokenHandler } from './legacy'; describe('LegacyTokenHandler', () => { const key1 = randomBytes(24); const key2 = randomBytes(24); - const key3 = randomBytes(24); - const accessRestrictions1 = new Map( - Object.entries({ - scaffolder: {}, - }), - ); - const accessRestrictions2 = new Map( - Object.entries({ - catalog: { permissionNames: ['catalog.entity.read'] }, - }), - ); - const configs = [ - new ConfigReader({ - options: { - secret: key1.toString('base64'), - subject: 'key1', - }, - accessRestrictions: [{ plugin: 'scaffolder' }], - }), + const tokenHandler = legacyTokenHandler; - new ConfigReader({ - options: { - secret: key2.toString('base64'), - subject: 'key2', - }, - accessRestrictions: [ - { plugin: 'catalog', permission: 'catalog.entity.read' }, - ], + const context1 = tokenHandler.initialize({ + options: new ConfigReader({ + secret: key1.toString('base64'), + subject: 'key1', }), - { - legacy: true, - config: new ConfigReader({ - secret: key3.toString('base64'), - }), - }, - ]; - const tokenHandler = new LegacyTokenHandler(configs); + }); + + const context2 = tokenHandler.initialize({ + options: new ConfigReader({ + secret: key2.toString('base64'), + subject: 'key2', + }), + }); it('should verify valid tokens', async () => { const token1 = await new SignJWT({ @@ -70,9 +48,8 @@ describe('LegacyTokenHandler', () => { .setProtectedHeader({ alg: 'HS256' }) .sign(key1); - await expect(tokenHandler.verifyToken(token1)).resolves.toEqual({ + await expect(tokenHandler.verifyToken(token1, context1)).resolves.toEqual({ subject: 'key1', - allAccessRestrictions: accessRestrictions1, }); const token2 = await new SignJWT({ @@ -82,20 +59,8 @@ describe('LegacyTokenHandler', () => { .setProtectedHeader({ alg: 'HS256' }) .sign(key2); - await expect(tokenHandler.verifyToken(token2)).resolves.toEqual({ + await expect(tokenHandler.verifyToken(token2, context2)).resolves.toEqual({ subject: 'key2', - allAccessRestrictions: accessRestrictions2, - }); - - const token3 = await new SignJWT({ - sub: 'backstage-server', - exp: DateTime.now().plus({ minutes: 1 }).toUnixInteger(), - }) - .setProtectedHeader({ alg: 'HS256' }) - .sign(key3); - - await expect(tokenHandler.verifyToken(token3)).resolves.toEqual({ - subject: 'external:backstage-plugin', }); }); @@ -107,10 +72,18 @@ describe('LegacyTokenHandler', () => { .setProtectedHeader({ alg: 'HS256' }) .sign(key1); - await expect(tokenHandler.verifyToken(validToken)).resolves.toBeUndefined(); + await expect( + tokenHandler.verifyToken(validToken, context1), + ).resolves.toBeUndefined(); + await expect( + tokenHandler.verifyToken(validToken, context2), + ).resolves.toBeUndefined(); await expect( - tokenHandler.verifyToken('statickeyblaaa'), + tokenHandler.verifyToken('statickeyblaaa', context1), + ).resolves.toBeUndefined(); + await expect( + tokenHandler.verifyToken('statickeyblaaa', context2), ).resolves.toBeUndefined(); const randomToken = await new SignJWT({ @@ -120,7 +93,10 @@ describe('LegacyTokenHandler', () => { .setProtectedHeader({ alg: 'HS256' }) .sign(randomBytes(24)); await expect( - tokenHandler.verifyToken(randomToken), + tokenHandler.verifyToken(randomToken, context1), + ).resolves.toBeUndefined(); + await expect( + tokenHandler.verifyToken(randomToken, context2), ).resolves.toBeUndefined(); const mockPublicKey = { @@ -144,7 +120,10 @@ describe('LegacyTokenHandler', () => { .sign(await importJWK(mockPrivateKey)); await expect( - tokenHandler.verifyToken(keyWithWrongAlg), + tokenHandler.verifyToken(keyWithWrongAlg, context1), + ).resolves.toBeUndefined(); + await expect( + tokenHandler.verifyToken(keyWithWrongAlg, context2), ).resolves.toBeUndefined(); }); @@ -157,151 +136,134 @@ describe('LegacyTokenHandler', () => { .setProtectedHeader({ alg: 'HS256' }) .sign(key1); - await expect(tokenHandler.verifyToken(keyWithWrongExp)).rejects.toThrow( - /\"exp\" claim must be a number/, - ); + await expect( + tokenHandler.verifyToken(keyWithWrongExp, context1), + ).rejects.toThrow(/\"exp\" claim must be a number/); }); it('rejects bad config', () => { - const handler = new LegacyTokenHandler([]); + const handler = legacyTokenHandler; // new style add, bad secrets - expect( - () => - new LegacyTokenHandler([ - new ConfigReader({ - options: { _missingsecret: true, subject: 'ok' }, - }), - ]), - ).toThrowErrorMatchingInlineSnapshot( - `"Missing required config value at 'options.secret' in 'mock-config'"`, - ); - expect( - () => - new LegacyTokenHandler([ - new ConfigReader({ options: { secret: '', subject: 'ok' } }), - ]), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'options.secret' in 'mock-config', got empty-string, wanted string"`, - ); - expect( - () => - new LegacyTokenHandler([ - new ConfigReader({ - options: { secret: 'has spaces', subject: 'ok' }, - }), - ]), - ).toThrowErrorMatchingInlineSnapshot( - `"Illegal secret, must be a valid base64 string"`, - ); - expect( - () => - new LegacyTokenHandler([ - new ConfigReader({ - options: { secret: 'hasnewline\n', subject: 'ok' }, - }), - ]), - ).toThrowErrorMatchingInlineSnapshot( - `"Illegal secret, must be a valid base64 string"`, - ); - expect( - () => - new LegacyTokenHandler([ - new ConfigReader({ options: { secret: 3, subject: 'ok' } }), - ]), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'options.secret' in 'mock-config', got number, wanted string"`, - ); - - // new style add, bad subjects - expect( - () => - new LegacyTokenHandler([ - new ConfigReader({ - options: { secret: 'b2s=', _missingsubject: true }, - }), - ]), - ).toThrowErrorMatchingInlineSnapshot( - `"Missing required config value at 'options.subject' in 'mock-config'"`, - ); - expect( - () => - new LegacyTokenHandler([ - new ConfigReader({ options: { secret: 'b2s=', subject: '' } }), - ]), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'options.subject' in 'mock-config', got empty-string, wanted string"`, - ); - expect( - () => - new LegacyTokenHandler([ - new ConfigReader({ - options: { secret: 'b2s=', subject: 'has spaces' }, - }), - ]), - ).toThrowErrorMatchingInlineSnapshot( - `"Illegal subject, must be a set of non-space characters"`, - ); - expect( - () => - new LegacyTokenHandler([ - new ConfigReader({ - options: { secret: 'b2s=', subject: 'hasnewline\n' }, - }), - ]), - ).toThrowErrorMatchingInlineSnapshot( - `"Illegal subject, must be a set of non-space characters"`, - ); - expect( - () => - new LegacyTokenHandler([ - new ConfigReader({ options: { secret: 'b2s=', subject: 3 } }), - ]), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'options.subject' in 'mock-config', got number, wanted string"`, - ); - - // new style add, bad access restrictions - expect( - () => - new LegacyTokenHandler([ - new ConfigReader({ - options: { secret: 'b2s=', subject: 'subject' }, - accessRestrictions: [{ plugin: ['a'] }], - }), - ]), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'accessRestrictions[0].plugin' in 'mock-config', got array, wanted string"`, - ); - - // old style add expect(() => - handler.addOld(new ConfigReader({ secret: 'b2s=' })), - ).not.toThrow(); - expect(() => - handler.addOld(new ConfigReader({ _missingsecret: true })), + handler.initialize({ + options: new ConfigReader({ + _missingsecret: true, + subject: 'ok', + }), + }), ).toThrowErrorMatchingInlineSnapshot( `"Missing required config value at 'secret' in 'mock-config'"`, ); expect(() => - handler.addOld(new ConfigReader({ secret: '' })), + handler.initialize({ + options: new ConfigReader({ secret: '', subject: 'ok' }), + }), ).toThrowErrorMatchingInlineSnapshot( `"Invalid type in config for key 'secret' in 'mock-config', got empty-string, wanted string"`, ); expect(() => - handler.addOld(new ConfigReader({ secret: 'has spaces' })), + handler.initialize({ + options: new ConfigReader({ + secret: 'has spaces', + subject: 'ok', + }), + }), ).toThrowErrorMatchingInlineSnapshot( `"Illegal secret, must be a valid base64 string"`, ); expect(() => - handler.addOld(new ConfigReader({ secret: 'hasnewline\n' })), + handler.initialize({ + options: new ConfigReader({ + secret: 'hasnewline\n', + subject: 'ok', + }), + }), ).toThrowErrorMatchingInlineSnapshot( `"Illegal secret, must be a valid base64 string"`, ); expect(() => - handler.addOld(new ConfigReader({ secret: 3 })), + handler.initialize({ + options: new ConfigReader({ secret: 3, subject: 'ok' }), + }), ).toThrowErrorMatchingInlineSnapshot( `"Invalid type in config for key 'secret' in 'mock-config', got number, wanted string"`, ); + + // new style add, bad subjects + expect(() => + handler.initialize({ + options: new ConfigReader({ + secret: 'b2s=', + _missingsubject: true, + }), + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Missing required config value at 'subject' in 'mock-config'"`, + ); + expect(() => + handler.initialize({ + options: new ConfigReader({ secret: 'b2s=', subject: '' }), + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'subject' in 'mock-config', got empty-string, wanted string"`, + ); + expect(() => + handler.initialize({ + options: new ConfigReader({ + secret: 'b2s=', + subject: 'has spaces', + }), + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal subject, must be a set of non-space characters"`, + ); + expect(() => + handler.initialize({ + options: new ConfigReader({ + secret: 'b2s=', + subject: 'hasnewline\n', + }), + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal subject, must be a set of non-space characters"`, + ); + expect(() => + handler.initialize({ + options: new ConfigReader({ secret: 'b2s=', subject: 3 }), + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'subject' in 'mock-config', got number, wanted string"`, + ); + + // // old style add + // expect(() => + // handler.addOld(new ConfigReader({ secret: 'b2s=' })), + // ).not.toThrow(); + // expect(() => + // handler.addOld(new ConfigReader({ _missingsecret: true })), + // ).toThrowErrorMatchingInlineSnapshot( + // `"Missing required config value at 'secret' in 'mock-config'"`, + // ); + // expect(() => + // handler.addOld(new ConfigReader({ secret: '' })), + // ).toThrowErrorMatchingInlineSnapshot( + // `"Invalid type in config for key 'secret' in 'mock-config', got empty-string, wanted string"`, + // ); + // expect(() => + // handler.addOld(new ConfigReader({ secret: 'has spaces' })), + // ).toThrowErrorMatchingInlineSnapshot( + // `"Illegal secret, must be a valid base64 string"`, + // ); + // expect(() => + // handler.addOld(new ConfigReader({ secret: 'hasnewline\n' })), + // ).toThrowErrorMatchingInlineSnapshot( + // `"Illegal secret, must be a valid base64 string"`, + // ); + // expect(() => + // handler.addOld(new ConfigReader({ secret: 3 })), + // ).toThrowErrorMatchingInlineSnapshot( + // `"Invalid type in config for key 'secret' in 'mock-config', got number, wanted string"`, + // ); }); }); diff --git a/packages/backend-defaults/src/entrypoints/auth/external/legacy.ts b/packages/backend-defaults/src/entrypoints/auth/external/legacy.ts index 8be0de19e4..f9932a8735 100644 --- a/packages/backend-defaults/src/entrypoints/auth/external/legacy.ts +++ b/packages/backend-defaults/src/entrypoints/auth/external/legacy.ts @@ -16,126 +16,78 @@ import { Config } from '@backstage/config'; import { base64url, decodeJwt, decodeProtectedHeader, jwtVerify } from 'jose'; -import { readAccessRestrictionsFromConfig } from './helpers'; -import { AccessRestrictionsMap, TokenHandler } from './types'; + +import { createExternalTokenHandler } from './helpers'; export type LegacyConfigWrapper = { legacy: boolean; config: Config; }; -/** - * Handles `type: legacy` access. - * - * @internal - */ -export class LegacyTokenHandler implements TokenHandler { - #entries = new Array<{ - key: Uint8Array; - result: { - subject: string; - allAccessRestrictions?: AccessRestrictionsMap; - }; - }>(); - constructor(configs: (Config | LegacyConfigWrapper)[]) { - for (const config of configs) { - if (isLegacy(config)) { - this.addOld(config.config); - continue; +type LegacyTokenHandlerContext = { + key: Uint8Array; + + subject: string; +}; + +export const legacyTokenHandler = + createExternalTokenHandler({ + type: 'legacy', + initialize(ctx: { options: Config }): LegacyTokenHandlerContext { + const secret = ctx.options.getString('secret'); + const subject = ctx.options.getString('subject'); + + if (!secret.match(/^\S+$/)) { + throw new Error('Illegal secret, must be a valid base64 string'); + } else if (!subject.match(/^\S+$/)) { + throw new Error( + 'Illegal subject, must be a set of non-space characters', + ); } - this.add(config); - } - } - add(config: Config) { - const allAccessRestrictions = readAccessRestrictionsFromConfig(config); - this.#doAdd( - config.getString('options.secret'), - config.getString('options.subject'), - allAccessRestrictions, - ); - return this; - } - - // used only for the old backend.auth.keys array - addOld(config: Config) { - // This choice of subject is for compatibility reasons - this.#doAdd(config.getString('secret'), 'external:backstage-plugin'); - } - - #doAdd( - secret: string, - subject: string, - allAccessRestrictions?: AccessRestrictionsMap, - ) { - if (!secret.match(/^\S+$/)) { - throw new Error('Illegal secret, must be a valid base64 string'); - } else if (!subject.match(/^\S+$/)) { - throw new Error('Illegal subject, must be a set of non-space characters'); - } - - let key: Uint8Array; - try { - key = base64url.decode(secret); - } catch { - throw new Error('Illegal secret, must be a valid base64 string'); - } - - if (this.#entries.some(e => e.key === key)) { - throw new Error( - 'Legacy externalAccess token was declared more than once', - ); - } - - this.#entries.push({ - key, - result: { - subject, - allAccessRestrictions, - }, - }); - } - - async verifyToken(token: string) { - // First do a duck typing check to see if it remotely looks like a legacy token - try { - // We do a fair amount of checking upfront here. Since we aren't certain - // that it's even the right type of key that we're looking at, we can't - // defer eg the alg check to jwtVerify, because it won't be possible to - // discern different reasons for key verification failures from each other - // easily - const { alg } = decodeProtectedHeader(token); - if (alg !== 'HS256') { - return undefined; - } - const { sub, aud } = decodeJwt(token); - if (sub !== 'backstage-server' || aud) { - return undefined; - } - } catch (e) { - // Doesn't look like a jwt at all - return undefined; - } - - for (const { key, result } of this.#entries) { try { - await jwtVerify(token, key); - return result; - } catch (e) { - if (e.code !== 'ERR_JWS_SIGNATURE_VERIFICATION_FAILED') { - throw e; - } - // Otherwise continue to try the next key + return { + key: base64url.decode(secret), + subject, + }; + } catch { + throw new Error('Illegal secret, must be a valid base64 string'); } - } + }, - // None of the signing keys matched - return undefined; - } -} + async verifyToken(token: string, context: LegacyTokenHandlerContext) { + // First do a duck typing check to see if it remotely looks like a legacy token + try { + // We do a fair amount of checking upfront here. Since we aren't certain + // that it's even the right type of key that we're looking at, we can't + // defer eg the alg check to jwtVerify, because it won't be possible to + // discern different reasons for key verification failures from each other + // easily + const { alg } = decodeProtectedHeader(token); + if (alg !== 'HS256') { + return undefined; + } + const { sub, aud } = decodeJwt(token); + if (sub !== 'backstage-server' || aud) { + return undefined; + } + } catch (e) { + // Doesn't look like a jwt at all + return undefined; + } -function isLegacy( - config: Config | LegacyConfigWrapper, -): config is LegacyConfigWrapper { - return (config as LegacyConfigWrapper).legacy === true; -} + try { + await jwtVerify(token, context.key); + return { + subject: context.subject, + }; + } catch (error) { + if (error.code !== 'ERR_JWS_SIGNATURE_VERIFICATION_FAILED') { + throw error; + } + } + + // None of the signing keys matched + return undefined; + }, + }); diff --git a/packages/backend-defaults/src/entrypoints/auth/external/static.test.ts b/packages/backend-defaults/src/entrypoints/auth/external/static.test.ts index cf1c1a46ac..10e78d2dbf 100644 --- a/packages/backend-defaults/src/entrypoints/auth/external/static.test.ts +++ b/packages/backend-defaults/src/entrypoints/auth/external/static.test.ts @@ -15,148 +15,146 @@ */ import { ConfigReader } from '@backstage/config'; -import { StaticTokenHandler } from './static'; +import { staticTokenHandler } from './static'; describe('StaticTokenHandler', () => { it('accepts any of the added list of tokens', async () => { - const configs = [ - new ConfigReader({ - options: { token: 'abcabcabc', subject: 'one' }, - accessRestrictions: [{ plugin: 'scaffolder' }], + const context1 = staticTokenHandler.initialize({ + options: new ConfigReader({ + token: 'abcabcabc', + subject: 'one', }), - new ConfigReader({ - options: { token: 'defdefdef', subject: 'two' }, - accessRestrictions: [ - { plugin: 'catalog', permission: 'catalog.entity.read' }, - ], + }); + const context2 = staticTokenHandler.initialize({ + options: new ConfigReader({ + token: 'defdefdef', + subject: 'two', }), - ]; - const handler = new StaticTokenHandler(configs); - const accessRestrictionsOne = new Map(Object.entries({ scaffolder: {} })); - const accessRestrictionsTwo = new Map( - Object.entries({ - catalog: { - permissionNames: ['catalog.entity.read'], - }, - }), - ); + }); - await expect(handler.verifyToken('abcabcabc')).resolves.toEqual({ + await expect( + staticTokenHandler.verifyToken('abcabcabc', context1), + ).resolves.toEqual({ subject: 'one', - allAccessRestrictions: accessRestrictionsOne, }); - await expect(handler.verifyToken('defdefdef')).resolves.toEqual({ + await expect( + staticTokenHandler.verifyToken('defdefdef', context2), + ).resolves.toEqual({ subject: 'two', - allAccessRestrictions: accessRestrictionsTwo, }); - await expect(handler.verifyToken('ghighighi')).resolves.toBeUndefined(); + await expect( + staticTokenHandler.verifyToken('ghighighi', context1), + ).resolves.toBeUndefined(); }); it('gracefully handles no added tokens', async () => { - const handler = new StaticTokenHandler([]); - await expect(handler.verifyToken('ghi')).resolves.toBeUndefined(); + await expect( + staticTokenHandler.verifyToken('ghi', {} as any), + ).resolves.toBeUndefined(); }); it('rejects bad config', () => { - expect( - () => - new StaticTokenHandler([ - new ConfigReader({ options: { _missingtoken: true, subject: 'ok' } }), - ]), + expect(() => + staticTokenHandler.initialize({ + options: new ConfigReader({ _missingtoken: true, subject: 'ok' }), + }), ).toThrowErrorMatchingInlineSnapshot( - `"Missing required config value at 'options.token' in 'mock-config'"`, + `"Missing required config value at 'token' in 'mock-config'"`, ); - expect( - () => - new StaticTokenHandler([ - new ConfigReader({ options: { token: '', subject: 'ok' } }), - ]), + expect(() => + staticTokenHandler.initialize({ + options: new ConfigReader({ token: '', subject: 'ok' }), + }), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'options.token' in 'mock-config', got empty-string, wanted string"`, + `"Invalid type in config for key 'token' in 'mock-config', got empty-string, wanted string"`, ); - expect( - () => - new StaticTokenHandler([ - new ConfigReader({ options: { token: 'has spaces', subject: 'ok' } }), - ]), + expect(() => + staticTokenHandler.initialize({ + options: new ConfigReader({ + token: 'has spaces', + subject: 'ok', + }), + }), ).toThrowErrorMatchingInlineSnapshot( `"Illegal token, must be a set of non-space characters"`, ); - expect( - () => - new StaticTokenHandler([ - new ConfigReader({ - options: { - token: 'hasnewlinebutislongenough\n', - subject: 'ok', - }, - }), - ]), + expect(() => + staticTokenHandler.initialize({ + options: new ConfigReader({ + token: 'hasnewlinebutislongenough\n', + subject: 'ok', + }), + }), ).toThrowErrorMatchingInlineSnapshot( `"Illegal token, must be a set of non-space characters"`, ); - expect( - () => - new StaticTokenHandler([ - new ConfigReader({ options: { token: 'short', subject: 'ok' } }), - ]), + expect(() => + staticTokenHandler.initialize({ + options: new ConfigReader({ + token: 'short', + subject: 'ok', + }), + }), ).toThrowErrorMatchingInlineSnapshot( `"Illegal token, must be at least 8 characters length"`, ); - expect( - () => - new StaticTokenHandler([ - new ConfigReader({ options: { token: 3, subject: 'ok' } }), - ]), + expect(() => + staticTokenHandler.initialize({ + options: new ConfigReader({ token: 3, subject: 'ok' }), + }), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'options.token' in 'mock-config', got number, wanted string"`, + `"Invalid type in config for key 'token' in 'mock-config', got number, wanted string"`, ); - expect( - () => - new StaticTokenHandler([ - new ConfigReader({ - options: { token: 'validtoken', _missingsubject: true }, - }), - ]), + expect(() => + staticTokenHandler.initialize({ + options: new ConfigReader({ + token: 'validtoken', + _missingsubject: true, + }), + }), ).toThrowErrorMatchingInlineSnapshot( - `"Missing required config value at 'options.subject' in 'mock-config'"`, + `"Missing required config value at 'subject' in 'mock-config'"`, ); - expect( - () => - new StaticTokenHandler([ - new ConfigReader({ options: { token: 'validtoken', subject: '' } }), - ]), + expect(() => + staticTokenHandler.initialize({ + options: new ConfigReader({ + token: 'validtoken', + subject: '', + }), + }), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'options.subject' in 'mock-config', got empty-string, wanted string"`, + `"Invalid type in config for key 'subject' in 'mock-config', got empty-string, wanted string"`, ); - expect( - () => - new StaticTokenHandler([ - new ConfigReader({ - options: { token: 'validtoken', subject: 'has spaces' }, - }), - ]), + expect(() => + staticTokenHandler.initialize({ + options: new ConfigReader({ + token: 'validtoken', + subject: 'has spaces', + }), + }), ).toThrowErrorMatchingInlineSnapshot( `"Illegal subject, must be a set of non-space characters"`, ); - expect( - () => - new StaticTokenHandler([ - new ConfigReader({ - options: { token: 'validtoken', subject: 'hasnewline\n' }, - }), - ]), + expect(() => + staticTokenHandler.initialize({ + options: new ConfigReader({ + token: 'validtoken', + subject: 'hasnewline\n', + }), + }), ).toThrowErrorMatchingInlineSnapshot( `"Illegal subject, must be a set of non-space characters"`, ); - expect( - () => - new StaticTokenHandler([ - new ConfigReader({ options: { token: 'validtoken', subject: 3 } }), - ]), + expect(() => + staticTokenHandler.initialize({ + options: new ConfigReader({ + token: 'validtoken', + subject: 3, + }), + }), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'options.subject' in 'mock-config', got number, wanted string"`, + `"Invalid type in config for key 'subject' in 'mock-config', got number, wanted string"`, ); }); }); diff --git a/packages/backend-defaults/src/entrypoints/auth/external/static.ts b/packages/backend-defaults/src/entrypoints/auth/external/static.ts index 7764569aca..fe7b9995a4 100644 --- a/packages/backend-defaults/src/entrypoints/auth/external/static.ts +++ b/packages/backend-defaults/src/entrypoints/auth/external/static.ts @@ -15,34 +15,18 @@ */ import { Config } from '@backstage/config'; -import { readAccessRestrictionsFromConfig } from './helpers'; -import { AccessRestrictionsMap, TokenHandler } from './types'; +import { createExternalTokenHandler } from './helpers'; const MIN_TOKEN_LENGTH = 8; -/** - * Handles `type: static` access. - * - * @internal - */ -export class StaticTokenHandler implements TokenHandler { - #entries = new Map< - string, - { - subject: string; - allAccessRestrictions?: AccessRestrictionsMap; - } - >(); - - constructor(configs: Config[]) { - for (const config of configs) { - this.add(config); - } - } - private add(config: Config) { - const token = config.getString('options.token'); - const subject = config.getString('options.subject'); - const allAccessRestrictions = readAccessRestrictionsFromConfig(config); +export const staticTokenHandler = createExternalTokenHandler<{ + token: string; + subject: string; +}>({ + type: 'static', + initialize(ctx: { options: Config }): { token: string; subject: string } { + const token = ctx.options.getString('token'); + const subject = ctx.options.getString('subject'); if (!token.match(/^\S+$/)) { throw new Error('Illegal token, must be a set of non-space characters'); @@ -52,17 +36,64 @@ export class StaticTokenHandler implements TokenHandler { ); } else if (!subject.match(/^\S+$/)) { throw new Error('Illegal subject, must be a set of non-space characters'); - } else if (this.#entries.has(token)) { - throw new Error( - 'Static externalAccess token was declared more than once', - ); } - this.#entries.set(token, { subject, allAccessRestrictions }); - return this; - } + return { token, subject }; + }, + async verifyToken( + token: string, + context: { token: string; subject: string }, + ) { + if (token === context.token) { + return { subject: context.subject }; + } + return undefined; + }, +}); - async verifyToken(token: string) { - return this.#entries.get(token); - } -} +/** + * Handles `type: static` access. + * + * @internal + */ +// export class StaticTokenHandler implements TokenHandler { +// #entries = new Map< +// string, +// { +// subject: string; +// allAccessRestrictions?: AccessRestrictionsMap; +// } +// >(); + +// constructor(configs: Config[]) { +// for (const config of configs) { +// this.add(config); +// } +// } +// private add(config: Config) { +// const token = config.getString('options.token'); +// const subject = config.getString('options.subject'); +// const allAccessRestrictions = readAccessRestrictionsFromConfig(config); + +// if (!token.match(/^\S+$/)) { +// throw new Error('Illegal token, must be a set of non-space characters'); +// } else if (token.length < MIN_TOKEN_LENGTH) { +// throw new Error( +// `Illegal token, must be at least ${MIN_TOKEN_LENGTH} characters length`, +// ); +// } else if (!subject.match(/^\S+$/)) { +// throw new Error('Illegal subject, must be a set of non-space characters'); +// } else if (this.#entries.has(token)) { +// throw new Error( +// 'Static externalAccess token was declared more than once', +// ); +// } + +// this.#entries.set(token, { subject, allAccessRestrictions }); +// return this; +// } + +// async verifyToken(token: string) { +// return this.#entries.get(token); +// } +// } diff --git a/packages/backend-defaults/src/entrypoints/auth/external/types.ts b/packages/backend-defaults/src/entrypoints/auth/external/types.ts index 5e26b66da5..6ce1ca13b6 100644 --- a/packages/backend-defaults/src/entrypoints/auth/external/types.ts +++ b/packages/backend-defaults/src/entrypoints/auth/external/types.ts @@ -15,6 +15,7 @@ */ import { BackstagePrincipalAccessRestrictions } from '@backstage/backend-plugin-api'; +import type { Config } from '@backstage/config'; /** * @public @@ -29,27 +30,24 @@ export type AccessRestrictionsMap = Map< * This interface is used to handle external tokens. * It is used by the auth service to verify tokens and extract the subject. */ -export interface TokenHandler { - verifyToken(token: string): Promise< - | { - subject: string; - allAccessRestrictions?: AccessRestrictionsMap; - } - | undefined - >; +export interface ExternalTokenHandler { + type: string; + initialize(ctx: { options: Config }): TContext; + verifyToken( + token: string, + ctx: TContext, + ): Promise<{ subject: string } | undefined>; } -/** - * @public - * This interface is used to handle external tokens. - */ -export interface TokenTypeHandler { - type: string; - /** - * A factory function that takes all token configuration for a given type - * and returns a TokenHandler or an array of TokenHandlers. - */ - factory: ( - configs: import('@backstage/config').Config[], - ) => TokenHandler | TokenHandler[]; -} +// /** +// * @public +// * This interface is used to handle external tokens. +// */ +// export interface TokenTypeHandler { +// type: string; +// /** +// * A factory function that takes all token configuration for a given type +// * and returns a TokenHandler or an array of TokenHandlers. +// */ +// factory: (configs: Config[]) => TokenHandler | TokenHandler[]; +// } diff --git a/packages/backend-defaults/src/entrypoints/auth/index.ts b/packages/backend-defaults/src/entrypoints/auth/index.ts index 335fc2f62c..e205c8b566 100644 --- a/packages/backend-defaults/src/entrypoints/auth/index.ts +++ b/packages/backend-defaults/src/entrypoints/auth/index.ts @@ -19,9 +19,11 @@ export { pluginTokenHandlerDecoratorServiceRef, } from './authServiceFactory'; -export { externalTokenTypeHandlersRef } from './external/ExternalTokenHandler'; +export { createExternalTokenHandler } from './external/helpers'; -export type { TokenHandler, TokenTypeHandler } from './external/types'; +export { externalTokenTypeHandlersRef } from './external/ExternalAuthTokenHandler'; + +export type { ExternalTokenHandler } from './external/types'; export type { AccessRestrictionsMap } from './external/types'; export type { PluginTokenHandler } from './plugin/PluginTokenHandler';