paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(auth): memory leaks

Browse Source 
alb-provider: JWT verification block was wrapped in generic error
that turned 401 to 500 causing clients to retry the login

cimd: cimd clients are not registered in oidc_clients table
so inserting offline sessions for them violates the foreign key
constraint. dropping the fk.

offline: return access token even when refresh token issuing fails.
if the refresh token issue fails for some reason, it will return
500 which will then cause client to retry even it can get valid
access token without refresh token.

closes #33329
Signed-off-by: Hellgren Heikki <heikki.hellgren@op.fi>
This commit is contained in:
Hellgren Heikki
2026-03-17 09:13:39 +02:00
parent c08e4daf24
commit 634ededdc9
7 changed files with 170 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/thirty-steaks-serve.md
+9
View File
@@ -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.
plugins/auth-backend-module-aws-alb-provider/src/authenticator.test.ts
+32 -30
View File
@@ -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');
});
});
plugins/auth-backend-module-aws-alb-provider/src/authenticator.ts
+7 -1
View File
@@ -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,
);
}
},
});
plugins/auth-backend/migrations/20260317120000_drop_offline_sessions_fk.js
+49
View File
@@ -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');
});
};
plugins/auth-backend/src/service/OidcRouter.test.ts
+2
View File
@@ -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,
});
plugins/auth-backend/src/service/OidcService.test.ts
+49 -1
View File
@@ -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', () => {
plugins/auth-backend/src/service/OidcService.ts
+22 -5
View File
@@ -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 {