feat: per-issuer session cache — stop re-running the full auth flow (popup included) on every 401#11
feat: per-issuer session cache — stop re-running the full auth flow (popup included) on every 401#11jeswr wants to merge 2 commits into
Conversation
Previously every 401 re-ran the entire flow — discovery, dynamic client registration, a fresh DPoP key, and a new authorization popup — so each authenticated request could prompt the user again. DPoPTokenProvider now keeps a single-flight per-issuer session cache: - concurrent 401 upgrades share one authorization-code flow (one popup); - later upgrades reuse the established access token, signing a fresh DPoP proof per request; - the token's reported `expires_in` is tracked (with 30 s skew) and an expired session re-runs the flow — silently while the IdP cookie lives, thanks to the existing `prompt=none`-first behaviour; - a failed flow is not cached, so the next request can retry; - shared flow work is no longer tied to a single request's AbortSignal (aborting one request must not cancel the login that other concurrent upgrades are waiting on). The public API is unchanged. Also adds a minimal vitest setup (the repo had no test runner) with a compact in-memory authorization server covering the cache behaviour. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a per-issuer, single-flight session cache inside DPoPTokenProvider to avoid re-running the full authorization-code flow (including popups) on every 401, and adds a new Vitest-based test harness to validate the new behavior.
Changes:
- Cache authentication sessions per issuer so concurrent upgrades share one auth flow and later upgrades reuse the access token until expiry.
- Track token expiry (
expires_inwith skew) to trigger re-auth when needed, while still generating a fresh DPoP proof per request. - Add a minimal in-memory OIDC/OAuth AS plus Vitest tests covering single-flight, reuse, expiry re-auth, and failed-flow retry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/DPoPTokenProvider.ts |
Adds per-issuer single-flight session caching and expiry-based renewal for DPoP upgrades. |
test/fakeAuthorizationServer.ts |
Introduces an in-memory discovery/JWKS/registration/token endpoint to support deterministic unit tests. |
test/DPoPTokenProvider.test.ts |
Adds Vitest coverage for caching/reuse/expiry and failure retry behavior. |
package.json |
Adds vitest and a npm test script to run the new unit tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function expiresAt(token: oauth.TokenEndpointResponse): number | undefined { | ||
| return token.expires_in === undefined ? undefined : Date.now() + token.expires_in * 1000 - expirySkewMs | ||
| } | ||
|
|
||
| function hasExpired(session: IssuerSession): boolean { | ||
| return session.expiresAt !== undefined && Date.now() >= session.expiresAt | ||
| } |
There was a problem hiding this comment.
Agreed this needed a strategy. Fixed in #14 (stacked follow-up): TokenProvider gains an optional invalidate(), and ReactiveFetchManager retries exactly once with renewed credentials when an upgraded request is still rejected — covering both the missing-expires_in case and early revocation, without guessing a max age.
| if (params.get("grant_type") === "refresh_token") { | ||
| const presented = params.get("refresh_token") ?? "" | ||
| if (!activeRefreshTokens.has(presented)) { | ||
| return json({error: "invalid_grant"}, 400) | ||
| } | ||
| if (rotate) { | ||
| activeRefreshTokens.delete(presented) | ||
| } | ||
| return json(tokenBody(true)) | ||
| } |
There was a problem hiding this comment.
Fixed in 23e0a5d: a non-rotating server now keeps the presented token active and issues no replacement (RFC 6749 §6), and the refresh-token grant is rejected with unsupported_grant_type when issueRefreshTokens is off.
| id_token_signing_alg_values_supported: ["ES256"], | ||
| scopes_supported: options.scopesSupported ?? ["openid", "webid"], | ||
| grant_types_supported: options.grantTypesSupported ?? ["authorization_code"], | ||
| }) |
There was a problem hiding this comment.
Fixed in 23e0a5d: discovery now defaults grant_types_supported to include refresh_token exactly when issueRefreshTokens is on.
Review follow-ups: discovery now advertises the refresh_token grant exactly when the server issues refresh tokens; the refresh-token grant is rejected (unsupported_grant_type) when refresh tokens are disabled; and a non-rotating server keeps the presented token active without issuing a replacement (RFC 6749 §6) instead of silently rotating. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem
DPoPTokenProvider.upgrade()runs the entire flow on every 401: discovery, dynamic client registration, a fresh DPoP key, and a new authorization popup. In a real app (observed live with a Solid pod browser against a Solid-OIDC broker) that means:refresh_token, if any) is dropped on the floor after a single request.Change
DPoPTokenProvidernow keeps a single-flight, per-issuer session cache:expires_inis tracked (with 30 s skew); an expired session re-runs the flow, which stays silent while the IdP cookie lives thanks to the existingprompt=none-first behaviour;AbortSignal— aborting one request no longer cancels the login other concurrent upgrades are waiting on (small behaviour change, called out deliberately).Public API is unchanged.
Tests
The repo had no test runner, so this adds a minimal vitest setup (
npm test) with a compact in-memory authorization server (discovery + JWKS + registration + token endpoint, ES256-signed ID tokens) — 6 tests covering token attachment, single-flight, reuse, per-request proofs, expiry re-auth, and failed-flow retry.npm run build(tsc): cleannpm test: 6/6 passingStacked work: refresh-token support builds on this cache in a follow-up PR.
🤖 Generated with Claude Code