diff --git a/CLAUDE.md b/CLAUDE.md index 85c9af5..2d5deb2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -92,13 +92,24 @@ If we ever want to keep them strictly separate, the change goes in ## What's deferred (documented in SECURITY.md) -- Recovery-code lockout-DoS. The recovery endpoint has no server-side proof - of the recovery code; an attacker who knows the email can lock out the - user (but **not** read their data). Mitigations: rate limiting and email - confirmation. Out of scope for the scaffold. -- Server-side rate limiting in general. +- Server-side rate limiting on auth/recovery endpoints. The recovery + verifier closes the lockout-DoS; rate limiting reduces the online + brute-force surface on top of that. - CSP / SRI for the SPA. +## Recovery verifier — deviation from the spec + +The original spec stored only `kek_salt`, `wrapped_dek_pw`+nonce, `rec_salt`, +and `wrapped_dek_rec`+nonce. We additionally store `rec_auth_salt` and +`rec_auth_verifier_hash` so the server can verify the caller knows the +recovery code before `/auth/recovery-complete` writes anything. This is the +only deviation from the spec's stated storage model — documented in +SECURITY.md. + +If you find yourself "simplifying away" the rec_auth_* columns or the verifier +check, **stop**: that re-opens the lockout DoS. See the test in +`tests/auth.test.ts` for the regression case. + ## Run / test / typecheck - `bun install` diff --git a/README.md b/README.md index 7353f23..7a188e3 100644 --- a/README.md +++ b/README.md @@ -157,5 +157,6 @@ Explicitly **out of scope** for now: - comments, notifications, other social features - native/mobile apps - server-side full-text search over private data -- email-confirmation step for recovery (lockout-DoS mitigation noted in - `SECURITY.md`) +- rate limiting on auth/recovery endpoints (defense-in-depth — the recovery + verifier already closes the lockout-DoS hole; rate limiting reduces online + brute-force surface) diff --git a/SECURITY.md b/SECURITY.md index f03fffa..bfbe00e 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -55,16 +55,26 @@ old parameters must be stored per-user so unlock still works. For each user the server stores **exactly**: ``` -auth_salt (16 bytes, public) -- distinct from kek_salt -auth_verifier_hash (text) -- Bun.password.hash of the auth verifier -kek_salt (16 bytes, public) -- for password-derived KEK -wrapped_dek_pw (48 bytes) -- DEK encrypted under KEK_pw -dek_pw_nonce (24 bytes) -rec_salt (16 bytes, public) -- for recovery-code-derived KEK -wrapped_dek_rec (48 bytes) -- DEK encrypted under KEK_rec -dek_rec_nonce (24 bytes) +auth_salt (16 bytes, public) -- distinct from kek_salt +auth_verifier_hash (text) -- Bun.password.hash of the auth verifier +kek_salt (16 bytes, public) -- for password-derived KEK +wrapped_dek_pw (48 bytes) -- DEK encrypted under KEK_pw +dek_pw_nonce (24 bytes) +rec_salt (16 bytes, public) -- for recovery-code-derived KEK +wrapped_dek_rec (48 bytes) -- DEK encrypted under KEK_rec +dek_rec_nonce (24 bytes) +rec_auth_salt (16 bytes, public) -- distinct from rec_salt +rec_auth_verifier_hash (text) -- Bun.password.hash of the recovery verifier ``` +The recovery verifier is the proof-of-recovery-code that +`/api/auth/recovery-complete` requires before it changes any state. Without +it (the original spec's storage model), an attacker who knows only the email +could submit a junk new password wrap and lock the legitimate user out — the +data would still be safe but the account would be DoS'd. With it, recovery +requires knowledge of the recovery code; an attacker can no longer cause a +lockout. + The server never sees, derives, or stores: - The user's raw password. @@ -72,34 +82,40 @@ The server never sees, derives, or stores: - The DEK itself. - Plaintext title / tags / location / scheduled time for any **private** activity. -## Why three salts? +## Why four salts? -Three independently random salts ensure that knowing one derivation tells you +Four independently random salts ensure that knowing one derivation tells you nothing about another: - `auth_salt` — input to the **auth verifier** the server holds. - `kek_salt` — input to **KEK_pw**, which unwraps the DEK. - `rec_salt` — input to **KEK_rec**, the recovery-code-derived unwrap key. +- `rec_auth_salt` — input to the **recovery verifier**, the proof-of-knowledge + the server checks before completing a recovery. In particular, `auth_salt ≠ kek_salt` guarantees that even if a server-side breach leaks the verifier hash *and* an attacker brute-forces it, they still -need to redo Argon2id against `kek_salt` to derive the KEK. The verifier hash -is never sufficient on its own. +need to redo Argon2id against `kek_salt` to derive the KEK. The same property +holds for `rec_auth_salt ≠ rec_salt`: brute-forcing the recovery verifier +hash doesn't directly hand the attacker KEK_rec. ## Signup flow (client-driven) -1. Client generates `dek` (32 bytes), `kek_salt`, `rec_salt`, `auth_salt` (16 bytes each). +1. Client generates `dek` (32 bytes), `kek_salt`, `rec_salt`, `auth_salt`, + `rec_auth_salt` (16 bytes each). 2. Client generates a high-entropy `recovery_code` (≥120 bits), shows it to the user, and never sends it. 3. Client derives: - `kek_pw = pwhash(password, kek_salt)` - `kek_rec = pwhash(recovery_code, rec_salt)` - - `auth_verifier = pwhash(password, auth_salt)` (≠ kek_pw because salts differ) + - `auth_verifier = pwhash(password, auth_salt)` (≠ kek_pw because salts differ) + - `rec_auth_verifier = pwhash(recovery_code, rec_auth_salt)` (≠ kek_rec because salts differ) 4. Client wraps: - `wrapped_dek_pw = AEAD(kek_pw, dek, dek_pw_nonce)` - `wrapped_dek_rec = AEAD(kek_rec, dek, dek_rec_nonce)` -5. Client posts the salts, wraps, nonces, and `auth_verifier` to the server. -6. Server hashes `auth_verifier` with `Bun.password.hash` and stores the row. +5. Client posts the salts, wraps, nonces, `auth_verifier`, and + `rec_auth_verifier` to the server. +6. Server hashes both verifiers with `Bun.password.hash` and stores the row. ## Unlock / login flow @@ -124,34 +140,52 @@ is never sufficient on its own. ## Recovery flow -The recovery code path is intentionally symmetric to the password path. The -server cannot tell whether the submitted new wrap is "of the same DEK" — it -just stores what the client sends. Trust is anchored entirely in the -recovery-code holder. +The recovery code path is symmetric to the password path. The server cannot +tell whether the submitted new wrap is "of the same DEK" — it just stores what +the client sends. Trust is anchored in the recovery-code holder, with the +server using a recovery verifier as a proof-of-knowledge gate before any state +change. 1. Client posts `{ email }` to `/api/auth/recovery-challenge`; server returns - `{ rec_salt, wrapped_dek_rec, dek_rec_nonce }`. + `{ rec_salt, wrapped_dek_rec, dek_rec_nonce, rec_auth_salt }`. 2. Client derives `kek_rec = pwhash(recovery_code, rec_salt)` and unwraps the DEK. -3. Client chooses a new password, derives new salts/verifier/wrap as in signup, - and posts to `/api/auth/recovery-complete`. -4. Server replaces the password wrap, auth salt, and verifier in a single - transaction. The recovery wrap is unchanged (the same recovery code keeps - working). +3. Client also derives `rec_auth_verifier = pwhash(recovery_code, rec_auth_salt)` — + the proof the server will check. +4. Client chooses a new password, derives new password-side + salts/verifier/wrap as in signup. +5. Client posts to `/api/auth/recovery-complete` with `rec_auth_verifier` plus + the new password-side material. +6. **Server first calls `Bun.password.verify(rec_auth_verifier, rec_auth_verifier_hash)`.** + If it fails, the request is rejected with `401` and no state changes. To + keep the timing of "no such user" indistinguishable from "wrong code", the + server runs `Bun.password.verify` against a dummy hash even when the email + isn't found. +7. On success, the server replaces password-side material in a single + transaction. The recovery wrap, `rec_salt`, `rec_auth_salt`, and + `rec_auth_verifier_hash` are unchanged — the same recovery code keeps + working. +8. The server deletes all existing sessions for the user so any hijacked + session is invalidated. -### Known limitation: lockout DoS +### Why the recovery verifier closes the DoS -Anyone who knows a user's email can trigger `/api/auth/recovery-complete` and, -without the recovery code, submit a "junk" new password wrap. The data is **not -disclosed** (the attacker can't decrypt anything), but the legitimate user is -locked out unless they still hold a logged-in session or the recovery code. +The original spec stored only the recovery wrap. Anyone who knew the email +could submit a new password wrap and lock the legitimate user out (the data +itself remained safe — only the recovery-code holder could read it). -Mitigations (out of scope for the scaffold but intended): +With the recovery verifier in place, the server has a cryptographic proof that +the submitter knows the recovery code before any write happens. An attacker +without the code can no longer cause a lockout. The `auth_salt ≠ kek_salt` +property carries over: `rec_auth_salt ≠ rec_salt`, so brute-forcing the +verifier hash doesn't give the attacker the unwrap key. -- Per-IP and per-email rate limiting on recovery endpoints. -- Email confirmation before activating the new password wrap. -- A "recovery verifier" stored server-side so the server can reject submissions - that don't prove knowledge of the recovery code. This is a deviation from the - spec's stated storage model and is therefore deferred. +### Remaining recovery-flow caveats + +- Per-IP and per-email rate limiting is still desirable on the recovery + endpoints (and on login) to slow online brute-force; out of scope for the + scaffold. +- Email-confirmation flows would add a second factor for additional defense + in depth; also out of scope. ## Private activity encryption @@ -208,8 +242,9 @@ server flags: ## Things to flag, not silently change -The spec invites flagging anything cryptographically unsound. The above design -follows the spec exactly. The one place where I would push back if asked to go -to production (not deferred for this scaffold) is the recovery lockout DoS — -without a server-side proof of the recovery code, the recovery endpoint is a -soft-DoS vector. Documented above, deferred for the scaffold. +The spec invites flagging anything cryptographically unsound. The original +spec stored only `kek_salt`, `wrapped_dek_pw`+nonce, `rec_salt`, +`wrapped_dek_rec`+nonce. We deviate by additionally storing `rec_auth_salt` +and `rec_auth_verifier_hash` to close the lockout DoS on `/auth/recovery-complete`. +The deviation is documented above. Salts and verifier hashes are not secret; +the storage shape is otherwise unchanged. diff --git a/frontend/src/components/Recovery.svelte b/frontend/src/components/Recovery.svelte index dea066b..164cd7e 100644 --- a/frontend/src/components/Recovery.svelte +++ b/frontend/src/components/Recovery.svelte @@ -32,10 +32,16 @@ onAuthed(); } catch (err) { // libsodium throws a generic decrypt error if the code is wrong. + // The server returns 401 if the code derives a wrong verifier (defense + // in depth — usually the local decrypt fails first). if (err instanceof Error && err.message.toLowerCase().includes('decrypt')) { error = 'Feil gjenopprettingskode.'; + } else if (err instanceof ApiError && err.status === 401) { + error = 'Feil gjenopprettingskode.'; } else if (err instanceof ApiError && err.status === 404) { error = 'Ingen bruker med den eposten.'; + } else if (err instanceof ApiError && err.status === 409) { + error = 'Denne kontoen mangler gjenopprettingsverifikator. Opprett konto på nytt.'; } else { error = 'Gjenoppretting feilet.'; } diff --git a/frontend/src/lib/auth.ts b/frontend/src/lib/auth.ts index cffbe57..e235eb6 100644 --- a/frontend/src/lib/auth.ts +++ b/frontend/src/lib/auth.ts @@ -37,14 +37,19 @@ export async function signup(email: string, password: string): Promise { 'email', 'auth_salt', 'auth_verifier', 'kek_salt', 'wrapped_dek_pw', 'dek_pw_nonce', 'rec_salt', 'wrapped_dek_rec', 'dek_rec_nonce', + 'rec_auth_salt', 'rec_auth_verifier', ]); if (miss) return c.json({ error: miss }, 400); const email = body.email.trim().toLowerCase(); @@ -72,17 +73,21 @@ authRoutes.post('/signup', async (c) => { // Server-side hash of the client-derived verifier. The verifier is already // expensive to brute-force (Argon2id-MODERATE), so Bun.password adds a second // hardening layer in case the DB leaks but the verifier salt is still public. - const verifierHash = await Bun.password.hash(body.auth_verifier, { - algorithm: 'argon2id', - }); + // The recovery verifier is hashed the same way for the same reasons — it's + // the proof-of-recovery-code on /auth/recovery-complete. + const [verifierHash, recVerifierHash] = await Promise.all([ + Bun.password.hash(body.auth_verifier, { algorithm: 'argon2id' }), + Bun.password.hash(body.rec_auth_verifier, { algorithm: 'argon2id' }), + ]); const id = newId(); const now = Date.now(); db.prepare(` INSERT INTO users (id, email, auth_salt, auth_verifier_hash, kek_salt, - wrapped_dek_pw, dek_pw_nonce, wrapped_dek_rec, rec_salt, dek_rec_nonce, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + wrapped_dek_pw, dek_pw_nonce, wrapped_dek_rec, rec_salt, dek_rec_nonce, + rec_auth_salt, rec_auth_verifier_hash, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `).run( id, email, @@ -94,6 +99,8 @@ authRoutes.post('/signup', async (c) => { b64ToBuffer(body.wrapped_dek_rec), b64ToBuffer(body.rec_salt), b64ToBuffer(body.dek_rec_nonce), + b64ToBuffer(body.rec_auth_salt), + recVerifierHash, now, ); @@ -224,38 +231,65 @@ authRoutes.post('/recovery-challenge', async (c) => { const email = body.email.trim().toLowerCase(); const row = getDb() - .prepare('SELECT rec_salt, wrapped_dek_rec, dek_rec_nonce FROM users WHERE email = ?') + .prepare(` + SELECT rec_salt, wrapped_dek_rec, dek_rec_nonce, rec_auth_salt + FROM users WHERE email = ? + `) .get(email) as - | { rec_salt: Uint8Array; wrapped_dek_rec: Uint8Array; dek_rec_nonce: Uint8Array } + | { + rec_salt: Uint8Array; + wrapped_dek_rec: Uint8Array; + dek_rec_nonce: Uint8Array; + rec_auth_salt: Uint8Array | null; + } | null; if (!row) return c.json({ error: 'no_such_user' }, 404); + if (!row.rec_auth_salt) { + // Account from a pre-fix scaffold session — no recovery verifier set. + // Refuse rather than silently downgrade. + return c.json({ error: 'recovery_not_provisioned' }, 409); + } const resp: RecoveryChallengeResponse = { rec_salt: bufferToB64(row.rec_salt), wrapped_dek_rec: bufferToB64(row.wrapped_dek_rec), dek_rec_nonce: bufferToB64(row.dek_rec_nonce), + rec_auth_salt: bufferToB64(row.rec_auth_salt), }; return c.json(resp); }); // --- POST /auth/recovery-complete ------------------------------------------- -// Replaces password-side material AND auth verifier. Recovery wrap is -// untouched (same recovery code keeps working). +// Replaces password-side material AND auth verifier. Recovery wrap and the +// rec_auth_* columns are untouched — the same recovery code keeps working. // -// Known limitation: this endpoint has no proof-of-recovery-code; see SECURITY.md. +// We require a recovery-side verifier proof: an attacker who knows the email +// but not the recovery code can no longer lock the user out (or read data — +// reading was already impossible). See SECURITY.md. authRoutes.post('/recovery-complete', async (c) => { const body = (await c.req.json().catch(() => null)) as RecoveryCompleteRequest | null; if (!body) return c.json({ error: 'invalid_json' }, 400); const miss = missingKey(body, [ - 'email', 'auth_salt', 'auth_verifier', 'kek_salt', 'wrapped_dek_pw', 'dek_pw_nonce', + 'email', 'rec_auth_verifier', + 'auth_salt', 'auth_verifier', 'kek_salt', 'wrapped_dek_pw', 'dek_pw_nonce', ]); if (miss) return c.json({ error: miss }, 400); const email = body.email.trim().toLowerCase(); const row = getDb() - .prepare('SELECT id FROM users WHERE email = ?') - .get(email) as { id: string } | null; - if (!row) return c.json({ error: 'no_such_user' }, 404); + .prepare('SELECT id, rec_auth_verifier_hash FROM users WHERE email = ?') + .get(email) as { id: string; rec_auth_verifier_hash: string | null } | null; + + // Same constant-work pattern as /auth/login: always run Bun.password.verify + // against *some* hash so an attacker can't distinguish "no such user" from + // "wrong recovery code" by timing the response. + const hashToVerify = row?.rec_auth_verifier_hash + ?? '$argon2id$v=19$m=65536,t=3,p=1$YWFhYWFhYWE$AAAAAAAAAAAAAAAAAAAAAA'; + const ok = await Bun.password.verify(body.rec_auth_verifier, hashToVerify).catch(() => false); + + if (!row || !row.rec_auth_verifier_hash || !ok) { + return c.json({ error: 'invalid_recovery' }, 401); + } const verifierHash = await Bun.password.hash(body.auth_verifier, { algorithm: 'argon2id', diff --git a/server/db.ts b/server/db.ts index 3bbadf2..f73d2d0 100644 --- a/server/db.ts +++ b/server/db.ts @@ -22,6 +22,11 @@ const SCHEMA_STATEMENTS: readonly string[] = [ wrapped_dek_rec BLOB NOT NULL, rec_salt BLOB NOT NULL, dek_rec_nonce BLOB NOT NULL, + -- Recovery-side verifier: closes the recovery lockout DoS by proving + -- knowledge of the recovery code before recovery-complete updates anything. + -- See SECURITY.md § "Recovery flow". + rec_auth_salt BLOB NOT NULL, + rec_auth_verifier_hash TEXT NOT NULL, created_at INTEGER NOT NULL )`, `CREATE TABLE IF NOT EXISTS activities ( @@ -77,6 +82,20 @@ function applyStatements(db: Database, statements: readonly string[]): void { } } +/** + * Idempotently add a column. SQLite has no `ALTER TABLE … ADD COLUMN IF NOT + * EXISTS`, so we probe `PRAGMA table_info`. Added columns are NULLABLE — SQLite + * can't add NOT NULL columns without a DEFAULT, and we don't have a sensible + * default. New rows still come through the schema in `SCHEMA_STATEMENTS` and + * fill the column; existing rows from older scaffold sessions remain NULL + * and the application-level handlers reject operations that need the column. + */ +function ensureColumn(db: Database, table: string, column: string, type: string): void { + const cols = db.prepare(`PRAGMA table_info(${table})`).all() as { name: string }[]; + if (cols.some((c) => c.name === column)) return; + db.prepare(`ALTER TABLE ${table} ADD COLUMN ${column} ${type}`).run(); +} + export function getDb(): Database { if (dbInstance) return dbInstance; @@ -88,6 +107,12 @@ export function getDb(): Database { applyStatements(db, PRAGMAS); applyStatements(db, SCHEMA_STATEMENTS); + // Forward-compat: a scaffold DB created before the recovery-verifier fix + // won't have these columns. Add them as nullable so the server still boots. + // Old users will need to re-sign-up to fully use recovery; see CLAUDE.md. + ensureColumn(db, 'users', 'rec_auth_salt', 'BLOB'); + ensureColumn(db, 'users', 'rec_auth_verifier_hash', 'TEXT'); + dbInstance = db; return db; } diff --git a/shared/types.ts b/shared/types.ts index f618dbe..b55660c 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -16,6 +16,12 @@ export interface SignupRequest { rec_salt: string; wrapped_dek_rec: string; dek_rec_nonce: string; + // Recovery-side verifier: derived client-side from the recovery code with + // its own salt, separate from `rec_salt`. The server stores Bun.password.hash + // of this verifier and uses it to gate recovery-complete; without a valid + // verifier, recovery-complete is rejected (closing the lockout DoS). + rec_auth_salt: string; + rec_auth_verifier: string; } export interface ChallengeResponse { @@ -34,6 +40,9 @@ export interface RecoveryChallengeResponse { rec_salt: string; wrapped_dek_rec: string; dek_rec_nonce: string; + // Salt for deriving the recovery verifier (separate from rec_salt above). + // The client returns the derived verifier on recovery-complete. + rec_auth_salt: string; } export interface PasswordChangeRequest { @@ -47,6 +56,10 @@ export interface PasswordChangeRequest { export interface RecoveryCompleteRequest { email: string; + // Proof that the caller knows the recovery code. Must verify against + // rec_auth_verifier_hash before the server touches any user state. + rec_auth_verifier: string; + // New password-side material; the recovery wrap and rec_auth_* are untouched. auth_salt: string; auth_verifier: string; kek_salt: string; diff --git a/tests/auth.test.ts b/tests/auth.test.ts new file mode 100644 index 0000000..b768e65 --- /dev/null +++ b/tests/auth.test.ts @@ -0,0 +1,204 @@ +/** + * Regression tests for the recovery verifier. + * + * The recovery lockout DoS was: an attacker who knew a user's email could + * POST to `/auth/recovery-complete` with junk material and lock the user + * out — the data stayed safe (the attacker couldn't decrypt anything) but + * the legitimate user couldn't log back in without the recovery code. + * + * The fix: server checks a recovery verifier (`Bun.password.verify` against + * `rec_auth_verifier_hash`) before touching any state. This test asserts the + * server rejects a recovery attempt without a valid verifier. + */ +import { afterAll, beforeAll, beforeEach, describe, expect, test } from 'bun:test'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + ready, + generateDek, + generateSalt, + generateRecoveryCode, + normalizeRecoveryCode, + deriveKey, + deriveAuthVerifier, + wrapDek, + bytesToBase64, +} from '../shared/crypto'; +import type { SignupRequest } from '../shared/types'; + +let tmpDir: string; +let baseUrl: string; +// Inferred from the Hono instance assigned in beforeAll. Avoid annotating — +// Hono's `fetch` is overloaded and tightening it here causes friction. +let app: Awaited>; + +async function loadApp() { + const { authRoutes } = await import('../server/auth'); + const { Hono } = await import('hono'); + const root = new Hono(); + root.route('/api/auth', authRoutes); + return root; +} + +beforeAll(async () => { + // Each test run gets its own DB file so failures don't bleed into reruns. + tmpDir = mkdtempSync(join(tmpdir(), 'vinterliste-test-')); + process.env.VINTERLISTE_DB = join(tmpDir, 'test.db'); + await ready(); + + // Dynamically import via the helper so the env var above is in place + // before db.ts opens the SQLite file. + app = await loadApp(); + baseUrl = 'http://test.local'; +}); + +afterAll(() => { + rmSync(tmpDir, { recursive: true, force: true }); +}); + +interface SignupOut { + email: string; + password: string; + recoveryCode: string; +} + +async function signupUser(email: string, password: string): Promise { + const recoveryCode = generateRecoveryCode(); + const dek = generateDek(); + const kekSalt = generateSalt(); + const authSalt = generateSalt(); + const recSalt = generateSalt(); + const recAuthSalt = generateSalt(); + + const kekPw = deriveKey(password, kekSalt); + const kekRec = deriveKey(normalizeRecoveryCode(recoveryCode), recSalt); + const authVerifier = deriveAuthVerifier(password, authSalt); + const recAuthVerifier = deriveAuthVerifier(normalizeRecoveryCode(recoveryCode), recAuthSalt); + + const wrappedPw = wrapDek(dek, kekPw); + const wrappedRec = wrapDek(dek, kekRec); + + const body: SignupRequest = { + email, + auth_salt: bytesToBase64(authSalt), + auth_verifier: authVerifier, + kek_salt: bytesToBase64(kekSalt), + wrapped_dek_pw: bytesToBase64(wrappedPw.ciphertext), + dek_pw_nonce: bytesToBase64(wrappedPw.nonce), + rec_salt: bytesToBase64(recSalt), + wrapped_dek_rec: bytesToBase64(wrappedRec.ciphertext), + dek_rec_nonce: bytesToBase64(wrappedRec.nonce), + rec_auth_salt: bytesToBase64(recAuthSalt), + rec_auth_verifier: recAuthVerifier, + }; + + const res = await app.fetch( + new Request(`${baseUrl}/api/auth/signup`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }), + ); + expect(res.status).toBe(200); + + return { email, password, recoveryCode }; +} + +async function recoveryComplete(opts: { + email: string; + recAuthVerifier: string; +}): Promise { + // We're testing the verifier gate; everything else can be arbitrary base64. + const dummy = bytesToBase64(generateSalt()); + return app.fetch( + new Request(`${baseUrl}/api/auth/recovery-complete`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + email: opts.email, + rec_auth_verifier: opts.recAuthVerifier, + auth_salt: dummy, + auth_verifier: 'AAAA', + kek_salt: dummy, + wrapped_dek_pw: 'AAAA', + dek_pw_nonce: 'AAAA', + }), + }), + ); +} + +describe('recovery verifier gate', () => { + let userEmail: string; + + beforeEach(async () => { + userEmail = `u-${Date.now()}-${Math.random().toString(36).slice(2, 8)}@test.invalid`; + await signupUser(userEmail, 'correct horse battery staple'); + }); + + test('recovery-complete with a junk verifier is rejected (no state changes)', async () => { + const junk = bytesToBase64(generateSalt()); // not derived from any code + + const res = await recoveryComplete({ email: userEmail, recAuthVerifier: junk }); + expect(res.status).toBe(401); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe('invalid_recovery'); + }); + + test('recovery-complete for an unknown email is also rejected with 401', async () => { + // We deliberately don't 404 on unknown emails so timing doesn't leak + // existence. The server runs Bun.password.verify against a dummy hash. + const junk = bytesToBase64(generateSalt()); + const res = await recoveryComplete({ + email: 'nobody-here@test.invalid', + recAuthVerifier: junk, + }); + expect(res.status).toBe(401); + }); + + test('challenge response includes rec_auth_salt', async () => { + const res = await app.fetch( + new Request(`${baseUrl}/api/auth/recovery-challenge`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: userEmail }), + }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as Record; + expect(typeof body.rec_auth_salt).toBe('string'); + expect(body.rec_auth_salt!.length).toBeGreaterThan(0); + }); + + test('a correctly-derived verifier passes the gate', async () => { + // Re-derive the verifier the way a legitimate recovery flow would. + const out = await signupUser( + `u2-${Date.now()}@test.invalid`, + 'correct horse battery staple', + ); + const challengeRes = await app.fetch( + new Request(`${baseUrl}/api/auth/recovery-challenge`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: out.email }), + }), + ); + const challenge = (await challengeRes.json()) as { rec_auth_salt: string }; + + // We don't have the rec_auth_salt at signup time on the client side here, + // so we re-sign-up with a known recovery code… actually simpler: derive + // the verifier using the salt we just got from the server. + const verifier = deriveAuthVerifier( + normalizeRecoveryCode(out.recoveryCode), + Buffer.from(challenge.rec_auth_salt, 'base64'), + ); + + const res = await recoveryComplete({ + email: out.email, + recAuthVerifier: verifier, + }); + expect(res.status).toBe(200); + const body = (await res.json()) as { ok: boolean }; + expect(body.ok).toBe(true); + }); +});