Repository navigation
fix: OAuth passkey requirement and race-free log out everywhere - #488
Merged
Merged
Conversation
finishOAuthLogin now answers 403 login_method_disabled when isPasskeyRequiredForUser applies, like the OTP, magic link and TOTP continuations. Session issuance applies the same rule as its backstop, not only phishing-resistant-only mode, so a path that misses the check fails closed. Closes #469
Every revoke-all path (logout all, DELETE /sessions, the admin revoke-all, device-replacement recovery and OAuth provider retirement) now revokes with one bulk update and records users.sessions_revoked_before first. validateSessionRecord and /refresh refuse a session created at or before that cutoff, and refresh re-reads it after claiming the rotation, so a rotation that started before the revoke cannot produce a live session. Closes #471
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.
Closes #469. Closes #471.
#469: OAuth sign-in skipped the per-account passkey requirement
finishOAuthLoginnow callsisPasskeyRequiredForUserafter resolving the user and before loggingoauth_login_success. When it applies (phishing-resistant-only mode, or the account holds a passkey andpasskey_login_fallback_enabledis off) the callback answers403 { error: 'login_method_disabled' }, recordsoauth_login_failedwith reasonpasskey_required, and issues no session.src/services/sessionIssuance.tsnow applies the same check (isPasskeyRequiredForUser) instead of only phishing-resistant-only mode, so a non-passkey path that forgets the gate fails closed.Why both, rather than moving the check into session issuance alone: issuance throws
PasskeyRequiredError, which the calling controllers turn into a 500, and it runs after each flow has already logged its success event. Keeping the explicit 403 in each continuation (now including OAuth) preserves the documentedlogin_method_disabledanswer and a clean audit trail, while the broadened backstop guarantees no new path can mint a session it should not. The extraCredential.countonly runs when fallback is off and passkey is enabled;isPasskeyRequiredForUsersettles every other case without a query. Every existing path's tests pass unchanged.Contract note: the OAuth callback can now answer
403 login_method_disabled, a status already in its declared 403 response schema. The route description andopenapi.jsonsay so. Server adapters pass 403 through; clients that already handlelogin_method_disabledfrom the other continuations need nothing new.#471: log out everywhere could miss a session created by a concurrent refresh
src/services/sessionRevocation.ts:revokeAllSessionsForUserswritesusers.sessions_revoked_beforeand then revokes with oneUPDATE sessions SET revoked_at ... WHERE user_id = ? AND revoked_at IS NULL.DELETE /logout/all,DELETE /sessions,DELETE /admin/sessions/:userId/revoke-all, device-replacement recovery (revokeSessions), and OAuth provider retirement (inside its existing transaction).validateSessionRecordrefuses a session created at or before the owner's cutoff, and a session whose owner row is gone.POST /refreshrefuses a session created at or before the cutoff, and re-reads the cutoff afterclaimSessionRotationsucceeds, so a rotation that began before the revoke cannot hand out a working session. Refusal:401 invalid_refresh_token, session revoked withrevokedReason: 'sessions_revoked',refresh_token_failedwithrefusal: 'sessions_revoked'.20261022130000-add-user-sessions-revoked-before.cjsadds the nullableusers.sessions_revoked_beforecolumn.userslookup per authenticated request (invalidateSessionRecord) and one per refresh.docs/security-posture.md(new "Log out everywhere" section),docs/admin-operations.md.Tests
DELETE /logout/allrun mid-refresh before the replacement row exists and after the rotation is claimed. Both end with 401 and no live session. Plus the early cutoff refusal and the owner-gone refusal.validateSessionRecordcutoff cases (before, at, missing createdAt, after), owner missing.tests/postgres/sessionRevocation.spec.tsagainst real Postgres.Confirmed failing before the fix by reverting
src/only: #469 6 failing (5 issuance cases plus the OAuth 403 case), #471 15 failing. Thebefore_replacementrace case passes on the old code too, because the conditional rotation claim already catches it; theafter_claimcase is the one that fails.Checks (real output)
npm run typecheck: passnpm run lint: passnpx prettier --check .: All matched files use Prettier code style!npm run build: passnpm run coverage: 142 files, 2001 passed, 1 todo. Statements 98.9%, Branches 95.42%, Functions 99.2%, Lines 99.14%npm run check:changeset-bump: No major changesets.TEST_DB=postgres, scratch database): 3 files, 17 passeddb:migrateaddssessions_revoked_before timestamp with time zone NULL,db:migrate:undoremoves it,db:migrateadds it again. Scratch database dropped afterwards.Changesets:
oauth-passkey-requirement.md(patch),revoke-all-sessions-cutoff.md(patch).