Skip to content

feat(auth): A1 — sign in with a mailed one-time code (server)#289

Open
oratis wants to merge 2 commits into
mainfrom
feat/auth-otp-server
Open

feat(auth): A1 — sign in with a mailed one-time code (server)#289
oratis wants to merge 2 commits into
mainfrom
feat/auth-otp-server

Conversation

@oratis

@oratis oratis commented Jul 23, 2026

Copy link
Copy Markdown
Owner

First of five stacked PRs adding passwordless email sign-in (OTP) and Google sign-in across web / iOS / Mac / CLI. Plan: docs/PLAN_AUTH_OTP_GOOGLE_v1.0.md (included here, following the #259 precedent of landing the plan with its first milestone).

What this does

Request a 6-digit code, spend it, you're in. Reading the mail is the proof of ownership, so a single step both registers and authenticates — and the address comes out verified, which levels the free window from $1 to $5 (src/billing/quota.ts).

Password sign-in is untouched beside it: App Review's demo account still needs user/pass, and for an existing account a code is an additional way in, not a replacement.

Pieces

  • src/web/otp.ts — the code store, behind the same file/Firestore seam as accounts.ts. One record per address, split in two halves:
    • the challenge (code hash, expiry, wrong-guess count), cleared the moment it's spent or burned;
    • the send budget (cooldown, sends today), which deliberately survives a spent code — otherwise redeeming one would reset the mail-bombing cap.
  • accounts.tsensureOtpAccount(); lookup and insert share one mutation so a Firestore CAS retry can't create an address twice. Password material is now explicitly optional.
  • mailer.ts — the code mail, sharing one transport with the verification link.
  • POST /api/auth/otp/request + /verify — pre-gate like the other sign-in surfaces, behind the same per-IP backstop as register/login.

Security notes for review

  • Codes are never stored: only SHA-256 of email:code (address-salted, so the same digits for two people don't collide), compared in constant time.
  • A code dies at the first of: expiry (10 min), five wrong guesses, or being spent once.
  • A passwordless account can never be password-authenticatedverifyEmailLogin runs the decoy scrypt params and fails, same as any bad credential.
  • The /request reply is identical whether or not an account exists for the address — no membership oracle.
  • Password lockouts are deliberately not cleared by a code sign-in: they guard the password credential only.
  • Deployment requirement: without RESEND_API_KEY the code degrades to a server log. A production instance must have a working key + verified sending domain, or nobody receives a code.

Tests

npm test1145 tests green (+22: 14 OTP store, 5 account seam, 3 mailer). Typecheck clean.

🤖 Generated with Claude Code

Passwordless sign-in for LISA accounts: request a 6-digit code, spend it,
and you are in. Reading the mail is the proof of ownership, so one step
both registers and authenticates — and the address comes out verified,
which is what levels the free window from $1 to $5.

- src/web/otp.ts: the code store behind the same file/Firestore seam as
  accounts.ts. One record per address, split into a challenge (hash,
  expiry, wrong-guess count) and a send budget (cooldown, daily cap) —
  the budget deliberately survives a spent code so redeeming one can't
  reset the mail-bombing cap. Codes are stored only as address-salted
  SHA-256 and compared in constant time; a code dies at the first of
  expiry, five wrong guesses, or being spent.
- accounts.ts: ensureOtpAccount() — lookup and insert inside one mutation
  so a Firestore CAS retry can't create an address twice. Password
  material is now explicitly optional; a code-only account fails the
  password path against the decoy params like any other bad credential.
- mailer.ts: the code mail, sharing one transport with the verification
  link. Without RESEND_API_KEY it degrades to a server log, so a real
  key is now a production requirement.
- POST /api/auth/otp/request + /verify, pre-gate like the other
  sign-in surfaces, behind the same per-IP backstop as register/login.
  The request answer is identical whether or not the address has an
  account — no membership oracle.

Password sign-in is untouched: App Review's demo account keeps working,
and a code is an additional way in rather than a replacement.

Plan: docs/PLAN_AUTH_OTP_GOOGLE_v1.0.md (A0/A1).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@oratis

oratis commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

Thorough, well-tested OTP core — CSPRNG randomInt, hash-at-rest + timingSafeEqual, single-use/expiry/attempt-burn, uniform /request (no enumeration), decoy-scrypt rejection of passwordless login, and a solid cookie posture. Two things before merge:

Blocker — account pre-hijacking. ensureOtpAccount() adopts a pre-existing email account: it flips verified=true, keeps the existing scrypt, and does not bump sessionVersion. Because /api/auth/register is open and unauthenticated (creates a verified:false account with an attacker-chosen password for any address, no ownership proof) and verifyEmailLogin never gates on verified:

  1. Attacker registers victim@x.com with a known password → em-U.
  2. Victim signs in by mailed code → adopts em-U, now verified; victim's credits/data land on it.
  3. Attacker logs in with the password → same uid → shared/hijacked account.

The test "the password still works — the code is an additional way in" pins this behavior. On the unverified→verified transition (proof of inbox control), please bump sessionVersion (kills pre-existing sessions) and invalidate the pre-set password (drop scrypt / force reset) so a password an attacker set before ownership was proven can't survive.

Recommendation (non-blocking) — mail amplification. /api/auth/otp/request mails any well-formed address with only per-address caps; the cross-address limiter is the shared per-IP auth: bucket over a spoofable x-forwarded-for. Add a global daily send ceiling + alerting so LISA can't be used to spam third parties / exhaust the Resend quota.

Minor: codeDigest is fast unsalted-except-email SHA-256 (a leaked otp.json → trivial offline brute of live codes; bounded by TTL/entropy), and the no-API-key path logs the raw code — ensure prod always has RESEND_API_KEY.

ensureOtpAccount() adopted a pre-existing email account by setting verified=true
but KEEPING its scrypt and NOT bumping sessionVersion. Because /api/auth/register
is open+unauthenticated, an attacker could pre-register victim@x.com with a known
password; when the victim later signed in by code, the attacker's password still
authenticated → shared/hijacked account.

Add markVerifiedByOwnershipProof(): on the unverified→verified transition (proof
of inbox control), drop any pre-set password and rotate sessionVersion, so a
credential set before ownership was proven can no longer authenticate and any
session minted from it is invalidated. A password the real owner sets AFTER
verifying is unaffected.

Updated the test that pinned the vulnerable "password still works" behavior; added
an explicit attacker-scenario test. (Google byEmail path fixed on #290; the
verify-link path + verifyEmailLogin-allows-unverified are flagged for review.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
oratis added a commit that referenced this pull request Jul 24, 2026
…nt (HIGH)

upsertGoogleAccount had two flaws:

- byEmail bind adopted a pre-existing email account (verified=true) while KEEPING
  its scrypt and NOT bumping sessionVersion — same account pre-hijacking as the
  OTP path (#289): an attacker pre-registers victim@x.com/password, the victim
  signs in with Google, the attacker's password still authenticates. Now routes
  through markVerifiedByOwnershipProof(): a password set before verification is
  dropped and sessionVersion rotated.

- bySub overwrote acct.email with no uniqueness check, so a Google email change
  onto an address another local account owns broke one-address-one-account and
  made getAccountByEmail order-dependent (split balance). Now refuses (email_taken)
  on a collision.

Updated the test that pinned the vulnerable "password still works" bind; added a
bySub-collision test. (Merges #289's OTP fix as the base of this stacked PR.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@oratis

oratis commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

Account-takeover fix pushed (a6be431) — held for your review, NOT merged (per the plan: this changes login semantics for existing users, so it's your call).

Fix: added markVerifiedByOwnershipProof() — on the unverified→verified transition (a mailed code proves inbox control), drop any pre-set password and rotate sessionVersion. This closes the pre-hijacking: an attacker who pre-registered victim@x.com with a known password can no longer authenticate after the victim signs in by code. Updated the test that pinned the vulnerable "password still works" behavior; added an explicit attacker-scenario test. typecheck + 14 accounts tests green.

Behavior change to confirm: a password set before verification is dropped (the owner sets a fresh one after signing in). OTP accounts normally carry no password, so this only affects a register-then-OTP sequence.

Related, deliberately NOT changed — your call:

  1. confirmEmailVerification (the email-verify link path) has the same shape, but there the password is usually the legit self-registrant's, so dropping it would break the normal register→verify→login flow. I left it; a phishing-dependent variant of the attack remains.
  2. verifyEmailLogin authenticates unverified accounts — you may prefer to require verification before password login (a cleaner root fix).
  3. Mail-amplification: /api/auth/otp/request has only per-address caps; consider a global daily send ceiling before wide launch.

⚠️ Needs reconciling with current main before merge (the KB v2 stack + a node_modules cleanup landed while this was open).

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.

1 participant