From 5f2721b30d99ad7c8c486b372ba7d56f1da97e4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Tue, 13 Oct 2026 11:03:48 +0200 Subject: [PATCH] feat(oidc): accept a function of the name as providers An app whose providers aren't known at startup (one OIDC client per customer, created at runtime) can now pass a function as `oidc.providers`. It is asked on the login and again on the callback, and returns the provider's config, or `undefined` for a 404. Each config it returns gets the credential checks the object form gets at startup. A client is now made for each call, so a rotated secret takes effect on the next one. Discovery documents and key sets moved to a cache the service shares, keyed by issuer and by URI: tenants on one issuer fetch them once, and the key set's refetch cooldown still holds. Closes #48 --- lib/interfaces/oidc.interface.ts | 24 +++++- lib/oidc/oidc.client.ts | 118 +++++++++++++++++--------- lib/oidc/oidc.service.ts | 80 ++++++++++++------ tests/mock-oidc.ts | 7 +- tests/oidc.e2e.spec.ts | 139 +++++++++++++++++++++++++++++++ 5 files changed, 298 insertions(+), 70 deletions(-) diff --git a/lib/interfaces/oidc.interface.ts b/lib/interfaces/oidc.interface.ts index cd21d37..29d97bd 100644 --- a/lib/interfaces/oidc.interface.ts +++ b/lib/interfaces/oidc.interface.ts @@ -51,7 +51,29 @@ export interface OidcOptions extends OidcClientOptions { * domain where it started. */ callbackUrl: string | ((request: OidcRequest) => string | Promise); - providers: Record; + /** + * The providers users can sign in with, by name: the `:provider` of your + * routes, and the `provider` of each profile they sign in with. + * + * Providers that aren't known at startup (one OIDC client per customer, + * created at runtime) come from a function of the name instead, called on + * the login and again on the callback: return the provider's config, or + * `undefined` (or `null`) for a name you don't serve, which answers 404. + * A rotated secret is used from the next call on. Each config it returns + * gets the checks the object form gets at startup. Caching your lookup is + * yours to do; discovery documents and key sets are cached by issuer and + * by URI, whatever the name. + * + * Identities are linked by (provider, subject), so a name must stand for + * one client of one provider: put the customer in the name (`acme`), + * rather than reusing one name for every customer's own provider. A + * provider a customer brings vouches for whatever addresses it likes, as + * verified ones too: link by email only to that customer's accounts, and + * check an issuer they enter against the providers you support. + */ + providers: + | Record + | ((name: string) => OidcProviderConfig | null | undefined | Promise); /** Where to land after sign-in when the login had no `redirectTo`. Default `/`. */ redirectAfterLogin?: string; /** Lifetime of a login between the redirect to the provider and the callback. Default `'10m'`. */ diff --git a/lib/oidc/oidc.client.ts b/lib/oidc/oidc.client.ts index 9f5451b..4316b05 100644 --- a/lib/oidc/oidc.client.ts +++ b/lib/oidc/oidc.client.ts @@ -22,8 +22,10 @@ const LOOPBACK = new Set(['localhost', '127.0.0.1', '[::1]']); /** * Only https, except loopback http (local IdPs and tests). The URLs come from the provider's - * discovery document or the app's configuration, never from the user: a bad one is a - * misconfiguration (`unavailable`, a 502), not a failed sign-in. + * discovery document or the app's configuration, never from the request: a bad one is a + * misconfiguration (`unavailable`, a 502), not a failed sign-in. Nor does this keep a fetch off + * the app's own network: an issuer that customers enter themselves is the app's to check + * against the providers it supports, before it stores it. */ function assertSecureUrl(value: string | undefined, what: string): string { let url: URL; @@ -39,6 +41,74 @@ function assertSecureUrl(value: string | undefined, what: string): string { return url.toString(); } +/** A request to the provider: no redirects, a 10 s timeout, and a network failure as an outage. */ +async function send(options: OidcClientOptions, url: string, init: RequestInit) { + try { + return await (options.fetch ?? fetch)(url, { ...init, redirect: 'error', signal: AbortSignal.timeout(10_000) }); + } catch (error) { + throw new OidcError(`${new URL(url).host} is unreachable (${(error as Error).message})`, 'unavailable'); + } +} + +/** + * What the clients of one `OidcService` share: discovery documents, by + * issuer, and key sets, by URI. Neither depends on the client id or secret, + * so a client made anew for each login (`oidc.providers` as a function) + * still fetches them once per TTL, and the key set's cooldown still holds. + */ +export class OidcMetadataCache { + private readonly documents = new Map; at: number }>(); + private readonly keySets = new Map(); + + constructor(private readonly options: OidcClientOptions = {}) {} + + /** The discovery document of `issuer`, as published: the endpoints a config overrides are its own to merge. */ + document(issuer: string): Promise { + const ttl = durationOr(this.options.discoveryTtl, '1h'); + let cached = this.documents.get(issuer); + if (!cached || this.now() - cached.at >= ttl) { + const entry = { value: this.discover(issuer), at: this.now() }; + this.documents.set(issuer, entry); + // Retry next time, unless a later fetch has taken its place. + entry.value.catch(() => this.documents.get(issuer) === entry && this.documents.delete(issuer)); + cached = entry; + } + + return cached.value; + } + + /** The key set at `uri`, an https URL. */ + jwks(uri: string): JwksClient { + let jwks = this.keySets.get(uri); + if (!jwks) { + jwks = new JwksClient(uri, { ...this.options.jwks, fetch: this.options.fetch, now: this.options.now }); + this.keySets.set(uri, jwks); + } + + return jwks; + } + + private async discover(issuer: string): Promise { + const url = `${issuer.replace(/\/$/, '')}/.well-known/openid-configuration`; + const response = await send(this.options, assertSecureUrl(url, 'issuer'), { headers: { accept: 'application/json' } }); + if (!response.ok) { + throw new OidcError(`discovery failed (${response.status})`, 'unavailable'); + } + + const doc = (await response.json().catch(() => ({}))) as Endpoints; + // OIDC Discovery §4.3: the document must name exactly the issuer we asked. + if (doc.issuer !== issuer) { + throw new OidcError('discovery issuer mismatch', 'unavailable'); + } + + return doc; + } + + private now() { + return this.options.now?.() ?? Date.now(); + } +} + /** * One OAuth 2.0 / OpenID Connect provider: authorization code flow with * PKCE (S256), `state` and `nonce`. @@ -49,14 +119,13 @@ function assertSecureUrl(value: string | undefined, what: string): string { * userinfo's `sub` matches the ID token's (OIDC Core §5.3.2). */ export class OidcClient { - private discovery?: { value: Promise; at: number }; - private jwks?: JwksClient; readonly kind: 'oidc' | 'oauth2'; constructor( readonly name: string, private readonly config: OidcProviderConfig, private readonly options: OidcClientOptions = {}, + private readonly metadata = new OidcMetadataCache(options), ) { this.kind = config.kind ?? 'oidc'; @@ -176,14 +245,8 @@ export class OidcClient { } private async verifyIdToken(endpoints: Endpoints, idToken: string, nonce: string): Promise { - this.jwks ??= new JwksClient(assertSecureUrl(endpoints.jwks_uri, 'jwks_uri'), { - ...this.options.jwks, - fetch: this.options.fetch, - now: this.options.now, - }); - const verifier = new JwtVerifier({ - jwks: this.jwks, + jwks: this.metadata.jwks(assertSecureUrl(endpoints.jwks_uri, 'jwks_uri')), algorithms: this.config.idTokenAlgorithms ?? ['RS256', 'ES256'], issuer: endpoints.issuer, audience: this.config.clientId, @@ -282,30 +345,7 @@ export class OidcClient { }; } - const ttl = durationOr(this.options.discoveryTtl, '1h'); - if (!this.discovery || this.now() - this.discovery.at >= ttl) { - const value = this.discover(); - this.discovery = { value, at: this.now() }; - value.catch(() => (this.discovery = undefined)); // retry next time - } - - return this.discovery.value; - } - - private async discover(): Promise { - const issuer = this.config.issuer!; - const url = `${issuer.replace(/\/$/, '')}/.well-known/openid-configuration`; - const response = await this.request(assertSecureUrl(url, 'issuer'), { headers: { accept: 'application/json' } }); - if (!response.ok) { - throw new OidcError(`discovery failed (${response.status})`, 'unavailable'); - } - - const doc = (await response.json().catch(() => ({}))) as Endpoints; - // OIDC Discovery §4.3: the document must name exactly the issuer we asked. - if (doc.issuer !== issuer) { - throw new OidcError('discovery issuer mismatch', 'unavailable'); - } - + const doc = await this.metadata.document(this.config.issuer!); return { ...doc, authorization_endpoint: assertSecureUrl(this.config.authorizationEndpoint ?? doc.authorization_endpoint, 'authorization_endpoint'), @@ -333,12 +373,8 @@ export class OidcClient { return response.json(); } - private async request(url: string, init: RequestInit) { - try { - return await (this.options.fetch ?? fetch)(url, { ...init, redirect: 'error', signal: AbortSignal.timeout(10_000) }); - } catch (error) { - throw new OidcError(`${new URL(url).host} is unreachable (${(error as Error).message})`, 'unavailable'); - } + private request(url: string, init: RequestInit) { + return send(this.options, url, init); } private now() { diff --git a/lib/oidc/oidc.service.ts b/lib/oidc/oidc.service.ts index 12341e7..57a0df0 100644 --- a/lib/oidc/oidc.service.ts +++ b/lib/oidc/oidc.service.ts @@ -24,7 +24,7 @@ import type { OidcTransaction } from '../interfaces/oidc.interface.js'; import { OidcError } from '../errors/oidc.error.js'; import type { OidcProviderConfig } from '../interfaces/oidc-provider.interface.js'; import type { OidcOptions, OidcRequest, OidcRedirect, OidcStartOptions } from '../interfaces/oidc.interface.js'; -import { OidcClient } from './oidc.client.js'; +import { OidcClient, OidcMetadataCache } from './oidc.client.js'; import { OidcAccountResolver } from './oidc-account.resolver.js'; type Headers = Record; @@ -76,7 +76,7 @@ const header = (headers: Headers, name: string) => { */ @Injectable() export class OidcService { - private readonly clients = new Map(); + private readonly metadata: OidcMetadataCache; private readonly options?: OidcOptions; private readonly transactionTtl: number; private readonly txCookieName: string; @@ -95,29 +95,16 @@ export class OidcService { if (typeof this.options.callbackUrl !== 'function') { assertCallbackUrl(this.options.callbackUrl); } - assertProviderCredentials(this.options.providers); + if (typeof this.options.providers !== 'function') { + assertProviderCredentials(this.options.providers); + } } + this.metadata = new OidcMetadataCache(this.options); this.transactionTtl = durationOr(this.options?.transactionTtl, '10m'); this.txCookieName = defaultCookieName('oidc_tx', this.options?.cookie); } - client(name: string): OidcClient { - const { options } = this.feature(); - const config = Object.hasOwn(options.providers, name) ? options.providers[name] : undefined; - if (!config) { - throw new NotFoundException(); - } - - let client = this.clients.get(name); - if (!client) { - client = new OidcClient(name, config, options); - this.clients.set(name, client); - } - - return client; - } - /** * Starts a login: stores the transaction, sets its cookie, and returns * the provider's authorization URL. 404 for a provider that isn't @@ -141,7 +128,8 @@ export class OidcService { linking = { userId: session.userId, sessionId: session.id }; } - const { url, transaction, binding } = await this.client(provider) + const client = await this.client(provider); + const { url, transaction, binding } = await client .authorizationRequest(await this.callbackUrl(provider, req), safeRedirectPath(redirectTo), this.transactionTtl) .catch(toHttp); await this.storage.oidcStates.saveOidcState({ ...transaction, ...(linking && { link: linking }) }); @@ -175,7 +163,8 @@ export class OidcService { // Asked again rather than stored with the login: the cookie keeps the callback on the login's host, // and a function that answers otherwise gets the code refused by the provider, not accepted. const callbackUrl = await this.callbackUrl(provider, req); - const result = await this.client(provider).callback(query, transaction, callbackUrl).catch(toHttp); + const client = await this.client(provider); + const result = await client.callback(query, transaction, callbackUrl).catch(toHttp); const location = transaction.redirectTo ?? options.redirectAfterLogin ?? '/'; if (transaction.link) { @@ -234,6 +223,30 @@ export class OidcService { return session && session.mfa !== 'pending' ? session : null; } + /** + * The client of the provider named `name`, made for this call from its + * config: a secret the `oidc.providers` function rotates is the one the + * next call uses. 404 for a provider that isn't configured, or that the + * function doesn't serve. + */ + private async client(name: string): Promise { + const { providers } = this.feature().options; + let config: OidcProviderConfig | null | undefined; + if (typeof providers === 'function') { + config = await providers(name); + if (config) { + assertResolvedCredentials(name, config); + } + } else { + config = Object.hasOwn(providers, name) ? providers[name] : undefined; + } + + if (!config) { + throw new NotFoundException(); + } + return new OidcClient(name, config, this.options, this.metadata); + } + /** `oidc.callbackUrl` for this request, with the provider's name in it. */ private async callbackUrl(provider: string, request: OidcRequest) { const { callbackUrl } = this.feature().options; @@ -273,6 +286,11 @@ function assertCallbackUrl(value: unknown, must = '`oidc.callbackUrl` must be') } } +const CREDENTIALS = { + clientId: 'the client id the provider issued', + clientSecret: "the client secret the provider issued; a public client sets `tokenEndpointAuthMethod: 'none'` instead", +}; + /** * `oidc.providers..clientId`, and `clientSecret` unless the provider * is a public client (`tokenEndpointAuthMethod: 'none'`), checked at @@ -280,14 +298,22 @@ function assertCallbackUrl(value: unknown, must = '`oidc.callbackUrl` must be') * confidential client into a public one, and fail at the token exchange. */ function assertProviderCredentials(providers: Record | undefined) { - requireOption(providers, 'oidc.providers', 'the providers users can sign in with, by name'); + requireOption(providers, 'oidc.providers', 'the providers users can sign in with, by name, or a function of the name'); for (const [name, config] of Object.entries(providers!)) { - requireOption(config?.clientId, `oidc.providers.${name}.clientId`, 'the client id the provider issued'); + requireOption(config?.clientId, `oidc.providers.${name}.clientId`, CREDENTIALS.clientId); if (config.tokenEndpointAuthMethod !== 'none') { - requireOption( - config.clientSecret, - `oidc.providers.${name}.clientSecret`, - "the client secret the provider issued; a public client sets `tokenEndpointAuthMethod: 'none'` instead", + requireOption(config.clientSecret, `oidc.providers.${name}.clientSecret`, CREDENTIALS.clientSecret); + } + } +} + +/** The same checks, on each config the `oidc.providers` function returns. */ +function assertResolvedCredentials(name: string, config: OidcProviderConfig) { + for (const key of ['clientId', 'clientSecret'] as const) { + if (!config[key] && (key === 'clientId' || config.tokenEndpointAuthMethod !== 'none')) { + throw new Error( + `OidcService: the \`oidc.providers\` function returned the provider '${name}' without its \`${key}\` ` + + `(${CREDENTIALS[key]}).`, ); } } diff --git a/tests/mock-oidc.ts b/tests/mock-oidc.ts index 1b5c48e..83c26b5 100644 --- a/tests/mock-oidc.ts +++ b/tests/mock-oidc.ts @@ -40,6 +40,8 @@ export class MockOidcProvider { readonly clientSecret = 's3cr3t:with/special+chars'; /** A public client (PKCE only, no secret). */ readonly publicClientId = 'nest-public-client'; + /** More confidential clients, one per tenant, by id: their secret, which a test may rotate. */ + readonly clients = new Map(); /** How each accepted token request authenticated the client. */ readonly clientAuthentications: ('client_secret_basic' | 'client_secret_post' | 'none')[] = []; issuer = ''; @@ -49,6 +51,7 @@ export class MockOidcProvider { private readonly codes = new Map(); private readonly accessTokens = new Map(); jwksRequests = 0; + discoveryRequests = 0; /** An endpoint that answers 503, as during an outage. */ down?: 'token' | 'userinfo'; @@ -101,6 +104,7 @@ export class MockOidcProvider { switch (`${req.method} ${url.pathname}`) { case 'GET /.well-known/openid-configuration': + this.discoveryRequests++; return { status: 200, body: { @@ -180,7 +184,8 @@ export class MockOidcProvider { : [form.get('client_id') ?? '', form.get('client_secret') ?? undefined]; const method = basic ? 'client_secret_basic' : secret === undefined ? 'none' : 'client_secret_post'; const publicClient = method === 'none' && id === this.publicClientId; - if (!publicClient && (id !== this.clientId || secret !== this.clientSecret)) { + const expected = id === this.clientId ? this.clientSecret : this.clients.get(id); + if (!publicClient && (expected === undefined || secret !== expected)) { return { status: 401, body: { error: 'invalid_client' } }; } this.clientAuthentications.push(method); diff --git a/tests/oidc.e2e.spec.ts b/tests/oidc.e2e.spec.ts index 3aed8a6..9acb47a 100644 --- a/tests/oidc.e2e.spec.ts +++ b/tests/oidc.e2e.spec.ts @@ -21,6 +21,7 @@ import { type AuthenticationEvent, type OidcOptions, type OidcProfile, + type OidcProviderConfig, type OidcRequest, type OidcResolveContext, } from '../lib/index.js'; @@ -598,6 +599,120 @@ describe.each(adapters.map((a) => a.name))('OIDC on several domains, callbackUrl }); }); +describe.each(adapters.map((a) => a.name))('OIDC providers from a function, a client per tenant (%s)', (adapter) => { + // The app's tenants table, which grows and changes while the app runs. + const tenants = new Map(); + const profiles: OidcProfile[] = []; + let app: INestApplication; + const http = () => request(app.getHttpServer()); + + beforeAll(async () => { + @Injectable() + class AnyoneIsAlice extends OidcAccountResolver { + constructor(registry: AuthenticationRegistry) { + super(); + registry.registerHandler('oidc', this); + } + resolveUser(profile: OidcProfile) { + profiles.push(profile); + return { id: 'u1' }; + } + } + + @Module({ + imports: [ + AuthenticationModule.forRoot({ + session: { cookie: { secure: false } }, + password: { logN: 10 }, + oidc: { + callbackUrl: 'http://app.test/auth/oidc/:provider/callback', + cookie: { secure: false }, + providers: async (name) => tenants.get(name), + }, + }), + UsersModule, + SessionAuthModule, + ], + controllers: [MeController, oidcController('auth/oidc')], + providers: [AnyoneIsAlice], + }) + class TenantsModule {} + + app = await createApp(adapter, TenantsModule); + }); + afterAll(() => app.close()); + afterEach(() => { + tenants.clear(); + idp.clients.clear(); + }); + + /** A tenant signs up, or rotates its secret: its client at the IdP, and its row in the app's table. */ + function saveTenant(key: string, secret = `${key}-secret`) { + idp.clients.set(`${key}-client`, secret); + tenants.set(key, { issuer: idp.issuer, clientId: `${key}-client`, clientSecret: secret }); + } + + async function begin(tenant: string) { + const login = await http().get(`/auth/oidc/${tenant}/login`).expect(302); + const params = new URL(login.headers.location).searchParams; + return { location: login.headers.location as string, params, tx: cookie(login, 'oidc_tx')! }; + } + + const finish = (tenant: string, { location, params, tx }: Awaited>) => + http() + .get(`/auth/oidc/${tenant}/callback`) + .set('Cookie', tx) + .query({ code: idp.approve(location, { sub: 'alice-sub' }), state: params.get('state')! }); + + it('signs in with a tenant added after startup, through its own client', async () => { + saveTenant('acme'); + + const login = await begin('acme'); + expect(login.params.get('client_id')).toBe('acme-client'); + expect(login.params.get('redirect_uri')).toBe('http://app.test/auth/oidc/acme/callback'); + + const res = await finish('acme', login).expect(302); + await http().get('/me').set('Cookie', cookie(res, 'sid')!).expect(200, { id: 'u1' }); + expect(profiles.at(-1)).toMatchObject({ provider: 'acme', subject: 'alice-sub' }); + }); + + it('answers 404 for a name the function does not serve, and for a tenant removed during a login', async () => { + await http().get('/auth/oidc/nope/login').expect(404); + await http().get('/auth/oidc/__proto__/login').expect(404); + + saveTenant('gone'); + const login = await begin('gone'); + tenants.delete('gone'); + await finish('gone', login).expect(404); + }); + + it('uses a rotated secret on the callback of a login under way, and on the next logins', async () => { + saveTenant('acme', 'old-secret'); + const underway = await begin('acme'); + + saveTenant('acme', 'new-secret'); + await finish('acme', underway).expect(302); + await finish('acme', await begin('acme')).expect(302); + + // Rotated at the IdP only: the app still sends the secret it has, and the IdP refuses it. + idp.clients.set('acme-client', 'newer-secret'); + await finish('acme', await begin('acme')).expect(401); + }); + + it('fetches the discovery document and the key set once for all the tenants of one issuer', async () => { + saveTenant('acme'); + saveTenant('globex'); + await finish('acme', await begin('acme')).expect(302); + const fetched = { discovery: idp.discoveryRequests, jwks: idp.jwksRequests }; + + await finish('globex', await begin('globex')).expect(302); + saveTenant('acme', 'rotated-secret'); + await finish('acme', await begin('acme')).expect(302); + + expect({ discovery: idp.discoveryRequests, jwks: idp.jwksRequests }).toEqual(fetched); + }); +}); + describe('OidcService', () => { const binding = 'S'.repeat(43); const state = sha256(binding); @@ -669,6 +784,30 @@ describe('OidcService', () => { ); }); + it('asks a providers function on the login and again on the callback, and checks what it returns', async () => { + const mock = { issuer: idp.issuer, clientId: idp.clientId, clientSecret: idp.clientSecret }; + const providers = vi.fn(async (name: string): Promise => (name === 'mock' ? mock : null)); + const { service } = setup({ providers }); + + const login = await service.start('mock', { request: { method: 'GET', headers: {} } }); + const code = idp.approve(login.url, { sub: 'alice-sub' }); + const request = { method: 'GET', headers: { cookie: login.cookies[0].split(';')[0] } }; + await service.finish('mock', { state: new URL(login.url).searchParams.get('state'), code }, { request }); + expect(providers.mock.calls).toEqual([['mock'], ['mock']]); + + await expect(service.start('nope', { request })).rejects.toThrow(NotFoundException); + + providers.mockResolvedValueOnce({ ...mock, clientSecret: '' }); + await expect(service.start('mock', { request })).rejects.toThrow( + "OidcService: the `oidc.providers` function returned the provider 'mock' without its `clientSecret` (the client " + + "secret the provider issued; a public client sets `tokenEndpointAuthMethod: 'none'` instead).", + ); + + providers.mockResolvedValueOnce({ issuer: idp.issuer, clientId: idp.publicClientId, tokenEndpointAuthMethod: 'none' }); + const publicLogin = await service.start('mock', { request }); + expect(new URL(publicLogin.url).searchParams.get('client_id')).toBe(idp.publicClientId); + }); + it('checks each provider’s credentials at startup; a public client says so', () => { const mock = { issuer: idp.issuer, clientId: idp.clientId }; expect(() => setup({ providers: { mock } })).toThrow(