From 60cb209fe23a1c2e47fdbd75ac1b4dc08a6fe9f6 Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Sat, 4 Jul 2026 03:08:49 +0100 Subject: [PATCH] feat: pin the providers' own OIDC requests to a caller-supplied fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Token providers perform their own network requests (discovery, dynamic client registration, the token grant) through oauth4webapi, which defaults to globalThis.fetch. When an application patches the global fetch with an authenticating wrapper — ReactiveFetchManager's registerGlobally(), or its own wrapper that single-flights concurrent requests onto one shared authentication attempt — those OIDC requests re-enter the wrapper mid-upgrade. A single-flighting wrapper then awaits the very authentication attempt its request is serving: a circular await that hangs login before the authorization popup/redirect ever opens. Add TokenProviderOptions.fetch (an optional trailing options argument on DPoPTokenProvider, BearerTokenProvider and ClientCredentialsTokenProvider) threading a caller-supplied pristine fetch into oauth4webapi via its customFetch option, so the providers' OIDC hops never ride the patched global. Defaults unchanged (globalThis.fetch resolved at call time). Includes a mock-OP regression test proving login resolves under a deadlocking patched global fetch when a pristine fetch is pinned, plus a backwards-compatibility test, and a README section documenting the capture-before-patch pattern. Model: claude-fable-5 Co-Authored-By: Claude Fable Signed-off-by: Jesse Wright <63333554+jeswr@users.noreply.github.com> --- README.md | 14 ++ package.json | 3 +- src/BearerTokenProvider.ts | 16 +- src/ClientCredentialsTokenProvider.ts | 17 +- src/DPoPTokenProvider.ts | 16 +- src/TokenProviderOptions.ts | 23 +++ src/customFetchOption.ts | 27 +++ src/mod.ts | 1 + test/DPoPTokenProvider.customFetch.test.ts | 189 +++++++++++++++++++++ 9 files changed, 292 insertions(+), 14 deletions(-) create mode 100644 src/TokenProviderOptions.ts create mode 100644 src/customFetchOption.ts create mode 100644 test/DPoPTokenProvider.customFetch.test.ts diff --git a/README.md b/README.md index c22a9a6..d71bd1a 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,20 @@ manager.registerGlobally() const response = await fetch(requestUri) ``` +### Pin the providers' own OIDC requests to a pristine `fetch` + +The token providers perform their own network requests (discovery, dynamic client registration, the token grant) which default to `globalThis.fetch`. If the application patches the global `fetch` with an authenticating wrapper — `registerGlobally()`, or its own wrapper that single-flights concurrent requests onto one shared authentication attempt — those OIDC requests re-enter the wrapper mid-login. A single-flighting wrapper then awaits the very authentication attempt its request is serving: a circular await that hangs login before the authorization popup/redirect ever opens. + +To avoid this, capture the pristine `fetch` before patching and pin the providers to it: + +```js +const pristineFetch = globalThis.fetch.bind(globalThis) // capture BEFORE patching + +const provider = new DPoPTokenProvider(callbackUri, getCode, getIssuer, {fetch: pristineFetch}) +const manager = new ReactiveFetchManager([provider]) +manager.registerGlobally() +``` + ## Run the demo To compile, diff --git a/package.json b/package.json index db6dd7a..82cffe6 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,8 @@ "url": "git+https://github.com/solid-contrib/reactive-authentication.git" }, "scripts": { - "build": "tsc" + "build": "tsc", + "test": "npm run build && node --test --experimental-strip-types \"test/**/*.test.ts\"" }, "license": "MIT", "dependencies": { diff --git a/src/BearerTokenProvider.ts b/src/BearerTokenProvider.ts index 051af48..f7d3a88 100644 --- a/src/BearerTokenProvider.ts +++ b/src/BearerTokenProvider.ts @@ -1,15 +1,19 @@ import * as oauth from "oauth4webapi" import { GetCodeCallback } from "./GetCodeCallback.js" import { TokenProvider } from "./TokenProvider.js" +import type { TokenProviderOptions } from "./TokenProviderOptions.js" +import { customFetchOption } from "./customFetchOption.js" // TODO: Configure properly for insecure localhost only const oauthAllowInsecureRequests = true export class BearerTokenProvider implements TokenProvider { readonly #getCode: GetCodeCallback + readonly #fetch: typeof globalThis.fetch | undefined - constructor(getCodeCallback: GetCodeCallback) { + constructor(getCodeCallback: GetCodeCallback, options?: TokenProviderOptions) { this.#getCode = getCodeCallback + this.#fetch = options?.fetch } async #getIssuer(request: Request): Promise { @@ -43,12 +47,16 @@ export class BearerTokenProvider implements TokenProvider { async upgrade(request: Request): Promise { const issuer = await this.#getIssuer(request) - const discoveryResponse = await oauth.discoveryRequest(issuer, {[oauth.allowInsecureRequests]: oauthAllowInsecureRequests}) + // Pin the provider's own OIDC requests to the configured fetch (when given) + // so they never re-enter an authenticating wrapper patched over the global. + const oidcRequestOptions = {[oauth.allowInsecureRequests]: oauthAllowInsecureRequests, ...customFetchOption(this.#fetch)} + + const discoveryResponse = await oauth.discoveryRequest(issuer, oidcRequestOptions) const authorizationServer = await oauth.processDiscoveryResponse(issuer, discoveryResponse) const callbackUri = await this.#getCallback(request) - const registrationResponse = await oauth.dynamicClientRegistrationRequest(authorizationServer, {redirect_uris: [callbackUri]}, {[oauth.allowInsecureRequests]: oauthAllowInsecureRequests}) + const registrationResponse = await oauth.dynamicClientRegistrationRequest(authorizationServer, {redirect_uris: [callbackUri]}, oidcRequestOptions) const clientRegistration = await oauth.processDynamicClientRegistrationResponse(registrationResponse) const [registeredRedirectUri] = clientRegistration.redirect_uris as string[] const [registeredResponseType] = clientRegistration.response_types as string[] @@ -84,7 +92,7 @@ export class BearerTokenProvider implements TokenProvider { clientAuth = authenticationMethod(clientSecret) } - const tokenResponse = await oauth.authorizationCodeGrantRequest(authorizationServer, clientRegistration, clientAuth, authorizationCodeParams, callbackUri, codeVerifier, {[oauth.allowInsecureRequests]: oauthAllowInsecureRequests}) + const tokenResponse = await oauth.authorizationCodeGrantRequest(authorizationServer, clientRegistration, clientAuth, authorizationCodeParams, callbackUri, codeVerifier, oidcRequestOptions) // jwt nonce missing in igrant // const tokenResult = await oauth.processAuthorizationCodeResponse(authorizationServer, clientRegistration, tokenResponse, {expectedNonce: nonce}) diff --git a/src/ClientCredentialsTokenProvider.ts b/src/ClientCredentialsTokenProvider.ts index 2bc32b5..5e7ea9f 100644 --- a/src/ClientCredentialsTokenProvider.ts +++ b/src/ClientCredentialsTokenProvider.ts @@ -2,9 +2,14 @@ import * as oauth from "oauth4webapi" import { AuthorizationServer } from "oauth4webapi" import * as DPoP from "dpop" import type { TokenProvider } from "./TokenProvider.js" +import type { TokenProviderOptions } from "./TokenProviderOptions.js" +import { customFetchOption } from "./customFetchOption.js" export class ClientCredentialsTokenProvider implements TokenProvider { - constructor(private clientId: string, private clientSecret: string) { + readonly #fetch: typeof globalThis.fetch | undefined + + constructor(private clientId: string, private clientSecret: string, options?: TokenProviderOptions) { + this.#fetch = options?.fetch } async #getIssuer(request: Request): Promise { @@ -34,9 +39,11 @@ export class ClientCredentialsTokenProvider implements TokenProvider { async upgrade(request: Request): Promise { const issuer = await this.#getIssuer(request) - const discoveryResponse = await oauth.discoveryRequest(issuer, { - signal: request.signal - }) + // Pin the provider's own OIDC requests to the configured fetch (when given) + // so they never re-enter an authenticating wrapper patched over the global. + const oidcRequestOptions = {signal: request.signal, ...customFetchOption(this.#fetch)} + + const discoveryResponse = await oauth.discoveryRequest(issuer, oidcRequestOptions) const authorizationServer = await oauth.processDiscoveryResponse(issuer, discoveryResponse) const clientRegistration: oauth.Client = {client_id: this.clientId, client_secret: this.clientSecret} @@ -46,7 +53,7 @@ export class ClientCredentialsTokenProvider implements TokenProvider { const tokenResponse = await oauth.clientCredentialsGrantRequest(authorizationServer, clientRegistration, this.getClientAuth(authorizationServer, clientRegistration), {scope: "webid"}, { DPoP: dpop, - signal: request.signal + ...oidcRequestOptions }) const tokenResult = await oauth.processClientCredentialsResponse(authorizationServer, clientRegistration, tokenResponse) diff --git a/src/DPoPTokenProvider.ts b/src/DPoPTokenProvider.ts index 4760ae2..4dcb3ce 100644 --- a/src/DPoPTokenProvider.ts +++ b/src/DPoPTokenProvider.ts @@ -3,16 +3,20 @@ import * as DPoP from "dpop" import type { GetCodeCallback } from "./GetCodeCallback.js" import type { TokenProvider } from "./TokenProvider.js" import type { GetIssuerCallback } from "./GetIssuerCallback.js" +import type { TokenProviderOptions } from "./TokenProviderOptions.js" +import { customFetchOption } from "./customFetchOption.js" export class DPoPTokenProvider implements TokenProvider { readonly #getCode: GetCodeCallback readonly #callbackUri: string readonly #getIssuer: GetIssuerCallback + readonly #fetch: typeof globalThis.fetch | undefined - constructor(callbackUri: string, getCodeCallback: GetCodeCallback, getIssuerCallback: GetIssuerCallback) { + constructor(callbackUri: string, getCodeCallback: GetCodeCallback, getIssuerCallback: GetIssuerCallback, options?: TokenProviderOptions) { this.#getCode = getCodeCallback this.#callbackUri = callbackUri this.#getIssuer = getIssuerCallback + this.#fetch = options?.fetch } async matches(request: Request): Promise { @@ -22,10 +26,14 @@ export class DPoPTokenProvider implements TokenProvider { async upgrade(request: Request): Promise { const issuer = await this.#getIssuer(request) - const discoveryResponse = await oauth.discoveryRequest(issuer, {signal: request.signal}) + // Pin the provider's own OIDC requests to the configured fetch (when given) + // so they never re-enter an authenticating wrapper patched over the global. + const oidcRequestOptions = {signal: request.signal, ...customFetchOption(this.#fetch)} + + const discoveryResponse = await oauth.discoveryRequest(issuer, oidcRequestOptions) const authorizationServer = await oauth.processDiscoveryResponse(issuer, discoveryResponse) - const registrationResponse = await oauth.dynamicClientRegistrationRequest(authorizationServer, {redirect_uris: [this.#callbackUri]}, {signal: request.signal}) + const registrationResponse = await oauth.dynamicClientRegistrationRequest(authorizationServer, {redirect_uris: [this.#callbackUri]}, oidcRequestOptions) const clientRegistration = await oauth.processDynamicClientRegistrationResponse(registrationResponse) const [registeredRedirectUri] = clientRegistration.redirect_uris as string[] const [registeredResponseType] = clientRegistration.response_types as string[] @@ -79,7 +87,7 @@ export class DPoPTokenProvider implements TokenProvider { } } - const tokenResponse = await oauth.authorizationCodeGrantRequest(authorizationServer, clientRegistration, this.getClientAuth(authorizationServer.issuer, clientRegistration), authorizationCodeParams, this.#callbackUri, authorizationServer.code_challenge_methods_supported !== undefined ? codeVerifier : oauth.nopkce, {DPoP: dpop, signal: request.signal}) + const tokenResponse = await oauth.authorizationCodeGrantRequest(authorizationServer, clientRegistration, this.getClientAuth(authorizationServer.issuer, clientRegistration), authorizationCodeParams, this.#callbackUri, authorizationServer.code_challenge_methods_supported !== undefined ? codeVerifier : oauth.nopkce, {DPoP: dpop, ...oidcRequestOptions}) const tokenResult = await oauth.processAuthorizationCodeResponse(authorizationServer, clientRegistration, tokenResponse, {expectedNonce: this.nonceVerificationOverride(authorizationServer.issuer, nonce)}) diff --git a/src/TokenProviderOptions.ts b/src/TokenProviderOptions.ts new file mode 100644 index 0000000..d626515 --- /dev/null +++ b/src/TokenProviderOptions.ts @@ -0,0 +1,23 @@ +/** + * Options accepted by the token providers. + */ +export interface TokenProviderOptions { + /** + * The fetch implementation the provider uses for its own OIDC requests — + * discovery, dynamic client registration and the token grant. + * + * @remarks + * Defaults to `globalThis.fetch`, resolved at call time (oauth4webapi's + * default behaviour). + * + * Applications that patch the global fetch with an authenticating wrapper + * (`ReactiveFetchManager.registerGlobally()`, or their own wrapper that + * single-flights concurrent requests onto one shared authentication + * attempt) should pass the pristine, pre-patch fetch here. Otherwise the + * provider's own OIDC requests re-enter the wrapper mid-upgrade: a + * single-flighting wrapper then awaits the very authentication attempt + * those requests are serving — a circular await that hangs login before + * the authorization redirect/popup ever opens. + */ + fetch?: typeof globalThis.fetch +} diff --git a/src/customFetchOption.ts b/src/customFetchOption.ts new file mode 100644 index 0000000..e133996 --- /dev/null +++ b/src/customFetchOption.ts @@ -0,0 +1,27 @@ +import * as oauth from "oauth4webapi" + +/** + * Adapts an optional Fetch API implementation to oauth4webapi's `customFetch` + * request option, as a spreadable options fragment. + * + * @remarks + * Returns an empty fragment when no fetch is given, preserving oauth4webapi's + * default (`globalThis.fetch`, resolved at call time). + * + * @see {@link TokenProviderOptions.fetch} + */ +export function customFetchOption(fetchImplementation: typeof globalThis.fetch | undefined): {[oauth.customFetch]?: (url: string, options: oauth.CustomFetchOptions) => Promise} { + if (fetchImplementation === undefined) { + return {} + } + + return { + [oauth.customFetch]: (url, options) => fetchImplementation(url, { + method: options.method, + headers: options.headers, + body: options.body ?? null, + redirect: options.redirect, + signal: options.signal ?? null, + }), + } +} diff --git a/src/mod.ts b/src/mod.ts index 0342cf6..e58915e 100644 --- a/src/mod.ts +++ b/src/mod.ts @@ -9,6 +9,7 @@ export * from "./ClientCredentialsTokenProvider.js" export * from "./GetCodeCallback.js" export * from "./issuerFrom.js" export * from "./TokenProvider.js" +export * from "./TokenProviderOptions.js" export * from "./GetIssuerCallback.js" export * from "./IdpPicker.js" export * from "./WebIdPicker.js" diff --git a/test/DPoPTokenProvider.customFetch.test.ts b/test/DPoPTokenProvider.customFetch.test.ts new file mode 100644 index 0000000..a6fe0dd --- /dev/null +++ b/test/DPoPTokenProvider.customFetch.test.ts @@ -0,0 +1,189 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import { DPoPTokenProvider } from "../dist/DPoPTokenProvider.js" +import { ClientCredentialsTokenProvider } from "../dist/ClientCredentialsTokenProvider.js" +import type { GetCodeCallback } from "../dist/GetCodeCallback.js" + +// Regression tests for the OIDC re-entrancy deadlock: token providers perform +// their own network requests (discovery, dynamic client registration, the token +// grant) through oauth4webapi, which defaults to `globalThis.fetch`. When an +// application patches the global fetch with an authenticating wrapper — e.g. +// `ReactiveFetchManager.registerGlobally()`, or a wrapper that single-flights +// concurrent requests onto one shared authentication attempt — those OIDC +// requests re-enter the wrapper mid-upgrade. A single-flighting wrapper then +// awaits the very authentication attempt its request is serving: a circular +// await that hangs login before the authorization popup/redirect ever opens. +// +// The fix: `TokenProviderOptions.fetch` pins the provider's own OIDC requests +// to a caller-supplied (pristine, pre-patch) fetch via oauth4webapi's +// `customFetch`, so they never ride the patched global. + +const issuer = "https://op.example" +const callbackUri = "https://app.example/callback.html" +const clientId = "mock-client" + +function jsonResponse(status: number, body: unknown): Response { + return new Response(JSON.stringify(body), {status, headers: {"content-type": "application/json"}}) +} + +function base64url(value: string): string { + return Buffer.from(value).toString("base64url") +} + +/** An ID Token with valid claims and a placeholder signature (oauth4webapi does not validate the signature of tokens received directly from the token endpoint). */ +function idToken(nonce: string | undefined): string { + const now = Math.floor(Date.now() / 1000) + const header = {alg: "RS256", typ: "JWT"} + const payload = { + iss: issuer, + sub: "https://alice.example/profile/card#me", + aud: clientId, + exp: now + 600, + iat: now, + webid: "https://alice.example/profile/card#me", + ...(nonce === undefined ? {} : {nonce}), + } + return `${base64url(JSON.stringify(header))}.${base64url(JSON.stringify(payload))}.${base64url("mock-signature")}` +} + +/** An in-process mock OpenID Provider, exposed as a fetch implementation. */ +function mockOpFetch(state: {nonce?: string, requests: string[]}): typeof globalThis.fetch { + return async (input, init) => { + const url = new URL(input instanceof Request ? input.url : String(input)) + state.requests.push(url.pathname) + + switch (url.pathname) { + case "/.well-known/openid-configuration": + return jsonResponse(200, { + issuer, + authorization_endpoint: `${issuer}/authorize`, + token_endpoint: `${issuer}/token`, + registration_endpoint: `${issuer}/register`, + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["client_secret_basic"], + }) + case "/register": + return jsonResponse(201, { + client_id: clientId, + redirect_uris: [callbackUri], + response_types: ["code"], + token_endpoint_auth_method: "none", + }) + case "/token": + return jsonResponse(200, { + access_token: "mock-access-token", + token_type: "DPoP", + expires_in: 3600, + scope: "openid webid", + id_token: idToken(state.nonce), + }) + default: + throw new Error(`Unexpected request to mock OP: ${url}`) + } + } +} + +/** Echoes the authorization code straight back, capturing the nonce so the mock OP can bind it into the ID Token. */ +function getCodeCallback(state: {nonce?: string}): GetCodeCallback { + return async authorizationUri => { + state.nonce = authorizationUri.searchParams.get("nonce") ?? undefined + return `${callbackUri}?code=mock-code&state=${authorizationUri.searchParams.get("state")}` + } +} + +test("DPoP auth-code login resolves even when globalThis.fetch is patched with a deadlocking wrapper, when a pristine fetch is pinned", {timeout: 15_000}, async t => { + const state: {nonce?: string, requests: string[]} = {requests: []} + + // Simulate an app-installed single-flighting authenticated-fetch wrapper: + // mid-login, any request through it awaits the in-flight authentication + // attempt — i.e. from the provider's perspective, it never resolves. + const realFetch = globalThis.fetch + let patchedCalls = 0 + globalThis.fetch = (() => { + patchedCalls++ + return new Promise(() => {}) + }) as typeof globalThis.fetch + t.after(() => { + globalThis.fetch = realFetch + }) + + const provider = new DPoPTokenProvider( + callbackUri, + getCodeCallback(state), + async () => new URL(issuer), + {fetch: mockOpFetch(state)}, + ) + + const upgraded = await provider.upgrade(new Request("https://pod.example/private/resource")) + + assert.match(upgraded.headers.get("Authorization") ?? "", /^DPoP mock-access-token$/) + assert.ok(upgraded.headers.get("DPoP")) + assert.deepEqual(state.requests, ["/.well-known/openid-configuration", "/register", "/token"]) + assert.equal(patchedCalls, 0, "the provider's OIDC requests must not ride the patched global fetch") +}) + +test("client-credentials login resolves under a patched globalThis.fetch when a pristine fetch is pinned", {timeout: 15_000}, async t => { + const state: {nonce?: string, requests: string[]} = {requests: []} + + const realFetch = globalThis.fetch + let patchedCalls = 0 + globalThis.fetch = (() => { + patchedCalls++ + return new Promise(() => {}) + }) as typeof globalThis.fetch + t.after(() => { + globalThis.fetch = realFetch + }) + + // The client-credentials provider derives the issuer from the request URL. + const provider = new ClientCredentialsTokenProvider(clientId, "mock-secret", {fetch: mockOpFetchAt("https://solidcommunity.net", state)}) + + const upgraded = await provider.upgrade(new Request("https://alice.solidcommunity.net/private/resource")) + + assert.match(upgraded.headers.get("Authorization") ?? "", /^DPoP mock-access-token$/) + assert.equal(patchedCalls, 0, "the provider's OIDC requests must not ride the patched global fetch") +}) + +test("without options the provider still defaults to globalThis.fetch (backwards compatible)", {timeout: 15_000}, async t => { + const state: {nonce?: string, requests: string[]} = {requests: []} + + const realFetch = globalThis.fetch + globalThis.fetch = mockOpFetch(state) + t.after(() => { + globalThis.fetch = realFetch + }) + + const provider = new DPoPTokenProvider(callbackUri, getCodeCallback(state), async () => new URL(issuer)) + + const upgraded = await provider.upgrade(new Request("https://pod.example/private/resource")) + + assert.match(upgraded.headers.get("Authorization") ?? "", /^DPoP mock-access-token$/) + assert.deepEqual(state.requests, ["/.well-known/openid-configuration", "/register", "/token"]) +}) + +/** A mock OP for a different issuer (client-credentials flow: no registration, no ID Token nonce). */ +function mockOpFetchAt(opIssuer: string, state: {requests: string[]}): typeof globalThis.fetch { + return async (input, init) => { + const url = new URL(input instanceof Request ? input.url : String(input)) + state.requests.push(url.pathname) + + switch (url.pathname) { + case "/.well-known/openid-configuration": + return jsonResponse(200, { + issuer: opIssuer, + authorization_endpoint: `${opIssuer}/authorize`, + token_endpoint: `${opIssuer}/token`, + token_endpoint_auth_methods_supported: ["client_secret_basic"], + }) + case "/token": + return jsonResponse(200, { + access_token: "mock-access-token", + token_type: "DPoP", + expires_in: 3600, + scope: "webid", + }) + default: + throw new Error(`Unexpected request to mock OP: ${url}`) + } + } +}