diff --git a/packages/backend-app-api/config.d.ts b/packages/backend-app-api/config.d.ts index 4a54de1cb8..f84493828a 100644 --- a/packages/backend-app-api/config.d.ts +++ b/packages/backend-app-api/config.d.ts @@ -37,8 +37,6 @@ export interface Config { * Configures methods of external access, ie ways for callers outside of * the Backstage ecosystem to get authorized for access to APIs that do * not permit unauthorized access. - * - * @deepVisibility secret */ externalAccess: Array< | { @@ -79,6 +77,8 @@ export interface Config { * ```sh * node -p 'require("crypto").randomBytes(24).toString("base64")' * ``` + * + * @visibility secret */ secret: string; @@ -119,6 +119,8 @@ export interface Config { * adding a `freben-local-dev-` prefix for debugging purposes to a * token that you know will be handed out for use as a personal * access token during development. + * + * @visibility secret */ token: string; 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 eeb6100e75..cbb66891ba 100644 --- a/packages/backend-app-api/src/services/implementations/auth/DefaultAuthService.ts +++ b/packages/backend-app-api/src/services/implementations/auth/DefaultAuthService.ts @@ -82,10 +82,7 @@ export class DefaultAuthService implements AuthService { const externalResult = await this.externalTokenHandler.verifyToken(token); if (externalResult) { - return createCredentialsWithServicePrincipal( - externalResult.subject, - externalResult.token, - ); + return createCredentialsWithServicePrincipal(externalResult.subject); } throw new AuthenticationError('Illegal token'); 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 bb0dd02d46..588a1f1794 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 @@ -77,9 +77,7 @@ export class ExternalTokenHandler { constructor(private readonly handlers: TokenHandler[]) {} - async verifyToken( - token: string, - ): Promise<{ subject: string; token?: string } | undefined> { + async verifyToken(token: string): Promise<{ subject: string } | undefined> { for (const handler of this.handlers) { const result = await handler.verifyToken(token); if (result) { 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 6952449c9a..8b2469f6b7 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 @@ -54,7 +54,6 @@ describe('LegacyTokenHandler', () => { await expect(tokenHandler.verifyToken(token1)).resolves.toEqual({ subject: 'key1', - token: token1, }); const token2 = await new SignJWT({ @@ -66,7 +65,6 @@ describe('LegacyTokenHandler', () => { await expect(tokenHandler.verifyToken(token2)).resolves.toEqual({ subject: 'key2', - token: token2, }); const token3 = await new SignJWT({ @@ -78,7 +76,6 @@ describe('LegacyTokenHandler', () => { await expect(tokenHandler.verifyToken(token3)).resolves.toEqual({ subject: 'external:backstage-plugin', - token: token3, }); }); 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 bc7105fa88..8448f295a6 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 @@ -55,9 +55,7 @@ export class LegacyTokenHandler implements TokenHandler { this.#entries.push({ key, subject }); } - async verifyToken( - token: string, - ): Promise<{ subject: string; token?: string } | undefined> { + 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 @@ -81,10 +79,7 @@ export class LegacyTokenHandler implements TokenHandler { for (const entry of this.#entries) { try { await jwtVerify(token, entry.key); - return { - subject: entry.subject, - token: token, - }; + return { subject: entry.subject }; } 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 5e9c6b83dd..86bdf0bdee 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 @@ -20,18 +20,16 @@ import { StaticTokenHandler } from './static'; describe('StaticTokenHandler', () => { it('accepts any of the added list of tokens', async () => { const handler = new StaticTokenHandler(); - handler.add(new ConfigReader({ token: 'abc', subject: 'one' })); - handler.add(new ConfigReader({ token: 'def', subject: 'two' })); + handler.add(new ConfigReader({ token: 'abcabcabc', subject: 'one' })); + handler.add(new ConfigReader({ token: 'defdefdef', subject: 'two' })); - await expect(handler.verifyToken('abc')).resolves.toEqual({ + await expect(handler.verifyToken('abcabcabc')).resolves.toEqual({ subject: 'one', - token: 'abc', }); - await expect(handler.verifyToken('def')).resolves.toEqual({ + await expect(handler.verifyToken('defdefdef')).resolves.toEqual({ subject: 'two', - token: 'def', }); - await expect(handler.verifyToken('ghi')).resolves.toBeUndefined(); + await expect(handler.verifyToken('ghighighi')).resolves.toBeUndefined(); }); it('gracefully handles no added tokens', async () => { @@ -44,34 +42,70 @@ describe('StaticTokenHandler', () => { expect(() => handler.add(new ConfigReader({ _missingtoken: true, subject: 'ok' })), - ).toThrow(/token/); + ).toThrowErrorMatchingInlineSnapshot( + `"Missing required config value at 'token' in 'mock-config'"`, + ); expect(() => handler.add(new ConfigReader({ token: '', subject: 'ok' })), - ).toThrow(/token/); + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'token' in 'mock-config', got empty-string, wanted string"`, + ); expect(() => handler.add(new ConfigReader({ token: 'has spaces', subject: 'ok' })), - ).toThrow(/token/); + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal token, must be a set of non-space characters"`, + ); expect(() => - handler.add(new ConfigReader({ token: 'hasnewline\n', subject: 'ok' })), - ).toThrow(/token/); + handler.add( + new ConfigReader({ + token: 'hasnewlinebutislongenough\n', + subject: 'ok', + }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal token, must be a set of non-space characters"`, + ); + expect(() => + handler.add(new ConfigReader({ token: 'short', subject: 'ok' })), + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal token, must be at least 8 characters length"`, + ); expect(() => handler.add(new ConfigReader({ token: 3, subject: 'ok' })), - ).toThrow(/token/); + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'token' in 'mock-config', got number, wanted string"`, + ); expect(() => - handler.add(new ConfigReader({ token: 'ok', _missingsubject: true })), - ).toThrow(/subject/); + handler.add( + new ConfigReader({ token: 'validtoken', _missingsubject: true }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Missing required config value at 'subject' in 'mock-config'"`, + ); expect(() => - handler.add(new ConfigReader({ token: 'ok', subject: '' })), - ).toThrow(/subject/); + handler.add(new ConfigReader({ token: 'validtoken', subject: '' })), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'subject' in 'mock-config', got empty-string, wanted string"`, + ); expect(() => - handler.add(new ConfigReader({ token: 'ok', subject: 'has spaces' })), - ).toThrow(/subject/); + handler.add( + new ConfigReader({ token: 'validtoken', subject: 'has spaces' }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal subject, must be a set of non-space characters"`, + ); expect(() => - handler.add(new ConfigReader({ token: 'ok', subject: 'hasnewline\n' })), - ).toThrow(/subject/); + handler.add( + new ConfigReader({ token: 'validtoken', subject: 'hasnewline\n' }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal subject, must be a set of non-space characters"`, + ); expect(() => - handler.add(new ConfigReader({ token: 'ok', subject: 3 })), - ).toThrow(/subject/); + handler.add(new ConfigReader({ token: 'validtoken', subject: 3 })), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'subject' in 'mock-config', got number, wanted string"`, + ); }); }); 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 6325253708..bae8e05f2b 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 @@ -17,6 +17,8 @@ import { Config } from '@backstage/config'; import { TokenHandler } from './types'; +const MIN_TOKEN_LENGTH = 8; + /** * Handles `type: static` access. * @@ -30,6 +32,11 @@ export class StaticTokenHandler implements TokenHandler { if (!token.match(/^\S+$/)) { throw new Error('Illegal token, must be a set of non-space characters'); } + if (token.length < MIN_TOKEN_LENGTH) { + throw new Error( + `Illegal token, must be at least ${MIN_TOKEN_LENGTH} characters length`, + ); + } const subject = options.getString('subject'); if (!subject.match(/^\S+$/)) { @@ -39,17 +46,12 @@ export class StaticTokenHandler implements TokenHandler { this.#entries.push({ token, subject }); } - async verifyToken( - token: string, - ): Promise<{ subject: string; token: string } | undefined> { + async verifyToken(token: string) { const entry = this.#entries.find(e => e.token === token); if (!entry) { return undefined; } - return { - subject: entry.subject, - token: token, - }; + return { subject: entry.subject }; } } 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 27b294824e..5d33f09a22 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 @@ -18,7 +18,5 @@ import { Config } from '@backstage/config'; export interface TokenHandler { add(options: Config): void; - verifyToken( - token: string, - ): Promise<{ subject: string; token?: string } | undefined>; + verifyToken(token: string): Promise<{ subject: string } | undefined>; }