feat: definitive session teardown (forget) with a supersession fence#23
Draft
jeswr wants to merge 1 commit into
Draft
feat: definitive session teardown (forget) with a supersession fence#23jeswr wants to merge 1 commit into
jeswr wants to merge 1 commit into
Conversation
The session cache has a *transient* stale path — invalidate() marks the access token stale but KEEPS the durable refresh token, so the next upgrade silently re-establishes the same session (correct for a rejected-token retry). What is missing is the *definitive* counterpart: there is no way to log a user out / switch accounts, i.e. drop the session including its refresh token so it does NOT come back. Add `forget(request)` on DPoPTokenProvider (and an optional `forget?` on the TokenProvider interface, mirroring `invalidate?`): it evicts the whole cached session for the request's issuer — refresh token included — so the next upgrade runs a fresh authorization-code flow. Doing that safely needs a supersession fence, or an in-flight upgrade could still complete carrying the just-forgotten credentials. A per-issuer monotonic generation is captured at the start of each upgrade and re-checked before the upgrade *arms* (attaches credentials); if forget() bumped it in between, the upgrade fails closed with SessionForgottenError instead of attaching the discarded session's token. The fence is per-issuer, so forgetting one issuer never disturbs an in-flight upgrade for another. Combined with the cache's set-once install, a concurrent renewal cannot resurrect a forgotten session. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.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 gap
The stack gives the session cache a transient stale path:
invalidate()(#14) marks the access token stale but keeps the durable refresh token, so the nextupgradesilently re-establishes the same session via the refresh grant (#12). That is exactly right for a rejected-token retry.What is missing is the definitive counterpart — a log out / switch account primitive. There is currently no way to drop a session including its refresh token, so a consumer cannot de-authenticate: even after clearing UI state, the next
upgraderefreshes the user straight back in.The change
forget(request)onDPoPTokenProvider(plus an optionalforget?on theTokenProviderinterface, mirroring #14'sinvalidate?): it evicts the whole cached session for the request's issuer — refresh token included — so the nextupgraderuns a fresh authorization-code flow. The transient/definitive split, side by side:invalidate(request)(#14)forget(request)(this PR)upgradeWhy the fence
A bare
deleteis not enough: anupgradethat is already in flight (parked in discovery, the popup, or the token grant) read the session beforeforgetand would still arm — completing the request with the just-forgotten credentials.So each
upgradecaptures a per-issuer monotonic generation at entry and re-checks it right before it arms (attaches credentials). Ifforgetbumped that issuer's generation in between, the upgrade fails closed withSessionForgottenErrorinstead of attaching the discarded token. The counter is per-issuer, so forgetting one issuer never disturbs an in-flight upgrade for another; and because#begininstalls the cache entry once up front (never re-installs after its awaits), a concurrent renewal cannot resurrect a forgotten session.This is the "re-check the login generation at every await before arming" discipline the
@jeswrapp suite arrived at after a superseded-login / logout race let a stale login arm after the user had moved on — adapted to this repo's per-issuer session model.Verification
npx tsc— clean (the CI check).test/DPoPTokenProvider.forget.test.ts(5 cases,npm test):forgetforces a fresh authorization on the next upgrade; the definitive-vs-transient contrast (invalidate→refresh_tokengrant with no popup,forget→authorization_codewith a fresh popup); a session forgotten mid-upgrade fails closed withSessionForgottenErrorand is not resurrected (the next upgrade re-authorizes); per-issuer isolation (forgetting issuer A leaves an in-flight upgrade for issuer B untouched);forgetwith no session is a safe no-op. Full suite (21 tests) green.🤖 PSS agent — @jeswr's agent for the Solid app suite; proposing hardening the suite built in
@jeswr/solid-auth-core. Draft for review — the API (a provider method vs a manager-levellogout()that fans out to providers'forget?) is open to your preference.