Repository navigation
fix(oauth2,oidc): authorization-server and logout hardening - #78
Merged
Merged
Conversation
authorize and client_credentials used the client's registered scope instead of the server's advertised list whenever the client declared one, so a client registering scope "admin" could be granted it. The device authorization endpoint did not validate the requested scope at all. Every grant now goes through one scopeCeiling() helper: the intersection of client.scope and config.scopes (either may be absent).
A registrant could ask for any grant_types and any scope: the server stored them as-is, so an initial-access-token holder could register a client_credentials client with scope "admin superuser" and mint tokens. Registrant-supplied values are now checked: - grant_types against registration.allowedGrants (default authorization_code + refresh_token) - scope against the server's advertised scopes - token_endpoint_auth_method against registration.allowedAuthMethods (default every non-mTLS method) Anything else is invalid_client_metadata. The deployment's own registration.defaults stay trusted.
defineClient only checked that each redirect URI was a non-empty string, so a dynamic registration could register javascript:, plain http or a fragment-bearing URI. Because authorize redirects its errors to a registered URI, that made the error path an open redirect. A redirect URI must now be absolute and fragment-free, and use https, http on a loopback host, or a reverse-domain private-use scheme (RFC 8252 §7.1).
authorize decided whether a request came through PAR by reading
params._fromPar, but the same object held the raw query string, form
body and request-object claims. Sending ?_fromPar=1 skipped a PAR
requirement (security.par.required, requirePushedAuthorizationRequests,
FAPI 2.0) and returned a code.
resolveRequestParams now returns { params, fromPar } and the flag never
lives in the parameters. Also:
- request_uri must be redeemed with the client_id that pushed it
(RFC 9126 §4); a missing or different client_id is invalid_request
- a request object pushed to PAR is verified at push time and its
claims are stored, instead of storing the unverified JWT
The adapters read getPeerCertificate() but never socket.authorized, and client auth trusted the subject DN string alone. A listener run with rejectUnauthorized:false (which self_signed_tls_client_auth needs) therefore accepted any self-signed certificate carrying the registered subject. The DN was also built by joining unescaped values, so CN=foo,O=Bank and CN="foo,O=Bank" compared equal. - normalizeRequest exposes clientCertificateAuthorized; the express and fastify adapters set it from socket.authorized, and tls_client_auth refuses a certificate whose chain the TLS layer did not verify - the registered DN is parsed as RFC 4514 and compared attribute by attribute with the certificate subject (order-insensitive) - self_signed_tls_client_auth compares thumbprints in constant time
The Redis record store rewrote an updated record with a plain SET when
no ttlMs was passed, which clears the key's expiry. oauth2's refresh
rotation calls update(old, { used: true }) that way, so every rotated
refresh token (and its reuse-detection marker) became immortal.
A TTL store now rewrites with SET … KEEPTTL (dispatched per driver
dialect) unless the caller passes a new TTL.
The refresh grant built the successor record without expiresAt, so the first rotation produced a token that never expired: the memory store treated it as valid forever and the Redis store saved it with no TTL. The successor now inherits the family's absolute expiry (a record written before expiries were carried gets now + refreshTokenTtl). Covered for the grant, the fake Redis client (which now honours PX / KEEPTTL like real Redis) and the live Redis suite on both drivers.
SET … PX … NX dispatched per driver dialect, resolving true only for the caller that created the key — a cross-process compare-and-set for single-winner claims. Adds unit coverage for the dialect dispatch of setIfAbsent and setKeepTTL.
The refresh grant ran get → reuse check → rotate as separate steps, so two simultaneous redemptions of the same token both passed the check and forked the family into two live branches. rotate() is now a compare-and-set that resolves false when the token was already claimed: the memory store flips used in one synchronous step, the Redis store claims a per-token key with SET NX that lives as long as the family. The grant rotates before issuing anything and treats a lost race as reuse (the family is revoked). Custom stores that resolve nothing keep working as before. The memory refresh store also sweeps expired records in the background instead of keeping them forever.
A refresh request with a narrower scope also narrowed the successor refresh token, permanently losing the rest of the grant. RFC 6749 §6 requires the new refresh token's scope to equal the presented one's; the narrowing now applies to the issued access token only.
The token-exchange grant accepted any active access token: no check that it was issued to (or for) the exchanging client, any audience was accepted, and a DPoP- or certificate-bound subject token came back bound to the exchanger's key instead. Any client allowed the grant could launder a token it had picked up into a fresh one for any audience. - the subject token's client_id or aud must name the exchanger, or its may_act claim must permit it (RFC 8693 §4.4) - a sender-constrained subject token (cnf.jkt / cnf.x5t#S256) needs a request proving the same key or certificate - resource / audience must be listed in tokenExchange.audiences (default: none) - scope also stays within the exchanger's own scope ceiling - certificate-bound exchangers get certificate-bound tokens, and an existing act chain nests under the new actor (RFC 8693 §4.1)
The token endpoint computes cnf.x5t#S256 for a certificate-authenticated client, but only the code and client_credentials grants passed it on. Access tokens from the device-code and refresh grants came out as plain bearer tokens for mTLS clients (RFC 8705 §3).
Every poll (including slow_down) rewrote the device record with a fresh 10-minute TTL, so a device that kept polling kept its code alive indefinitely. The in-memory user_code → device_code index had no TTL at all, and two concurrent polls of an approved code could both get tokens. - the stores stamp expiresAt on save and update without extending it (memory: remaining TTL, Redis: SET … KEEPTTL); the reverse index lives on the same TTL - new optional DeviceStore.redeem() flips approved → redeemed atomically (memory: synchronous check-and-flip, Redis: SET NX claim); the grant uses it when present and falls back to update() for custom stores
resolveClientKeys built a fresh remote JWKS resolver on every call, so the JWKS response cache never survived: each private_key_jwt token request and each unauthenticated /authorize?request= triggered a fetch of a client-chosen URL. Resolvers are now cached per server — remote sets by jwks_uri (bounded LRU), inline sets by client descriptor. When dynamic registration is enabled and no security.allowJwksHost gate is configured, jwks_uri hosts that are loopback, private, link-local or metadata addresses are refused by default.
The DPoP proof, client-assertion and request-object jti caches and the DPoP nonce map were module-level: shared by every createServer in the process, invisible to other nodes, and not injectable. Behind a load balancer a replayed proof or assertion succeeded on another node and dpop.nonce:true broke (a nonce issued by one node was unknown to the next). The nonce map also swept itself O(n) on every issue past 1024. - stores.replay accepts any IncrStore-shaped store (incr(key, ttlMs)); without one each server gets its own in-memory guard - verifyDpopForResource takes an optional replayStore for multi-node resource servers - DPoP nonces are HMAC time-window tokens: every node configured with the same security.dpop.nonceSecret (≥ 32 bytes) issues and accepts the same nonces, with nothing stored or swept
A second redemption of an authorization code was rejected, but the tokens already issued from it stayed valid. RFC 6749 §4.1.2 says they SHOULD be revoked, since a reused code means it was intercepted. After a successful redemption the grant leaves a short-lived marker under the same code key (plain save, so any auth-code store works). A replay finds the marker and revokes the refresh family that redemption started. Consume-first ordering is kept: it is what makes redemption atomic.
A public client authenticates by client_id alone, so anyone who knew a public client's id could use the introspection endpoint to query that client's tokens. Public clients are now refused unless the deployment sets introspection.allowPublicClients.
endSessionHandler decoded id_token_hint without checking its signature and handed the hint's sub to onLogout. Anyone could mint an unsigned token with the OP's iss and any sub, send a victim's browser (or call the endpoint directly) and log that user out everywhere. - the hint is verified against the published JWKS (or the signing key) with the configured alg and issuer; an expired hint is still accepted for logout.idTokenHintMaxAge (default 30d) past exp - an unverifiable hint names no subject; onLogout also gets clientId - a client_id parameter must match the hint's audience - postLogoutRedirectUris may be a client_id → URIs map, checked against the verified hint's client
authorize() forwarded maxAge / acrValues to the OP, but handleCallback never looked at auth_time or acr. An OP that ignored the request (or an attacker replaying an older login through it) passed a stale or weaker authentication as if it met the requirement. handleCallback now requires, when they were requested (per call, or via the new client-level maxAge / acrValues defaults that authorize() also sends): - an auth_time no older than max_age (+ clockTolerance) - an acr from the requested acr_values list Failures throw OidcError AUTHENTICATION_REQUIREMENTS_NOT_MET (401).
checkSessionIframeHtml placed session.cookieName into an inline script with JSON.stringify (which does not escape </script>) and into new RegExp unescaped. A configured name containing </script> broke out of the script; one containing regex metacharacters matched other cookies. The name must now be an RFC 6265 cookie-name token (INVALID_ARGUMENT otherwise) and is regex-escaped before it reaches the RegExp.
The UserInfo endpoint fell back to an access_token query parameter, so tokens could arrive in URLs — and from there in access logs, browser history and Referer headers (RFC 6750 §2.3 discourages the query form). Only the Authorization: Bearer header is accepted now.
buildAuthorization applied caller-supplied params last, so
authorize(id, { params }) could replace state, nonce, redirect_uri,
scope or the PKCE challenge. Anything that lets request input reach
params then detached the authorization request from the flow session
that verifies the callback.
Those keys are now reserved; passing one throws INVALID_ARGUMENT.
In api mode without a secret the login middleware handed the client an unsigned flow session (PKCE verifier, nonce, state, createdAt) and took it back as-is, so the client could rewrite any of it — including the createdAt the freshness check relies on. api mode now requires a secret, and any configured flow-session secret (api secret or web cookie.secret) must be at least 32 bytes.
A custom defineProvider({ kind: 'oidc' }) without an issuer verified
the id_token with issuer undefined, so the iss claim was never checked
— any OP whose keys the JWKS held could mint accepted tokens. Defining
an OIDC provider without an issuer (string or validator) is now an
INVALID_ARGUMENT; every built-in OIDC preset already sets one.
EMAIL_UNVERIFIED fired only when a provider said email_verified: false. Providers that omit the flag — notably Entra ID, whose email claim is mutable and unverified (the nOAuth account-takeover vector) — produced no warning, so apps keyed accounts on an email nobody had verified. The warning now fires unless emailVerified is exactly true.
A Redis-backed stores.replay (INCR with a TTL per id) so DPoP proofs, client assertions and request objects replayed against another node are caught. Covered by the live suite on both drivers.
README and web docs for: the scope ceiling, dynamic-registration limits (allowedGrants / allowedAuthMethods, private jwks_uri refusal), redirect-URI rules, tls_client_auth chain verification, token-exchange subject checks and the tokenExchange.audiences allowlist, compare-and-set refresh/device store contracts, the replay store and shared DPoP nonce secret, reserved authorize params, the api-mode secret requirement, the required OIDC issuer and the broader EMAIL_UNVERIFIED warning.
Logout docs describe id_token_hint verification, idTokenHintMaxAge and per-client post-logout URIs; client docs cover max_age / acr_values enforcement and the AUTHENTICATION_REQUIREMENTS_NOT_MET code; provider and session docs note the header-only UserInfo token and the RFC 6265 cookie-name rule.
This was referenced Oct 24, 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.
What / why
Closes a set of escalation and bypass paths in the authorization server, the OpenID Provider add-on and the RP login flow. Each fix lands with a regression test that reproduces the attack.
Authorization server (
@exortek/oauth2/server)scope∩ the server'sscopesin every grant. Previously a client's ownscopereplaced the server list, so a dynamically registered client could grant itselfadmin.registration.allowedGrants, default code + refresh), and redirect URIs must behttps, loopbackhttpor a private-use scheme without a fragment (closes an open redirect on the authorize error path).?_fromPar=1no longer satisfies a PAR requirement (the flag now travels out of band); arequest_urimust be redeemed by the client that pushed it; a request object pushed to PAR is verified at push time.tls_client_authrequires a TLS-verified chain (socket.authorized) and compares the DN structurally.tokenExchange.audiences.jwks_urihosts are refused by default.stores.replay(+createRedisReplayStore) and stateless HMAC nonces keyed bysecurity.dpop.nonceSecret.OpenID Provider / RP (
@exortek/oidc)id_token_hintis signature-verified before itssubreachesonLogout(an unsigned hint could force any user's logout); per-client post-logout URIs.max_age/acr_valuesare enforced at the callback.RP flow (
@exortek/oauth2)params; api-mode login needs a ≥ 32-byte secret; OIDC providers must declare an issuer;EMAIL_UNVERIFIEDalso fires when the provider omitsemail_verified(Entra).@exortek/sharedgainssetKeepTTL/setIfAbsent, and the Redis record store keeps a key's TTL on update.Which package(s)?
@exortek/oauth2@exortek/oidcType of change
Checklist
yarn verifypasses locally (typecheck + lint + format:check + test), including the Redis integration suites on ioredis and node-redisweb/content/…where relevant)@exortek/oauth2major,@exortek/oidcmajor, each with a migration sectionAdditional notes
Not released on merge — the changesets accumulate and ship together with the remaining hardening PRs.