Skip to content

Security: refuse a two-factor challenge as a session - #62

Merged
davior merged 1 commit into
mainfrom
claude/beautiful-wright-og4xur
Nov 4, 2026
Merged

davior merged 1 commit into
mainfrom
claude/beautiful-wright-og4xur

Conversation

@davior

@davior davior commented Nov 4, 2026

Copy link
Copy Markdown
Owner

Summary

A 2FA bypass, live in production. When someone with two-factor on signs in to Notes, the password step returns a five-minute challenge token for the second step. It is signed with the shared JWT_SECRET_KEY and carries the person's sub. GAM's current_user checks signature, expiry and subject, but not the twofa_pending claim, so the challenge was accepted as a full session: the password alone opened GAM, as a bearer header or as the gecko_session cookie.

current_user now refuses any token with twofa_pending (401 unauthorized, "This sign-in still needs its second step"). Normal sessions are unaffected; they never carry the claim.

Found while porting sign-in to the suite (M6). The same gap is in Notes itself, GVC and the suite, each fixed in its own PR.

Test plan

  • New test_a_two_factor_challenge_is_not_a_session: a challenge-shaped token is refused both as a bearer header and as the cookie.
  • Full pytest -q: 1544 passed.

Deployment

  • Services rebuilt: backend only (docker compose up --build -d). No configuration, no migrations.
  • Verify: curl -f http://localhost:${APP_PORT}/api/health; signing in through Notes still reaches GAM as before.
  • Roll back: redeploy the previous commit (which reopens the gap).

Generated by Claude Code

Notes returns a five-minute challenge token after the password step of a 2FA sign-in,
signed with the shared JWT secret and carrying the person's sub. It was accepted here as
a session, so a password alone got past the second factor. It now answers 401.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V1geD2ELWgL2sDpxYbx3Aw
@davior
davior marked this pull request as ready for review November 4, 2026 18:44
@davior
davior merged commit cb099cf into main Nov 4, 2026
3 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

Development

Successfully merging this pull request may close these issues.

2 participants