Skip to content

feat: per-issuer session cache — stop re-running the full auth flow (popup included) on every 401 - #11

Open
jeswr wants to merge 6 commits into
mainfrom
feat/dpop-session-cache
Open

feat: per-issuer session cache — stop re-running the full auth flow (popup included) on every 401#11
jeswr wants to merge 6 commits into
mainfrom
feat/dpop-session-cache

Conversation

@jeswr

@jeswr jeswr commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

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:

  • every authenticated request that races another one opens its own popup;
  • nothing is remembered between requests, so users keep being prompted;
  • the token (and its refresh_token, if any) is dropped on the floor after a single request.

Change

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 and only sign a fresh per-request DPoP proof;
  • expires_in is tracked (with 30 s skew); an expired session re-runs the flow, which stays silent while the IdP cookie lives thanks to the existing prompt=none-first behaviour;
  • a failed flow is not cached, so the next request retries;
  • the shared flow work is provider-owned rather than tied to one request's 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): clean
  • npm test: 6/6 passing
  • Verified live: Playwright-driven login against a deployed pod manager + Solid-OIDC broker (https://app.solid-test.jeswr.org) — one popup per session, repeat reads reuse the token.

Stacked work: refresh-token support builds on this cache in a follow-up PR.

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_in with 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.

Comment thread src/DPoPTokenProvider.ts
Comment thread test/fakeAuthorizationServer.ts Outdated
Comment thread test/fakeAuthorizationServer.ts Outdated
jeswr and others added 2 commits July 29, 2026 11:16
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>
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>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a library by @panva which does this out of the box rather than needing to entirely re-define our own authorisation server here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spiked this rather than guessing. Short answer: yes for the signing, no for the whole server.

Signing — adopted. The hand-rolled base64url + subtle.sign JWT construction is gone, replaced with panva jose (46e4362). Zero-dependency, and it removes the only real crypto we were writing ourselves.

The whole server — oidc-provider is the candidate, and it does work. I stood it up and drove a full code grant against it. Discovery, dynamic registration, PKCE and DPoP are all supported out of the box, and it is the certified reference implementation. But the costs are concrete:

  1. You do not escape the fetch stub. It is a Node HTTP listener, so it serves plain http, and oauth4webapi refuses non-https issuers. The test still has to stub globalThis.fetch to rewrite https://as.testhttp://127.0.0.1:PORT — or take the insecure opt-in from fix: allow opting in to insecure OAuth requests #18.
  2. The authorization endpoint is interactive. It 303s to /interaction/:uid. Getting a code took 3 redirect hops, a cookie jar, and regex-scraping the HTML login and consent forms out of devInteractions — a feature it prints a startup warning telling you to replace. My working spike was ~90 lines against the fake’s ~190, and the 90 are the fragile kind.
  3. Four startup warnings on a default config: dev in-memory adapter, dev signing keys, devInteractions, and unsupported runtime.
  4. ~680 KB and ten transitive packages (koa, @koa/router, @koa/cors, eta, raw-body, quick-lru, nanoid, jsesc, debug, jose).

The deciding factor is that the fake is a test double, not a server. The suite needs to force expires_in, toggle refresh-token rotation, and count how many times the user was prompted — that is what the tests actually assert on. With oidc-provider those become configuration archaeology; here they are constructor options.

So I have kept the fake, cut its header comment down, and taken jose for the part that was genuinely reinventing a library. oidc-provider is the right tool for a conformance or integration suite against the real client, and I would happily use it there — just not as the unit-test double.

Comment thread src/DPoPTokenProvider.ts Outdated
Comment thread src/DPoPTokenProvider.ts Outdated
jeswr and others added 4 commits August 6, 2026 10:16
Sessions were held in a private per-issuer Map, which fixed both where they
live and how they are keyed.

- SessionCache<T> is the storage seam, async so a session can live in
  IndexedDB or an editor secrets API rather than only in memory.
  MemorySessionCache is the default.
- GetSessionKeyCallback derives the key, defaulting to the issuer. Callers
  that must not share one session per authorization server can scope narrower.
- Single-flight moves to a separate in-memory map of in-flight flows, since a
  pending Promise cannot be persisted. One popup per key is unchanged.
- IssuerSession becomes the exported DPoPSession, so caches can be typed.

The fake authorization server now signs ID tokens with jose instead of
hand-rolled base64url and subtle.sign.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MemorySessionCache loses everything on reload, so the popup comes back on
every page load.

IndexedDbSessionCache is the one to reach for with DPoP. IndexedDB stores by
structured clone, which keeps a non extractable CryptoKey intact, so the key
survives a browser restart while staying unreadable by script on the origin.
Verified by round tripping a key and signing with it afterwards.

WebStorageSessionCache takes localStorage or sessionStorage for sessions that
really are just JSON, such as a bare refresh token. It cannot hold a DPoP
session: JSON.stringify turns a CryptoKey into {} without complaining, which
would fail much later inside generateProof, so set() throws instead and names
the alternative.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TypeDoc 0.28 does not support TypeScript 7, so npm rejects the dependency tree before any CI task can run. Return to TypeScript 6.0.3 and run the Vitest suite in the existing test matrix.
Replace Vitest with node:test and node:assert, building first so the TypeScript tests exercise the package output without another loader. Replace the handwritten authorization server with oauth2-mock-server plus a small dynamic-registration and HTTPS-routing adapter. This keeps all 24 tests while removing Vitest, direct jose usage, and more than 100 lines of test code.
@jeswr
jeswr marked this pull request as ready for review August 17, 2026 10:06
@jeswr

jeswr commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Pause, awaiting #28 to be resolved first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants