diff --git a/packages/backend-app-api/src/services/implementations/auth/DefaultAuthService.ts b/packages/backend-app-api/src/services/implementations/auth/DefaultAuthService.ts index d0c11a4eee..b4feb46cce 100644 --- a/packages/backend-app-api/src/services/implementations/auth/DefaultAuthService.ts +++ b/packages/backend-app-api/src/services/implementations/auth/DefaultAuthService.ts @@ -23,11 +23,7 @@ import { BackstageServicePrincipal, BackstageUserPrincipal, } from '@backstage/backend-plugin-api'; -import { - AuthenticationError, - ForwardedError, - NotAllowedError, -} from '@backstage/errors'; +import { AuthenticationError, ForwardedError } from '@backstage/errors'; import { JsonObject } from '@backstage/types'; import { decodeJwt } from 'jose'; import { ExternalTokenHandler } from './external/ExternalTokenHandler'; @@ -86,20 +82,10 @@ export class DefaultAuthService implements AuthService { const externalResult = await this.externalTokenHandler.verifyToken(token); if (externalResult) { - const restrictions = externalResult.accessRestrictions; - if (restrictions) { - if (!restrictions.has(this.pluginId)) { - const valid = [...restrictions.keys()].map(k => `'${k}'`).join(', '); - throw new NotAllowedError( - `This token's access is restricted to plugin(s) ${valid}`, - ); - } - } - return createCredentialsWithServicePrincipal( externalResult.subject, undefined, - restrictions?.get(this.pluginId), + externalResult.accessRestrictions, ); } diff --git a/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.ts b/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.ts index 7bbd012c01..ad4fb17d88 100644 --- a/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.ts +++ b/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.ts @@ -39,19 +39,7 @@ export const authServiceFactory = createServiceFactory({ // new auth services in the new backend system. tokenManager: coreServices.tokenManager, }, - async createRootContext({ config, logger }) { - const externalTokens = ExternalTokenHandler.create({ - config, - logger, - }); - return { - externalTokens, - }; - }, - async factory( - { config, discovery, plugin, tokenManager, logger, database }, - { externalTokens }, - ) { + async factory({ config, discovery, plugin, tokenManager, logger, database }) { const disableDefaultAuthPolicy = Boolean( config.getOptionalBoolean( 'backend.auth.dangerouslyDisableDefaultAuthPolicy', @@ -73,6 +61,11 @@ export const authServiceFactory = createServiceFactory({ publicKeyStore, discovery, }); + const externalTokens = ExternalTokenHandler.create({ + ownPluginId: plugin.getId(), + config, + logger, + }); return new DefaultAuthService( userTokens, 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 new file mode 100644 index 0000000000..1e82f76134 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.test.ts @@ -0,0 +1,55 @@ +/* + * Copyright 2024 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 { BackstagePrincipalAccessRestrictions } from '@backstage/backend-plugin-api'; +import { ExternalTokenHandler } from './ExternalTokenHandler'; +import { TokenHandler } from './types'; + +describe('ExternalTokenHandler', () => { + it('skips over inner handlers that do not match, and applies plugin restrictions', async () => { + const handler1: TokenHandler = { + add: jest.fn(), + verifyToken: jest.fn().mockResolvedValue(undefined), + }; + + const handler2: TokenHandler = { + add: jest.fn(), + verifyToken: jest.fn().mockResolvedValue({ + subject: 'sub', + allAccessRestrictions: new Map( + Object.entries({ + plugin1: { + permissionNames: ['do.it'], + } satisfies BackstagePrincipalAccessRestrictions, + }), + ), + }), + }; + + const plugin1 = new ExternalTokenHandler('plugin1', [handler1, handler2]); + const plugin2 = new ExternalTokenHandler('plugin2', [handler1, handler2]); + + await expect(plugin1.verifyToken('token')).resolves.toEqual({ + subject: 'sub', + accessRestrictions: { permissionNames: ['do.it'] }, + }); + await expect( + plugin2.verifyToken('token'), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"This token's access is restricted to plugin(s) 'plugin1'"`, + ); + }); +}); diff --git a/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.ts b/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.ts index 82603953d0..72eeae6fcf 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.ts @@ -15,16 +15,19 @@ */ import { + BackstagePrincipalAccessRestrictions, LoggerService, RootConfigService, } from '@backstage/backend-plugin-api'; +import { NotAllowedError } from '@backstage/errors'; import { LegacyTokenHandler } from './legacy'; import { StaticTokenHandler } from './static'; import { JWKSHandler } from './jwks'; -import { AccessRestriptionsMap, TokenHandler } from './types'; +import { TokenHandler } from './types'; const NEW_CONFIG_KEY = 'backend.auth.externalAccess'; const OLD_CONFIG_KEY = 'backend.auth.keys'; +let loggedDeprecationWarning = false; /** * Handles all types of external caller token types (i.e. not Backstage user @@ -34,10 +37,11 @@ const OLD_CONFIG_KEY = 'backend.auth.keys'; */ export class ExternalTokenHandler { static create(options: { + ownPluginId: string; config: RootConfigService; logger: LoggerService; }): ExternalTokenHandler { - const { config, logger } = options; + const { ownPluginId, config, logger } = options; const staticHandler = new StaticTokenHandler(); const legacyHandler = new LegacyTokenHandler(); @@ -66,7 +70,8 @@ export class ExternalTokenHandler { // Load the old keys too const legacyConfigs = config.getOptionalConfigArray(OLD_CONFIG_KEY) ?? []; - if (legacyConfigs.length) { + 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`, ); @@ -75,24 +80,48 @@ export class ExternalTokenHandler { legacyHandler.addOld(handlerConfig); } - return new ExternalTokenHandler(Object.values(handlers)); + return new ExternalTokenHandler(ownPluginId, Object.values(handlers)); } - constructor(private readonly handlers: TokenHandler[]) {} + constructor( + private readonly ownPluginId: string, + private readonly handlers: TokenHandler[], + ) {} async verifyToken(token: string): Promise< | { subject: string; - accessRestrictions?: AccessRestriptionsMap; + accessRestrictions?: BackstagePrincipalAccessRestrictions; } | undefined > { for (const handler of this.handlers) { const result = await handler.verifyToken(token); if (result) { - return result; + const { allAccessRestrictions, ...rest } = result; + if (allAccessRestrictions) { + const accessRestrictions = allAccessRestrictions.get( + this.ownPluginId, + ); + if (!accessRestrictions) { + const valid = [...allAccessRestrictions.keys()] + .map(k => `'${k}'`) + .join(', '); + throw new NotAllowedError( + `This token's access is restricted to plugin(s) ${valid}`, + ); + } + + return { + ...rest, + accessRestrictions, + }; + } + + return rest; } } + return undefined; } } diff --git a/packages/backend-app-api/src/services/implementations/auth/external/legacy.test.ts b/packages/backend-app-api/src/services/implementations/auth/external/legacy.test.ts index 503157acc7..c674e46ec9 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/legacy.test.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/legacy.test.ts @@ -72,7 +72,7 @@ describe('LegacyTokenHandler', () => { await expect(tokenHandler.verifyToken(token1)).resolves.toEqual({ subject: 'key1', - accessRestrictions: accessRestrictions1, + allAccessRestrictions: accessRestrictions1, }); const token2 = await new SignJWT({ @@ -84,7 +84,7 @@ describe('LegacyTokenHandler', () => { await expect(tokenHandler.verifyToken(token2)).resolves.toEqual({ subject: 'key2', - accessRestrictions: accessRestrictions2, + allAccessRestrictions: accessRestrictions2, }); const token3 = await new SignJWT({ diff --git a/packages/backend-app-api/src/services/implementations/auth/external/legacy.ts b/packages/backend-app-api/src/services/implementations/auth/external/legacy.ts index ba56d3a213..4eb982314b 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/legacy.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/legacy.ts @@ -27,16 +27,18 @@ import { AccessRestriptionsMap, TokenHandler } from './types'; export class LegacyTokenHandler implements TokenHandler { #entries = new Array<{ key: Uint8Array; - subject: string; - accessRestrictions?: AccessRestriptionsMap; + result: { + subject: string; + allAccessRestrictions?: AccessRestriptionsMap; + }; }>(); add(config: Config) { - const accessRestrictions = readAccessRestrictionsFromConfig(config); + const allAccessRestrictions = readAccessRestrictionsFromConfig(config); this.#doAdd( config.getString('options.secret'), config.getString('options.subject'), - accessRestrictions, + allAccessRestrictions, ); } @@ -49,10 +51,12 @@ export class LegacyTokenHandler implements TokenHandler { #doAdd( secret: string, subject: string, - accessRestrictions?: AccessRestriptionsMap, + allAccessRestrictions?: AccessRestriptionsMap, ) { 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; @@ -62,14 +66,12 @@ export class LegacyTokenHandler implements TokenHandler { throw new Error('Illegal secret, must be a valid base64 string'); } - if (!subject.match(/^\S+$/)) { - throw new Error('Illegal subject, must be a set of non-space characters'); - } - this.#entries.push({ key, - subject, - accessRestrictions, + result: { + subject, + allAccessRestrictions, + }, }); } @@ -94,13 +96,10 @@ export class LegacyTokenHandler implements TokenHandler { return undefined; } - for (const entry of this.#entries) { + for (const { key, result } of this.#entries) { try { - await jwtVerify(token, entry.key); - return { - subject: entry.subject, - accessRestrictions: entry.accessRestrictions, - }; + await jwtVerify(token, key); + return result; } catch (e) { if (e.code !== 'ERR_JWS_SIGNATURE_VERIFICATION_FAILED') { throw e; diff --git a/packages/backend-app-api/src/services/implementations/auth/external/static.test.ts b/packages/backend-app-api/src/services/implementations/auth/external/static.test.ts index 458c3f06f3..a5945daba9 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/static.test.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/static.test.ts @@ -45,11 +45,11 @@ describe('StaticTokenHandler', () => { await expect(handler.verifyToken('abcabcabc')).resolves.toEqual({ subject: 'one', - accessRestrictions: accessRestrictionsOne, + allAccessRestrictions: accessRestrictionsOne, }); await expect(handler.verifyToken('defdefdef')).resolves.toEqual({ subject: 'two', - accessRestrictions: accessRestrictionsTwo, + allAccessRestrictions: accessRestrictionsTwo, }); await expect(handler.verifyToken('ghighighi')).resolves.toBeUndefined(); }); diff --git a/packages/backend-app-api/src/services/implementations/auth/external/static.ts b/packages/backend-app-api/src/services/implementations/auth/external/static.ts index 8242b87dc0..1e89d8c6a1 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/static.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/static.ts @@ -26,46 +26,37 @@ const MIN_TOKEN_LENGTH = 8; * @internal */ export class StaticTokenHandler implements TokenHandler { - #entries = new Array<{ - token: string; - subject: string; - accessRestrictions?: AccessRestriptionsMap; - }>(); + #entries = new Map< + string, + { + subject: string; + allAccessRestrictions?: AccessRestriptionsMap; + } + >(); 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'); - } - if (token.length < MIN_TOKEN_LENGTH) { + } else if (token.length < MIN_TOKEN_LENGTH) { throw new Error( `Illegal token, must be at least ${MIN_TOKEN_LENGTH} characters length`, ); - } - - const subject = config.getString('options.subject'); - if (!subject.match(/^\S+$/)) { + } 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', + ); } - const accessRestrictions = readAccessRestrictionsFromConfig(config); - - this.#entries.push({ - token, - subject, - accessRestrictions, - }); + this.#entries.set(token, { subject, allAccessRestrictions }); } async verifyToken(token: string) { - const entry = this.#entries.find(e => e.token === token); - if (!entry) { - return undefined; - } - - return { - subject: entry.subject, - accessRestrictions: entry.accessRestrictions, - }; + return this.#entries.get(token); } } diff --git a/packages/backend-app-api/src/services/implementations/auth/external/types.ts b/packages/backend-app-api/src/services/implementations/auth/external/types.ts index 54e51c1ef6..6a1fd084ed 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/types.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/types.ts @@ -27,7 +27,7 @@ export interface TokenHandler { verifyToken(token: string): Promise< | { subject: string; - accessRestrictions?: AccessRestriptionsMap; + allAccessRestrictions?: AccessRestriptionsMap; } | undefined >;