diff --git a/.changeset/oauth2-server-hardening.md b/.changeset/oauth2-server-hardening.md new file mode 100644 index 00000000..8c8aceb7 --- /dev/null +++ b/.changeset/oauth2-server-hardening.md @@ -0,0 +1,35 @@ +--- +'@exortek/oauth2': major +--- + +Authorization-server and RP hardening. Several defaults are stricter, so this is a major release. + +**Security fixes** + +- Every grant caps the granted scope at the client's registered `scope` ∩ the server's `scopes`; previously a client's own `scope` replaced the server list, and the device endpoint did not validate scope at all. +- Dynamic registration only accepts `grant_types` from `registration.allowedGrants` (default `authorization_code` + `refresh_token`), scopes the server advertises, and auth methods from `registration.allowedAuthMethods` (default: every non-mTLS method). Redirect URIs must be `https`, loopback `http`, or a reverse-domain private-use scheme, with no fragment. +- `?_fromPar=1` no longer bypasses a PAR requirement; `request_uri` must be redeemed with the `client_id` that pushed it, and a request object pushed to PAR is verified at push time. +- `tls_client_auth` requires a chain the TLS layer verified (`socket.authorized`) and compares the subject DN attribute by attribute. +- Rotated refresh tokens keep the family's expiry (they previously never expired), rotation is a compare-and-set (a lost race revokes the family), and a narrowed refresh request no longer narrows the successor token. +- Token exchange checks that the subject token was issued to / for the exchanging client (`client_id`, `aud` or `may_act`), requires the bound key for a DPoP- or certificate-bound subject token, and only exchanges for targets in `tokenExchange.audiences`. +- Device codes keep their original expiry while polled and are redeemed exactly once. +- A replayed authorization code revokes the refresh family it produced; introspection requires a confidential client. +- Client JWKS resolvers are cached per server; with registration enabled, `jwks_uri` hosts on loopback / private / link-local addresses are refused unless `security.allowJwksHost` says otherwise. +- RP: reserved authorize params (`state`, `nonce`, `redirect_uri`, `scope`, PKCE, …) can no longer be overridden via `params`; api-mode login requires a flow-session secret and every such secret must be ≥ 32 bytes; an OIDC provider must declare an `issuer`; `EMAIL_UNVERIFIED` also fires when a provider omits `email_verified`. + +**New** + +- `stores.replay` (any `incr(key, ttlMs)` store) and `createRedisReplayStore(redis)` share DPoP / client-assertion / request-object jti replay checks across nodes; `verifyDpopForResource` accepts a `replayStore`. +- DPoP nonces are stateless; set the same `security.dpop.nonceSecret` (≥ 32 bytes) on every node. +- `DeviceStore.redeem()` (optional) for atomic device-code redemption; `introspection.allowPublicClients`. + +**Migration** + +- List every scope a client may receive in the server's `scopes`. +- If self-registered clients need `client_credentials`, device or token exchange, add them to `registration.allowedGrants`. +- Replace plain-`http` (non-loopback) or single-word custom-scheme redirect URIs. +- Add `tokenExchange: { audiences: [...] }` if you exchange tokens for a `resource` / `audience`. +- On a bare `node:http` listener with `tls_client_auth`, pass `clientCertificateAuthorized: socket.authorized` in the request descriptor. +- Custom refresh stores: `rotate` should resolve `false` when the token was already rotated (resolving nothing keeps the old behaviour). +- Behind a load balancer with `dpop.nonce: true`, configure `security.dpop.nonceSecret`; add `stores.replay` for cross-node replay checks. +- RP: give api-mode login a `secret`, make every flow-session secret ≥ 32 bytes, add an `issuer` to custom OIDC providers, and stop passing reserved keys in `authorize(..., { params })`. diff --git a/.changeset/oidc-logout-auth-requirements.md b/.changeset/oidc-logout-auth-requirements.md new file mode 100644 index 00000000..76c23eba --- /dev/null +++ b/.changeset/oidc-logout-auth-requirements.md @@ -0,0 +1,16 @@ +--- +'@exortek/oidc': major +--- + +OpenID Provider and relying-party hardening. + +- `endSessionHandler` verifies `id_token_hint` against the OP's published JWKS (or signing key) before using its `sub`; previously an unsigned hint could name any user and force their logout. Expired hints are accepted for `logout.idTokenHintMaxAge` (default `'30d'`). A `client_id` parameter must match the hint's audience, `onLogout` also receives `clientId`, and `postLogoutRedirectUris` may be a `{ [clientId]: uris }` map. +- `handleCallback` enforces requested `max_age` (`auth_time`) and `acr_values` (`acr`), throwing the new `AUTHENTICATION_REQUIREMENTS_NOT_MET` (401). `createClient` accepts `maxAge` / `acrValues` defaults that are sent and enforced on every flow. +- The UserInfo endpoint only reads the access token from the `Authorization` header (no `access_token` query parameter). +- `session.cookieName` must be an RFC 6265 token and is regex-escaped in the check-session iframe. + +**Migration** + +- Publish the OP's verification keys via `jwks` (or keep the signing key), and handle `onLogout` calls with `sub: undefined` for unverifiable hints. +- Send UserInfo requests with `Authorization: Bearer`. +- If you pass `maxAge` / `acrValues` to `authorize()` per call, pass the same values to `handleCallback()` (or set them on `createClient`). diff --git a/packages/oauth2/README.md b/packages/oauth2/README.md index f82c45bf..e4008757 100644 --- a/packages/oauth2/README.md +++ b/packages/oauth2/README.md @@ -161,13 +161,28 @@ challenge), PKCE, PAR (RFC 9126), resource indicators (RFC 8707), RAR profile. Resource servers get `verifyDpopForResource` for the `ath` / `cnf.jkt` check. +A few of those are deliberately strict: + +- **Scope ceiling.** Every grant caps the scope at the client's registered + `scope` ∩ the server's `scopes`. +- **Token exchange** only accepts a subject token issued to the exchanging + client, naming it in `aud`, or permitting it via `may_act`; a DPoP- or + certificate-bound subject token needs the same key; and the target must be + listed in `tokenExchange: { audiences: [...] }`. +- **`tls_client_auth`** requires a chain the TLS layer verified. The express / + fastify adapters pass `socket.authorized` through; on a bare `node:http` + listener set `clientCertificateAuthorized: socket.authorized` on the request + descriptor yourself. +- **Redirect URIs** must be `https`, loopback `http`, or a reverse-domain + private-use scheme (`com.example.app:/cb`), with no fragment. + ### Storage (read this before you run two instances) `createServer` defaults every store to memory: authorization codes, PAR -request URIs, refresh families, device codes, and the DPoP replay cache. That -is fine for one process and wrong for two — a code issued on instance A cannot -be redeemed on instance B, and a DPoP proof replayed against a different -instance is not seen as a replay. +request URIs, refresh families, device codes, and the jti replay checks (DPoP +proofs, client assertions, request objects). That is fine for one process and +wrong for two — a code issued on instance A cannot be redeemed on instance B, +and a DPoP proof replayed against a different instance is not seen as a replay. Pass Redis-backed stores for any deployment with more than one process: @@ -178,6 +193,7 @@ import { createRedisRefreshStore, createRedisParStore, createRedisDeviceStore, + createRedisReplayStore, } from '@exortek/oauth2/server'; const server = createServer({ @@ -187,10 +203,18 @@ const server = createServer({ refresh: createRedisRefreshStore(redis), par: createRedisParStore(redis), device: createRedisDeviceStore(redis), + replay: createRedisReplayStore(redis), // or any store with incr(key, ttlMs) }, + // With DPoP nonces on, every instance must share the nonce secret (≥ 32 bytes). + security: { dpop: { nonce: true, nonceSecret: process.env.DPOP_NONCE_SECRET } }, }); ``` +Refresh rotation and device-code redemption are compare-and-set in the +built-in stores: of two simultaneous redemptions exactly one wins (a lost +refresh race is treated as reuse and revokes the family). A custom refresh +store's `rotate` should resolve `false` when the token was already rotated. + `redis` is your own client — `ioredis` or `node-redis`, both supported and both covered by the integration suite. They are peer dependencies, so install whichever you already use. Any object matching the store interface works too, @@ -238,6 +262,12 @@ const server = createServer({ // → 201 { client_id, client_secret, … } ``` +A registrant may only ask for `authorization_code` + `refresh_token` unless you +widen `registration.allowedGrants`, only for scopes the server advertises, and +only for the non-mTLS auth methods (`registration.allowedAuthMethods`). With +registration enabled and no `allowJwksHost`, `jwks_uri` hosts on loopback, +private or link-local addresses are refused. + ## Modules | Subpath | Status | Purpose | diff --git a/packages/oauth2/src/middleware/core.js b/packages/oauth2/src/middleware/core.js index 5423b7ef..6642b29d 100644 --- a/packages/oauth2/src/middleware/core.js +++ b/packages/oauth2/src/middleware/core.js @@ -54,6 +54,8 @@ const DEFAULT_MAX_AGE_SEC = 600; // 10 minutes — matches the default maxAuthAg * @property {string} [attach='oauth'] */ +const MIN_SECRET_BYTES = 32; + /** * @param {LoginConfig} config */ @@ -72,6 +74,18 @@ export function normalizeLoginConfig(config) { if (seal === 'jwe' && !isNonEmptyString(secret)) { throw new OAuth2Error(ErrorCode.INVALID_ARGUMENT, "seal: 'jwe' requires a secret"); } + // api mode hands the flow session (PKCE verifier, nonce, state, + // createdAt) to the client to bring back — unsigned, the client could + // rewrite any of it, including the createdAt the freshness check trusts. + if (mode === 'api' && !isNonEmptyString(secret)) { + throw new OAuth2Error(ErrorCode.INVALID_ARGUMENT, "mode: 'api' requires a secret to sign the flow session"); + } + if (isNonEmptyString(secret) && Buffer.byteLength(secret, 'utf8') < MIN_SECRET_BYTES) { + throw new OAuth2Error( + ErrorCode.INVALID_ARGUMENT, + `the flow-session secret must be at least ${MIN_SECRET_BYTES} bytes`, + ); + } return Object.freeze({ oauth: config.oauth, diff --git a/packages/oauth2/src/providers/_base.js b/packages/oauth2/src/providers/_base.js index edf06e5f..b05eb4c1 100644 --- a/packages/oauth2/src/providers/_base.js +++ b/packages/oauth2/src/providers/_base.js @@ -81,6 +81,11 @@ export function defineProvider(def) { if (!isFunction(def.mapUser)) { throw new OAuth2Error(ErrorCode.INVALID_ARGUMENT, `provider ${def.id} must supply a mapUser function`); } + // An OIDC provider's id_token is verified against its issuer; with none + // configured the `iss` check would silently be skipped. + if (def.kind === 'oidc' && !isNonEmptyString(def.issuer) && !isFunction(def.issuer)) { + throw new OAuth2Error(ErrorCode.INVALID_ARGUMENT, `OIDC provider ${def.id} requires an issuer`); + } return function createProvider(appOptions = {}) { if (!isNonEmptyString(appOptions.clientId)) { @@ -165,6 +170,17 @@ export async function resolveEndpoints(provider) { return ep; } +const RESERVED_AUTHORIZATION_PARAMS = new Set([ + 'response_type', + 'client_id', + 'redirect_uri', + 'state', + 'nonce', + 'scope', + 'code_challenge', + 'code_challenge_method', +]); + /** * Build the authorization-request URL and the flow session. * @@ -219,6 +235,15 @@ export async function buildAuthorization(provider, opts) { params.set(k, v); } for (const [k, v] of Object.entries(opts.params ?? {})) { + if (RESERVED_AUTHORIZATION_PARAMS.has(k)) { + // These carry the flow's security bindings — overriding `state`, + // `nonce`, `redirect_uri` or the PKCE challenge from caller input + // would silently detach the request from the session that checks it. + throw new OAuth2Error( + ErrorCode.INVALID_ARGUMENT, + `authorize params must not set ${JSON.stringify(k)}; it is managed by the flow`, + ); + } params.set(k, v); } @@ -395,9 +420,18 @@ export async function handleCallback(provider, opts) { throw new OAuth2Error(ErrorCode.SUB_MISMATCH, `${provider.id} did not yield a stable subject id`); } - // surface an unverified email rather than trusting it. - if (isNonEmptyString(mapped.email) && mapped.emailVerified === false) { - warnings.push(warn(WarningCode.EMAIL_UNVERIFIED, `${provider.id} reports the email as unverified`)); + // Surface an email that is not positively verified rather than trusting + // it. A provider that simply omits the flag (Entra's mutable `email`, the + // nOAuth account-takeover vector) counts as unverified too. + if (isNonEmptyString(mapped.email) && mapped.emailVerified !== true) { + warnings.push( + warn( + WarningCode.EMAIL_UNVERIFIED, + mapped.emailVerified === false + ? `${provider.id} reports the email as unverified` + : `${provider.id} does not report the email as verified`, + ), + ); } /** @type {NormalizedUser} */ diff --git a/packages/oauth2/src/server/clients.js b/packages/oauth2/src/server/clients.js index 8aab6764..25779880 100644 --- a/packages/oauth2/src/server/clients.js +++ b/packages/oauth2/src/server/clients.js @@ -36,7 +36,7 @@ export const AUTH_METHODS = Object.freeze([ * @typedef {Object} ClientConfig * @property {string} clientId * @property {string} [clientSecret] confidential clients only - * @property {string[]} redirectUris exact-match allowlist (RFC 6749 §3.1.2) + * @property {string[]} redirectUris exact-match allowlist (RFC 6749 §3.1.2); https, loopback http, or a reverse-domain private-use scheme * @property {string[]} [grantTypes] default `['authorization_code','refresh_token']` * @property {string[]} [responseTypes] default `['code']` * @property {string} [tokenEndpointAuthMethod] one of {@link AUTH_METHODS} @@ -76,9 +76,7 @@ export function defineClient(config) { ); } for (const uri of config.redirectUris) { - if (!isNonEmptyString(uri)) { - throw new OAuth2Error(ErrorCode.INVALID_ARGUMENT, `client ${config.clientId} has an invalid redirectUri`); - } + assertRedirectUri(uri, config.clientId); } const authMethod = @@ -132,6 +130,52 @@ export function defineClient(config) { }); } +const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]']); + +/** + * A redirect URI must be absolute with no fragment (RFC 6749 §3.1.2) and use + * a transport that cannot be intercepted (OAuth 2.1 §2.3.1 / RFC 8252): + * `https`, `http` on a loopback host, or a reverse-domain private-use scheme + * such as `com.example.app:/cb`. Anything else — `javascript:`, `data:`, + * plain `http`, a fragment — is refused at definition time, so neither a + * static client nor a dynamic registration can turn the authorize error + * redirect into an open redirect. + * + * @param {unknown} uri + * @param {string} clientId + */ +function assertRedirectUri(uri, clientId) { + const invalid = reason => + new OAuth2Error(ErrorCode.INVALID_ARGUMENT, `client ${clientId} has an invalid redirectUri: ${reason}`); + if (!isNonEmptyString(uri)) { + throw invalid('must be a non-empty string'); + } + let url; + try { + url = new URL(uri); + } catch { + throw invalid(`${JSON.stringify(uri)} is not an absolute URI`); + } + if (uri.includes('#')) { + throw invalid('must not contain a fragment'); + } + const scheme = url.protocol.slice(0, -1); + if (scheme === 'https') { + return; + } + if (scheme === 'http') { + if (LOOPBACK_HOSTS.has(url.hostname)) { + return; + } + throw invalid('http is only allowed for a loopback host'); + } + // RFC 8252 §7.1: a private-use scheme is a reverse domain name. + if (scheme.includes('.')) { + return; + } + throw invalid(`scheme ${JSON.stringify(scheme)} is not allowed`); +} + /** * @typedef {Object} ClientRegistry * @property {(clientId: string) => (Client | undefined) | Promise} getClient diff --git a/packages/oauth2/src/server/grants/authorization-code.js b/packages/oauth2/src/server/grants/authorization-code.js index 557c85de..e1b13ae2 100644 --- a/packages/oauth2/src/server/grants/authorization-code.js +++ b/packages/oauth2/src/server/grants/authorization-code.js @@ -7,6 +7,7 @@ */ import { isNonEmptyString } from '@exortek/shared/predicates'; +import { randomState } from '../../internal/state.js'; import { ProtocolError, ServerError } from '../errors.js'; import { verifyCodeVerifier } from '../security/pkce.js'; import { resolveTokenAudience } from '../security/resource.js'; @@ -29,8 +30,19 @@ export async function authorizationCodeGrant(req, ctx) { throw new ServerError(ProtocolError.INVALID_REQUEST, 'missing code'); } - // Single-use: consuming the code here means a replay finds nothing. + // Single-use: consuming the code here is what makes redemption atomic. + // A redeemed code leaves a short-lived marker under the same key, so a + // replay finds it and revokes the refresh family the code produced + // (RFC 6749 §4.1.2: tokens issued for a reused code SHOULD be revoked). + // Refresh tokens are revocable; self-contained access tokens simply run + // out their (short) lifetime. const record = await config.stores.authCode.consume(code); + if (record?.redeemed === true) { + if (isNonEmptyString(record.familyId)) { + await config.stores.refresh.revokeFamily(record.familyId); + } + throw new ServerError(ProtocolError.INVALID_GRANT, 'authorization code has already been used'); + } if (!record) { throw new ServerError(ProtocolError.INVALID_GRANT, 'authorization code is invalid, expired, or already used'); } @@ -81,8 +93,12 @@ export async function authorizationCodeGrant(req, ctx) { authTime: record.authTime, }); + /** @type {string | undefined} */ + let familyId; if (refreshAllowed(config, client)) { + familyId = randomState(); body.refresh_token = await mintRefreshToken(config, { + familyId, subject: record.subject, clientId: client.clientId, scope: record.scope, @@ -90,6 +106,7 @@ export async function authorizationCodeGrant(req, ctx) { dpopJkt, }); } + await config.stores.authCode.save(code, { redeemed: true, familyId }, config.authorizationCodeTtlMs); return body; } diff --git a/packages/oauth2/src/server/grants/client-credentials.js b/packages/oauth2/src/server/grants/client-credentials.js index b6dd3500..9cc05f0b 100644 --- a/packages/oauth2/src/server/grants/client-credentials.js +++ b/packages/oauth2/src/server/grants/client-credentials.js @@ -8,6 +8,7 @@ import { isArray, isNonEmptyString } from '@exortek/shared/predicates'; import { ProtocolError, ServerError } from '../errors.js'; import { resolveTokenAudience } from '../security/resource.js'; +import { assertWithinCeiling, parseScope, scopeCeiling } from '../security/scope.js'; import { buildAccessResponse } from './_issue.js'; /** @@ -40,10 +41,11 @@ export async function clientCredentialsGrant(req, ctx) { } /** - * A requested scope is validated against the client's registered `scope` (or - * the server's advertised `scopes` when the client declares none). With no - * `scope` requested, the client's registered scope is granted — or an empty - * scope when it registers none; never the full server scope list by default. + * A requested scope is validated against the client's scope ceiling (its + * registered `scope` ∩ the server's advertised `scopes`). With no `scope` + * requested, the client's registered scope (within that ceiling) is granted — + * or an empty scope when it registers none; never the full server scope list + * by default. * * @param {string | undefined} requested * @param {import('../clients.js').Client} client @@ -51,20 +53,11 @@ export async function clientCredentialsGrant(req, ctx) { * @returns {string[]} */ function resolveScope(requested, client, config) { + const ceiling = scopeCeiling(client, config); if (!isNonEmptyString(requested)) { - return isArray(client.scope) ? client.scope : []; - } - const asked = requested.split(/\s+/).filter(Boolean); - const allowed = isArray(client.scope) ? client.scope : config.scopes; - if (isArray(allowed)) { - for (const s of asked) { - if (!allowed.includes(s)) { - throw new ServerError( - ProtocolError.INVALID_SCOPE, - `scope ${JSON.stringify(s)} is not permitted for this client`, - ); - } - } + return isArray(client.scope) ? (ceiling ?? []) : []; } + const asked = parseScope(requested); + assertWithinCeiling(asked, ceiling); return asked; } diff --git a/packages/oauth2/src/server/grants/device-code.js b/packages/oauth2/src/server/grants/device-code.js index 38be9c19..40b08ae4 100644 --- a/packages/oauth2/src/server/grants/device-code.js +++ b/packages/oauth2/src/server/grants/device-code.js @@ -6,7 +6,7 @@ * the code is redeemed exactly once for tokens; a denied or expired * request returns the matching error. */ -import { isNonEmptyString, isNumber } from '@exortek/shared/predicates'; +import { isFunction, isNonEmptyString, isNumber } from '@exortek/shared/predicates'; import { ProtocolError, ServerError } from '../errors.js'; import { buildAccessResponse, refreshAllowed, mintRefreshToken } from './_issue.js'; @@ -66,13 +66,22 @@ export async function deviceCodeGrant(req, ctx) { } // Redeem once: flip to a terminal state so a repeat poll can't re-issue. - await config.stores.device.update(deviceCode, { status: 'redeemed' }); + // The built-in stores do it atomically, so two concurrent polls of an + // approved code cannot both get tokens. + if (isFunction(config.stores.device.redeem)) { + if (!(await config.stores.device.redeem(deviceCode))) { + throw new ServerError(ProtocolError.INVALID_GRANT, 'the device_code has already been used'); + } + } else { + await config.stores.device.update(deviceCode, { status: 'redeemed' }); + } const body = await buildAccessResponse(config, { subject: record.subject, clientId: client.clientId, scope: record.scope, dpopJkt: ctx.dpopJkt, + confirmation: ctx.confirmation, }); if (refreshAllowed(config, client)) { diff --git a/packages/oauth2/src/server/grants/refresh-token.js b/packages/oauth2/src/server/grants/refresh-token.js index 7eb6557a..0defb0bc 100644 --- a/packages/oauth2/src/server/grants/refresh-token.js +++ b/packages/oauth2/src/server/grants/refresh-token.js @@ -5,7 +5,7 @@ * revokes the whole family. Scope may be narrowed but never widened, and * a DPoP-bound token stays bound to the same key. */ -import { isNonEmptyString } from '@exortek/shared/predicates'; +import { isNonEmptyString, isNumber } from '@exortek/shared/predicates'; import { ProtocolError, ServerError } from '../errors.js'; import { randomState } from '../../internal/state.js'; @@ -55,32 +55,43 @@ export async function refreshTokenGrant(req, ctx) { const scope = narrowScope(req.form.scope, record.scope); const audience = resolveTokenAudience(req.form.resource, record.resource, config); - const body = await buildAccessResponse(config, { - subject: record.subject, - clientId: client.clientId, - scope, - audience, - dpopJkt: record.dpopJkt, - }); - - // Rotate: mint the successor in the same family and mark the presented - // token used so a later presentation trips the reuse check above. + // Rotate before issuing anything: mint the successor in the same family + // and claim the presented token. The store's rotate is a compare-and-set, + // so of two simultaneous redemptions of the SAME token exactly one wins; + // the loser is handled exactly like a replay and burns the family. // - // The get→check→rotate sequence assumes a single in-flight redemption per - // token (the normal client behaviour). It is not a compare-and-set: two - // truly-simultaneous redemptions of the SAME token could both pass the - // reuse check before either marks it used. The security guarantee that - // still holds unconditionally is the important one — any later replay of - // the now-rotated token trips reuse detection and burns the family. + // The successor inherits the family's absolute expiry: rotation renews the + // token, not the grant, so a refresh family still dies `refreshTokenTtl` + // after the user authorized it. (A record written before expiries were + // carried gets a fresh one rather than none.) const nextToken = randomState(); - await config.stores.refresh.rotate(presented, { + const rotated = await config.stores.refresh.rotate(presented, { token: nextToken, familyId: record.familyId, clientId: client.clientId, - scope, + // RFC 6749 §6: the new refresh token's scope MUST be identical to the + // presented one's — a narrowed request narrows this access token only. + scope: record.scope, subject: record.subject, resource: record.resource, dpopJkt: record.dpopJkt, + expiresAt: isNumber(record.expiresAt) ? record.expiresAt : Date.now() + config.refreshTokenTtlMs, + }); + if (rotated === false) { + await config.stores.refresh.revokeFamily(record.familyId); + throw new ServerError( + ProtocolError.INVALID_GRANT, + 'refresh token reuse detected; the token family has been revoked', + ); + } + + const body = await buildAccessResponse(config, { + subject: record.subject, + clientId: client.clientId, + scope, + audience, + dpopJkt: record.dpopJkt, + confirmation: ctx.confirmation, }); body.refresh_token = nextToken; diff --git a/packages/oauth2/src/server/grants/token-exchange.js b/packages/oauth2/src/server/grants/token-exchange.js index 66e1a6ef..b4223f08 100644 --- a/packages/oauth2/src/server/grants/token-exchange.js +++ b/packages/oauth2/src/server/grants/token-exchange.js @@ -4,13 +4,20 @@ * and exchanges it for a new access token, optionally scoped down and * re-audienced for a downstream `resource`/`audience`. The subject token * is validated through the issuer strategy; a valid one yields a token - * for the same subject, never a privilege escalation (scope can only - * narrow, audience must be an authorized target). + * for the same subject, never a privilege escalation: + * - the subject token must have been issued to the exchanging client, + * name it in `aud`, or permit it through `may_act` (RFC 8693 §4.4) — + * a token picked up elsewhere cannot be laundered into a fresh one; + * - a sender-constrained subject token (`cnf`) is exchanged only by the + * holder of the bound key / certificate; + * - scope only narrows, and stays within the exchanger's own ceiling; + * - the target audience must be on `tokenExchange.audiences`. */ -import { isNonEmptyString } from '@exortek/shared/predicates'; +import { isArray, isNonEmptyString, isObject } from '@exortek/shared/predicates'; import { ProtocolError, ServerError } from '../errors.js'; import { validateResource } from '../security/resource.js'; +import { assertWithinCeiling, parseScope, scopeCeiling } from '../security/scope.js'; import { buildAccessResponse } from './_issue.js'; const ACCESS_TOKEN_TYPE = 'urn:ietf:params:oauth:token-type:access_token'; @@ -46,9 +53,19 @@ export async function tokenExchangeGrant(req, ctx) { throw new ServerError(ProtocolError.INVALID_GRANT, 'subject_token is not valid'); } - const grantedScope = isNonEmptyString(claims.scope) ? claims.scope.split(/\s+/).filter(Boolean) : []; - const scope = narrowScope(req.form.scope, grantedScope); - const audience = resolveTarget(req.form.resource, req.form.audience); + assertMayExchange(claims, client); + assertHolderOfKey(claims, ctx); + + const grantedScope = parseScope(claims.scope); + const ceiling = scopeCeiling(client, config); + let scope; + if (isNonEmptyString(req.form.scope)) { + scope = narrowScope(req.form.scope, grantedScope); + assertWithinCeiling(scope, ceiling); + } else { + scope = isArray(ceiling) ? grantedScope.filter(s => ceiling.includes(s)) : grantedScope; + } + const audience = resolveTarget(req.form.resource, req.form.audience, config); const body = await buildAccessResponse(config, { subject: claims.sub, @@ -56,8 +73,10 @@ export async function tokenExchangeGrant(req, ctx) { scope, audience, dpopJkt: ctx.dpopJkt, - // RFC 8693 §4.1: record the acting party in the issued token. - extra: { act: { sub: client.clientId } }, + confirmation: ctx.confirmation, + // RFC 8693 §4.1: the current actor is the top-level `act`; any prior + // actor chain from the subject token nests beneath it. + extra: { act: isObject(claims.act) ? { sub: client.clientId, act: claims.act } : { sub: client.clientId } }, }); // RFC 8693 §2.2.1 requires issued_token_type on the response. body.issued_token_type = ACCESS_TOKEN_TYPE; @@ -85,20 +104,75 @@ function narrowScope(requested, granted) { return asked; } +/** + * The exchanging client must be the subject token's own client, one of its + * audiences, or the actor its `may_act` claim authorizes. + * + * @param {Record} claims + * @param {import('../clients.js').Client} client + */ +function assertMayExchange(claims, client) { + const id = client.clientId; + const audiences = isArray(claims.aud) ? claims.aud : [claims.aud]; + const mayAct = isObject(claims.may_act) ? claims.may_act : undefined; + if (claims.client_id === id || audiences.includes(id) || mayAct?.client_id === id || mayAct?.sub === id) { + return; + } + throw new ServerError(ProtocolError.INVALID_GRANT, 'subject_token was not issued to or for this client'); +} + +/** + * A sender-constrained subject token can only be exchanged by a request + * that proves the same key (DPoP `cnf.jkt`, RFC 9449) or certificate + * (`cnf.x5t#S256`, RFC 8705) — otherwise exchanging would strip the + * binding off a stolen token. + * + * @param {Record} claims + * @param {{ dpopJkt?: string, confirmation?: Record }} ctx + */ +function assertHolderOfKey(claims, ctx) { + if (!isObject(claims.cnf)) { + return; + } + const { jkt } = claims.cnf; + const x5t = claims.cnf['x5t#S256']; + if (isNonEmptyString(jkt) && ctx.dpopJkt !== jkt) { + throw new ServerError( + ProtocolError.INVALID_GRANT, + 'subject_token is DPoP-bound to a key this request did not prove', + ); + } + if (isNonEmptyString(x5t) && ctx.confirmation?.['x5t#S256'] !== x5t) { + throw new ServerError( + ProtocolError.INVALID_GRANT, + 'subject_token is bound to a client certificate this request did not present', + ); + } +} + /** * The exchange target — a `resource` URI (RFC 8707) or an `audience` - * identifier (RFC 8693). A malformed `resource` is `invalid_target`. + * identifier (RFC 8693). A malformed `resource` is `invalid_target`, and so + * is any target missing from `tokenExchange.audiences`: without an + * allowlist, a token can only be exchanged for one addressed to the client. * * @param {string | undefined} resource * @param {string | undefined} audience + * @param {Record} config * @returns {string | undefined} */ -function resolveTarget(resource, audience) { +function resolveTarget(resource, audience, config) { + let target; if (isNonEmptyString(resource)) { - return validateResource(resource); + target = validateResource(resource); + } else if (isNonEmptyString(audience)) { + target = audience; + } else { + return undefined; } - if (isNonEmptyString(audience)) { - return audience; + const allowed = config.tokenExchange?.audiences; + if (!isArray(allowed) || !allowed.includes(target)) { + throw new ServerError(ProtocolError.INVALID_TARGET, `exchange target ${JSON.stringify(target)} is not permitted`); } - return undefined; + return target; } diff --git a/packages/oauth2/src/server/handlers/authorize.js b/packages/oauth2/src/server/handlers/authorize.js index 1706fa41..3206fddf 100644 --- a/packages/oauth2/src/server/handlers/authorize.js +++ b/packages/oauth2/src/server/handlers/authorize.js @@ -11,7 +11,7 @@ * handler only once the user has authenticated and approved the request. * A `null` result is treated as the user declining (`access_denied`). */ -import { isArray, isNonEmptyString } from '@exortek/shared/predicates'; +import { isNonEmptyString } from '@exortek/shared/predicates'; import { randomState } from '../../internal/state.js'; import { ProtocolError, ServerError } from '../errors.js'; @@ -21,6 +21,7 @@ import { resolvePkceChallenge } from '../security/pkce.js'; import { validateResource } from '../security/resource.js'; import { validateAuthorizationDetails } from '../security/rar.js'; import { verifyRequestObject, signJarmResponse } from '../security/jar-jarm.js'; +import { assertWithinCeiling, parseScope, scopeCeiling } from '../security/scope.js'; /** * @param {import('../index.js').ResolvedServerConfig & Record} config @@ -32,7 +33,7 @@ export function createAuthorizeHandler(config) { // Untrusted until the client + redirect URI are validated. let redirectUri; try { - const params = await resolveRequestParams(req, config); + const { params, fromPar } = await resolveRequestParams(req, config); const client = await requireClient(params.client_id, config); redirectUri = requireRegisteredRedirectUri(params.redirect_uri, client); @@ -68,7 +69,7 @@ export function createAuthorizeHandler(config) { } } // PAR is required by the server OR by this client's registration. - if ((config.requirePar || client.requirePushedAuthorizationRequests === true) && !params._fromPar) { + if ((config.requirePar || client.requirePushedAuthorizationRequests === true) && !fromPar) { throw new ServerError(ProtocolError.INVALID_REQUEST, 'this client requires a pushed authorization request', { redirectable: true, state, @@ -142,9 +143,14 @@ export function createAuthorizeHandler(config) { * (RFC 9126); the JAR signed-request-object path (RFC 9101) is layered in * `security/jar-jarm.js`. Plain front-channel query params are the base. * + * Whether the params came from PAR is returned out of band (`fromPar`), + * never as a parameter: anything inside `params` is attacker-reachable + * (query string, form body, request object), so a flag carried there could + * be forged to skip a PAR requirement. + * * @param {import('../request.js').ServerRequest} req * @param {Record} config - * @returns {Promise>} + * @returns {Promise<{ params: Record, fromPar: boolean }>} */ async function resolveRequestParams(req, config) { // RFC 6749 §3.1 allows the authorization request over POST too; Apple's @@ -154,17 +160,25 @@ async function resolveRequestParams(req, config) { const requestUri = src.request_uri; if (isNonEmptyString(requestUri)) { + // RFC 9126 §4: client_id is still sent on the front channel and must + // name the client that pushed the request. + if (!isNonEmptyString(src.client_id)) { + throw new ServerError(ProtocolError.INVALID_REQUEST, 'client_id is required alongside request_uri'); + } const pushed = await config.stores.par.consume(requestUri); if (!pushed) { throw new ServerError(ProtocolError.INVALID_REQUEST, 'request_uri is unknown, expired, or already used'); } - return { ...pushed, _fromPar: true }; + if (pushed.client_id !== src.client_id) { + throw new ServerError(ProtocolError.INVALID_REQUEST, 'request_uri was pushed by a different client'); + } + return { params: { ...pushed }, fromPar: true }; } // JAR (RFC 9101): a signed request object supersedes the query params. if (isNonEmptyString(src.request)) { - return verifyRequestObject(src.request, src.client_id, config); + return { params: await verifyRequestObject(src.request, src.client_id, config), fromPar: false }; } - return { ...src }; + return { params: { ...src }, fromPar: false }; } /** @@ -202,9 +216,9 @@ function requireRegisteredRedirectUri(redirectUri, client) { } /** - * Requested scope must be a subset of the server's advertised scopes and, - * when the client registers its own `scope`, of that too (RFC 6749 - * §4.1.2.1). An out-of-bounds scope is `invalid_scope`. + * Requested scope must be a subset of both the server's advertised scopes + * and the client's registered `scope` (RFC 6749 §4.1.2.1). An + * out-of-bounds scope is `invalid_scope`. * * @param {string | undefined} raw * @param {import('../clients.js').Client} client @@ -213,20 +227,8 @@ function requireRegisteredRedirectUri(redirectUri, client) { * @returns {string[]} */ function resolveScope(raw, client, config, state) { - const requested = isNonEmptyString(raw) ? raw.split(/\s+/).filter(Boolean) : []; - // The client's registered scope is the tighter ceiling when present; - // otherwise the server-wide list gates. - const allowed = isArray(client.scope) ? client.scope : config.scopes; - if (isArray(allowed)) { - for (const s of requested) { - if (!allowed.includes(s)) { - throw new ServerError(ProtocolError.INVALID_SCOPE, `scope ${JSON.stringify(s)} is not permitted`, { - redirectable: true, - state, - }); - } - } - } + const requested = parseScope(raw); + assertWithinCeiling(requested, scopeCeiling(client, config), { redirectable: true, state }); return requested; } diff --git a/packages/oauth2/src/server/handlers/device.js b/packages/oauth2/src/server/handlers/device.js index a9cca167..b4dde023 100644 --- a/packages/oauth2/src/server/handlers/device.js +++ b/packages/oauth2/src/server/handlers/device.js @@ -20,6 +20,7 @@ import { randomState } from '../../internal/state.js'; import { normalizeRequest } from '../request.js'; import { json, jsonError } from '../response.js'; import { authenticateClient } from '../security/client-auth.js'; +import { assertWithinCeiling, parseScope, scopeCeiling } from '../security/scope.js'; const DEFAULT_TTL_MS = 600_000; // 10 minutes const DEFAULT_INTERVAL_SEC = 5; @@ -51,7 +52,8 @@ export function createDeviceHandler(config) { throw new ServerError(ProtocolError.UNAUTHORIZED_CLIENT, 'client may not use the device grant'); } - const scope = isNonEmptyString(req.form.scope) ? req.form.scope.split(/\s+/).filter(Boolean) : []; + const scope = parseScope(req.form.scope); + assertWithinCeiling(scope, scopeCeiling(client, config)); const deviceCode = randomState(); const userCode = generateUserCode(); diff --git a/packages/oauth2/src/server/handlers/introspect.js b/packages/oauth2/src/server/handlers/introspect.js index 1765b79f..c9eca32b 100644 --- a/packages/oauth2/src/server/handlers/introspect.js +++ b/packages/oauth2/src/server/handlers/introspect.js @@ -13,6 +13,10 @@ * user identities). Deployments where trusted resource servers must * introspect any client's token opt in with * `introspection: { allowCrossClient: true }`. + * + * A public client authenticates by `client_id` alone, so it is refused + * unless `introspection.allowPublicClients` is set — otherwise anyone who + * knows a public client's id could query its tokens. */ import { isArray, isNonEmptyString, isObject } from '@exortek/shared/predicates'; @@ -36,6 +40,9 @@ export function createIntrospectHandler(config) { } const client = await authenticateClient(req, config); + if (client.isPublic && config.introspection?.allowPublicClients !== true) { + throw new ServerError(ProtocolError.INVALID_CLIENT, 'introspection requires a confidential client'); + } const token = req.form.token; if (!isNonEmptyString(token)) { diff --git a/packages/oauth2/src/server/handlers/par.js b/packages/oauth2/src/server/handlers/par.js index ebf855f2..42b35a5c 100644 --- a/packages/oauth2/src/server/handlers/par.js +++ b/packages/oauth2/src/server/handlers/par.js @@ -19,6 +19,7 @@ import { normalizeRequest } from '../request.js'; import { json, jsonError } from '../response.js'; import { authenticateClient } from '../security/client-auth.js'; import { resolveDpopBinding } from '../security/dpop.js'; +import { verifyRequestObject } from '../security/jar-jarm.js'; const REQUEST_URI_PREFIX = 'urn:ietf:params:oauth:request_uri:'; @@ -56,7 +57,13 @@ export function createParHandler(config) { // (RFC 9449 §10.1) via `dpop_jkt`. const { dpopJkt } = await resolveDpopBinding(req, config, client, { htu: config.endpoints.par }); - const params = collectParams(req.form, client.clientId, dpopJkt); + // RFC 9126 §3: a pushed request may itself be a JAR request object. + // Verify it now, while the client is authenticated, and store the + // parameters it carries — never the unverified JWT. + const source = isNonEmptyString(req.form.request) + ? await verifyRequestObject(req.form.request, client.clientId, config) + : req.form; + const params = collectParams(source, client.clientId, dpopJkt); const requestUri = REQUEST_URI_PREFIX + randomState(); await config.stores.par.save(requestUri, params, config.parTtlMs); @@ -74,7 +81,7 @@ export function createParHandler(config) { } /** - * @param {Record} form + * @param {Record} form * @param {string} clientId * @param {string | undefined} dpopJkt * @returns {Record} @@ -83,7 +90,7 @@ function collectParams(form, clientId, dpopJkt) { /** @type {Record} */ const params = {}; for (const [key, value] of Object.entries(form)) { - if (!CLIENT_AUTH_PARAMS.has(key)) { + if (!CLIENT_AUTH_PARAMS.has(key) && key !== 'request') { params[key] = value; } } diff --git a/packages/oauth2/src/server/handlers/register.js b/packages/oauth2/src/server/handlers/register.js index d30c6aab..bce6e5a5 100644 --- a/packages/oauth2/src/server/handlers/register.js +++ b/packages/oauth2/src/server/handlers/register.js @@ -22,6 +22,21 @@ import { json, jsonError } from '../response.js'; const SECRET_BASED = new Set(['client_secret_basic', 'client_secret_post', 'client_secret_jwt']); +// What a registrant may ask for unless the deployment widens it. The +// client_credentials / device / token-exchange grants act without a user +// in the loop, so a self-registered client gets them only when the +// deployment lists them in `registration.allowedGrants`. +const DEFAULT_ALLOWED_GRANTS = ['authorization_code', 'refresh_token']; +// mTLS methods need key material (subject DN / thumbprint) that RFC 7591 +// metadata here does not carry, so they are registered out of band. +const DEFAULT_ALLOWED_AUTH_METHODS = [ + 'none', + 'client_secret_basic', + 'client_secret_post', + 'client_secret_jwt', + 'private_key_jwt', +]; + /** * @param {Record} config * @returns {(raw: import('../request.js').RawRequest) => Promise} @@ -50,7 +65,7 @@ export function createRegisterHandler(config) { throw new ServerError(ProtocolError.INVALID_CLIENT_METADATA, 'client metadata is required'); } - const clientConfig = buildClientConfig(meta, reg); + const clientConfig = buildClientConfig(meta, reg, config); let client; try { client = config.registry.register(clientConfig); @@ -106,18 +121,46 @@ async function authorizeRegistration(req, reg) { * the deployment's `registration.defaults` under the request. The server owns * the generated `client_id` / `client_secret`. * + * Registrant-supplied `grant_types`, `scope` and `token_endpoint_auth_method` + * are checked against what the deployment allows; the deployment's own + * `defaults` are trusted as configured. + * * @param {Record} meta * @param {Record} reg + * @param {Record} config * @returns {import('../clients.js').ClientConfig} */ -function buildClientConfig(meta, reg) { +function buildClientConfig(meta, reg, config) { const defaults = isObject(reg.defaults) ? reg.defaults : {}; const redirectUris = meta.redirect_uris ?? defaults.redirectUris; - const authMethod = meta.token_endpoint_auth_method ?? defaults.tokenEndpointAuthMethod ?? 'client_secret_basic'; + const requestedAuthMethod = meta.token_endpoint_auth_method; + if (requestedAuthMethod !== undefined) { + const allowed = isArray(reg.allowedAuthMethods) ? reg.allowedAuthMethods : DEFAULT_ALLOWED_AUTH_METHODS; + if (!allowed.includes(requestedAuthMethod)) { + throw new ServerError( + ProtocolError.INVALID_CLIENT_METADATA, + `token_endpoint_auth_method ${JSON.stringify(requestedAuthMethod)} is not permitted for registration`, + ); + } + } + const authMethod = requestedAuthMethod ?? defaults.tokenEndpointAuthMethod ?? 'client_secret_basic'; + + const requestedGrants = toArray(meta.grant_types); + if (requestedGrants) { + const allowed = isArray(reg.allowedGrants) ? reg.allowedGrants : DEFAULT_ALLOWED_GRANTS; + for (const grant of requestedGrants) { + if (!allowed.includes(grant)) { + throw new ServerError( + ProtocolError.INVALID_CLIENT_METADATA, + `grant_type ${JSON.stringify(grant)} is not permitted for registration`, + ); + } + } + } // RFC 7591 §2 / §3.2.2: a client using a redirect-based grant must supply // at least one redirect URI. - const grantTypes = toArray(meta.grant_types) ?? defaults.grantTypes ?? ['authorization_code']; + const grantTypes = requestedGrants ?? defaults.grantTypes ?? ['authorization_code']; const usesRedirect = grantTypes.includes('authorization_code'); if (usesRedirect && (!isArray(redirectUris) || redirectUris.length === 0)) { throw new ServerError(ProtocolError.INVALID_REDIRECT_URI, 'redirect_uris is required for this grant'); @@ -137,7 +180,18 @@ function buildClientConfig(meta, reg) { if (responseTypes) { cfg.responseTypes = responseTypes; } - const scope = isNonEmptyString(meta.scope) ? meta.scope.split(/\s+/).filter(Boolean) : defaults.scope; + const requestedScope = isNonEmptyString(meta.scope) ? meta.scope.split(/\s+/).filter(Boolean) : undefined; + if (requestedScope && isArray(config.scopes)) { + for (const s of requestedScope) { + if (!config.scopes.includes(s)) { + throw new ServerError( + ProtocolError.INVALID_CLIENT_METADATA, + `scope ${JSON.stringify(s)} is not supported by this server`, + ); + } + } + } + const scope = requestedScope ?? defaults.scope; if (isArray(scope)) { cfg.scope = scope; } diff --git a/packages/oauth2/src/server/index.js b/packages/oauth2/src/server/index.js index 50cca383..36906eaf 100644 --- a/packages/oauth2/src/server/index.js +++ b/packages/oauth2/src/server/index.js @@ -28,6 +28,9 @@ import { createDeviceHandler, createDeviceApproval } from './handlers/device.js' import { createRegisterHandler } from './handlers/register.js'; import { applyFapiProfile } from './security/fapi.js'; import { createIdTokenSigner } from './issuers/id-token.js'; +import { createClientKeyResolver } from './security/client-keys.js'; +import { createDpopNonces } from './security/dpop.js'; +import { createReplayGuard, isReplayStore } from './security/replay.js'; export { defineClient, createClientRegistry, AUTH_METHODS } from './clients.js'; export { jwtIssuer } from './issuers/jwt.js'; @@ -40,6 +43,7 @@ export { createRedisRefreshStore, createRedisParStore, createRedisDeviceStore, + createRedisReplayStore, } from './stores/redis.js'; const DEFAULT_GRANTS = ['authorization_code', 'refresh_token', 'client_credentials']; @@ -54,10 +58,10 @@ const DEFAULT_REFRESH_TTL = '30d'; * @typedef {Object} ServerSecurity * @property {string|number} [authorizationCodeTtl='1m'] * @property {string|number} [refreshTokenTtl='30d'] lifetime of an issued refresh token - * @property {{ required?: boolean, algs?: string[], nonce?: boolean }} [dpop] RFC 9449 + * @property {{ required?: boolean, algs?: string[], nonce?: boolean, nonceSecret?: string | Uint8Array }} [dpop] RFC 9449; give every node the same ≥32-byte `nonceSecret` when nonces are on behind a load balancer * @property {{ required?: boolean, ttl?: string|number }} [par] RFC 9126 * @property {boolean} [fapi] FAPI 2.0 profile (bundles PAR + PKCE + DPoP/mTLS + iss) - * @property {(hostname: string, url: URL) => boolean} [allowJwksHost] gate the hosts the server will fetch a client's `jwks_uri` from; return false to refuse. Recommended wherever dynamic client registration is enabled, since the URI is then chosen by the registrant. + * @property {(hostname: string, url: URL) => boolean} [allowJwksHost] gate the hosts the server will fetch a client's `jwks_uri` from; return false to refuse. With dynamic registration enabled and no gate set, loopback / private / link-local hosts are refused by default, since the URI is then chosen by the registrant. * * @typedef {Object} ServerConfig * @property {string} issuer AS issuer identifier (https, no trailing slash) @@ -72,12 +76,15 @@ const DEFAULT_REFRESH_TTL = '30d'; * @property {ServerSecurity} [security] * @property {OidcConfig} [oidc] make this an OpenID Provider (issue id_token on the `openid` scope) * @property {RegistrationConfig} [registration] enable RFC 7591 dynamic client registration (opt-in) + * @property {{ audiences?: string[] }} [tokenExchange] RFC 8693: the `resource` / `audience` values a token may be exchanged for (default: none) * * @typedef {Object} RegistrationConfig * @property {string} [initialAccessToken] bearer required to register (secure default; omit only with `open`) * @property {boolean} [open] allow unauthenticated registration (NOT recommended) * @property {(req: import('./request.js').ServerRequest) => boolean | Promise} [authorize] custom gate * @property {Partial} [defaults] defaults layered under each registration + * @property {string[]} [allowedGrants] grant types a registrant may request (default `['authorization_code','refresh_token']`) + * @property {string[]} [allowedAuthMethods] token-endpoint auth methods a registrant may request (default: every non-mTLS method) * * @typedef {Object} OidcConfig * @property {import('./issuers/id-token.js').IdTokenSignerConfig} idToken id_token JWS signer (signingKey + alg) @@ -102,6 +109,7 @@ const DEFAULT_REFRESH_TTL = '30d'; * @property {ReturnType} refresh * @property {ReturnType} par * @property {ReturnType} device + * @property {import('./security/replay.js').ReplayStore} [replay] IncrStore-shaped (`incr(key, ttlMs)`) — shares DPoP / client-assertion / request-object jti replay checks across nodes * * @typedef {Object} ResolvedServerConfig * @property {string} issuer @@ -156,6 +164,11 @@ export function createServer(config) { ]), }; + const replayStore = config.stores?.replay; + if (replayStore !== undefined && !isReplayStore(replayStore)) { + throw new OAuth2Error(ErrorCode.INVALID_ARGUMENT, 'stores.replay must implement incr(key, ttlMs)'); + } + // FAPI 2.0 tightens several defaults at once (PLAN Tier C). const { fapi, requirePkce, requirePar, dpopRequired } = applyFapiProfile(security); @@ -188,6 +201,9 @@ export function createServer(config) { requirePkce, dpopRequired, dpopNonceRequired: security.dpop?.nonce === true, + dpopNonces: createDpopNonces(security.dpop?.nonceSecret), + // Per-server single-use jti guard (DPoP, client assertions, JAR). + replay: createReplayGuard(replayStore), fapi, authenticateUser: config.authenticateUser, authorizationCodeTtlMs: parseDuration(security.authorizationCodeTtl ?? DEFAULT_AUTH_CODE_TTL), @@ -195,6 +211,7 @@ export function createServer(config) { refreshTokenTtlMs: parseDuration(security.refreshTokenTtl ?? DEFAULT_REFRESH_TTL), authorizationDetailsTypes: config.authorizationDetailsTypes, introspection: isObject(config.introspection) ? config.introspection : {}, + tokenExchange: isObject(config.tokenExchange) ? config.tokenExchange : {}, // JARM is enabled only when fully configured: without a signing key // `signJarmResponse` can't sign, so advertising / accepting jwt mode would // fail at runtime. An under-configured `jarm` is treated as absent. @@ -206,6 +223,11 @@ export function createServer(config) { // `registration` config is present AND the registry can persist. registration: isObject(config.registration) ? config.registration : undefined, registrationEnabled: isObject(config.registration) && isFunction(registry.register), + // Per-server client JWKS resolver cache (private_key_jwt, JAR). + clientKeys: createClientKeyResolver({ + allowJwksHost: security.allowJwksHost, + registrationEnabled: isObject(config.registration) && isFunction(registry.register), + }), }; return { diff --git a/packages/oauth2/src/server/middleware/express.js b/packages/oauth2/src/server/middleware/express.js index 43aa8f0d..bca2833f 100644 --- a/packages/oauth2/src/server/middleware/express.js +++ b/packages/oauth2/src/server/middleware/express.js @@ -148,6 +148,7 @@ function toRawRequest(req) { query: req.query, body: req.body, clientCertificate: readPeerCertificate(req), + clientCertificateAuthorized: peerCertificateAuthorized(req), }; } @@ -166,3 +167,17 @@ function readPeerCertificate(req) { } return undefined; } + +/** + * Whether the TLS layer verified the client certificate against the + * server's trusted CAs. A listener run with `rejectUnauthorized: false` + * (needed for `self_signed_tls_client_auth`) still accepts unverified + * certificates, so `tls_client_auth` must not trust the subject DN alone. + * + * @param {any} req + * @returns {boolean} + */ +function peerCertificateAuthorized(req) { + const socket = req.socket ?? req.connection; + return socket?.authorized === true; +} diff --git a/packages/oauth2/src/server/middleware/fastify.js b/packages/oauth2/src/server/middleware/fastify.js index f383f2bf..773961da 100644 --- a/packages/oauth2/src/server/middleware/fastify.js +++ b/packages/oauth2/src/server/middleware/fastify.js @@ -138,6 +138,7 @@ function adapt(handler) { query: request.query, body: request.body, clientCertificate: readPeerCertificate(request), + clientCertificateAuthorized: peerCertificateAuthorized(request), }); } catch (err) { out = errorResponse(err); @@ -165,6 +166,20 @@ function readPeerCertificate(request) { return undefined; } +/** + * Whether the TLS layer verified the client certificate against the + * server's trusted CAs. A listener run with `rejectUnauthorized: false` + * (needed for `self_signed_tls_client_auth`) still accepts unverified + * certificates, so `tls_client_auth` must not trust the subject DN alone. + * + * @param {any} request + * @returns {boolean} + */ +function peerCertificateAuthorized(request) { + const socket = request.socket ?? request.raw?.socket; + return socket?.authorized === true; +} + export const oauth2ServerPlugin = fastifyPlugin(oauth2ServerPluginFn, { fastify: '>=4', name: '@exortek/oauth2-server', diff --git a/packages/oauth2/src/server/request.js b/packages/oauth2/src/server/request.js index d4bc1442..0ae718e3 100644 --- a/packages/oauth2/src/server/request.js +++ b/packages/oauth2/src/server/request.js @@ -24,6 +24,7 @@ import { ProtocolError, ServerError } from './errors.js'; * @property {Record} [query] pre-parsed query, else derived from `url` * @property {string | Record} [body] raw string or a framework-parsed object * @property {import('node:tls').PeerCertificate | Record} [clientCertificate] mTLS client cert (RFC 8705) + * @property {boolean} [clientCertificateAuthorized] true only when the TLS layer verified the cert chain (`socket.authorized`) * * @typedef {Object} ServerRequest * @property {string} method @@ -33,6 +34,7 @@ import { ProtocolError, ServerError } from './errors.js'; * @property {(name: string) => string | undefined} header * @property {(name: string) => string | undefined} param query ∪ form (form wins) * @property {object | undefined} clientCertificate + * @property {boolean} clientCertificateAuthorized chain verified by the TLS layer (required for `tls_client_auth`) */ /** @@ -57,6 +59,7 @@ export function normalizeRequest(raw) { query, form, clientCertificate: isObject(raw.clientCertificate) ? raw.clientCertificate : undefined, + clientCertificateAuthorized: raw.clientCertificateAuthorized === true, header(name) { return headers[String(name).toLowerCase()]; }, diff --git a/packages/oauth2/src/server/security/client-auth.js b/packages/oauth2/src/server/security/client-auth.js index 2bace8b0..d3ca2483 100644 --- a/packages/oauth2/src/server/security/client-auth.js +++ b/packages/oauth2/src/server/security/client-auth.js @@ -24,13 +24,11 @@ import { isNonEmptyString, isNumber, isObject } from '@exortek/shared/predicates import { ProtocolError, ServerError } from '../errors.js'; import { readClientCredentials } from '../request.js'; -import { resolveClientKeys } from './client-keys.js'; -import { createReplayCache } from './replay-cache.js'; +import { parseDn, sameDn, subjectAttributes } from './dn.js'; const JWT_BEARER_ASSERTION = 'urn:ietf:params:oauth:client-assertion-type:jwt-bearer'; -// RFC 7523 §3 requires `exp`; bound its reach so the (process-local, -// opportunistically-evicted) jti replay cache actually covers the token's -// whole life. +// RFC 7523 §3 requires `exp`; bound its reach so the jti replay guard +// actually covers the token's whole life. const MAX_ASSERTION_LIFETIME_MS = 300_000; // 5 minutes const ASSERTION_ALGS = new Set([ 'ES256', @@ -45,10 +43,6 @@ const ASSERTION_ALGS = new Set([ 'EdDSA', ]); -// Assertion replay cache — a `jti` is single-use within its lifetime. The -// 5-minute retention matches the accepted assertion lifetime bound above. -const seenAssertionJti = createReplayCache(MAX_ASSERTION_LIFETIME_MS); - /** * Resolve and authenticate the client behind a request. * @@ -188,7 +182,7 @@ async function authenticateAssertion(req, config, assertionType) { `private_key_jwt alg ${JSON.stringify(alg)} is not an accepted asymmetric algorithm`, ); } - keyish = await resolveClientKeys(client, { allowJwksHost: config.security?.allowJwksHost }); + keyish = await config.clientKeys(client); } else { throw new ServerError( ProtocolError.INVALID_CLIENT, @@ -217,16 +211,21 @@ async function authenticateAssertion(req, config, assertionType) { throw new ServerError(ProtocolError.INVALID_CLIENT, 'client_assertion lifetime exceeds the accepted maximum'); } - assertSingleUse(payload.jti); + await assertSingleUse(payload.jti, config.replay); return client; } -/** @param {unknown} jti */ -function assertSingleUse(jti) { +/** + * A `jti` is single-use within the accepted assertion lifetime. + * + * @param {unknown} jti + * @param {import('./replay.js').ReplayGuard} replay + */ +async function assertSingleUse(jti, replay) { if (!isNonEmptyString(jti)) { throw new ServerError(ProtocolError.INVALID_CLIENT, 'client_assertion is missing jti'); } - if (seenAssertionJti.seen(jti)) { + if (await replay.seen('assertion', jti, MAX_ASSERTION_LIFETIME_MS)) { throw new ServerError(ProtocolError.INVALID_CLIENT, 'client_assertion jti has already been used (replay)'); } } @@ -245,10 +244,17 @@ async function authenticateByCertificate(req, client) { } if (client.tokenEndpointAuthMethod === 'tls_client_auth') { - // PKI mTLS: the certificate's subject DN must match the registered - // expected value (RFC 8705 §2.1). - const subject = certSubjectString(cert); - if (!isNonEmptyString(client.tlsClientAuthSubjectDn) || client.tlsClientAuthSubjectDn !== subject) { + // PKI mTLS (RFC 8705 §2.1): the chain must have been verified by the TLS + // layer — otherwise any self-signed certificate carrying the right + // subject would pass — and the subject DN must match the registered one. + if (req.clientCertificateAuthorized !== true) { + throw new ServerError(ProtocolError.INVALID_CLIENT, 'client certificate chain was not verified'); + } + const presented = subjectAttributes(/** @type {any} */ (cert).subject); + const registered = isNonEmptyString(client.tlsClientAuthSubjectDn) + ? parseDn(client.tlsClientAuthSubjectDn) + : undefined; + if (!presented || !registered || !sameDn(presented, registered)) { throw new ServerError( ProtocolError.INVALID_CLIENT, 'client certificate subject does not match the registered value', @@ -257,10 +263,10 @@ async function authenticateByCertificate(req, client) { return client; } if (client.tokenEndpointAuthMethod === 'self_signed_tls_client_auth') { - // Self-signed mTLS: the certificate thumbprint must be present in the - // client's registered JWKS (RFC 8705 §2.2). + // Self-signed mTLS: the certificate itself is the credential, so its + // thumbprint must equal the registered one (RFC 8705 §2.2). const thumb = certThumbprint(cert); - if (!certInJwks(client, thumb)) { + if (!thumbprintMatches(client, thumb)) { throw new ServerError(ProtocolError.INVALID_CLIENT, 'client certificate is not registered for this client'); } return client; @@ -283,29 +289,19 @@ function certThumbprint(cert) { return base64urlEncode(createHash('sha256').update(der).digest()); } -/** @param {object} cert @returns {string} */ -function certSubjectString(cert) { - const subject = /** @type {any} */ (cert).subject; - if (isNonEmptyString(subject)) { - return subject; - } - if (isObject(subject)) { - // Node parses the subject into an object; render a stable DN string. - return Object.entries(subject) - .map(([k, v]) => `${k}=${v}`) - .join(','); - } - throw new ServerError(ProtocolError.INVALID_CLIENT, 'client certificate has no subject'); -} - /** + * Compare the presented certificate's thumbprint with the one registered + * for a `self_signed_tls_client_auth` client (RFC 8705 §2.2). + * * @param {import('../clients.js').Client} client * @param {string} thumb * @returns {boolean} */ -function certInJwks(client, thumb) { - if (isNonEmptyString(client.certificateThumbprint)) { - return client.certificateThumbprint === thumb; +function thumbprintMatches(client, thumb) { + if (!isNonEmptyString(client.certificateThumbprint)) { + return false; } - return false; + const a = Buffer.from(client.certificateThumbprint, 'utf8'); + const b = Buffer.from(thumb, 'utf8'); + return a.length === b.length && timingSafeEqual(a, b); } diff --git a/packages/oauth2/src/server/security/client-keys.js b/packages/oauth2/src/server/security/client-keys.js index eccd1412..d0672310 100644 --- a/packages/oauth2/src/server/security/client-keys.js +++ b/packages/oauth2/src/server/security/client-keys.js @@ -7,59 +7,146 @@ * inline set becomes a `kid`-aware resolver so a signed token's `kid` * selects the right key (importing to bare KeyObjects would drop the * `kid` and break selection). + * + * Resolvers are cached per server: a remote set by its `jwks_uri` (so the + * JWKS response cache actually survives between requests instead of every + * token request and every `/authorize?request=` triggering a fetch) and an + * inline set by the client descriptor. */ +import { isIP } from 'node:net'; + import { createRemoteJWKS } from '@exortek/jwks'; import { importJWK } from '@exortek/jwk'; -import { isArray, isNonEmptyString, isObject } from '@exortek/shared/predicates'; +import { isArray, isFunction, isNonEmptyString, isObject } from '@exortek/shared/predicates'; import { ProtocolError, ServerError } from '../errors.js'; +const MAX_REMOTE_RESOLVERS = 1000; + /** - * @param {import('../clients.js').Client} client - * @param {{ allowJwksHost?: (hostname: string, url: URL) => boolean }} [options] + * @typedef {(client: import('../clients.js').Client) => Promise} ClientKeyResolver + */ + +/** + * @param {{ allowJwksHost?: (hostname: string, url: URL) => boolean, registrationEnabled?: boolean }} [options] * `allowJwksHost` gates the destination of a `jwksUri` fetch. A client's - * `jwks_uri` is client-supplied — under open or token-gated dynamic - * registration it is chosen by whoever registered — so a deployment can - * constrain which hosts the server will reach out to. - * @returns {Promise} a `keyish` resolver for `jwt.verify` + * `jwks_uri` is client-supplied — under dynamic registration it is chosen + * by whoever registered — so when registration is enabled and no gate is + * configured, loopback, private and link-local hosts are refused by + * default (an SSRF lever into the server's own network). + * @returns {ClientKeyResolver} */ -export async function resolveClientKeys(client, options = {}) { - if (isNonEmptyString(client.jwksUri)) { - try { - return createRemoteJWKS(client.jwksUri, { - ...(options.allowJwksHost ? { allowHost: options.allowJwksHost } : {}), - }); - } catch (err) { - throw new ServerError( - ProtocolError.INVALID_CLIENT, - `client jwks_uri was refused: ${err instanceof Error ? err.message : String(err)}`, - ); +export function createClientKeyResolver(options = {}) { + const allowHost = isFunction(options.allowJwksHost) + ? options.allowJwksHost + : options.registrationEnabled === true + ? isPublicHost + : undefined; + + /** @type {Map} jwks_uri → remote resolver (insertion-ordered LRU) */ + const remote = new Map(); + /** @type {WeakMap>} client descriptor → inline resolver */ + const inline = new WeakMap(); + + return async function resolveClientKeys(client) { + if (isNonEmptyString(client.jwksUri)) { + const cached = remote.get(client.jwksUri); + if (cached) { + remote.delete(client.jwksUri); + remote.set(client.jwksUri, cached); + return cached; + } + let resolver; + try { + resolver = createRemoteJWKS(client.jwksUri, allowHost ? { allowHost } : {}); + } catch (err) { + throw new ServerError( + ProtocolError.INVALID_CLIENT, + `client jwks_uri was refused: ${err instanceof Error ? err.message : String(err)}`, + ); + } + if (remote.size >= MAX_REMOTE_RESOLVERS) { + remote.delete(remote.keys().next().value); + } + remote.set(client.jwksUri, resolver); + return resolver; } - } - if (isObject(client.jwks) && isArray(client.jwks.keys) && client.jwks.keys.length > 0) { - const entries = await Promise.all( - client.jwks.keys.map(async jwk => [isNonEmptyString(jwk.kid) ? jwk.kid : undefined, await importJWK(jwk)]), - ); - const byKid = new Map(entries.filter(([kid]) => kid !== undefined)); - const all = entries.map(([, key]) => key); - return header => { - const kid = header?.kid; - if (isNonEmptyString(kid)) { - const match = byKid.get(kid); - if (match) { - return match; - } - // A named kid that isn't in the set is a hard miss. - throw new ServerError(ProtocolError.INVALID_CLIENT, `no client key matches kid ${JSON.stringify(kid)}`); + if (isObject(client.jwks) && isArray(client.jwks.keys) && client.jwks.keys.length > 0) { + let pending = inline.get(client); + if (!pending) { + pending = inlineResolver(client.jwks.keys); + inline.set(client, pending); + // A key that fails to import must not poison the cache. + pending.catch(() => inline.delete(client)); } - if (all.length === 1) { - return all[0]; + return pending; + } + throw new ServerError(ProtocolError.INVALID_CLIENT, `client ${JSON.stringify(client.clientId)} has no JWKS`); + }; +} + +/** + * @param {object[]} keys + * @returns {Promise} + */ +async function inlineResolver(keys) { + const entries = await Promise.all( + keys.map(async jwk => [isNonEmptyString(jwk.kid) ? jwk.kid : undefined, await importJWK(jwk)]), + ); + const byKid = new Map(entries.filter(([kid]) => kid !== undefined)); + const all = entries.map(([, key]) => key); + return header => { + const kid = header?.kid; + if (isNonEmptyString(kid)) { + const match = byKid.get(kid); + if (match) { + return match; } - throw new ServerError( - ProtocolError.INVALID_CLIENT, - 'the signed token has no kid but the client has multiple keys', - ); - }; + // A named kid that isn't in the set is a hard miss. + throw new ServerError(ProtocolError.INVALID_CLIENT, `no client key matches kid ${JSON.stringify(kid)}`); + } + if (all.length === 1) { + return all[0]; + } + throw new ServerError(ProtocolError.INVALID_CLIENT, 'the signed token has no kid but the client has multiple keys'); + }; +} + +/** + * Default `jwks_uri` host gate under dynamic registration: refuse names and + * IP literals that point back into the server's own or private networks. + * This is a literal check — a public name that resolves to a private + * address is only stopped by a deployment-supplied `allowJwksHost` (or an + * egress proxy). + * + * @param {string} hostname + * @returns {boolean} + */ +export function isPublicHost(hostname) { + const host = hostname.replace(/^\[|\]$/g, '').toLowerCase(); + if (host === 'localhost' || host.endsWith('.localhost') || host.endsWith('.local') || host.endsWith('.internal')) { + return false; + } + const family = isIP(host); + if (family === 4) { + const [a, b] = host.split('.').map(Number); + return !( + a === 0 || + a === 10 || + a === 127 || + (a === 100 && b >= 64 && b <= 127) || + (a === 169 && b === 254) || + (a === 172 && b >= 16 && b <= 31) || + (a === 192 && b === 168) || + a >= 224 + ); + } + if (family === 6) { + const mapped = host.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/); + if (mapped) { + return isPublicHost(mapped[1]); + } + return !(host === '::' || host === '::1' || /^f[cd]/.test(host) || /^fe[89ab]/.test(host) || /^ff/.test(host)); } - throw new ServerError(ProtocolError.INVALID_CLIENT, `client ${JSON.stringify(client.clientId)} has no JWKS`); + return true; } diff --git a/packages/oauth2/src/server/security/dn.js b/packages/oauth2/src/server/security/dn.js new file mode 100644 index 00000000..b88a32c6 --- /dev/null +++ b/packages/oauth2/src/server/security/dn.js @@ -0,0 +1,135 @@ +/** + * Distinguished-name comparison for `tls_client_auth` (RFC 8705 §2.1.2). + * The registered `tlsClientAuthSubjectDn` is an RFC 4514 string; Node hands + * the certificate subject over as an object of attribute values. Both are + * reduced to a set of `TYPE=value` attributes and compared structurally, so + * an escaped comma inside a value (`CN=foo\,O=Bank`) can never collide with + * an extra attribute (`CN=foo,O=Bank`), and attribute order does not matter. + */ +import { isArray, isNonEmptyString, isObject, isString } from '@exortek/shared/predicates'; + +/** + * Parse an RFC 4514 DN string into attributes. + * + * @param {string} dn + * @returns {{ type: string, value: string }[] | undefined} undefined when malformed + */ +export function parseDn(dn) { + /** @type {{ type: string, value: string }[]} */ + const out = []; + let type = ''; + /** @type {number[] | null} value bytes while reading a value, null while reading a type */ + let value = null; + let trailingSpaces = 0; + + const flush = () => { + if (value === null) { + return false; + } + const t = type.trim(); + if (t.length === 0) { + return false; + } + const bytes = trailingSpaces > 0 ? value.slice(0, value.length - trailingSpaces) : value; + out.push({ type: t.toUpperCase(), value: Buffer.from(bytes).toString('utf8') }); + type = ''; + value = null; + trailingSpaces = 0; + return true; + }; + + for (let i = 0; i < dn.length; i++) { + const ch = dn[i]; + if (value === null) { + if (ch === '=') { + value = []; + } else if (ch === ',' || ch === '+' || ch === '\\') { + return undefined; + } else { + type += ch; + } + continue; + } + if (ch === '\\') { + const next = dn[i + 1]; + if (next === undefined) { + return undefined; + } + if (/[0-9a-fA-F]/.test(next) && /[0-9a-fA-F]/.test(dn[i + 2] ?? '')) { + value.push(Number.parseInt(dn.slice(i + 1, i + 3), 16)); + i += 2; + } else { + value.push(...Buffer.from(next, 'utf8')); + i += 1; + } + trailingSpaces = 0; + continue; + } + if (ch === ',' || ch === '+') { + if (!flush()) { + return undefined; + } + continue; + } + if (ch === ' ' && value.length === 0) { + // Unescaped leading space after `=` is insignificant. + continue; + } + if (ch === '#' && value.length === 0) { + // Hex-encoded BER values are not supported for matching. + return undefined; + } + value.push(...Buffer.from(ch, 'utf8')); + trailingSpaces = ch === ' ' ? trailingSpaces + 1 : 0; + } + if (!flush()) { + return undefined; + } + return out; +} + +/** + * Attributes of a certificate subject — Node's parsed object (multi-valued + * attributes arrive as arrays) or an RFC 4514 string. + * + * @param {unknown} subject + * @returns {{ type: string, value: string }[] | undefined} + */ +export function subjectAttributes(subject) { + if (isNonEmptyString(subject)) { + return parseDn(subject); + } + if (!isObject(subject)) { + return undefined; + } + /** @type {{ type: string, value: string }[]} */ + const out = []; + for (const [type, raw] of Object.entries(subject)) { + for (const value of isArray(raw) ? raw : [raw]) { + if (!isString(value)) { + return undefined; + } + out.push({ type: type.toUpperCase(), value }); + } + } + return out; +} + +/** + * Whether two attribute lists name the same DN (order-insensitive). + * + * @param {{ type: string, value: string }[]} a + * @param {{ type: string, value: string }[]} b + * @returns {boolean} + */ +export function sameDn(a, b) { + if (a.length === 0 || a.length !== b.length) { + return false; + } + const key = attrs => + attrs + .map(({ type, value }) => JSON.stringify([type, value])) + .sort() + .join('\n'); + return key(a) === key(b); +} diff --git a/packages/oauth2/src/server/security/dpop.js b/packages/oauth2/src/server/security/dpop.js index 4188bfdf..290b4479 100644 --- a/packages/oauth2/src/server/security/dpop.js +++ b/packages/oauth2/src/server/security/dpop.js @@ -12,15 +12,17 @@ * proof is allowed only when neither the client nor the server requires * DPoP — otherwise it is `invalid_dpop_proof`. */ -import { createHash, randomBytes } from 'node:crypto'; +import { createHash, createHmac, randomBytes } from 'node:crypto'; import { importJWK, thumbprint } from '@exortek/jwk'; import { verify as jwsVerify, decodeProtectedHeader } from '@exortek/jws'; -import { encode as base64urlEncode } from '@exortek/shared/base64url'; +import { encode as base64urlEncode, decode as base64urlDecode } from '@exortek/shared/base64url'; import { isNonEmptyString, isNumber, isObject } from '@exortek/shared/predicates'; +import { timingSafeEqual } from '@exortek/shared/timing-safe'; +import { ErrorCode, OAuth2Error } from '../../internal/errors.js'; import { ProtocolError, ServerError } from '../errors.js'; -import { createReplayCache } from './replay-cache.js'; +import { createReplayGuard } from './replay.js'; // RFC 9449 §4.2 — the proof `alg` must be an asymmetric JWS alg; symmetric // and `none` are rejected so the embedded JWK is a real public key. @@ -39,52 +41,71 @@ const ALLOWED_ALGS = new Set([ // Proofs are single-use and short-lived; §4.3 recommends a small window. const MAX_PROOF_AGE_MS = 60_000; -// Replay cache: a proof `jti` may be used once. A deployment fronting -// multiple nodes overrides this with a shared store if it needs cross-node -// replay defense. Retention covers the full freshness window (a proof's -// iat is accepted within ±MAX_PROOF_AGE_MS). -const seenJti = createReplayCache(2 * MAX_PROOF_AGE_MS); +// A proof `jti` may be used once; retention covers the full freshness +// window (a proof's iat is accepted within ±MAX_PROOF_AGE_MS). The server +// passes its own replay guard (shareable across nodes via `stores.replay`); +// `verifyDpopForResource` without one falls back to this process-local guard. +const REPLAY_TTL_MS = 2 * MAX_PROOF_AGE_MS; +let defaultReplay = createReplayGuard(); // Server-provided DPoP nonce (RFC 9449 §8-9). When the server requires a // nonce, the client must echo the most recent `DPoP-Nonce` value as the // proof's `nonce` claim; a missing/stale nonce is answered with // `use_dpop_nonce` and a fresh `DPoP-Nonce` header so the client retries. -// Issued nonces are accepted for a short window (module-level, single- -// process — a multi-node deployment supplies a shared nonce service). -const NONCE_TTL_MS = 300_000; -/** @type {Map} nonce → expiry epoch ms */ -const issuedNonces = new Map(); +// +// Nonces are stateless: `base64url(window ‖ HMAC(secret, window)[0..16])` +// for a 150 s time window, accepted for the current and the previous window. +// Every node holding the same `security.dpop.nonceSecret` issues and accepts +// the same nonces, so nonce mode works behind a load balancer with no shared +// state and nothing to sweep. +const NONCE_WINDOW_MS = 150_000; +const NONCE_MAC_BYTES = 16; /** - * Mint a fresh DPoP nonce, remember it for the acceptance window, and - * return the value to hand back in the `DPoP-Nonce` header. - * - * @returns {string} + * @param {string | Uint8Array | undefined} secret ≥ 32 bytes; random per server when omitted */ -function issueNonce() { - const now = Date.now(); - if (issuedNonces.size > 1024) { - for (const [value, exp] of issuedNonces) { - if (exp <= now) { - issuedNonces.delete(value); - } +export function createDpopNonces(secret) { + let key; + if (secret === undefined) { + key = randomBytes(32); + } else { + key = Buffer.from(/** @type {any} */ (secret)); + if (key.length < 32) { + throw new OAuth2Error(ErrorCode.INVALID_ARGUMENT, 'security.dpop.nonceSecret must be at least 32 bytes'); } } - const nonce = base64urlEncode(randomBytes(16)); - issuedNonces.set(nonce, now + NONCE_TTL_MS); - return nonce; -} - -/** - * @param {unknown} nonce - * @returns {boolean} - */ -function isValidNonce(nonce) { - if (!isNonEmptyString(nonce)) { - return false; - } - const exp = issuedNonces.get(nonce); - return exp !== undefined && exp > Date.now(); + const mac = window => { + const w = Buffer.alloc(4); + w.writeUInt32BE(window); + return Buffer.concat([w, createHmac('sha256', key).update(w).digest().subarray(0, NONCE_MAC_BYTES)]); + }; + const currentWindow = () => Math.floor(Date.now() / NONCE_WINDOW_MS); + return { + issue() { + return base64urlEncode(mac(currentWindow())); + }, + /** @param {unknown} nonce */ + isValid(nonce) { + if (!isNonEmptyString(nonce)) { + return false; + } + let raw; + try { + raw = base64urlDecode(nonce); + } catch { + return false; + } + if (raw.length !== 4 + NONCE_MAC_BYTES) { + return false; + } + const window = raw.readUInt32BE(0); + const now = currentWindow(); + if (window !== now && window !== now - 1) { + return false; + } + return timingSafeEqual(raw, mac(window)); + }, + }; } /** @@ -113,24 +134,26 @@ export async function resolveDpopBinding(req, config, client, options = {}) { // The proof binds to the endpoint it was sent to — the token endpoint, // or the PAR endpoint for a `dpop_jkt` pre-binding (RFC 9449 §10.1). htu: isNonEmptyString(options.htu) ? options.htu : config.endpoints.token, + replay: config.replay, }); // RFC 9449 §8-9: when the server requires a nonce, a proof that omits it // or carries a stale one is answered with `use_dpop_nonce` plus a fresh // `DPoP-Nonce` header, and the client retries with the new value. - if (nonceRequired && !isValidNonce(nonce)) { + if (nonceRequired && !config.dpopNonces.isValid(nonce)) { throw new ServerError( ProtocolError.USE_DPOP_NONCE, 'a DPoP proof nonce is required; retry with the supplied nonce', { - headers: { 'DPoP-Nonce': issueNonce() }, + headers: { 'DPoP-Nonce': config.dpopNonces.issue() }, }, ); } - // On success, hand the client a fresh nonce to use on its next request - // (kept rotating so a captured nonce ages out). Only when nonces are on. - return { dpopJkt: jkt, nonceHeaders: nonceRequired ? { 'DPoP-Nonce': issueNonce() } : {} }; + // On success, hand the client the current nonce for its next request + // (it rotates every window, so a captured nonce ages out). Only when + // nonces are on. + return { dpopJkt: jkt, nonceHeaders: nonceRequired ? { 'DPoP-Nonce': config.dpopNonces.issue() } : {} }; } /** @@ -138,7 +161,7 @@ export async function resolveDpopBinding(req, config, client, options = {}) { * `nonce` claim (if any) for the caller's nonce-challenge check. * * @param {string} proof - * @param {{ htm: string, htu: string, ath?: string }} binding + * @param {{ htm: string, htu: string, ath?: string, replay?: import('./replay.js').ReplayGuard }} binding * @returns {Promise<{ jkt: string, nonce: string | undefined }>} */ export async function verifyDpopProof(proof, binding) { @@ -193,7 +216,7 @@ export async function verifyDpopProof(proof, binding) { throw new ServerError(ProtocolError.INVALID_DPOP_PROOF, 'DPoP proof ath does not match the access token'); } assertFresh(payload.iat); - assertUnique(payload.jti); + await assertUnique(payload.jti, binding.replay ?? defaultReplay); return { jkt, nonce: isNonEmptyString(payload.nonce) ? payload.nonce : undefined }; } @@ -206,8 +229,11 @@ export async function verifyDpopProof(proof, binding) { * key is the one the token was issued to — the missing high-level helper * for the resource-server half of DPoP. * + * A multi-node resource server passes `replayStore` (IncrStore-shaped, + * e.g. Redis-backed) so a proof replayed against a different node is caught. + * * @param {string} proof the `DPoP` request header - * @param {{ htm: string, htu: string, accessToken: string, cnfJkt: string }} binding + * @param {{ htm: string, htu: string, accessToken: string, cnfJkt: string, replayStore?: import('./replay.js').ReplayStore }} binding * @returns {Promise<{ jkt: string }>} */ export async function verifyDpopForResource(proof, binding) { @@ -218,7 +244,8 @@ export async function verifyDpopForResource(proof, binding) { throw new ServerError(ProtocolError.INVALID_DPOP_PROOF, 'verifyDpopForResource requires the access token'); } const ath = base64urlEncode(createHash('sha256').update(binding.accessToken).digest()); - const { jkt } = await verifyDpopProof(proof, { htm: binding.htm, htu: binding.htu, ath }); + const replay = binding.replayStore !== undefined ? createReplayGuard(binding.replayStore) : undefined; + const { jkt } = await verifyDpopProof(proof, { htm: binding.htm, htu: binding.htu, ath, replay }); // The proving key must be the key the token is bound to. if (!isNonEmptyString(binding.cnfJkt) || jkt !== binding.cnfJkt) { throw new ServerError(ProtocolError.INVALID_DPOP_PROOF, 'DPoP proof key does not match the token cnf.jkt'); @@ -255,23 +282,20 @@ function assertFresh(iat) { } } -/** @param {unknown} jti */ -function assertUnique(jti) { +/** + * @param {unknown} jti + * @param {import('./replay.js').ReplayGuard} replay + */ +async function assertUnique(jti, replay) { if (!isNonEmptyString(jti)) { throw new ServerError(ProtocolError.INVALID_DPOP_PROOF, 'DPoP proof is missing jti'); } - if (seenJti.seen(jti)) { + if (await replay.seen('dpop', jti, REPLAY_TTL_MS)) { throw new ServerError(ProtocolError.INVALID_DPOP_PROOF, 'DPoP proof jti has already been used (replay)'); } } -/** Test hook — clear the replay + nonce caches between cases. */ +/** Test hook — reset the process-local fallback replay guard. */ export function _clearDpopReplayCache() { - seenJti.clear(); - issuedNonces.clear(); -} - -/** Test hook — mint a nonce as the server would (to seed a proof). */ -export function _issueDpopNonce() { - return issueNonce(); + defaultReplay = createReplayGuard(); } diff --git a/packages/oauth2/src/server/security/jar-jarm.js b/packages/oauth2/src/server/security/jar-jarm.js index 11c2a419..89d0896c 100644 --- a/packages/oauth2/src/server/security/jar-jarm.js +++ b/packages/oauth2/src/server/security/jar-jarm.js @@ -15,16 +15,13 @@ import { sign as jwtSign, verify as jwtVerify, decode as jwtDecode, JwtError } f import { isNonEmptyString, isNumber, isObject } from '@exortek/shared/predicates'; import { ProtocolError, ServerError } from '../errors.js'; -import { resolveClientKeys } from './client-keys.js'; -import { createReplayCache } from './replay-cache.js'; // A JAR request object is short-lived; bound its accepted lifetime so a // captured one cannot be replayed indefinitely, and remember its `jti` -// (when present) for that window so a single-use request object cannot be -// replayed at all. Module-level / single-process, like the DPoP and -// client-assertion caches; a multi-node deployment fronts it with PAR. +// (when present) for that window — through the server's replay guard, +// shareable across nodes via `stores.replay` — so a single-use request +// object cannot be replayed at all. const MAX_REQUEST_OBJECT_LIFETIME_MS = 300_000; // 5 minutes -const seenRequestJti = createReplayCache(MAX_REQUEST_OBJECT_LIFETIME_MS); const REQUEST_OBJECT_ALGS = new Set([ 'ES256', @@ -74,7 +71,7 @@ export async function verifyRequestObject(requestJwt, outerClientId, config) { let payload; try { - const keys = await resolveClientKeys(client, { allowJwksHost: config.security?.allowJwksHost }); + const keys = await config.clientKeys(client); payload = (await jwtVerify(requestJwt, keys, { alg: [alg], audience: config.issuer })).payload; } catch (err) { if (err instanceof JwtError) { @@ -92,7 +89,7 @@ export async function verifyRequestObject(requestJwt, outerClientId, config) { if (payload.exp * 1000 - Date.now() > MAX_REQUEST_OBJECT_LIFETIME_MS) { throw new ServerError(ProtocolError.INVALID_REQUEST, 'request object lifetime exceeds the accepted maximum'); } - if (isNonEmptyString(payload.jti) && seenRequestJti.seen(payload.jti)) { + if (isNonEmptyString(payload.jti) && (await config.replay.seen('jar', payload.jti, MAX_REQUEST_OBJECT_LIFETIME_MS))) { throw new ServerError(ProtocolError.INVALID_REQUEST, 'request object jti has already been used (replay)'); } diff --git a/packages/oauth2/src/server/security/replay.js b/packages/oauth2/src/server/security/replay.js new file mode 100644 index 00000000..64ad98fa --- /dev/null +++ b/packages/oauth2/src/server/security/replay.js @@ -0,0 +1,60 @@ +/** + * Replay guard — one place that decides whether a single-use identifier + * (a DPoP proof `jti`, a client-assertion `jti`, a request-object `jti`) has + * been seen before. Each server gets its own guard: + * + * - with `stores.replay` — any IncrStore-shaped store (`incr(key, ttlMs)` + * → `{ count }` or a number, e.g. `@exortek/shared/incr-store` over + * Redis) — the check is shared by every node behind a load balancer; + * - without it, per-server in-memory two-bucket caches (single process). + * + * The first `incr` of a key returns 1; anything higher is a replay. + */ +import { isFunction, isNumber, isObject } from '@exortek/shared/predicates'; + +import { createReplayCache } from './replay-cache.js'; + +/** + * @typedef {Object} ReplayStore + * @property {(key: string, ttlMs: number) => Promise<{ count: number } | number> | { count: number } | number} incr + * + * @typedef {Object} ReplayGuard + * @property {(namespace: string, id: string, ttlMs: number) => Promise} seen + * record `id` under `namespace` for `ttlMs`; resolve true when it was already present + */ + +/** + * @param {unknown} value + * @returns {value is ReplayStore} + */ +export function isReplayStore(value) { + return isObject(value) && isFunction(/** @type {Record} */ (value).incr); +} + +/** + * @param {ReplayStore} [store] + * @returns {ReplayGuard} + */ +export function createReplayGuard(store) { + if (store !== undefined) { + return { + async seen(namespace, id, ttlMs) { + const res = await store.incr(`${namespace}:${id}`, ttlMs); + const count = isNumber(res) ? res : res?.count; + return !isNumber(count) || count > 1; + }, + }; + } + /** @type {Map>} */ + const caches = new Map(); + return { + async seen(namespace, id, ttlMs) { + let cache = caches.get(namespace); + if (!cache) { + cache = createReplayCache(ttlMs); + caches.set(namespace, cache); + } + return cache.seen(id); + }, + }; +} diff --git a/packages/oauth2/src/server/security/scope.js b/packages/oauth2/src/server/security/scope.js new file mode 100644 index 00000000..e19f9907 --- /dev/null +++ b/packages/oauth2/src/server/security/scope.js @@ -0,0 +1,60 @@ +/** + * Scope ceiling — the single rule every grant applies to a requested scope. + * A client may only ever be granted scopes that BOTH the server advertises + * (`config.scopes`) and its own registration lists (`client.scope`). Taking + * either one alone is an escalation lever: a dynamically-registered client + * picks its own `scope`, so trusting it instead of the server list would let + * a registrant grant itself anything. + */ +import { isArray, isNonEmptyString } from '@exortek/shared/predicates'; + +import { ProtocolError, ServerError } from '../errors.js'; + +/** + * The scopes `client` may be granted, or `undefined` when neither the server + * nor the client declares a list (no ceiling configured). + * + * @param {import('../clients.js').Client} client + * @param {Record} config + * @returns {string[] | undefined} + */ +export function scopeCeiling(client, config) { + const own = isArray(client.scope) ? client.scope : undefined; + const server = isArray(config.scopes) ? config.scopes : undefined; + if (own && server) { + return own.filter(s => server.includes(s)); + } + return own ?? server; +} + +/** + * Split a space-delimited `scope` parameter (RFC 6749 §3.3). + * + * @param {unknown} raw + * @returns {string[]} + */ +export function parseScope(raw) { + return isNonEmptyString(raw) ? raw.split(/\s+/).filter(Boolean) : []; +} + +/** + * Throw `invalid_scope` for the first requested scope outside the ceiling. + * + * @param {string[]} requested + * @param {string[] | undefined} ceiling + * @param {{ redirectable?: boolean, state?: string }} [errorOptions] + */ +export function assertWithinCeiling(requested, ceiling, errorOptions) { + if (!isArray(ceiling)) { + return; + } + for (const s of requested) { + if (!ceiling.includes(s)) { + throw new ServerError( + ProtocolError.INVALID_SCOPE, + `scope ${JSON.stringify(s)} is not permitted for this client`, + errorOptions, + ); + } + } +} diff --git a/packages/oauth2/src/server/stores.js b/packages/oauth2/src/server/stores.js index 976a581a..db6a4d2a 100644 --- a/packages/oauth2/src/server/stores.js +++ b/packages/oauth2/src/server/stores.js @@ -106,6 +106,8 @@ function createTtlMap({ sweepIntervalMs = 60_000 } = {}) { * @property {string} [resource] * @property {unknown} [authorizationDetails] * @property {string} [dpopJkt] DPoP `jkt` bound at the authorize step (RFC 9449 §10) + * @property {boolean} [redeemed] marker left after redemption so a replay can revoke the family + * @property {string} [familyId] the refresh family a redeemed code produced * * @typedef {Object} AuthCodeStore * @property {(code: string, record: AuthCodeRecord, ttlMs: number) => void | Promise} save @@ -143,34 +145,63 @@ export function createAuthCodeStore() { * @typedef {Object} RefreshStore * @property {(record: RefreshRecord) => Promise} save * @property {(token: string) => Promise} get - * @property {(oldToken: string, next: RefreshRecord) => Promise} rotate + * @property {(oldToken: string, next: RefreshRecord) => Promise} rotate + * Atomically claim `oldToken` and persist `next`. Resolve `false` when the + * token was already rotated or revoked (a concurrent redemption won), so the + * grant treats it as reuse. A store that resolves nothing is treated as a + * successful rotation (pre-CAS custom stores). * @property {(familyId: string) => Promise} revokeFamily */ -/** @returns {RefreshStore} */ -export function createRefreshStore() { +/** + * @param {{ sweepIntervalMs?: number }} [options] + * @returns {RefreshStore} + */ +export function createRefreshStore({ sweepIntervalMs = 60_000 } = {}) { const store = createMemoryRecordStore({ idField: 'token', indexField: 'familyId' }); + + // The record store has no TTL of its own: evict expired tokens in the + // background so a family that is never presented again cannot leak. + const sweep = setInterval(() => { + const now = Date.now(); + for (const [token, record] of store.byId) { + const { expiresAt, familyId } = /** @type {RefreshRecord} */ (record); + if (isNumber(expiresAt) && expiresAt <= now) { + store.byId.delete(token); + const members = store.byIndex.get(familyId); + members?.delete(token); + if (members?.size === 0) { + store.byIndex.delete(familyId); + } + } + } + }, sweepIntervalMs); + if (isFunction(sweep.unref)) { + sweep.unref(); + } + return { async save(record) { await store.put(record); }, async get(token) { const record = /** @type {RefreshRecord | null} */ (await store.getById(token)); - // An expired refresh token is treated as absent (the memory record - // store has no TTL of its own); a Redis-backed store would let the - // key expire instead. + // An expired refresh token is treated as absent. if (record && isNumber(record.expiresAt) && record.expiresAt <= Date.now()) { return null; } return record; }, - // Mark the presented token used and persist its successor. If the - // presented token was ALREADY used, the caller detects reuse from - // `get` first and calls `revokeFamily` instead — this only runs on a - // legitimate rotation. + // Compare-and-set: the check and the `used` flip happen in one + // synchronous step, so of two concurrent redemptions exactly one wins. async rotate(oldToken, next) { - await store.update(oldToken, { used: true }); + const existing = /** @type {RefreshRecord | undefined} */ (store.byId.get(oldToken)); + if (!existing || existing.used === true || existing.revoked === true) { + return false; + } + existing.used = true; await store.put(next); + return true; }, async revokeFamily(familyId) { const members = await store.listByIndex(familyId); @@ -213,51 +244,47 @@ export function createParStore() { * @property {'pending' | 'approved' | 'denied' | 'redeemed'} status * @property {string} [subject] set on approval * @property {number} [lastPolledAt] for slow_down enforcement + * @property {number} [expiresAt] epoch ms; set by the store on save and never extended * * @typedef {Object} DeviceStore * @property {(record: DeviceRecord, ttlMs: number) => void | Promise} save * @property {(deviceCode: string) => (DeviceRecord | undefined) | Promise} getByDeviceCode * @property {(userCode: string) => (DeviceRecord | undefined) | Promise} getByUserCode * @property {(deviceCode: string, patch: Partial) => void | Promise} update + * Patch a record without extending its lifetime. + * @property {(deviceCode: string) => boolean | Promise} [redeem] + * Atomically flip an `approved` record to `redeemed`; resolve whether this + * call did it. Optional for custom stores (the grant then falls back to a + * non-atomic `update`), implemented by the built-in stores. */ /** @returns {DeviceStore} */ export function createDeviceStore() { const byDeviceCode = createTtlMap(); - /** @type {Map} user_code → device_code */ - const userToDevice = new Map(); + // user_code → device_code, on the same TTL as the record it points at. + const userToDevice = createTtlMap(); return { save(record, ttlMs) { - byDeviceCode.set(record.deviceCode, record, ttlMs); - userToDevice.set(record.userCode, record.deviceCode); + const expiresAt = Date.now() + ttlMs; + byDeviceCode.set(record.deviceCode, { ...record, expiresAt }, ttlMs); + userToDevice.set(record.userCode, record.deviceCode, ttlMs); }, getByDeviceCode(deviceCode) { return /** @type {DeviceRecord | undefined} */ (byDeviceCode.get(deviceCode)); }, getByUserCode(userCode) { - const deviceCode = userToDevice.get(userCode); - if (!deviceCode) { - return undefined; - } - const record = this.getByDeviceCode(deviceCode); - // The device record expired out from under the reverse index — drop - // the dangling `user_code` mapping so it cannot leak. - if (!record) { - userToDevice.delete(userCode); - return undefined; - } - return record; + const deviceCode = /** @type {string | undefined} */ (userToDevice.get(userCode)); + return deviceCode ? this.getByDeviceCode(deviceCode) : undefined; }, update(deviceCode, patch) { const existing = /** @type {DeviceRecord | undefined} */ (byDeviceCode.get(deviceCode)); if (!existing) { return; } - // Preserve the remaining TTL by not resetting it — re-save with the - // original expiry window is unnecessary here since getByDeviceCode - // already evicts past expiry; a short re-arm is acceptable. - byDeviceCode.set(deviceCode, { ...existing, ...patch }, DEVICE_REEARM_MS); + // Keep the original expiry: polling must never extend a device code. + const remaining = /** @type {number} */ (existing.expiresAt) - Date.now(); + byDeviceCode.set(deviceCode, { ...existing, ...patch }, remaining); // Once the code is spent (redeemed/denied), the one-time `user_code` // is done — free the reverse index eagerly rather than waiting for // TTL, so a verified device flow leaves nothing behind. @@ -265,13 +292,18 @@ export function createDeviceStore() { userToDevice.delete(existing.userCode); } }, + // Check-and-flip in one synchronous step: exactly one poll redeems. + redeem(deviceCode) { + const existing = /** @type {DeviceRecord | undefined} */ (byDeviceCode.get(deviceCode)); + if (!existing || existing.status !== 'approved') { + return false; + } + this.update(deviceCode, { status: 'redeemed' }); + return true; + }, }; } -// A device flow is minutes-long; re-arming an update for 10 min keeps a -// pending record alive across polls without a separate expiry field. -const DEVICE_REEARM_MS = 600_000; - /** * @param {unknown} store * @param {string[]} methods diff --git a/packages/oauth2/src/server/stores/redis.js b/packages/oauth2/src/server/stores/redis.js index 1457cea3..0666e10d 100644 --- a/packages/oauth2/src/server/stores/redis.js +++ b/packages/oauth2/src/server/stores/redis.js @@ -24,9 +24,12 @@ */ import { createRedisHelpers } from '@exortek/shared/redis-helpers'; import { createRedisRecordStore } from '@exortek/shared/record-store'; +import { createRedisIncrStore } from '@exortek/shared/incr-store'; import { isFunction } from '@exortek/shared/predicates'; const DEFAULT_PREFIX = 'oauth2:'; +// Claim-key lifetime for a family that carries no expiry (legacy records). +const CLAIM_FALLBACK_TTL_MS = 30 * 24 * 60 * 60 * 1000; /** * GET-then-DEL a key, atomically when the client supports `GETDEL` @@ -117,6 +120,7 @@ export function createRedisRefreshStore(client, options = {}) { /** @param {{ expiresAt?: number }} record */ const ttlOf = record => typeof record.expiresAt === 'number' ? Math.max(1, record.expiresAt - Date.now()) : undefined; + const claimKey = token => `${options.keyPrefix ?? DEFAULT_PREFIX}refresh:claim:${token}`; return { async save(record) { @@ -129,9 +133,18 @@ export function createRedisRefreshStore(client, options = {}) { } return record; }, + // Compare-and-set across nodes: the first redemption creates the claim + // key (SET NX) and wins; a concurrent one finds it and resolves false, + // which the grant treats as reuse. The claim lives as long as the + // family, so a late replay is still caught after the record is gone. async rotate(oldToken, next) { + const won = await store.helpers.setIfAbsent(claimKey(oldToken), '1', ttlOf(next) ?? CLAIM_FALLBACK_TTL_MS); + if (!won) { + return false; + } await store.update(oldToken, { used: true }); await store.put(next, ttlOf(next)); + return true; }, async revokeFamily(familyId) { const members = await store.listByIndex(familyId); @@ -156,13 +169,11 @@ export function createRedisDeviceStore(client, options = {}) { const dk = deviceCode => `${prefix}d:${deviceCode}`; const uk = userCode => `${prefix}u:${userCode}`; const helpers = createRedisHelpers(client); - // Re-arm window for an update that carries no explicit TTL (mirrors the - // in-memory store): a device flow is minutes-long. - const REEARM_MS = 600_000; const self = { async save(record, ttlMs) { - await helpers.setWithTTL(dk(record.deviceCode), JSON.stringify(record), ttlMs); + const expiresAt = Date.now() + ttlMs; + await helpers.setWithTTL(dk(record.deviceCode), JSON.stringify({ ...record, expiresAt }), ttlMs); await helpers.setWithTTL(uk(record.userCode), record.deviceCode, ttlMs); }, async getByDeviceCode(deviceCode) { @@ -175,16 +186,45 @@ export function createRedisDeviceStore(client, options = {}) { } return self.getByDeviceCode(deviceCode); }, + // KEEPTTL: an update (every poll writes lastPolledAt) never extends the + // code's life. async update(deviceCode, patch) { const existing = await self.getByDeviceCode(deviceCode); if (!existing) { return; } - await helpers.setWithTTL(dk(deviceCode), JSON.stringify({ ...existing, ...patch }), REEARM_MS); + await helpers.setKeepTTL(dk(deviceCode), JSON.stringify({ ...existing, ...patch })); if (patch.status === 'redeemed' || patch.status === 'denied') { await client.del(uk(existing.userCode)); } }, + // Single redemption across nodes: the first poll to create the redeem + // claim (SET NX) wins; any concurrent poll resolves false. + async redeem(deviceCode) { + const existing = await self.getByDeviceCode(deviceCode); + if (!existing || existing.status !== 'approved') { + return false; + } + const ttlMs = Math.max(1, (existing.expiresAt ?? Date.now() + 1000) - Date.now()); + if (!(await helpers.setIfAbsent(`${dk(deviceCode)}:redeemed`, '1', ttlMs))) { + return false; + } + await self.update(deviceCode, { status: 'redeemed' }); + return true; + }, }; return self; } + +/** + * Shared jti replay store (`stores.replay`) — DPoP proofs, client assertions + * and request objects replayed against a different node are caught. Each id + * is an INCR with a TTL; the first sighting counts 1. + * + * @param {any} client + * @param {{ keyPrefix?: string }} [options] + * @returns {import('../security/replay.js').ReplayStore} + */ +export function createRedisReplayStore(client, options = {}) { + return createRedisIncrStore(client, { keyPrefix: `${options.keyPrefix ?? DEFAULT_PREFIX}replay:` }); +} diff --git a/packages/oauth2/tests/audit.test.js b/packages/oauth2/tests/audit.test.js index 1195b22c..8d8a3352 100644 --- a/packages/oauth2/tests/audit.test.js +++ b/packages/oauth2/tests/audit.test.js @@ -373,7 +373,7 @@ test('D3: async (Promise-returning) PAR / auth-code stores drive a full flow', a ), ); assert.ok(pushed.request_uri.startsWith('urn:ietf:params:oauth:request_uri:')); - const authed = await server.authorize(get('/authorize', { request_uri: pushed.request_uri })); + const authed = await server.authorize(get('/authorize', { client_id: 'app', request_uri: pushed.request_uri })); assert.ok(new URL(authed.headers.location).searchParams.get('code')); }); diff --git a/packages/oauth2/tests/login.test.js b/packages/oauth2/tests/login.test.js index cdc7568b..489111f5 100644 --- a/packages/oauth2/tests/login.test.js +++ b/packages/oauth2/tests/login.test.js @@ -12,7 +12,7 @@ import { } from '../src/middleware/core.js'; import { mountOAuthLogin } from '../src/middleware/express.js'; -const SECRET = 'flow-cookie-secret-value'; +const SECRET = 'flow-cookie-secret-value-32-bytes!'; /** A hermetic stand-in for a `createOAuth` hub. */ function fakeOauth() { @@ -186,3 +186,11 @@ test('express web callback runs end-to-end over mock req/res', async () => { assert.equal(onSuccessArg.user.sub, 'user-1'); assert.equal(onSuccessArg.provider, 'google'); }); + +test('api mode requires a secret, and every secret needs 32 bytes', () => { + assert.throws(() => normalizeLoginConfig({ oauth: fakeOauth(), mode: 'api' }), /requires a secret/); + assert.throws(() => normalizeLoginConfig({ oauth: fakeOauth(), mode: 'api', secret: 'short' }), /32 bytes/); + assert.throws(() => normalizeLoginConfig({ oauth: fakeOauth(), cookie: { secret: 'short' } }), /32 bytes/); + // web mode without a secret still uses the server-side store. + assert.equal(normalizeLoginConfig({ oauth: fakeOauth() }).cookieMode, false); +}); diff --git a/packages/oauth2/tests/oauth.test.js b/packages/oauth2/tests/oauth.test.js index 81d096ff..778e0b20 100644 --- a/packages/oauth2/tests/oauth.test.js +++ b/packages/oauth2/tests/oauth.test.js @@ -233,3 +233,45 @@ test('authorize on an unknown provider rejects', async () => { return true; }); }); + +test('caller params cannot override the flow-managed authorize parameters', async () => { + const oauth = mkOAuth(); + for (const key of ['state', 'nonce', 'redirect_uri', 'code_challenge', 'code_challenge_method', 'scope']) { + await assert.rejects(oauth.authorize('test', { params: { [key]: 'attacker' } }), { + code: ErrorCode.INVALID_ARGUMENT, + }); + } + const { url } = await oauth.authorize('test', { params: { prompt: 'consent' } }); + assert.equal(new URL(url).searchParams.get('prompt'), 'consent'); +}); + +test('an OIDC provider without an issuer is a definition error', () => { + assert.throws( + () => + defineProvider({ + id: 'bare', + kind: 'oidc', + authorizationEndpoint: 'https://x/a', + tokenEndpoint: 'https://x/t', + mapUser: r => r, + }), + { code: ErrorCode.INVALID_ARGUMENT }, + ); +}); + +test('an email the provider does not report as verified is flagged', async () => { + const provider = testProvider({ mapUser: raw => ({ sub: raw.sub, email: raw.email }) }); + const oauth = createOAuth({ + baseUrl: 'https://app.com', + callback: '/auth/{provider}/callback', + providers: [provider({ clientId: CLIENT_ID, clientSecret: 'secret' })], + }); + const { session } = await oauth.authorize('test'); + const parsed = JSON.parse(Buffer.from(session, 'base64url').toString()); + idTokenToServe = await signer.mint( + { iss: as.issuer, sub: 'user-1', aud: CLIENT_ID, nonce: parsed.nonce }, + { expiresIn: '5m' }, + ); + const { warnings } = await oauth.callback('test', { code: 'c', state: parsed.state }, { session }); + assert.ok(warnings.some(w => w.code === 'EMAIL_UNVERIFIED')); +}); diff --git a/packages/oauth2/tests/registration.test.js b/packages/oauth2/tests/registration.test.js index 0a57487e..a3cdc58a 100644 --- a/packages/oauth2/tests/registration.test.js +++ b/packages/oauth2/tests/registration.test.js @@ -108,3 +108,67 @@ test('open registration is allowed only when explicitly opened', async () => { const reg = body(await server.register(jsonPost({ redirect_uris: ['https://o/cb'] }))); assert.ok(reg.client_id); }); + +test('a registrant cannot pick client_credentials or an unadvertised scope', async () => { + const { server } = buildRegistrationServer({ initialAccessToken: IAT }); + const auth = { authorization: `Bearer ${IAT}` }; + + const grant = await server.register( + jsonPost({ grant_types: ['client_credentials'], token_endpoint_auth_method: 'client_secret_post' }, auth), + ); + assert.equal(grant.status, 400); + assert.equal(body(grant).error, 'invalid_client_metadata'); + + const scope = await server.register( + jsonPost({ redirect_uris: ['https://c.example.com/cb'], scope: 'admin superuser' }, auth), + ); + assert.equal(scope.status, 400); + assert.equal(body(scope).error, 'invalid_client_metadata'); + + const method = await server.register( + jsonPost({ redirect_uris: ['https://c.example.com/cb'], token_endpoint_auth_method: 'tls_client_auth' }, auth), + ); + assert.equal(method.status, 400); + assert.equal(body(method).error, 'invalid_client_metadata'); +}); + +test('registration.allowedGrants opts a deployment into more grants', async () => { + const { server } = buildRegistrationServer({ + initialAccessToken: IAT, + allowedGrants: ['client_credentials'], + }); + const reg = body( + await server.register( + jsonPost( + { + redirect_uris: ['https://c.example.com/cb'], + grant_types: ['client_credentials'], + token_endpoint_auth_method: 'client_secret_post', + scope: 'read', + }, + { authorization: `Bearer ${IAT}` }, + ), + ), + ); + assert.ok(reg.client_id); + const tok = body( + await server.token( + post({ grant_type: 'client_credentials', client_id: reg.client_id, client_secret: reg.client_secret }), + ), + ); + assert.equal(tok.scope, 'read'); +}); + +test('a registered redirect_uri must be https, loopback http, or a private-use scheme', async () => { + const { server } = buildRegistrationServer({ initialAccessToken: IAT }); + const auth = { authorization: `Bearer ${IAT}` }; + for (const uri of ['javascript:alert(1)', 'http://evil.example/cb', 'https://c.example.com/cb#frag', 'myapp:/cb']) { + const res = await server.register(jsonPost({ redirect_uris: [uri] }, auth)); + assert.equal(res.status, 400, uri); + assert.equal(body(res).error, 'invalid_client_metadata', uri); + } + for (const uri of ['http://127.0.0.1:8080/cb', 'com.example.app:/oauth/cb']) { + const res = await server.register(jsonPost({ redirect_uris: [uri] }, auth)); + assert.equal(res.status, 201, uri); + } +}); diff --git a/packages/oauth2/tests/server-hardening.test.js b/packages/oauth2/tests/server-hardening.test.js new file mode 100644 index 00000000..606c844c --- /dev/null +++ b/packages/oauth2/tests/server-hardening.test.js @@ -0,0 +1,594 @@ +/** + * Authorization-server hardening: the scope ceiling, dynamic-registration + * limits, PAR integrity, mTLS chain checks, refresh / device lifetimes and + * token-exchange subject checks. Each test reproduces an escalation or + * bypass that used to succeed. + */ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; + +import { buildServer, getAuthorizationCode, dpopClient, get, post, body, ISSUER } from './helpers/server.js'; + +// SCOPE CEILING + +const WIDE_CLIENT = { + clientId: 'wide', + clientSecret: 's3cret-value-strong', + tokenEndpointAuthMethod: 'client_secret_post', + redirectUris: ['https://wide.example.com/cb'], + grantTypes: ['authorization_code', 'client_credentials', 'urn:ietf:params:oauth:grant-type:device_code'], + // Registers a scope the server never advertises. + scope: ['read', 'admin'], +}; + +test('scope ceiling: authorize refuses a client scope the server does not advertise', async () => { + const { server } = buildServer({ clients: [WIDE_CLIENT], scopes: ['read', 'write'] }); + const { res, code } = await getAuthorizationCode(server, { + clientId: 'wide', + redirectUri: 'https://wide.example.com/cb', + scope: 'read admin', + }); + assert.equal(code, null); + assert.equal(new URL(res.headers.location).searchParams.get('error'), 'invalid_scope'); +}); + +test('scope ceiling: client_credentials refuses, and never defaults to, an unadvertised scope', async () => { + const { server } = buildServer({ clients: [WIDE_CLIENT], scopes: ['read', 'write'] }); + const creds = { client_id: 'wide', client_secret: 's3cret-value-strong' }; + + const asked = await server.token(post({ grant_type: 'client_credentials', scope: 'admin', ...creds })); + assert.equal(asked.status, 400); + assert.equal(body(asked).error, 'invalid_scope'); + + const implicit = body(await server.token(post({ grant_type: 'client_credentials', ...creds }))); + assert.equal(implicit.scope, 'read'); +}); + +test('scope ceiling: device authorization validates the requested scope', async () => { + const { server } = buildServer({ + clients: [WIDE_CLIENT], + scopes: ['read', 'write'], + grants: ['urn:ietf:params:oauth:grant-type:device_code'], + }); + const res = await server.deviceAuthorization( + post({ client_id: 'wide', client_secret: 's3cret-value-strong', scope: 'admin' }), + ); + assert.equal(res.status, 400); + assert.equal(body(res).error, 'invalid_scope'); +}); + +// PAR + +const PKCE_PARAMS = { + redirect_uri: 'https://app.example.com/cb', + response_type: 'code', + code_challenge: 'a'.repeat(43), + code_challenge_method: 'S256', +}; + +test('PAR: a forged _fromPar parameter does not satisfy a PAR requirement', async () => { + const { server } = buildServer({ security: { par: { required: true } } }); + const res = await server.authorize(get('/authorize', { client_id: 'app', _fromPar: '1', ...PKCE_PARAMS })); + const location = new URL(res.headers.location); + assert.equal(location.searchParams.get('code'), null); + assert.equal(location.searchParams.get('error'), 'invalid_request'); +}); + +test('PAR: request_uri must be redeemed with the client_id that pushed it', async () => { + const other = { + clientId: 'other', + clientSecret: 'other-secret-value-strong', + tokenEndpointAuthMethod: 'client_secret_post', + redirectUris: ['https://app.example.com/cb'], + }; + const { server } = buildServer({ + clients: [ + { + clientId: 'app', + clientSecret: 's3cret-value-strong', + tokenEndpointAuthMethod: 'client_secret_post', + redirectUris: ['https://app.example.com/cb'], + }, + other, + ], + }); + const pushed = body( + await server.par(post({ client_id: 'app', client_secret: 's3cret-value-strong', ...PKCE_PARAMS })), + ); + + const missing = await server.authorize(get('/authorize', { request_uri: pushed.request_uri })); + assert.equal(body(missing).error, 'invalid_request'); + + const pushedAgain = body( + await server.par(post({ client_id: 'app', client_secret: 's3cret-value-strong', ...PKCE_PARAMS })), + ); + const mismatch = await server.authorize( + get('/authorize', { client_id: 'other', request_uri: pushedAgain.request_uri }), + ); + assert.equal(body(mismatch).error, 'invalid_request'); +}); + +test('PAR: a pushed request object is verified at push time', async () => { + const { server } = buildServer(); + const res = await server.par(post({ client_id: 'app', client_secret: 's3cret-value-strong', request: 'not.a.jwt' })); + assert.equal(res.status, 400); + assert.equal(body(res).error, 'invalid_request'); +}); + +// mTLS (tls_client_auth) + +const PKI_CLIENT = { + clientId: 'pki', + tokenEndpointAuthMethod: 'tls_client_auth', + tlsClientAuthSubjectDn: 'CN=pki-client,O=Bank', + redirectUris: ['https://x.example.com/cb'], + grantTypes: ['client_credentials'], + scope: ['read'], +}; + +function mtlsRequest(subject, authorized) { + return { + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ grant_type: 'client_credentials', client_id: 'pki' }).toString(), + clientCertificate: { raw: Buffer.from('der'), subject }, + clientCertificateAuthorized: authorized, + }; +} + +test('tls_client_auth: an unverified certificate with a matching DN is refused', async () => { + const { server } = buildServer({ clients: [PKI_CLIENT], grants: ['client_credentials'] }); + const res = await server.token(mtlsRequest({ CN: 'pki-client', O: 'Bank' }, false)); + assert.equal(res.status, 401); + assert.equal(body(res).error, 'invalid_client'); + + const ok = await server.token(mtlsRequest({ O: 'Bank', CN: 'pki-client' }, true)); + assert.equal(ok.status, 200); +}); + +test('tls_client_auth: DN values are compared structurally, not as joined strings', async () => { + const { server } = buildServer({ + clients: [{ ...PKI_CLIENT, tlsClientAuthSubjectDn: 'CN=foo\\,O=Bank' }], + grants: ['client_credentials'], + }); + // Joined naively this subject renders as "CN=foo,O=Bank" too. + const forged = await server.token(mtlsRequest({ CN: 'foo', O: 'Bank' }, true)); + assert.equal(forged.status, 401); + + const real = await server.token(mtlsRequest({ CN: 'foo,O=Bank' }, true)); + assert.equal(real.status, 200); +}); + +test('express adapter passes socket.authorized through', async () => { + const { server } = buildServer({ clients: [PKI_CLIENT], grants: ['client_credentials'] }); + const { expressHandler } = await import('../src/server/middleware/express.js'); + const handle = expressHandler(server.token); + const statuses = []; + for (const authorized of [false, true]) { + const socket = { + authorized, + getPeerCertificate: () => ({ raw: Buffer.from('der'), subject: { CN: 'pki-client', O: 'Bank' } }), + }; + const req = { + method: 'POST', + url: '/token', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body: { grant_type: 'client_credentials', client_id: 'pki' }, + query: {}, + socket, + }; + const res = { + status(code) { + statuses.push(code); + return this; + }, + setHeader() {}, + send() {}, + }; + await handle(req, res); + } + assert.deepEqual(statuses, [401, 200]); +}); + +// REFRESH ROTATION + +async function issueRefresh(server) { + const { code, pkce, redirectUri } = await getAuthorizationCode(server, { scope: 'read write' }); + return body( + await server.token( + post({ + grant_type: 'authorization_code', + code, + redirect_uri: redirectUri, + client_id: 'app', + client_secret: 's3cret-value-strong', + code_verifier: pkce.codeVerifier, + }), + ), + ); +} + +function refresh(server, token, extra = {}) { + return server.token( + post({ + grant_type: 'refresh_token', + refresh_token: token, + client_id: 'app', + client_secret: 's3cret-value-strong', + ...extra, + }), + ); +} + +test('refresh: a rotated token keeps the family expiry', async () => { + const { server } = buildServer({ security: { refreshTokenTtl: 80 } }); + const first = await issueRefresh(server); + const rotated = body(await refresh(server, first.refresh_token)); + assert.ok(rotated.refresh_token); + + const record = await server._config.stores.refresh.get(rotated.refresh_token); + assert.equal(typeof record.expiresAt, 'number'); + + await new Promise(r => setTimeout(r, 100)); + assert.equal(body(await refresh(server, rotated.refresh_token)).error, 'invalid_grant'); +}); + +test('refresh: two simultaneous redemptions cannot fork the family', async () => { + const { server } = buildServer(); + const first = await issueRefresh(server); + const results = (await Promise.all([refresh(server, first.refresh_token), refresh(server, first.refresh_token)])).map( + body, + ); + const winners = results.filter(r => r.refresh_token); + assert.equal(winners.length, 1); + assert.equal(results.find(r => !r.refresh_token).error, 'invalid_grant'); + // The race is treated as reuse: the winner's successor is burned too. + assert.equal(body(await refresh(server, winners[0].refresh_token)).error, 'invalid_grant'); +}); + +test('refresh: the memory store evicts expired tokens in the background', async () => { + const { createRefreshStore } = await import('../src/server/stores.js'); + const store = createRefreshStore({ sweepIntervalMs: 10 }); + await store.save({ token: 'r1', familyId: 'f1', clientId: 'app', scope: [], expiresAt: Date.now() + 5 }); + await new Promise(r => setTimeout(r, 40)); + assert.equal(await store.get('r1'), null); + await store.revokeFamily('f1'); + assert.equal(await store.get('r1'), null); +}); + +test('refresh: a narrowed request does not narrow the successor refresh token', async () => { + const { server } = buildServer(); + const first = await issueRefresh(server); + const narrowed = body(await refresh(server, first.refresh_token, { scope: 'read' })); + assert.equal(narrowed.scope, 'read'); + const next = body(await refresh(server, narrowed.refresh_token, { scope: 'write' })); + assert.equal(next.scope, 'write'); +}); + +// TOKEN EXCHANGE + +const EXCHANGE = 'urn:ietf:params:oauth:grant-type:token-exchange'; +const ACCESS_TOKEN_TYPE = 'urn:ietf:params:oauth:token-type:access_token'; + +function exchangeServer(extra = {}) { + const svc = id => ({ + clientId: id, + clientSecret: `${id}-secret-value-strong`, + tokenEndpointAuthMethod: 'client_secret_post', + redirectUris: ['https://x.example.com/cb'], + grantTypes: ['client_credentials', EXCHANGE], + scope: ['api'], + }); + return buildServer({ + clients: [svc('alice'), svc('mallory')], + grants: ['client_credentials', EXCHANGE], + scopes: ['api'], + tokenExchange: { audiences: ['https://downstream.example.com'] }, + ...extra, + }); +} + +const creds = id => ({ client_id: id, client_secret: `${id}-secret-value-strong` }); + +async function ccToken(server, id, headers = {}) { + return body(await server.token(post({ grant_type: 'client_credentials', scope: 'api', ...creds(id) }, headers))); +} + +function exchange(server, id, subjectToken, extra = {}, headers = {}) { + return server.token( + post( + { + grant_type: EXCHANGE, + subject_token: subjectToken, + subject_token_type: ACCESS_TOKEN_TYPE, + ...creds(id), + ...extra, + }, + headers, + ), + ); +} + +test('token exchange: a token issued to another client cannot be exchanged', async () => { + const { server } = exchangeServer(); + const alice = await ccToken(server, 'alice'); + const res = await exchange(server, 'mallory', alice.access_token); + assert.equal(res.status, 400); + assert.equal(body(res).error, 'invalid_grant'); + + assert.equal((await exchange(server, 'alice', alice.access_token)).status, 200); +}); + +test('token exchange: a DPoP-bound subject token needs the same key', async () => { + const { server } = exchangeServer(); + const dpop = await dpopClient(); + const htu = `${ISSUER}/token`; + const bound = await ccToken(server, 'alice', { dpop: await dpop.proof('POST', htu) }); + assert.equal(bound.token_type, 'DPoP'); + + const stripped = await exchange(server, 'alice', bound.access_token); + assert.equal(body(stripped).error, 'invalid_grant'); + + const proven = await exchange(server, 'alice', bound.access_token, {}, { dpop: await dpop.proof('POST', htu) }); + assert.equal(proven.status, 200); +}); + +test('token exchange: the target audience must be allowlisted', async () => { + const { server } = exchangeServer(); + const alice = await ccToken(server, 'alice'); + const res = await exchange(server, 'alice', alice.access_token, { audience: 'https://evil.example.com' }); + assert.equal(body(res).error, 'invalid_target'); + const ok = await exchange(server, 'alice', alice.access_token, { resource: 'https://downstream.example.com' }); + assert.equal(ok.status, 200); +}); + +test('token exchange: a prior actor chain nests under the new act', async () => { + const { server } = exchangeServer(); + const alice = await ccToken(server, 'alice'); + const once = body(await exchange(server, 'alice', alice.access_token)); + const twice = body(await exchange(server, 'alice', once.access_token)); + const claims = JSON.parse(Buffer.from(twice.access_token.split('.')[1], 'base64url').toString()); + assert.deepEqual(claims.act, { sub: 'alice', act: { sub: 'alice' } }); +}); + +// CERTIFICATE BINDING + +test('refresh and device grants keep mTLS certificate binding', async () => { + const der = Buffer.from('device-cert-der'); + const { createHash } = await import('node:crypto'); + const thumb = createHash('sha256').update(der).digest('base64url'); + const DEVICE = 'urn:ietf:params:oauth:grant-type:device_code'; + const { server } = buildServer({ + clients: [ + { + clientId: 'mtls', + tokenEndpointAuthMethod: 'self_signed_tls_client_auth', + certificateThumbprint: thumb, + redirectUris: ['https://x.example.com/cb'], + grantTypes: [DEVICE, 'refresh_token'], + scope: ['read'], + }, + ], + grants: [DEVICE, 'refresh_token'], + }); + const call = (handler, params) => + handler({ + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ client_id: 'mtls', ...params }).toString(), + clientCertificate: { raw: der }, + }); + const cnfOf = tok => JSON.parse(Buffer.from(tok.split('.')[1], 'base64url').toString()).cnf; + + const da = body(await call(server.deviceAuthorization, { scope: 'read' })); + await server.device.approve(da.user_code, { subject: 'user-1' }); + const tok = body(await call(server.token, { grant_type: DEVICE, device_code: da.device_code })); + assert.equal(cnfOf(tok.access_token)['x5t#S256'], thumb); + + const refreshed = body(await call(server.token, { grant_type: 'refresh_token', refresh_token: tok.refresh_token })); + assert.equal(cnfOf(refreshed.access_token)['x5t#S256'], thumb); +}); + +// DEVICE CODE LIFETIME + +function deviceServer(device) { + const DEVICE = 'urn:ietf:params:oauth:grant-type:device_code'; + const { server } = buildServer({ + clients: [ + { + clientId: 'tv', + tokenEndpointAuthMethod: 'none', + redirectUris: ['https://x.example.com/cb'], + grantTypes: [DEVICE], + }, + ], + grants: [DEVICE], + device, + }); + const poll = deviceCode => server.token(post({ grant_type: DEVICE, device_code: deviceCode, client_id: 'tv' })); + return { server, poll }; +} + +test('device: polling never extends the device code past its lifetime', async () => { + const { server, poll } = deviceServer({ ttl: 80, interval: 0 }); + const da = body(await server.deviceAuthorization(post({ client_id: 'tv' }))); + assert.equal(body(await poll(da.device_code)).error, 'authorization_pending'); + await new Promise(r => setTimeout(r, 40)); + assert.equal(body(await poll(da.device_code)).error, 'authorization_pending'); + await new Promise(r => setTimeout(r, 60)); + assert.equal(body(await poll(da.device_code)).error, 'expired_token'); + assert.equal(await server.device.getByUserCode(da.user_code), undefined); +}); + +test('device: two concurrent polls of an approved code redeem it once', async () => { + const { server, poll } = deviceServer({ interval: 0 }); + const da = body(await server.deviceAuthorization(post({ client_id: 'tv' }))); + await server.device.approve(da.user_code, { subject: 'user-1' }); + const results = (await Promise.all([poll(da.device_code), poll(da.device_code)])).map(body); + assert.equal(results.filter(r => r.access_token).length, 1); + assert.equal(results.find(r => !r.access_token).error, 'invalid_grant'); +}); + +// CLIENT JWKS + +test('client jwks_uri: the resolver is cached across token requests', async () => { + const { assertionClient } = await import('./helpers/server.js'); + const pk = await assertionClient('pk'); + const { server } = buildServer({ + clients: [ + { + clientId: 'pk', + tokenEndpointAuthMethod: 'private_key_jwt', + jwksUri: 'https://keys.example.com/jwks.json', + redirectUris: ['https://x.example.com/cb'], + grantTypes: ['client_credentials'], + scope: ['read'], + }, + ], + grants: ['client_credentials'], + }); + const realFetch = globalThis.fetch; + let fetches = 0; + globalThis.fetch = async () => { + fetches += 1; + return new Response(JSON.stringify({ keys: [pk.jwk] }), { headers: { 'content-type': 'application/json' } }); + }; + try { + for (const jti of ['a1', 'a2', 'a3']) { + const res = await server.token( + post({ + grant_type: 'client_credentials', + client_assertion_type: 'urn:ietf:params:oauth:client-assertion-type:jwt-bearer', + client_assertion: await pk.assertion(jti), + }), + ); + assert.equal(res.status, 200); + } + } finally { + globalThis.fetch = realFetch; + } + assert.equal(fetches, 1); +}); + +test('client jwks_uri: private hosts are refused when registration is enabled', async () => { + const { isPublicHost } = await import('../src/server/security/client-keys.js'); + for (const host of [ + 'localhost', + '127.0.0.1', + '10.1.2.3', + '172.20.0.1', + '192.168.1.1', + '169.254.169.254', + '[::1]', + '[fd00::1]', + '[::ffff:127.0.0.1]', + ]) { + assert.equal(isPublicHost(host), false, host); + } + for (const host of ['keys.example.com', '8.8.8.8', '[2606:4700::1111]']) { + assert.equal(isPublicHost(host), true, host); + } + + const { createClientRegistry } = await import('../src/server/index.js'); + const { server } = buildServer({ + clients: createClientRegistry([ + { + clientId: 'ssrf', + tokenEndpointAuthMethod: 'private_key_jwt', + jwksUri: 'https://169.254.169.254/latest/meta-data', + redirectUris: ['https://x.example.com/cb'], + grantTypes: ['client_credentials'], + }, + ]), + grants: ['client_credentials'], + registration: { initialAccessToken: 'iat-value-strong-enough' }, + }); + const { assertionClient } = await import('./helpers/server.js'); + const attacker = await assertionClient('ssrf'); + const res = await server.token( + post({ + grant_type: 'client_credentials', + client_assertion_type: 'urn:ietf:params:oauth:client-assertion-type:jwt-bearer', + client_assertion: await attacker.assertion('x1'), + }), + ); + assert.equal(res.status, 401); + assert.equal(body(res).error, 'invalid_client'); +}); + +// MULTI-NODE REPLAY + NONCES + +test('stores.replay shares client-assertion replay checks across servers', async () => { + const { createMemoryIncrStore } = await import('@exortek/shared/incr-store'); + const { assertionClient } = await import('./helpers/server.js'); + const pk = await assertionClient('pk'); + const replay = createMemoryIncrStore(); + const node = () => + buildServer({ + clients: [ + { + clientId: 'pk', + tokenEndpointAuthMethod: 'private_key_jwt', + jwks: { keys: [pk.jwk] }, + redirectUris: ['https://x.example.com/cb'], + grantTypes: ['client_credentials'], + scope: ['read'], + }, + ], + grants: ['client_credentials'], + stores: { replay }, + }).server; + const assertion = await pk.assertion('only-once'); + const req = () => + post({ + grant_type: 'client_credentials', + client_assertion_type: 'urn:ietf:params:oauth:client-assertion-type:jwt-bearer', + client_assertion: assertion, + }); + assert.equal((await node().token(req())).status, 200); + const replayed = await node().token(req()); + assert.equal(replayed.status, 401); + assert.match(body(replayed).error_description, /replay/); +}); + +test('DPoP nonces issued by one node are accepted by another with the same secret', async () => { + const nonceSecret = 'n'.repeat(32); + const a = buildServer({ security: { dpop: { nonce: true, nonceSecret } } }).server; + const b = buildServer({ security: { dpop: { nonce: true, nonceSecret } } }).server; + const nonce = a._config.dpopNonces.issue(); + assert.equal(b._config.dpopNonces.isValid(nonce), true); + assert.equal(buildServer({ security: { dpop: { nonce: true } } }).server._config.dpopNonces.isValid(nonce), false); + assert.equal(b._config.dpopNonces.isValid(`${nonce.slice(0, -2)}AA`), false); + assert.throws(() => buildServer({ security: { dpop: { nonce: true, nonceSecret: 'short' } } }), /32 bytes/); +}); + +// AUTHORIZATION CODE REUSE + INTROSPECTION + +test('a replayed authorization code revokes the refresh family it produced', async () => { + const { server } = buildServer(); + const { code, pkce, redirectUri } = await getAuthorizationCode(server, { scope: 'read' }); + const redeem = () => + server.token( + post({ + grant_type: 'authorization_code', + code, + redirect_uri: redirectUri, + client_id: 'app', + client_secret: 's3cret-value-strong', + code_verifier: pkce.codeVerifier, + }), + ); + const first = body(await redeem()); + assert.ok(first.refresh_token); + assert.equal(body(await redeem()).error, 'invalid_grant'); + assert.equal(body(await refresh(server, first.refresh_token)).error, 'invalid_grant'); +}); + +test('introspection refuses a public client unless allowed', async () => { + const pub = { clientId: 'spa', tokenEndpointAuthMethod: 'none', redirectUris: ['https://spa.example.com/cb'] }; + const refused = buildServer({ clients: [pub] }).server; + const res = await refused.introspect(post({ token: 'x', client_id: 'spa' })); + assert.equal(res.status, 401); + const allowed = buildServer({ clients: [pub], introspection: { allowPublicClients: true } }).server; + assert.equal((await allowed.introspect(post({ token: 'x', client_id: 'spa' }))).status, 200); +}); diff --git a/packages/oauth2/tests/server-tierc.test.js b/packages/oauth2/tests/server-tierc.test.js index 970d007c..e4757ab7 100644 --- a/packages/oauth2/tests/server-tierc.test.js +++ b/packages/oauth2/tests/server-tierc.test.js @@ -26,6 +26,7 @@ test('client_credentials issues a token with no refresh (RFC 6749 §4.4)', async }, ], grants: [CC], + scopes: ['api'], }); const tok = body( await server.token( @@ -40,6 +41,7 @@ test('a public client cannot use client_credentials', async () => { const { server } = buildServer({ clients: [{ clientId: 'pub', tokenEndpointAuthMethod: 'none', redirectUris: ['https://x/cb'], grantTypes: [CC] }], grants: [CC], + scopes: ['api'], }); const res = body(await server.token(post({ grant_type: CC, client_id: 'pub' }))); assert.equal(res.error, 'unauthorized_client'); @@ -59,6 +61,7 @@ test('private_key_jwt client auth with assertion replay defense (RFC 7523, threa }, ], grants: [CC], + scopes: ['api'], }); const ok = await server.token( @@ -112,6 +115,7 @@ test('client_secret_jwt client auth (HS256)', async () => { }, ], grants: [CC], + scopes: ['api'], }); const now = Math.floor(Date.now() / 1000); const assertion = await jwtSign( @@ -140,6 +144,7 @@ test('mTLS self_signed client auth binds a certificate-bound token (RFC 8705)', }, ], grants: [CC], + scopes: ['api'], }); const req = { @@ -218,6 +223,8 @@ test('token exchange delegates a subject token to a new audience (RFC 8693)', as }, ], grants: [CC, EXCHANGE], + scopes: ['api'], + tokenExchange: { audiences: ['https://downstream.example.com'] }, }); const subjectTok = body( @@ -309,6 +316,7 @@ test('a confidential client cannot authenticate with the wrong method', async () }, ], grants: [CC], + scopes: ['api'], }); // Registered basic, presenting post → refused. const res = body( diff --git a/packages/oauth2/tests/server.test.js b/packages/oauth2/tests/server.test.js index 196a9108..c2f34a34 100644 --- a/packages/oauth2/tests/server.test.js +++ b/packages/oauth2/tests/server.test.js @@ -271,10 +271,10 @@ test('PAR pushes params and the request_uri is single-use', async () => { ); assert.ok(pushed.request_uri.startsWith('urn:ietf:params:oauth:request_uri:')); - const ok = await server.authorize(get('/authorize', { request_uri: pushed.request_uri })); + const ok = await server.authorize(get('/authorize', { client_id: 'app', request_uri: pushed.request_uri })); assert.ok(new URL(ok.headers.location).searchParams.get('code')); - const reuse = await server.authorize(get('/authorize', { request_uri: pushed.request_uri })); + const reuse = await server.authorize(get('/authorize', { client_id: 'app', request_uri: pushed.request_uri })); assert.equal(body(reuse).error, 'invalid_request'); }); diff --git a/packages/oauth2/tests/stores/redis.integration.test.js b/packages/oauth2/tests/stores/redis.integration.test.js index 311d4d20..f379f45d 100644 --- a/packages/oauth2/tests/stores/redis.integration.test.js +++ b/packages/oauth2/tests/stores/redis.integration.test.js @@ -9,7 +9,13 @@ import assert from 'node:assert/strict'; import { forEachRedisDriver } from '@exortek/shared/test-helpers/redis-drivers'; -import { createRedisAuthCodeStore, createRedisRefreshStore, createRedisDeviceStore } from '../../src/server/index.js'; +import { + createRedisAuthCodeStore, + createRedisRefreshStore, + createRedisDeviceStore, + createRedisReplayStore, +} from '../../src/server/index.js'; +import { createReplayGuard } from '../../src/server/security/replay.js'; forEachRedisDriver('oauth2 server stores', ({ test, client, ns, raw }) => { test('auth-code single-use consume', async () => { @@ -44,12 +50,61 @@ forEachRedisDriver('oauth2 server stores', ({ test, client, ns, raw }) => { assert.equal((await store.get('t2')).revoked, true); }); + test('refresh rotation keeps both tokens on a TTL', async () => { + const keyPrefix = ns('c2'); + const store = createRedisRefreshStore(client(), { keyPrefix }); + const base = { familyId: 'f1', clientId: 'app', scope: [], expiresAt: Date.now() + 60_000 }; + + await store.save({ ...base, token: 't1' }); + await store.rotate('t1', { ...base, token: 't2' }); + for (const token of ['t1', 't2']) { + const pttl = await raw.pttl(`${keyPrefix}refresh:${token}`); + assert.ok(pttl > 0 && pttl <= 60_000, `${token} must keep an expiry; got PTTL ${pttl}`); + } + }); + + test('refresh rotate is a compare-and-set across clients', async () => { + const store = createRedisRefreshStore(client(), { keyPrefix: ns('c3') }); + const base = { familyId: 'f1', clientId: 'app', scope: [], expiresAt: Date.now() + 60_000 }; + await store.save({ ...base, token: 't1' }); + + const outcomes = await Promise.all([ + store.rotate('t1', { ...base, token: 't2a' }), + store.rotate('t1', { ...base, token: 't2b' }), + ]); + assert.deepEqual(outcomes.sort(), [false, true]); + }); + test('an expired refresh record reads as absent', async () => { const store = createRedisRefreshStore(client(), { keyPrefix: ns('d') }); await store.save({ token: 't1', familyId: 'f1', clientId: 'app', scope: [], expiresAt: Date.now() - 1 }); assert.equal(await store.get('t1'), null); }); + test('device updates keep the original expiry and redeem once', async () => { + const keyPrefix = ns('e2'); + const store = createRedisDeviceStore(client(), { keyPrefix }); + await store.save( + { deviceCode: 'd2', userCode: 'BCDF-2345', clientId: 'app', scope: [], status: 'pending' }, + 60_000, + ); + await store.update('d2', { status: 'approved', subject: 'u' }); + const pttl = await raw.pttl(`${keyPrefix}device:d:d2`); + assert.ok(pttl > 0 && pttl <= 60_000, `update must keep the expiry; got PTTL ${pttl}`); + assert.deepEqual((await store.getByDeviceCode('d2')).scope, []); + + const outcomes = await Promise.all([store.redeem('d2'), store.redeem('d2')]); + assert.deepEqual(outcomes.sort(), [false, true]); + }); + + test('replay store: a jti is seen once across guards', async () => { + const store = createRedisReplayStore(client(), { keyPrefix: ns('f') }); + const nodeA = createReplayGuard(store); + const nodeB = createReplayGuard(store); + assert.equal(await nodeA.seen('dpop', 'jti-1', 60_000), false); + assert.equal(await nodeB.seen('dpop', 'jti-1', 60_000), true); + }); + test('device lookup + reverse-index cleanup', async () => { const store = createRedisDeviceStore(client(), { keyPrefix: ns('e') }); await store.save({ deviceCode: 'd1', userCode: 'WXYZ-2345', clientId: 'app', scope: [], status: 'pending' }, 5_000); diff --git a/packages/oauth2/tests/stores/redis.test.js b/packages/oauth2/tests/stores/redis.test.js index 7690bfc2..67b9a01f 100644 --- a/packages/oauth2/tests/stores/redis.test.js +++ b/packages/oauth2/tests/stores/redis.test.js @@ -27,11 +27,23 @@ function fakeClient() { async get(k) { return live(k) ? kv.get(k) : null; }, + // Mirrors real SET semantics for both driver call forms: NX refuses an + // existing key, PX sets an expiry, KEEPTTL keeps it, anything else + // clears it. async set(k, v, ...rest) { + const nx = rest.includes('NX') || rest[0]?.NX === true; + if (nx && live(k)) { + return null; + } kv.set(k, v); + const opts = typeof rest[0] === 'object' && rest[0] !== null ? rest[0] : null; const ix = rest.indexOf('PX'); if (ix !== -1) ttl.set(k, Date.now() + Number(rest[ix + 1])); + else if (opts?.PX !== undefined) ttl.set(k, Date.now() + Number(opts.PX)); + else if (!rest.includes('KEEPTTL') && !opts?.KEEPTTL) ttl.delete(k); + return 'OK'; }, + _ttl: ttl, async del(k) { kv.delete(k); ttl.delete(k); @@ -80,6 +92,16 @@ test('redis refresh store: rotate marks used and revokeFamily burns the family', assert.equal((await store.get('t2')).revoked, true); }); +test('redis refresh store: rotation keeps both tokens on a TTL', async () => { + const client = fakeClient(); + const store = createRedisRefreshStore(client, { keyPrefix: 't:' }); + const expiresAt = Date.now() + 60_000; + await store.save({ token: 'r1', familyId: 'f', clientId: 'app', scope: [], expiresAt }); + await store.rotate('r1', { token: 'r2', familyId: 'f', clientId: 'app', scope: [], expiresAt }); + assert.ok(client._ttl.has('t:refresh:r1'), 'rotated-away token lost its TTL'); + assert.ok(client._ttl.has('t:refresh:r2'), 'successor stored without a TTL'); +}); + test('redis refresh store: an expired record reads as absent', async () => { const store = createRedisRefreshStore(fakeClient()); await store.save({ token: 't1', familyId: 'f1', clientId: 'app', scope: [], expiresAt: Date.now() - 1 }); diff --git a/packages/oidc/src/client/index.js b/packages/oidc/src/client/index.js index a1dcd3c9..296ce5c0 100644 --- a/packages/oidc/src/client/index.js +++ b/packages/oidc/src/client/index.js @@ -16,9 +16,10 @@ */ import { createOAuth, defineProvider } from '@exortek/oauth2'; import { decode } from '@exortek/jwt'; -import { isArray, isNonEmptyString, isObject } from '@exortek/shared/predicates'; +import { parseDuration } from '@exortek/shared/duration'; +import { isArray, isNonEmptyString, isNumber, isObject } from '@exortek/shared/predicates'; -import { invalidArgument } from '../internal/errors.js'; +import { authenticationRequirementsNotMet, invalidArgument } from '../internal/errors.js'; import { buildEndSessionUrl } from '../internal/logout.js'; import { resolveIssuerMetadata } from '../internal/issuer-discovery.js'; @@ -57,7 +58,9 @@ const AUTH_PARAM_NAMES = { * @property {string} redirectUri Registered redirect URI for the callback. * @property {string[]} [scope] Requested scopes; `openid` is enforced. * @property {string[]} [idTokenAlgs] Signature alg allowlist for the id_token. - * @property {string|number} [clockTolerance] Leeway for `exp`/`nbf`/`iat`. + * @property {string|number} [clockTolerance] Leeway for `exp`/`nbf`/`iat` (and `auth_time` against `maxAge`). + * @property {number} [maxAge] Default `max_age` (seconds) for every authorization request; enforced on `auth_time` at the callback. + * @property {string} [acrValues] Default `acr_values` for every authorization request; the returned `acr` must be one of them. * @property {import('@exortek/jwks').RemoteJWKSOptions} [jwksOptions] Forwarded to the id_token JWKS resolver. * @property {string} [endSessionEndpoint] RP-Initiated Logout endpoint; auto-discovered from OP metadata when omitted. * @property {typeof fetch} [fetch] Override used only for the logout-endpoint metadata fetch (tests / proxies). @@ -132,8 +135,9 @@ export function createClient(config) { async authorize(options = {}) { /** @type {Record} */ const params = { ...options.params }; + const withDefaults = { maxAge: config.maxAge, acrValues: config.acrValues, ...options }; for (const [key, wire] of Object.entries(AUTH_PARAM_NAMES)) { - const value = /** @type {Record} */ (options)[key]; + const value = /** @type {Record} */ (withDefaults)[key]; if (value !== undefined) { params[wire] = String(value); } @@ -148,16 +152,28 @@ export function createClient(config) { * are already checked inside the oauth2 hub; `decode` here only reads the * now-trusted payload (no second network hop). * + * When the request asked for `max_age` / `acr_values` (pass the same + * values here, or set them once on the client config), the id_token must + * show an `auth_time` within `max_age` and an `acr` from the list (OIDC + * Core §3.1.2.1, §3.1.3.7) — otherwise the OP silently ignoring them + * would let a stale or weaker login through. + * * @param {Record} query the callback query params - * @param {{ session?: string }} [options] + * @param {{ session?: string, maxAge?: number, acrValues?: string }} [options] * @returns {Promise<{ idToken: string, claims: Record, userinfo: Record, user: object, tokens: Record, warnings: object[] }>} */ async handleCallback(query, options = {}) { const { tokens, user, warnings } = await oauth.callback('oidc', query, { session: options.session }); const idToken = /** @type {string} */ (tokens.id_token); + const claims = decode(idToken).payload; + assertAuthenticationRequirements(claims, { + maxAge: options.maxAge ?? config.maxAge, + acrValues: options.acrValues ?? config.acrValues, + clockToleranceMs: config.clockTolerance === undefined ? 0 : parseDuration(config.clockTolerance), + }); return { idToken, - claims: decode(idToken).payload, + claims, userinfo: /** @type {Record} */ (user).raw, user, tokens, @@ -215,3 +231,26 @@ export function createClient(config) { }, }; } + +/** + * @param {Record} claims + * @param {{ maxAge?: number, acrValues?: string, clockToleranceMs: number }} required + */ +function assertAuthenticationRequirements(claims, { maxAge, acrValues, clockToleranceMs }) { + if (maxAge !== undefined) { + const authTime = claims.auth_time; + if (!isNumber(authTime)) { + authenticationRequirementsNotMet('max_age was requested but the id_token has no auth_time'); + } + const ageMs = Date.now() - authTime * 1000; + if (ageMs > Number(maxAge) * 1000 + clockToleranceMs) { + authenticationRequirementsNotMet(`the end user authenticated more than max_age (${maxAge}s) ago`); + } + } + if (isNonEmptyString(acrValues)) { + const allowed = acrValues.split(/\s+/).filter(Boolean); + if (!isNonEmptyString(claims.acr) || !allowed.includes(claims.acr)) { + authenticationRequirementsNotMet(`the id_token acr is not one of the requested acr_values (${acrValues})`); + } + } +} diff --git a/packages/oidc/src/internal/errors.js b/packages/oidc/src/internal/errors.js index 57ed9165..96fa2fa1 100644 --- a/packages/oidc/src/internal/errors.js +++ b/packages/oidc/src/internal/errors.js @@ -18,6 +18,11 @@ export const ErrorCode = Object.freeze({ // `end_session_endpoint` for RP-Initiated Logout — the main login flow's // discovery is delegated to @exortek/oauth2 and surfaces as OAuth2Error). DISCOVERY_FAILED: 'DISCOVERY_FAILED', + + // The id_token verified, but the authentication it describes does not meet + // what the client asked for: `auth_time` older than the requested + // `max_age`, or an `acr` outside the requested `acr_values`. + AUTHENTICATION_REQUIREMENTS_NOT_MET: 'AUTHENTICATION_REQUIREMENTS_NOT_MET', }); /** @@ -31,6 +36,7 @@ export class OidcError extends BaseError { static statuses = { INVALID_ARGUMENT: 400, DISCOVERY_FAILED: 502, + AUTHENTICATION_REQUIREMENTS_NOT_MET: 401, }; static defaultStatus = 500; @@ -45,3 +51,11 @@ export class OidcError extends BaseError { export function invalidArgument(message) { throw new OidcError(ErrorCode.INVALID_ARGUMENT, message); } + +/** + * @param {string} message + * @returns {never} + */ +export function authenticationRequirementsNotMet(message) { + throw new OidcError(ErrorCode.AUTHENTICATION_REQUIREMENTS_NOT_MET, message); +} diff --git a/packages/oidc/src/internal/logout.js b/packages/oidc/src/internal/logout.js index 5b7b10aa..66570b33 100644 --- a/packages/oidc/src/internal/logout.js +++ b/packages/oidc/src/internal/logout.js @@ -7,7 +7,7 @@ * redirects back. These are the pure builders/validators shared by the * client's `endSessionUrl` and the provider's `endSessionHandler`. */ -import { decode } from '@exortek/jwt'; +import { verify } from '@exortek/jwt'; import { isArray, isNonEmptyString } from '@exortek/shared/predicates'; /** @@ -40,31 +40,34 @@ export function buildEndSessionUrl(endpoint, params) { } /** - * Read the (unverified) `sub` / `aud` from an `id_token_hint`. A logout hint - * is frequently expired, so the OP validates issuer/audience rather than the - * full signature+exp; the return lets the handler match the session to clear. + * Verify an `id_token_hint` and read its `sub` / `aud`. The hint is the only + * thing that tells the OP whose session to end, so it must carry the OP's + * own signature — a decoded-but-unverified hint lets anyone name any `sub` + * and force that user's logout. A logout hint is frequently expired, so + * `exp` is tolerated up to `maxAge` past expiry; signature, `iss` and `alg` + * are always enforced. * - * @param {string} idTokenHint - * @param {string} issuer the OP's own issuer — the hint's `iss` must match - * @returns {{ sub?: string, aud?: string | string[] } | null} null when the hint is unusable / not ours + * @param {string | undefined} idTokenHint + * @param {{ issuer: string, keys: unknown, alg: string, maxAge: string | number }} options + * @returns {Promise<{ sub?: string, aud?: string | string[] } | null>} null when the hint is absent or not genuinely ours */ -export function readIdTokenHint(idTokenHint, issuer) { +export async function verifyIdTokenHint(idTokenHint, { issuer, keys, alg, maxAge }) { if (!isNonEmptyString(idTokenHint)) { return null; } - let payload; try { - payload = decode(idTokenHint).payload; + const { payload } = await verify(idTokenHint, /** @type {any} */ (keys), { + alg: [alg], + issuer, + clockTolerance: maxAge, + }); + return { + sub: /** @type {string|undefined} */ (payload.sub), + aud: /** @type {string|string[]|undefined} */ (payload.aud), + }; } catch { return null; } - if (payload.iss !== issuer) { - return null; - } - return { - sub: /** @type {string|undefined} */ (payload.sub), - aud: /** @type {string|string[]|undefined} */ (payload.aud), - }; } /** diff --git a/packages/oidc/src/internal/session.js b/packages/oidc/src/internal/session.js index 6dbc3bd8..0e5fb685 100644 --- a/packages/oidc/src/internal/session.js +++ b/packages/oidc/src/internal/session.js @@ -16,6 +16,11 @@ import { createHash, randomBytes } from 'node:crypto'; import { encode as base64urlEncode } from '@exortek/shared/base64url'; +import { invalidArgument } from './errors.js'; + +// RFC 6265 §4.1.1 cookie-name = token (RFC 2616 §2.2). +const COOKIE_NAME_TOKEN = /^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$/; + /** * Compute a `session_state` value (OIDC Session Management §4.2): * `base64url(sha256(client_id + " " + origin + " " + op_browser_state + " " + @@ -42,19 +47,23 @@ export function computeSessionState(input) { */ export function checkSessionIframeHtml(options = {}) { const cookieName = options.cookieName ?? 'op_browser_state'; - // The cookie name is the only injected value; JSON.stringify keeps it a safe - // string literal inside the script. - const cookieLiteral = JSON.stringify(cookieName); + // The cookie name is the only injected value. Restricting it to an + // RFC 6265 token keeps `<`, quotes and whitespace out of the script + // entirely, and it is regex-escaped before it reaches `new RegExp`. + if (!COOKIE_NAME_TOKEN.test(cookieName)) { + invalidArgument('session.cookieName must be an RFC 6265 cookie-name token.'); + } + const cookieLiteral = JSON.stringify(cookieName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')); return ` check_session_iframe