Skip to content

fix: OAuth passkey requirement and race-free log out everywhere - #488

Merged
Bccorb merged 3 commits into
mainfrom
fix/oauth-passkey-requirement-and-revoke-all
Oct 23, 2026
Merged

Bccorb merged 3 commits into
mainfrom
fix/oauth-passkey-requirement-and-revoke-all

Conversation

@Bccorb

@Bccorb Bccorb commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Closes #469. Closes #471.

#469: OAuth sign-in skipped the per-account passkey requirement

  • finishOAuthLogin now calls isPasskeyRequiredForUser after resolving the user and before logging oauth_login_success. When it applies (phishing-resistant-only mode, or the account holds a passkey and passkey_login_fallback_enabled is off) the callback answers 403 { error: 'login_method_disabled' }, records oauth_login_failed with reason passkey_required, and issues no session.
  • The session-issuance backstop in src/services/sessionIssuance.ts now 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 documented login_method_disabled answer and a clean audit trail, while the broadened backstop guarantees no new path can mint a session it should not. The extra Credential.count only runs when fallback is off and passkey is enabled; isPasskeyRequiredForUser settles 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 and openapi.json say so. Server adapters pass 403 through; clients that already handle login_method_disabled from the other continuations need nothing new.

#471: log out everywhere could miss a session created by a concurrent refresh

  • New src/services/sessionRevocation.ts: revokeAllSessionsForUsers writes users.sessions_revoked_before and then revokes with one UPDATE sessions SET revoked_at ... WHERE user_id = ? AND revoked_at IS NULL.
  • Used by DELETE /logout/all, DELETE /sessions, DELETE /admin/sessions/:userId/revoke-all, device-replacement recovery (revokeSessions), and OAuth provider retirement (inside its existing transaction).
  • validateSessionRecord refuses a session created at or before the owner's cutoff, and a session whose owner row is gone.
  • POST /refresh refuses a session created at or before the cutoff, and re-reads the cutoff after claimSessionRotation succeeds, so a rotation that began before the revoke cannot hand out a working session. Refusal: 401 invalid_refresh_token, session revoked with revokedReason: 'sessions_revoked', refresh_token_failed with refusal: 'sessions_revoked'.
  • Migration 20261022130000-add-user-sessions-revoked-before.cjs adds the nullable users.sessions_revoked_before column.
  • Cost: one extra indexed users lookup per authenticated request (in validateSessionRecord) and one per refresh.
  • Docs: docs/security-posture.md (new "Log out everywhere" section), docs/admin-operations.md.

Tests

  • OAuth: refused with 403 for a passkey holder when fallback is off (no session, failure event, no success event); allowed for an account with no passkey.
  • Session issuance: refuses each non-passkey method for a passkey holder when fallback is off; issues for an account without one; passkey sessions do not count credentials.
  • Refresh racing log out everywhere: an in-memory store shared by both handlers, with DELETE /logout/all run 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.
  • validateSessionRecord cutoff cases (before, at, missing createdAt, after), owner missing.
  • Bulk-revoke assertions on every revoke-all path, and tests/postgres/sessionRevocation.spec.ts against 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. The before_replacement race case passes on the old code too, because the conditional rotation claim already catches it; the after_claim case is the one that fails.

Checks (real output)

  • npm run typecheck: pass
  • npm run lint: pass
  • npx prettier --check .: All matched files use Prettier code style!
  • npm run build: pass
  • npm 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.
  • Postgres suite (TEST_DB=postgres, scratch database): 3 files, 17 passed
  • Migration chain on a scratch Postgres database: db:migrate adds sessions_revoked_before timestamp with time zone NULL, db:migrate:undo removes it, db:migrate adds it again. Scratch database dropped afterwards.

Changesets: oauth-passkey-requirement.md (patch), revoke-all-sessions-cutoff.md (patch).

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
@Bccorb
Bccorb merged commit afbb857 into main Oct 23, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant