Skip to content

feat: renew the session once when an upgraded request is still rejected (401)#14

Open
jeswr wants to merge 3 commits into
feat/refresh-tokensfrom
feat/renew-on-rejected-token
Open

feat: renew the session once when an upgraded request is still rejected (401)#14
jeswr wants to merge 3 commits into
feat/refresh-tokensfrom
feat/renew-on-rejected-token

Conversation

@jeswr

@jeswr jeswr commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Stacked on #12 (which stacks on #11) — review only the last commit; the base auto-retargets as those merge.

Problem

A cached access token can stop working before its reported expiry — or have no reported expiry at all (expires_in is optional): revocation, server-side invalidation, key rollover. With the per-issuer session cache (#11), the manager would replay the same rejected token on every 401, locking the user out until a page reload. (Raised by review on #11.)

Change

  • TokenProvider gains an optional invalidate(request): mark cached credentials for the rejected upgraded request stale. Optional, so existing/third-party providers are unaffected — public API stays backwards-compatible.
  • ReactiveFetchManager retries exactly once: when the upgraded request still comes back 401, it invalidates and re-upgrades; a still-rejected retry surfaces the 401 unchanged (bounded, never a loop).
  • DPoPTokenProvider.invalidate marks the session expired only when the rejected token is still the cached one (a concurrent renewal may already have replaced it), so the next upgrade renews — refresh grant (feat: refresh-token support — renew expired sessions without user interaction #12) first, popup flow as fallback.

Verification

  • npm run build (tsc) clean; npm test 15/15 — three new tests: renew-and-retry-once on revocation (no popup, refresh grant used), bounded give-up surfacing the 401, and stale-invalidation ignored after renewal.
  • Live: exercised against https://app.solid-test.jeswr.org with the broker at idp.solid-test.jeswr.org (DPoP + rotating refresh tokens).

🤖 Generated with Claude Code

A cached access token can stop working before its reported expiry — or
have no reported expiry at all (expires_in is optional): revocation,
server-side invalidation, key rollover. Previously the manager replayed
the same rejected token on every 401, locking the user out until reload.

ReactiveFetchManager now asks the provider to invalidate its credentials
when the upgraded request still comes back 401, and retries exactly once
with renewed ones (refresh grant first, popup flow as fallback). A
still-rejected retry surfaces the 401 unchanged — bounded, never a loop.
TokenProvider.invalidate is optional, so existing providers are
unaffected (public API stays backwards-compatible).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jeswr jeswr requested a review from matthieubosquet as a code owner June 11, 2026 20:47
Copilot AI review requested due to automatic review settings June 11, 2026 20:47
@jeswr jeswr requested a review from langsamu as a code owner June 11, 2026 20:47

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 adds a bounded recovery path for cases where an upgraded request is still rejected with 401 (e.g., server-side revocation or early invalidation of an otherwise “unexpired” access token), by allowing providers to invalidate cached credentials and retry the request exactly once.

Changes:

  • Extend the TokenProvider interface with an optional invalidate(request) hook to mark credentials for a rejected upgraded request as stale.
  • Update ReactiveFetchManager to invalidate and re-upgrade/retry exactly once when an upgraded request still returns 401.
  • Implement invalidate() in DPoPTokenProvider to expire the cached session only if the rejected token is still the cached token; add tests covering the new behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/TokenProvider.ts Adds optional invalidate() API for cache invalidation on rejected upgraded requests.
src/ReactiveFetchManager.ts Adds a single bounded retry path: invalidate + re-upgrade when upgraded request still returns 401.
src/DPoPTokenProvider.ts Implements invalidation by expiring the cached session only when the rejected token matches the cached token.
test/DPoPTokenProvider.test.ts Adds tests covering renew-and-retry-once behavior, bounded give-up, and concurrent-renewal safety.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ReactiveFetchManager.ts
Comment thread test/DPoPTokenProvider.test.ts Outdated
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