chore: start a refactor of the existing oidc implementation

Signed-off-by: benjdlambert <ben@blam.sh>
This commit is contained in:
benjdlambert
2025-07-01 15:09:47 +02:00
parent ab539cc72b
commit 2d1ebc1e25
11 changed files with 111 additions and 61 deletions
@@ -17,7 +17,8 @@
import { resolvePackagePath } from '@backstage/backend-plugin-api';
import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils';
import { Knex } from 'knex';
import { UserInfoDatabaseHandler } from './UserInfoDatabaseHandler';
import { UserInfoDatabase } from './UserInfoDatabase';
import { AuthDatabase } from './AuthDatabase';
const migrationsDir = resolvePackagePath(
'@backstage/plugin-auth-backend',
@@ -26,7 +27,7 @@ const migrationsDir = resolvePackagePath(
jest.setTimeout(60_000);
describe('UserInfoDatabaseHandler', () => {
describe('UserInfoDatabase', () => {
const databases = TestDatabases.create();
async function createDatabaseHandler(databaseId: TestDatabaseId) {
@@ -38,7 +39,11 @@ describe('UserInfoDatabaseHandler', () => {
return {
knex,
dbHandler: new UserInfoDatabaseHandler(knex),
dbHandler: await UserInfoDatabase.create({
database: AuthDatabase.create({
getClient: async () => knex,
}),
}),
};
}
@@ -46,7 +51,7 @@ describe('UserInfoDatabaseHandler', () => {
'should support database %p',
databaseId => {
let knex: Knex;
let dbHandler: UserInfoDatabaseHandler;
let dbHandler: UserInfoDatabase;
beforeEach(async () => {
({ knex, dbHandler } = await createDatabaseHandler(databaseId));
@@ -17,7 +17,8 @@
import { DateTime } from 'luxon';
import { Knex } from 'knex';
import { BackstageTokenPayload } from './TokenFactory';
import { BackstageTokenPayload } from '../identity/TokenFactory';
import { AuthDatabase } from './AuthDatabase';
const TABLE = 'user_info';
@@ -31,8 +32,8 @@ type UserInfo = {
claims: Omit<BackstageTokenPayload, 'aud' | 'iat' | 'iss' | 'uip'>;
};
export class UserInfoDatabaseHandler {
constructor(private readonly client: Knex) {}
export class UserInfoDatabase {
private constructor(private readonly client: Knex) {}
async addUserInfo(userInfo: UserInfo): Promise<void> {
await this.client<Row>(TABLE)
@@ -59,4 +60,9 @@ export class UserInfoDatabaseHandler {
const userInfo = JSON.parse(info.user_info);
return userInfo;
}
static async create(options: { database: AuthDatabase }) {
const client = await options.database.get();
return new UserInfoDatabase(client);
}
}
@@ -18,7 +18,7 @@ import { createLocalJWKSet, jwtVerify } from 'jose';
import { stringifyEntityRef } from '@backstage/catalog-model';
import { StaticKeyStore } from './StaticKeyStore';
import { mockServices } from '@backstage/backend-test-utils';
import { UserInfoDatabaseHandler } from './UserInfoDatabaseHandler';
import { UserInfo } from '../database/UserInfoDatabase';
import { omit } from 'lodash';
const logger = mockServices.logger.mock();
@@ -29,9 +29,9 @@ const entityRef = stringifyEntityRef({
});
describe('StaticTokenIssuer', () => {
const mockUserInfoDatabaseHandler = {
const mockUserInfo = {
addUserInfo: jest.fn().mockResolvedValue(undefined),
} as unknown as UserInfoDatabaseHandler;
} as unknown as UserInfo;
const staticKeyStore = {
listKeys: () => {
@@ -85,7 +85,7 @@ describe('StaticTokenIssuer', () => {
logger,
issuer: 'my-issuer',
sessionExpirationSeconds: keyDurationSeconds,
userInfoDatabaseHandler: mockUserInfoDatabaseHandler,
UserInfo: mockUserInfo,
},
staticKeyStore as unknown as StaticKeyStore,
);
@@ -119,7 +119,7 @@ describe('StaticTokenIssuer', () => {
expect(verifyResult.payload.exp).toBe(
verifyResult.payload.iat! + keyDurationSeconds,
);
expect(mockUserInfoDatabaseHandler.addUserInfo).toHaveBeenCalledWith({
expect(mockUserInfo.addUserInfo).toHaveBeenCalledWith({
claims: omit(verifyResult.payload, ['aud', 'iat', 'iss', 'uip']),
});
});
@@ -131,7 +131,7 @@ describe('StaticTokenIssuer', () => {
logger,
issuer: 'my-issuer',
sessionExpirationSeconds: keyDurationSeconds,
userInfoDatabaseHandler: mockUserInfoDatabaseHandler,
UserInfo: mockUserInfo,
omitClaimsFromToken: ['ent'],
},
staticKeyStore as unknown as StaticKeyStore,
@@ -165,7 +165,7 @@ describe('StaticTokenIssuer', () => {
expect(verifyResult.payload.exp).toBe(
verifyResult.payload.iat! + keyDurationSeconds,
);
expect(mockUserInfoDatabaseHandler.addUserInfo).toHaveBeenCalledWith({
expect(mockUserInfo.addUserInfo).toHaveBeenCalledWith({
claims: {
...omit(verifyResult.payload, ['aud', 'iat', 'iss', 'uip']),
ent: [entityRef],
@@ -22,7 +22,7 @@ import {
BackstageSignInResult,
TokenParams,
} from '@backstage/plugin-auth-node';
import { UserInfoDatabaseHandler } from './UserInfoDatabaseHandler';
import { UserInfoDatabase } from '../database/UserInfoDatabase';
import { issueUserToken } from './issueUserToken';
export type Config = {
@@ -42,7 +42,7 @@ export type Options = {
* A list of claims to omit from issued tokens and only store in the user info database
*/
omitClaimsFromToken?: string[];
userInfoDatabaseHandler: UserInfoDatabaseHandler;
userInfo: UserInfoDatabase;
};
/**
@@ -55,7 +55,7 @@ export class StaticTokenIssuer implements TokenIssuer {
private readonly keyStore: StaticKeyStore;
private readonly sessionExpirationSeconds: number;
private readonly omitClaimsFromToken?: string[];
private readonly userInfoDatabaseHandler: UserInfoDatabaseHandler;
private readonly userInfo: UserInfoDatabase;
public constructor(options: Options, keyStore: StaticKeyStore) {
this.issuer = options.issuer;
@@ -63,7 +63,7 @@ export class StaticTokenIssuer implements TokenIssuer {
this.sessionExpirationSeconds = options.sessionExpirationSeconds;
this.keyStore = keyStore;
this.omitClaimsFromToken = options.omitClaimsFromToken;
this.userInfoDatabaseHandler = options.userInfoDatabaseHandler;
this.userInfo = options.userInfo;
}
public async issueToken(params: TokenParams): Promise<BackstageSignInResult> {
@@ -76,7 +76,7 @@ export class StaticTokenIssuer implements TokenIssuer {
logger: this.logger,
omitClaimsFromToken: this.omitClaimsFromToken,
params,
userInfoDatabaseHandler: this.userInfoDatabaseHandler,
userInfo: this.userInfo,
});
}
@@ -24,7 +24,7 @@ import {
import { omit } from 'lodash';
import { MemoryKeyStore } from './MemoryKeyStore';
import { TokenFactory } from './TokenFactory';
import { UserInfoDatabaseHandler } from './UserInfoDatabaseHandler';
import { UserInfoDatabaseHandler } from '../database/UserInfoDatabase';
import { tokenTypes } from '@backstage/plugin-auth-node';
import { mockServices } from '@backstage/backend-test-utils';
@@ -25,7 +25,7 @@ import {
} from '@backstage/plugin-auth-node';
import { AnyJWK, KeyStore, TokenIssuer } from './types';
import { JsonValue } from '@backstage/types';
import { UserInfoDatabaseHandler } from './UserInfoDatabaseHandler';
import { UserInfoDatabase } from '../database/UserInfoDatabase';
import { issueUserToken } from './issueUserToken';
/**
@@ -92,7 +92,7 @@ type Options = {
* A list of claims to omit from issued tokens and only store in the user info database
*/
omitClaimsFromToken?: string[];
userInfoDatabaseHandler: UserInfoDatabaseHandler;
userInfo: UserInfoDatabase;
};
/**
@@ -116,7 +116,7 @@ export class TokenFactory implements TokenIssuer {
private readonly keyDurationSeconds: number;
private readonly algorithm: string;
private readonly omitClaimsFromToken?: string[];
private readonly userInfoDatabaseHandler: UserInfoDatabaseHandler;
private readonly userInfo: UserInfoDatabase;
private keyExpiry?: Date;
private privateKeyPromise?: Promise<JWK>;
@@ -128,7 +128,7 @@ export class TokenFactory implements TokenIssuer {
this.keyDurationSeconds = options.keyDurationSeconds;
this.algorithm = options.algorithm ?? 'ES256';
this.omitClaimsFromToken = options.omitClaimsFromToken;
this.userInfoDatabaseHandler = options.userInfoDatabaseHandler;
this.userInfo = options.userInfo;
}
async issueToken(params: TokenParams): Promise<BackstageSignInResult> {
@@ -141,7 +141,7 @@ export class TokenFactory implements TokenIssuer {
logger: this.logger,
omitClaimsFromToken: this.omitClaimsFromToken,
params,
userInfoDatabaseHandler: this.userInfoDatabaseHandler,
userInfo: this.userInfo,
});
}
@@ -22,7 +22,7 @@ import {
tokenTypes,
} from '@backstage/plugin-auth-node';
import { omit } from 'lodash';
import { UserInfoDatabaseHandler } from './UserInfoDatabaseHandler';
import { UserInfoDatabase } from '../database/UserInfoDatabase';
import { LoggerService } from '@backstage/backend-plugin-api';
import { GeneralSign, importJWK, JWK, KeyLike, SignJWT } from 'jose';
import { BackstageTokenPayload } from './TokenFactory';
@@ -37,7 +37,7 @@ export async function issueUserToken({
logger,
omitClaimsFromToken,
params,
userInfoDatabaseHandler,
userInfo,
}: {
issuer: string;
key: JWK;
@@ -45,7 +45,7 @@ export async function issueUserToken({
logger: LoggerService;
omitClaimsFromToken?: string[];
params: TokenParams;
userInfoDatabaseHandler: UserInfoDatabaseHandler;
userInfo: UserInfoDatabase;
}): Promise<BackstageSignInResult> {
const { sub, ent = [sub], ...additionalClaims } = params.claims;
const aud = tokenTypes.user.audClaim;
@@ -111,7 +111,7 @@ export async function issueUserToken({
// Store the user info in the database upon successful token
// issuance so that it can be retrieved later by limited user tokens
await userInfoDatabaseHandler.addUserInfo({
await userInfo.addUserInfo({
claims: omit(claims, ['aud', 'iat', 'iss', 'uip']),
});
@@ -22,19 +22,19 @@ import { mockServices, startTestBackend } from '@backstage/backend-test-utils';
import Router from 'express-promise-router';
import request from 'supertest';
import { bindOidcRouter } from './router';
import { UserInfoDatabaseHandler } from './UserInfoDatabaseHandler';
import { UserInfoDatabase } from '../database/UserInfoDatabase';
describe('bindOidcRouter', () => {
it('should return user info for full tokens', async () => {
const auth = mockServices.auth.mock();
const mockUserInfoDatabaseHandler = {
const mockUserInfo = {
getUserInfo: jest.fn().mockResolvedValue({
claims: {
sub: 'k/ns:n',
ent: ['k/ns:a', 'k/ns:b'],
},
}),
} as unknown as UserInfoDatabaseHandler;
} as unknown as UserInfoDatabase;
const { server } = await startTestBackend({
features: [
@@ -49,7 +49,7 @@ describe('bindOidcRouter', () => {
baseUrl: 'http://localhost:7000',
auth,
tokenIssuer: {} as any,
userInfoDatabaseHandler: mockUserInfoDatabaseHandler,
userInfo: mockUserInfo,
});
httpRouter.use(router);
httpRouter.addAuthPolicy({
@@ -81,21 +81,19 @@ describe('bindOidcRouter', () => {
},
});
expect(mockUserInfoDatabaseHandler.getUserInfo).toHaveBeenCalledWith(
'k/ns:n',
);
expect(mockUserInfo.getUserInfo).toHaveBeenCalledWith('k/ns:n');
});
it('should return user info for limited tokens', async () => {
const auth = mockServices.auth.mock();
const mockUserInfoDatabaseHandler = {
const mockUserInfo = {
getUserInfo: jest.fn().mockResolvedValue({
claims: {
sub: 'k/ns:n',
ent: ['k/ns:a', 'k/ns:b'],
},
}),
} as unknown as UserInfoDatabaseHandler;
} as unknown as UserInfoDatabase;
const { server } = await startTestBackend({
features: [
@@ -110,7 +108,7 @@ describe('bindOidcRouter', () => {
baseUrl: 'http://localhost:7000',
auth,
tokenIssuer: {} as any,
userInfoDatabaseHandler: mockUserInfoDatabaseHandler,
userInfo: mockUserInfo,
});
httpRouter.use(router);
httpRouter.addAuthPolicy({
@@ -140,8 +138,6 @@ describe('bindOidcRouter', () => {
},
});
expect(mockUserInfoDatabaseHandler.getUserInfo).toHaveBeenCalledWith(
'k/ns:n',
);
expect(mockUserInfo.getUserInfo).toHaveBeenCalledWith('k/ns:n');
});
});
+6 -6
View File
@@ -20,7 +20,7 @@ import { TokenIssuer } from './types';
import { AuthService } from '@backstage/backend-plugin-api';
import { decodeJwt } from 'jose';
import { AuthenticationError, InputError } from '@backstage/errors';
import { UserInfoDatabaseHandler } from './UserInfoDatabaseHandler';
import { UserInfoDatabase } from '../database/UserInfoDatabase';
export function bindOidcRouter(
targetRouter: express.Router,
@@ -28,10 +28,10 @@ export function bindOidcRouter(
baseUrl: string;
auth: AuthService;
tokenIssuer: TokenIssuer;
userInfoDatabaseHandler: UserInfoDatabaseHandler;
userInfo: UserInfoDatabase;
},
) {
const { baseUrl, auth, tokenIssuer, userInfoDatabaseHandler } = options;
const { baseUrl, auth, tokenIssuer, userInfo } = options;
const router = Router();
targetRouter.use(router);
@@ -99,12 +99,12 @@ export function bindOidcRouter(
throw new Error('Invalid user token, user entity ref must be a string');
}
const userInfo = await userInfoDatabaseHandler.getUserInfo(userEntityRef);
if (!userInfo) {
const info = await userInfo.getUserInfo(userEntityRef);
if (!info) {
res.status(404).send('User info not found');
return;
}
res.json(userInfo);
res.json(info);
});
}
@@ -0,0 +1,41 @@
/*
* Copyright 2025 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.
*/
import { AuthService } from '@backstage/backend-plugin-api';
import { TokenIssuer } from '../identity/types';
import { UserInfoDatabase } from '../database/UserInfoDatabase';
export class OidcService {
constructor(
private readonly auth: AuthService,
private readonly tokenIssuer: TokenIssuer,
private readonly baseUrl: string,
private readonly userInfo: UserInfoDatabase,
) {}
static create(options: {
auth: AuthService;
tokenIssuer: TokenIssuer;
baseUrl: string;
userInfo: UserInfoDatabase;
}) {
return new OidcService(
options.auth,
options.tokenIssuer,
options.baseUrl,
options.userInfo,
);
}
}
+15 -13
View File
@@ -27,10 +27,9 @@ import {
import { AuthOwnershipResolver } from '@backstage/plugin-auth-node';
import { CatalogService } from '@backstage/plugin-catalog-node';
import { NotFoundError } from '@backstage/errors';
import { bindOidcRouter } from '../identity/router';
import { KeyStores } from '../identity/KeyStores';
import { TokenFactory } from '../identity/TokenFactory';
import { UserInfoDatabaseHandler } from '../identity/UserInfoDatabaseHandler';
import { UserInfoDatabase } from '../database/UserInfoDatabase';
import session from 'express-session';
import connectSessionKnex from 'connect-session-knex';
import passport from 'passport';
@@ -40,6 +39,7 @@ import { TokenIssuer } from '../identity/types';
import { StaticTokenIssuer } from '../identity/StaticTokenIssuer';
import { StaticKeyStore } from '../identity/StaticKeyStore';
import { bindProviderRouters, ProviderFactories } from '../providers/router';
import { OidcService } from './OidcService';
interface RouterOptions {
logger: LoggerService;
@@ -60,7 +60,7 @@ export async function createRouter(
logger,
config,
discovery,
database,
database: db,
tokenFactoryAlgorithm,
providerFactories = {},
} = options;
@@ -70,16 +70,16 @@ export async function createRouter(
const appUrl = config.getString('app.baseUrl');
const authUrl = await discovery.getExternalBaseUrl('auth');
const backstageTokenExpiration = readBackstageTokenExpiration(config);
const authDb = AuthDatabase.create(database);
const database = AuthDatabase.create(db);
const keyStore = await KeyStores.fromConfig(config, {
logger,
database: authDb,
database,
});
const userInfoDatabaseHandler = new UserInfoDatabaseHandler(
await authDb.get(),
);
const userInfo = await UserInfoDatabase.create({
database,
});
const omitClaimsFromToken = config.getOptionalBoolean(
'auth.omitIdentityTokenOwnershipClaim',
@@ -94,7 +94,7 @@ export async function createRouter(
logger: logger.child({ component: 'token-factory' }),
issuer: authUrl,
sessionExpirationSeconds: backstageTokenExpiration,
userInfoDatabaseHandler,
userInfo,
omitClaimsFromToken,
},
keyStore as StaticKeyStore,
@@ -108,7 +108,7 @@ export async function createRouter(
algorithm:
tokenFactoryAlgorithm ??
config.getOptionalString('auth.identityTokenAlgorithm'),
userInfoDatabaseHandler,
userInfo,
omitClaimsFromToken,
});
}
@@ -126,7 +126,7 @@ export async function createRouter(
cookie: { secure: enforceCookieSSL ? 'auto' : false },
store: new KnexSessionStore({
createtable: false,
knex: await authDb.get(),
knex: await database.get(),
}),
}),
);
@@ -148,13 +148,15 @@ export async function createRouter(
auth: options.auth,
});
bindOidcRouter(router, {
const oidcService = OidcService.create({
auth: options.auth,
tokenIssuer,
baseUrl: authUrl,
userInfoDatabaseHandler,
userInfo,
});
router.use(oidcService.getRouter());
// Gives a more helpful error message than a plain 404
router.use('/:provider/', req => {
const { provider } = req.params;