diff --git a/.changeset/thirty-steaks-serve.md b/.changeset/thirty-steaks-serve.md new file mode 100644 index 0000000000..d17cb0c2c9 --- /dev/null +++ b/.changeset/thirty-steaks-serve.md @@ -0,0 +1,9 @@ +--- +'@backstage/plugin-auth-backend-module-aws-alb-provider': patch +'@backstage/plugin-auth-backend': patch +--- + +Fixed a foreign key constraint violation when issuing refresh tokens for CIMD clients, and +prevented a failed refresh token issuance from failing the entire token exchange. +Fixed AWS ALB auth provider incorrectly returning HTTP 500 instead of 401 for JWT validation failures, +which caused retry loops and memory pressure under load. diff --git a/plugins/auth-backend-module-aws-alb-provider/src/authenticator.test.ts b/plugins/auth-backend-module-aws-alb-provider/src/authenticator.test.ts index 73f859df52..e7e8fb96a5 100644 --- a/plugins/auth-backend-module-aws-alb-provider/src/authenticator.test.ts +++ b/plugins/auth-backend-module-aws-alb-provider/src/authenticator.test.ts @@ -136,14 +136,15 @@ describe('AwsAlbProvider', () => { }); it('JWT is invalid', async () => { - await expect( - awsAlbAuthenticator.authenticate( + const err = await awsAlbAuthenticator + .authenticate( { req: mockRequestWithInvalidJwt }, { issuer: 'ISSUER_URL', signer: 'SIGNER_ARN', getKey: jest.fn() }, - ), - ).rejects.toThrow( - 'Exception occurred during JWT processing: JWSInvalid: Invalid Compact JWS', - ); + ) + .catch(e => e); + expect(err).toBeInstanceOf(AuthenticationError); + expect(err.message).toContain('Exception occurred during JWT processing'); + expect(err.cause).toBeDefined(); }); it('Email is missing', async () => { @@ -160,18 +161,19 @@ describe('AwsAlbProvider', () => { return undefined; }), } as unknown as express.Request; - await expect( - awsAlbAuthenticator.authenticate( + + const err = await awsAlbAuthenticator + .authenticate( { req }, { issuer: 'ISSUER_URL', signer: undefined, getKey: jest.fn().mockResolvedValue(signingKey), }, - ), - ).rejects.toThrow( - 'Exception occurred during JWT processing: AuthenticationError: Missing email in the JWT token', - ); + ) + .catch(e => e); + expect(err).toBeInstanceOf(AuthenticationError); + expect(err.message).toContain('Missing email in the JWT token'); }); it('issuer is missing', async () => { @@ -189,18 +191,18 @@ describe('AwsAlbProvider', () => { }), } as unknown as express.Request; - await expect( - awsAlbAuthenticator.authenticate( + const err = await awsAlbAuthenticator + .authenticate( { req }, { issuer: 'ISSUER_URL', signer: undefined, getKey: jest.fn().mockResolvedValue(signingKey), }, - ), - ).rejects.toThrow( - 'Exception occurred during JWT processing: AuthenticationError: Issuer mismatch on JWT token', - ); + ) + .catch(e => e); + expect(err).toBeInstanceOf(AuthenticationError); + expect(err.message).toContain('Issuer mismatch on JWT token'); }); it('issuer is invalid', async () => { @@ -218,18 +220,18 @@ describe('AwsAlbProvider', () => { }), } as unknown as express.Request; - await expect( - awsAlbAuthenticator.authenticate( + const err = await awsAlbAuthenticator + .authenticate( { req }, { issuer: 'ISSUER_URL', signer: 'SIGNER_ARN', getKey: jest.fn().mockResolvedValue(signingKey), }, - ), - ).rejects.toThrow( - 'Exception occurred during JWT processing: AuthenticationError: Issuer mismatch on JWT token', - ); + ) + .catch(e => e); + expect(err).toBeInstanceOf(AuthenticationError); + expect(err.message).toContain('Issuer mismatch on JWT token'); }); it('signer is invalid', async () => { @@ -247,18 +249,18 @@ describe('AwsAlbProvider', () => { }), } as unknown as express.Request; - await expect( - awsAlbAuthenticator.authenticate( + const err = await awsAlbAuthenticator + .authenticate( { req }, { issuer: 'ISSUER_URL', signer: 'SIGNER_ARN', getKey: jest.fn().mockResolvedValue(signingKey), }, - ), - ).rejects.toThrow( - 'Exception occurred during JWT processing: AuthenticationError: Issuer mismatch on JWT token', - ); + ) + .catch(e => e); + expect(err).toBeInstanceOf(AuthenticationError); + expect(err.message).toContain('Issuer mismatch on JWT token'); }); }); diff --git a/plugins/auth-backend-module-aws-alb-provider/src/authenticator.ts b/plugins/auth-backend-module-aws-alb-provider/src/authenticator.ts index b6969c5432..5f7eb62219 100644 --- a/plugins/auth-backend-module-aws-alb-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-aws-alb-provider/src/authenticator.ts @@ -98,7 +98,13 @@ export const awsAlbAuthenticator = createProxyAuthenticator({ }, }; } catch (e) { - throw new Error(`Exception occurred during JWT processing: ${e}`); + if (e.name === 'AuthenticationError') { + throw e; + } + throw new AuthenticationError( + 'Exception occurred during JWT processing', + e, + ); } }, }); diff --git a/plugins/auth-backend/migrations/20260317120000_drop_offline_sessions_fk.js b/plugins/auth-backend/migrations/20260317120000_drop_offline_sessions_fk.js new file mode 100644 index 0000000000..d67f069950 --- /dev/null +++ b/plugins/auth-backend/migrations/20260317120000_drop_offline_sessions_fk.js @@ -0,0 +1,49 @@ +/* + * Copyright 2026 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. + */ + +// @ts-check + +/** + * Drop the foreign key constraint on offline_sessions.oidc_client_id + * to allow CIMD (Client ID Metadata Document) clients which are not stored + * in the oidc_clients table. + * + * @param {import('knex').Knex} knex + */ +exports.up = async function up(knex) { + await knex.schema.alterTable('offline_sessions', table => { + table.dropForeign(['oidc_client_id']); + }); +}; + +/** + * @param {import('knex').Knex} knex + */ +exports.down = async function down(knex) { + // Delete sessions with CIMD client_ids (not in oidc_clients) before re-adding FK + await knex('offline_sessions') + .whereNotNull('oidc_client_id') + .whereNotIn('oidc_client_id', knex('oidc_clients').select('client_id')) + .delete(); + + await knex.schema.alterTable('offline_sessions', table => { + table + .foreign('oidc_client_id') + .references('client_id') + .inTable('oidc_clients') + .onDelete('CASCADE'); + }); +}; diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index dea9efbaaa..6027f27406 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -109,6 +109,7 @@ describe('OidcRouter', () => { userInfo: userInfoDatabase, oidc: oidcDatabase, config: mockConfig, + logger: mockServices.logger.mock(), }); const oidcRouter = OidcRouter.create({ @@ -194,6 +195,7 @@ describe('OidcRouter', () => { userInfo: userInfoDatabase, oidc: oidcDatabase, config: mockConfig, + logger: mockServices.logger.mock(), offlineAccess, }); diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index 78c5d88830..b6b77d7146 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -20,6 +20,7 @@ import { } from '@backstage/backend-test-utils'; import { JsonObject } from '@backstage/types'; import { OidcService } from './OidcService'; +import { OfflineAccessService } from './OfflineAccessService'; import { BackstageCredentials, BackstageServicePrincipal, @@ -53,10 +54,11 @@ describe('OidcService', () => { interface CreateOidcServiceOptions { databaseId: TestDatabaseId; config?: JsonObject; + offlineAccess?: OfflineAccessService; } async function createOidcService(options: CreateOidcServiceOptions) { - const { databaseId, config: configData = {} } = options; + const { databaseId, config: configData = {}, offlineAccess } = options; const knex = await databases.init(databaseId); @@ -94,6 +96,8 @@ describe('OidcService', () => { userInfo: mockUserInfo, oidc: oidcDatabase, config, + logger: mockServices.logger.mock(), + offlineAccess, }), mocks: { auth: mockAuth, @@ -809,6 +813,50 @@ describe('OidcService', () => { }), ).rejects.toThrow('Invalid code verifier'); }); + + it('should still return access token when refresh token issuance fails', async () => { + const mockOfflineAccess = { + issueRefreshToken: jest + .fn() + .mockRejectedValue(new Error('DB constraint violation')), + } as unknown as OfflineAccessService; + + const { service, mocks } = await createOidcService({ + databaseId, + offlineAccess: mockOfflineAccess, + }); + const mockToken = 'mock-jwt-token'; + mocks.tokenIssuer.issueToken.mockResolvedValue({ token: mockToken }); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid offline_access', + }); + + const authResult = await service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }); + + const code = new URL(authResult.redirectUrl).searchParams.get('code')!; + + const tokenResult = await service.exchangeCodeForToken({ + code, + redirectUri: 'https://example.com/callback', + grantType: 'authorization_code', + }); + + expect(tokenResult.accessToken).toBe(mockToken); + expect(tokenResult.refreshToken).toBeUndefined(); + expect(mockOfflineAccess.issueRefreshToken).toHaveBeenCalledTimes(1); + }); }); describe('CIMD (Client ID Metadata Document) support', () => { diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index a48a38bfb2..ace9d306b4 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -13,7 +13,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { AuthService, RootConfigService } from '@backstage/backend-plugin-api'; +import { + AuthService, + LoggerService, + RootConfigService, +} from '@backstage/backend-plugin-api'; import { TokenIssuer } from '../identity/types'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; import { @@ -36,6 +40,7 @@ export class OidcService { private readonly userInfo: UserInfoDatabase; private readonly oidc: OidcDatabase; private readonly config: RootConfigService; + private readonly logger: LoggerService; private readonly offlineAccess?: OfflineAccessService; private constructor( @@ -45,6 +50,7 @@ export class OidcService { userInfo: UserInfoDatabase, oidc: OidcDatabase, config: RootConfigService, + logger: LoggerService, offlineAccess?: OfflineAccessService, ) { this.auth = auth; @@ -53,6 +59,7 @@ export class OidcService { this.userInfo = userInfo; this.oidc = oidc; this.config = config; + this.logger = logger; this.offlineAccess = offlineAccess; } @@ -63,6 +70,7 @@ export class OidcService { userInfo: UserInfoDatabase; oidc: OidcDatabase; config: RootConfigService; + logger: LoggerService; offlineAccess?: OfflineAccessService; }) { return new OidcService( @@ -72,6 +80,7 @@ export class OidcService { options.userInfo, options.oidc, options.config, + options.logger, options.offlineAccess, ); } @@ -509,10 +518,18 @@ export class OidcService { let refreshToken: string | undefined; const scopes = session.scope?.split(' ') ?? []; if (scopes.includes('offline_access') && this.offlineAccess) { - refreshToken = await this.offlineAccess.issueRefreshToken({ - userEntityRef: session.userEntityRef, - oidcClientId: session.clientId, - }); + try { + refreshToken = await this.offlineAccess.issueRefreshToken({ + userEntityRef: session.userEntityRef, + oidcClientId: session.clientId, + }); + } catch (err) { + // Don't fail the entire token exchange if refresh token issuance fails. + // The access token is still valid and should be returned. + this.logger.warn( + `Failed to issue refresh token for user ${session.userEntityRef}, offline_access will not be available: ${err}`, + ); + } } return {