From 0f9d673e82e748b8064d385b98b7ebb209c5d9b9 Mon Sep 17 00:00:00 2001 From: Ben Lambert Date: Wed, 11 Mar 2026 13:33:00 +0100 Subject: [PATCH] Merge commit from fork * Fix redirect URI allowlist bypass via URL userinfo syntax * Validate redirect URIs against normalized origin+pathname --- .../fix-redirect-uri-userinfo-bypass.md | 5 ++++ .../src/service/OidcService.test.ts | 27 ++++++++++++++++++ .../auth-backend/src/service/OidcService.ts | 28 +++++++++---------- 3 files changed, 46 insertions(+), 14 deletions(-) create mode 100644 .changeset/fix-redirect-uri-userinfo-bypass.md diff --git a/.changeset/fix-redirect-uri-userinfo-bypass.md b/.changeset/fix-redirect-uri-userinfo-bypass.md new file mode 100644 index 0000000000..622baf6867 --- /dev/null +++ b/.changeset/fix-redirect-uri-userinfo-bypass.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Improved redirect URI validation in the experimental OIDC provider to match against normalized URLs rather than raw strings. diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index 78c5d88830..7d206b3b05 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -283,6 +283,33 @@ describe('OidcService', () => { ); }); + it('should reject redirect URIs containing userinfo', async () => { + const { service } = await createOidcService({ + databaseId, + config: { + auth: { + experimentalDynamicClientRegistration: { + allowedRedirectUriPatterns: ['http://localhost:*'], + }, + }, + }, + }); + + await expect( + service.registerClient({ + clientName: 'Evil Client', + redirectUris: ['http://localhost:3000@attacker.example/callback'], + }), + ).rejects.toThrow('Invalid redirect_uri'); + + await expect( + service.registerClient({ + clientName: 'Evil Client', + redirectUris: ['http://user:pass@example.com/callback'], + }), + ).rejects.toThrow('Invalid redirect_uri'); + }); + it('should create a client with default values', async () => { const { service } = await createOidcService({ databaseId }); diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index a48a38bfb2..ba975774bd 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -29,6 +29,18 @@ import matcher from 'matcher'; import { OfflineAccessService } from './OfflineAccessService'; import { validateCimdUrl, fetchCimdMetadata } from './CimdClient'; +function validateRedirectUri( + redirectUri: string, + allowedPatterns: string[], +): void { + const parsed = new URL(redirectUri); + const normalized = `${parsed.protocol}//${parsed.host}${parsed.pathname}`; + + if (!allowedPatterns.some(pattern => matcher.isMatch(normalized, pattern))) { + throw new InputError('Invalid redirect_uri'); + } +} + export class OidcService { private readonly auth: AuthService; private readonly tokenIssuer: TokenIssuer; @@ -162,13 +174,7 @@ export class OidcService { ) ?? ['*']; for (const redirectUri of opts.redirectUris ?? []) { - if ( - !allowedRedirectUriPatterns.some(pattern => - matcher.isMatch(redirectUri, pattern), - ) - ) { - throw new InputError('Invalid redirect_uri'); - } + validateRedirectUri(redirectUri, allowedRedirectUriPatterns); } return await this.oidc.createClient({ @@ -305,13 +311,7 @@ export class OidcService { }); if (opts.redirectUri) { - if ( - !cimd.allowedRedirectUriPatterns.some(pattern => - matcher.isMatch(opts.redirectUri!, pattern), - ) - ) { - throw new InputError('Invalid redirect_uri'); - } + validateRedirectUri(opts.redirectUri, cimd.allowedRedirectUriPatterns); if (!cimdClient.redirectUris.includes(opts.redirectUri)) { throw new InputError('Redirect URI not registered');