Repository navigation
feat(oidc): accept a function of the name as providers - #52
Open
kamilmysliwiec wants to merge 1 commit into
Open
kamilmysliwiec wants to merge 1 commit into
kamilmysliwiec wants to merge 1 commit into
Conversation
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
This was referenced Oct 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Closes #48
oidc.providersis a record read at startup. An app that creates providers at runtime (one OIDC client per customer, stored in its database) has to restart to add one.The workaround in the issue, adding entries to the record at runtime, only half works.
OidcServicecaches anOidcClientper name with the config it was first made from. So a new tenant works, but a rotated secret never takes effect: the cached client keeps sending the old one, and every sign-in fails with 401. Entries added this way also skip the startup checks.What is the new behavior?
oidc.providersalso accepts a function of the provider's name,(name) => OidcProviderConfig | null | undefined | Promise<…>:start()andfinish()each call it. The config is looked up again on the callback, not stored with the login, as withcallbackUrlin feat(oidc): accept a function of the request as callbackUrl #44, soOidcStateStorekeeps its shape. A secret rotated during a login is used on that login's callback.finish()asks only after the state has matched the browser's cookie and the transaction has been consumed, so a forged callback doesn't trigger a lookup.undefinedornullanswers 404, as an unknown name does today.clientId, andclientSecretunlesstokenEndpointAuthMethod: 'none'). A failure throws an error naming the function and the provider. The record form is still checked at startup.OidcClientkept two caches: the discovery document, which depends only on the issuer, and theJwksClient, which depends only on the key set's URI. Neither depends on the client id or secret. They now live in anOidcMetadataCachethat the service shares, keyed by issuer and by URI. A client is made for each call, which is cheap, so nothing needs invalidating:JwksClientfor each login would have dropped it.Why
(name)and not(name, request): identities are linked by(profile.provider, subject), andprofile.provideris the name. That's only safe while one name stands for one client of one IdP. With the request as an argument, an app could reuse one name (sayokta) for every customer's IdP and pick the tenant from the host. Two customers' IdPs could then issue the samesub, andresolveUser()couldn't tell them apart. The issue's example keys by name only, and host checks already fit in the controller or thecallbackUrlfunction. If a second argument is ever needed, adding it won't break anyone.Why a function and not
register()/unregister(): a registration call reaches only the instance it runs on. A function that reads the database answers the same on every instance.The option's doc comment covers naming (put the customer in the name) and two hazards that come with providers customers bring themselves:
email_verifiedfor any address, so linking by email has to stay within that customer's accounts.assertSecureUrlallows loopbackhttp, andhttpsto internal hosts, so it doesn't keep a fetch off the app's own network. Its comment is reworded to say so, since it no longer holds that these URLs never come from users.Does this PR introduce a breaking change?
The record form behaves as before. Two type-level edges:
OidcService.client(name)is now private and async. It was public but undocumented, and returnedOidcClient, which the package doesn't export.OidcOptions['providers']is now a union, so code that reads the option as a record needs to narrow it.Other information
Tests in
tests/oidc.e2e.spec.ts, on Express and Fastify, with the mock IdP extended to more clients (one per tenant) and a discovery counter:client_id, andprofile.provideris the tenant's name.__proto__included, and so does the callback of a login whose tenant was removed mid-login.clientSecretis refused with an error naming the function, and a public client without one is accepted.I checked that the tests catch regressions:
One behavior difference: the discovery cache now holds the document as published. Overrides and the https checks on its endpoints apply per client, on each call. So a document with a bad endpoint stays cached for
discoveryTtl, where before each login refetched it. A provider that fixes its document is picked up after the TTL.Memory: the cache has one entry per distinct issuer and key set URI that the function has returned. Nothing evicts them.
Docs: a section for docs.nestjs.com (
content/security/authentication.md) will follow as a PR there.