From c2ea75f4733e6f9bb3d9c8754aad405c3b4a3cf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 22 May 2024 16:40:53 +0200 Subject: [PATCH] update the jwks external auth to use singular nouns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- docs/auth/service-to-service-auth.md | 18 ++--- packages/backend-app-api/config.d.ts | 20 +++--- .../auth/external/helpers.test.ts | 68 ++++++++++++++++++- .../implementations/auth/external/helpers.ts | 11 ++- .../auth/external/jwks.test.ts | 34 +++++----- .../implementations/auth/external/jwks.ts | 7 +- 6 files changed, 112 insertions(+), 46 deletions(-) diff --git a/docs/auth/service-to-service-auth.md b/docs/auth/service-to-service-auth.md index a728bf064b..ee88e45305 100644 --- a/docs/auth/service-to-service-auth.md +++ b/docs/auth/service-to-service-auth.md @@ -96,29 +96,25 @@ backend: - type: jwks options: url: https://example.com/.well-known/jwks.json - issuers: - - https://example.com - algorithms: - - RS256 - audiences: - - example + issuer: https://example.com + algorithm: RS256 + audience: example, other-example subjectPrefix: custom-prefix - type: jwks options: url: https://another-example.com/.well-known/jwks.json - issuers: - - https://example.com + issuer: https://example.com ``` The URL should point at an unauthenticated endpoint that returns the JWKS. -Issuers specifies the issuer(s) of the JWT that the authenticating app will accept. +`issuer` specifies the issuer(s) of the JWT that the authenticating app will accept. Passed JWTs must have an `iss` claim which matches one of the specified issuers. -Algorithms specifies the algorithm(s) that are used to verify the JWT. The passed JWTs +`algorithm` specifies the algorithm(s) that are used to verify the JWT. The passed JWTs must have been signed using one of the listed algorithms. -Audiences specify the intended audience(s) of the JWT. The passed JWTs must have an "aud" +`audience` specifies the intended audience(s) of the JWT. The passed JWTs must have an "aud" claim that matches one of the audiences specified, or have no audience specified. For additional details regarding the JWKS configuration, please consult your authentication diff --git a/packages/backend-app-api/config.d.ts b/packages/backend-app-api/config.d.ts index 5b72fef54c..80f97cb4b5 100644 --- a/packages/backend-app-api/config.d.ts +++ b/packages/backend-app-api/config.d.ts @@ -226,30 +226,30 @@ export interface Config { type: 'jwks'; options: { /** - * Sets the algorithms that should be used to verify the JWT tokens. + * The full URL of the JWKS endpoint. + */ + url: string; + /** + * Sets the algorithm(s) that should be used to verify the JWT tokens. * The passed JWTs must have been signed using one of the listed algorithms. */ - algorithms?: string[]; + algorithm?: string | string[]; /** - * Sets the issuers that should be used to verify the JWT tokens. + * Sets the issuer(s) that should be used to verify the JWT tokens. * Passed JWTs must have an `iss` claim which matches one of the specified issuers. */ - issuers?: string[]; + issuer?: string | string[]; /** - * Sets the audiences that should be used to verify the JWT tokens. + * Sets the audience(s) that should be used to verify the JWT tokens. * The passed JWTs must have an "aud" claim that matches one of the audiences specified, * or have no audience specified. */ - audiences?: string[]; + audience?: string | string[]; /** * Sets an optional subject prefix. Passes the subject to called plugins. * Useful for debugging and tracking purposes. */ subjectPrefix?: string; - /** - * Sets the URL containing the JWKS endpoint. - */ - url: string; }; } >; diff --git a/packages/backend-app-api/src/services/implementations/auth/external/helpers.test.ts b/packages/backend-app-api/src/services/implementations/auth/external/helpers.test.ts index ee0dc3cb87..5b8e40ed4f 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/helpers.test.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/helpers.test.ts @@ -15,8 +15,74 @@ */ import { ConfigReader } from '@backstage/config'; -import { readAccessRestrictionsFromConfig } from './helpers'; +import { + readAccessRestrictionsFromConfig, + readStringOrStringArrayFromConfig, +} from './helpers'; import { JsonObject } from '@backstage/types'; +import { mockServices } from '@backstage/backend-test-utils'; + +describe('readStringOrStringArrayFromConfig', () => { + it('handles all cases correctly', () => { + const config = mockServices.rootConfig({ + data: { + wrongType: 1, + wrongTypeInArray: [1], + singleString: 'a', + spaceSeparatedString: 'a b c', + commaSeparatedString: 'a,b,c', + mixedSeparatorsString: 'a b,c ,, d', + emptyString: '', + emptyArray: [], + simpleArray: ['a', 'b', 'c'], + arrayWithSeparators: ['a b', 'c,d', 'e'], + complexDuplicates: ['a', 'a b', 'a', 'b, a'], + }, + }); + + expect(() => + readStringOrStringArrayFromConfig(config, 'wrongType'), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'wrongType' in 'mock-config', got number, wanted string"`, + ); + expect(() => + readStringOrStringArrayFromConfig(config, 'wrongTypeInArray'), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'wrongTypeInArray[0]' in 'mock-config', got number, wanted string-array"`, + ); + expect(readStringOrStringArrayFromConfig(config, 'singleString')).toEqual([ + 'a', + ]); + expect( + readStringOrStringArrayFromConfig(config, 'spaceSeparatedString'), + ).toEqual(['a', 'b', 'c']); + expect( + readStringOrStringArrayFromConfig(config, 'commaSeparatedString'), + ).toEqual(['a', 'b', 'c']); + expect( + readStringOrStringArrayFromConfig(config, 'mixedSeparatorsString'), + ).toEqual(['a', 'b', 'c', 'd']); + expect(() => + readStringOrStringArrayFromConfig(config, 'emptyString'), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'emptyString' in 'mock-config', got empty-string, wanted string"`, + ); + expect( + readStringOrStringArrayFromConfig(config, 'emptyArray'), + ).toBeUndefined(); + expect(readStringOrStringArrayFromConfig(config, 'simpleArray')).toEqual([ + 'a', + 'b', + 'c', + ]); + expect( + readStringOrStringArrayFromConfig(config, 'arrayWithSeparators'), + ).toEqual(['a', 'b', 'c', 'd', 'e']); + expect( + readStringOrStringArrayFromConfig(config, 'complexDuplicates'), + ).toEqual(['a', 'b']); + }); +}); describe('readAccessRestrictionsFromConfig', () => { function r(config: JsonObject) { diff --git a/packages/backend-app-api/src/services/implementations/auth/external/helpers.ts b/packages/backend-app-api/src/services/implementations/auth/external/helpers.ts index 5d199a5067..d6aa0a01ff 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/helpers.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/helpers.ts @@ -66,8 +66,10 @@ export function readAccessRestrictionsFromConfig( * splits by comma/space into a string array. Can also validate against a known * set of values. Returns undefined if the key didn't exist or if the array * would have ended up being empty. + * + * @internal */ -function stringOrStringArray( +export function readStringOrStringArrayFromConfig( root: Config, key: string, validValues?: readonly T[], @@ -108,7 +110,10 @@ function stringOrStringArray( } function readPermissionNames(externalAccessEntryConfig: Config) { - return stringOrStringArray(externalAccessEntryConfig, 'permission'); + return readStringOrStringArrayFromConfig( + externalAccessEntryConfig, + 'permission', + ); } function readPermissionAttributes(externalAccessEntryConfig: Config) { @@ -129,7 +134,7 @@ function readPermissionAttributes(externalAccessEntryConfig: Config) { } } - const action = stringOrStringArray(config, 'action', [ + const action = readStringOrStringArrayFromConfig(config, 'action', [ 'create', 'read', 'update', diff --git a/packages/backend-app-api/src/services/implementations/auth/external/jwks.test.ts b/packages/backend-app-api/src/services/implementations/auth/external/jwks.test.ts index 4cbdcb1cb0..0466cdf034 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/jwks.test.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/jwks.test.ts @@ -101,9 +101,9 @@ describe('JWKSHandler', () => { it('verifies token with valid entry', async () => { const validEntry = { url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithms: ['RS256'], - issuers: [mockBaseUrl], - audiences: ['backstage'], + algorithm: 'RS256', + issuer: mockBaseUrl, + audience: 'backstage', }; const jwksHandler = new JWKSHandler(); @@ -121,16 +121,16 @@ describe('JWKSHandler', () => { it('skips invalid entry and continues verification', async () => { const invalidEntry = { url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithms: ['RS256'], - issuers: ['fakeIssuer'], - audiences: ['fakeAud'], + algorithm: 'RS256', + issuer: ['fakeIssuer'], + audience: ['fakeAud'], }; const validEntry = { url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithms: ['RS256'], - issuers: ['multiple-issuers', mockBaseUrl], - audiences: ['multiple-audiences', 'backstage'], + algorithm: 'RS256', + issuer: ['multiple-issuers', mockBaseUrl], + audience: ['multiple-audiences', 'backstage'], }; const jwksHandler = new JWKSHandler(); @@ -149,16 +149,14 @@ describe('JWKSHandler', () => { it('returns undefined if no valid entry found', async () => { const invalidEntry1 = { url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithms: ['RS256'], - issuers: [mockBaseUrl], - audiences: [], + algorithm: 'RS256', + issuer: 'wrong', }; const invalidEntry2 = { url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithms: ['HS256'], - issuers: [], - audiences: ['backstage'], + algorithm: ['HS256'], + audience: 'wrong', }; const jwksHandler = new JWKSHandler(); @@ -201,9 +199,9 @@ describe('JWKSHandler', () => { it('uses custom subject prefix if provided', async () => { const validEntry = { url: `${mockBaseUrl}/.well-known/jwks.json`, - algorithms: ['RS256'], - issuers: [mockBaseUrl], - audiences: ['backstage'], + algorithm: 'RS256', + issuer: mockBaseUrl, + audience: 'backstage', subjectPrefix: 'custom-prefix', }; const jwksHandler = new JWKSHandler(); diff --git a/packages/backend-app-api/src/services/implementations/auth/external/jwks.ts b/packages/backend-app-api/src/services/implementations/auth/external/jwks.ts index ea62b3880c..8af44f4d54 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/jwks.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/jwks.ts @@ -16,6 +16,7 @@ import { jwtVerify, createRemoteJWKSet, JWTVerifyGetKey } from 'jose'; import { Config } from '@backstage/config'; +import { readStringOrStringArrayFromConfig } from './helpers'; import { TokenHandler } from './types'; /** @@ -34,9 +35,9 @@ export class JWKSHandler implements TokenHandler { }> = []; add(options: Config) { - const algorithms = options.getOptionalStringArray('algorithms'); - const issuers = options.getOptionalStringArray('issuers'); - const audiences = options.getOptionalStringArray('audiences'); + 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);