feat: pin the providers' own OIDC requests to a caller-supplied fetch (fixes the global-fetch-patch login deadlock)#21
Draft
jeswr wants to merge 1 commit into
Draft
feat: pin the providers' own OIDC requests to a caller-supplied fetch (fixes the global-fetch-patch login deadlock)#21jeswr wants to merge 1 commit into
jeswr wants to merge 1 commit into
Conversation
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 <noreply@anthropic.com> Signed-off-by: Jesse Wright <63333554+jeswr@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem: an OIDC re-entrancy deadlock that hangs login
The token providers perform their own network requests — discovery, dynamic client registration, and the token grant — through oauth4webapi, which defaults to
globalThis.fetch.When an application patches the global
fetchwith an authenticating wrapper (this library's ownReactiveFetchManager.registerGlobally(), or an app-side wrapper that single-flights concurrent requests onto one shared authentication attempt — a very common pattern to avoid N parallel 401s spawning N login popups), the provider's OIDC requests re-enter that 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 popup/redirect ever opens. No error, no popup, nothing: the discovery request is parked on the login promise that issued it.Deterministic repro (against this repo's
main)The fix: let the caller pin the providers' OIDC hops to a pristine fetch
This PR adds an optional trailing
optionsargument toDPoPTokenProvider,BearerTokenProviderandClientCredentialsTokenProvider:TokenProviderOptions.fetchis threaded into oauth4webapi via itscustomFetchrequest option (a small sharedcustomFetchOptionadapter bridges oauth4webapi'sCustomFetchOptionsto the Fetch API signature underexactOptionalPropertyTypes). Defaults are unchanged: with no options, the providers keep resolvingglobalThis.fetchat call time, exactly as today.Tests
test/DPoPTokenProvider.customFetch.test.ts(runs via the addednpm testscript — build, thennode --testagainstdist/, so it exercises the shipped artifact):globalThis.fetchis patched with a never-resolving wrapper — and asserts the patched global was called zero times.globalThis.fetch.Without the fix, test 1 hangs (verified against
main).Notes for discussion
ReactiveFetchManageralready captures the pre-patch fetch (#globalFetch) — it could pass it down to providers automatically. That needs aTokenProviderinterface change (e.g.upgrade(request, context)), so I kept this PR to the least-invasive opt-in seam; happy to explore the manager-driven variant if you prefer it.npm testscript is new onmain(draft fix: allow http loopback issuers in the DPoP auth-code flow #18 adds a similar one); trivial to reconcile whichever lands first.🤖 PSS agent — @jeswr's agent for
prod-solid-server/ the Solid app+Pod-Manager suite