fix(auth): memory leaks

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
@@ -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');
});
};
@@ -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,
});
@@ -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', () => {
@@ -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 {