From 9539836a8e9ad33653d493603f588b695b33e918 Mon Sep 17 00:00:00 2001 From: jagdeep sidhu Date: Sat, 25 Apr 2026 07:23:46 -0700 Subject: [PATCH 1/8] feat(auth): add TOTP MFA support Adds optional TOTP setup, login challenge enforcement, encrypted secret storage, and single-use recovery codes so account sessions can require a second factor after password verification. Made-with: Cursor --- db/migrations/001_init.sql | 26 +++ lib/appFactory.js | 3 + lib/kdf.js | 18 ++ lib/totp.js | 347 +++++++++++++++++++++++++++++++++++++ routes/auth.js | 163 +++++++++++++++-- tests/auth.routes.test.js | 101 +++++++++++ 6 files changed, 639 insertions(+), 19 deletions(-) create mode 100644 lib/totp.js diff --git a/db/migrations/001_init.sql b/db/migrations/001_init.sql index 769e763..b639fd4 100644 --- a/db/migrations/001_init.sql +++ b/db/migrations/001_init.sql @@ -42,6 +42,11 @@ -- sessions -- Cookie-backed session rows. Also hashed at rest. -- +-- user_totp, mfa_challenges +-- Optional TOTP second factor. Shared secrets are encrypted with the +-- server pepper before storage; recovery codes and login challenge +-- tokens are hashed at rest. +-- -- vaults -- 1:1 with users, created lazily on first PUT. No salt_v here — that -- lives on the user row (see above). @@ -141,6 +146,27 @@ CREATE TABLE sessions ( CREATE INDEX idx_sessions_user ON sessions(user_id); CREATE INDEX idx_sessions_expires ON sessions(expires_at); +CREATE TABLE user_totp ( + user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, + secret_enc TEXT, + pending_secret_enc TEXT, + recovery_hashes TEXT NOT NULL DEFAULT '[]', + enabled INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL +); + +CREATE TABLE mfa_challenges ( + token_hash TEXT PRIMARY KEY, + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + expires_at INTEGER NOT NULL, + attempts INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL +); + +CREATE INDEX idx_mfa_challenges_user ON mfa_challenges(user_id); +CREATE INDEX idx_mfa_challenges_expires ON mfa_challenges(expires_at); + CREATE TABLE vaults ( user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, blob TEXT NOT NULL, diff --git a/lib/appFactory.js b/lib/appFactory.js index d9c9acb..0607366 100644 --- a/lib/appFactory.js +++ b/lib/appFactory.js @@ -9,6 +9,7 @@ const { createVerificationsRepo } = require('./verifications'); const { createPendingRegistrationsRepo, } = require('./pendingRegistrations'); +const { createTotpRepo } = require('./totp'); const { createVaultsRepo } = require('./vaults'); const { createVoteReceiptsRepo } = require('./voteReceipts'); const { createProposalDraftsRepo } = require('./proposalDrafts'); @@ -97,6 +98,7 @@ function buildServices({ return { users: createUsersRepo(db, { now }), sessions: createSessionStore(db, { now }), + totp: createTotpRepo(db, { now }), verifications: createVerificationsRepo(db, { now }), pendingRegistrations: createPendingRegistrationsRepo(db, { now }), vaults: createVaultsRepo(db, { now }), @@ -193,6 +195,7 @@ function mountAuthAndVault( createAuthRouter({ users: services.users, sessions: services.sessions, + totp: services.totp, pendingRegistrations: services.pendingRegistrations, vaults: services.vaults, mailer, diff --git a/lib/kdf.js b/lib/kdf.js index 7d5f413..c7cf1b4 100644 --- a/lib/kdf.js +++ b/lib/kdf.js @@ -126,6 +126,22 @@ function assertPepperConfigured() { loadPepper(); } +function hmacServerSecret(value, context = 'default') { + return crypto + .createHmac('sha256', loadPepper()) + .update(String(context), 'utf8') + .update('\0') + .update(String(value), 'utf8') + .digest('hex'); +} + +function deriveServerKey(context = 'default') { + return crypto + .createHmac('sha256', loadPepper()) + .update(`sysnode:${context}`, 'utf8') + .digest(); +} + function constantTimeEqualHex(a, b) { if (typeof a !== 'string' || typeof b !== 'string') return false; const aLower = a.toLowerCase(); @@ -140,6 +156,8 @@ module.exports = { generateSalt, hashAuthHash, verifyAuthHash, + hmacServerSecret, + deriveServerKey, constantTimeEqualHex, assertPepperConfigured, KdfConfigError, diff --git a/lib/totp.js b/lib/totp.js new file mode 100644 index 0000000..7d32d00 --- /dev/null +++ b/lib/totp.js @@ -0,0 +1,347 @@ +const crypto = require('crypto'); +const { deriveServerKey, hmacServerSecret } = require('./kdf'); + +const ISSUER = 'Sysnode'; +const BASE32_ALPHABET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567'; +const CHALLENGE_TTL_MS = 5 * 60 * 1000; +const MAX_CHALLENGE_ATTEMPTS = 5; +const RECOVERY_CODE_COUNT = 10; +const TOTP_STEP_SECONDS = 30; +const TOTP_DIGITS = 6; + +function mkErr(code) { + const e = new Error(code); + e.code = code; + return e; +} + +function encryptSecret(secret) { + const key = deriveServerKey('totp-secret-v1'); + const iv = crypto.randomBytes(12); + const cipher = crypto.createCipheriv('aes-256-gcm', key, iv); + const ciphertext = Buffer.concat([ + cipher.update(String(secret), 'utf8'), + cipher.final(), + ]); + const tag = cipher.getAuthTag(); + return [ + 'v1', + iv.toString('base64url'), + tag.toString('base64url'), + ciphertext.toString('base64url'), + ].join('.'); +} + +function decryptSecret(envelope) { + const parts = String(envelope || '').split('.'); + if (parts.length !== 4 || parts[0] !== 'v1') throw mkErr('invalid_secret'); + const key = deriveServerKey('totp-secret-v1'); + const iv = Buffer.from(parts[1], 'base64url'); + const tag = Buffer.from(parts[2], 'base64url'); + const ciphertext = Buffer.from(parts[3], 'base64url'); + const decipher = crypto.createDecipheriv('aes-256-gcm', key, iv); + decipher.setAuthTag(tag); + return Buffer.concat([decipher.update(ciphertext), decipher.final()]).toString( + 'utf8' + ); +} + +function base32Encode(buf) { + let bits = 0; + let value = 0; + let output = ''; + for (const byte of buf) { + value = (value << 8) | byte; + bits += 8; + while (bits >= 5) { + output += BASE32_ALPHABET[(value >>> (bits - 5)) & 31]; + bits -= 5; + } + } + if (bits > 0) { + output += BASE32_ALPHABET[(value << (5 - bits)) & 31]; + } + return output; +} + +function base32Decode(value) { + const clean = String(value || '').toUpperCase().replace(/=+$/g, ''); + let bits = 0; + let acc = 0; + const out = []; + for (const ch of clean) { + const idx = BASE32_ALPHABET.indexOf(ch); + if (idx === -1) throw mkErr('invalid_secret'); + acc = (acc << 5) | idx; + bits += 5; + if (bits >= 8) { + out.push((acc >>> (bits - 8)) & 0xff); + bits -= 8; + } + } + return Buffer.from(out); +} + +function generateSecret() { + return base32Encode(crypto.randomBytes(20)); +} + +function generateTotpCode(secret, timeMs = Date.now()) { + const counter = Math.floor(timeMs / 1000 / TOTP_STEP_SECONDS); + const msg = Buffer.alloc(8); + msg.writeBigUInt64BE(BigInt(counter)); + const hmac = crypto.createHmac('sha1', base32Decode(secret)).update(msg).digest(); + const offset = hmac[hmac.length - 1] & 0x0f; + const binary = + ((hmac[offset] & 0x7f) << 24) | + ((hmac[offset + 1] & 0xff) << 16) | + ((hmac[offset + 2] & 0xff) << 8) | + (hmac[offset + 3] & 0xff); + return String(binary % 10 ** TOTP_DIGITS).padStart(TOTP_DIGITS, '0'); +} + +function normalizeCode(code) { + return String(code || '').replace(/\s+/g, ''); +} + +function constantTimeEqualText(a, b) { + if (typeof a !== 'string' || typeof b !== 'string') return false; + if (a.length !== b.length) return false; + return crypto.timingSafeEqual(Buffer.from(a), Buffer.from(b)); +} + +function verifyTotp(secret, code, timeMs = Date.now()) { + const token = normalizeCode(code); + if (!/^[0-9]{6}$/.test(token)) return false; + return [-1, 0, 1].some((offset) => + constantTimeEqualText( + token, + generateTotpCode(secret, timeMs + offset * TOTP_STEP_SECONDS * 1000) + ) + ); +} + +function formatRecoveryCode(raw) { + return `${raw.slice(0, 4)}-${raw.slice(4, 8)}-${raw.slice(8, 12)}`; +} + +function normalizeRecoveryCode(code) { + return String(code || '').toUpperCase().replace(/[^A-Z0-9]/g, ''); +} + +function hashRecoveryCode(code) { + return hmacServerSecret(normalizeRecoveryCode(code), 'totp-recovery-v1'); +} + +function generateRecoveryCodes() { + return Array.from({ length: RECOVERY_CODE_COUNT }, () => + formatRecoveryCode(crypto.randomBytes(6).toString('hex').toUpperCase()) + ); +} + +function mapRecoveryHashes(codes) { + return JSON.stringify(codes.map(hashRecoveryCode)); +} + +function readRecoveryHashes(json) { + try { + const parsed = JSON.parse(json || '[]'); + return Array.isArray(parsed) ? parsed.filter((v) => typeof v === 'string') : []; + } catch { + return []; + } +} + +function hashToken(token) { + return crypto.createHash('sha256').update(token, 'hex').digest('hex'); +} + +function createTotpRepo(db, opts = {}) { + const now = opts.now ?? (() => Date.now()); + + const selectTotp = db.prepare( + `SELECT user_id AS userId, + secret_enc AS secretEnc, + pending_secret_enc AS pendingSecretEnc, + recovery_hashes AS recoveryHashes, + enabled, + created_at AS createdAt, + updated_at AS updatedAt + FROM user_totp + WHERE user_id = ?` + ); + const upsertPending = db.prepare( + `INSERT INTO user_totp + (user_id, secret_enc, pending_secret_enc, recovery_hashes, enabled, created_at, updated_at) + VALUES (?, NULL, ?, '[]', 0, ?, ?) + ON CONFLICT(user_id) DO UPDATE SET + pending_secret_enc = excluded.pending_secret_enc, + updated_at = excluded.updated_at` + ); + const enable = db.prepare( + `UPDATE user_totp + SET secret_enc = pending_secret_enc, + pending_secret_enc = NULL, + recovery_hashes = ?, + enabled = 1, + updated_at = ? + WHERE user_id = ? + AND pending_secret_enc IS NOT NULL` + ); + const disable = db.prepare(`DELETE FROM user_totp WHERE user_id = ?`); + const updateRecoveryHashes = db.prepare( + `UPDATE user_totp SET recovery_hashes = ?, updated_at = ? WHERE user_id = ?` + ); + + const insertChallenge = db.prepare( + `INSERT INTO mfa_challenges + (token_hash, user_id, expires_at, attempts, created_at) + VALUES (?, ?, ?, 0, ?)` + ); + const selectChallenge = db.prepare( + `SELECT token_hash AS tokenHash, + user_id AS userId, + expires_at AS expiresAt, + attempts + FROM mfa_challenges + WHERE token_hash = ?` + ); + const bumpChallenge = db.prepare( + `UPDATE mfa_challenges SET attempts = attempts + 1 WHERE token_hash = ?` + ); + const deleteChallenge = db.prepare( + `DELETE FROM mfa_challenges WHERE token_hash = ?` + ); + const deleteExpiredChallenges = db.prepare( + `DELETE FROM mfa_challenges WHERE expires_at < ?` + ); + + function status(userId) { + const row = selectTotp.get(userId); + return { + enabled: !!(row && row.enabled === 1 && row.secretEnc), + pending: !!(row && row.pendingSecretEnc), + recoveryCodesRemaining: + row && row.recoveryHashes ? readRecoveryHashes(row.recoveryHashes).length : 0, + }; + } + + function beginSetup(user) { + if (!user || !Number.isInteger(user.id)) throw mkErr('invalid_user'); + const secret = generateSecret(); + const t = now(); + upsertPending.run(user.id, encryptSecret(secret), t, t); + const account = user.email || `user-${user.id}`; + const label = `${encodeURIComponent(ISSUER)}:${encodeURIComponent(account)}`; + const issuer = encodeURIComponent(ISSUER); + return { + secret, + otpauthUrl: `otpauth://totp/${label}?secret=${secret}&issuer=${issuer}&algorithm=SHA1&digits=${TOTP_DIGITS}&period=${TOTP_STEP_SECONDS}`, + }; + } + + function enableSetup(userId, code) { + const row = selectTotp.get(userId); + if (!row || !row.pendingSecretEnc) throw mkErr('totp_setup_not_started'); + const secret = decryptSecret(row.pendingSecretEnc); + if (!verifyTotp(secret, code)) throw mkErr('invalid_totp_code'); + const recoveryCodes = generateRecoveryCodes(); + const info = enable.run(mapRecoveryHashes(recoveryCodes), now(), userId); + if (info.changes !== 1) throw mkErr('totp_setup_not_started'); + return { recoveryCodes }; + } + + function getEnabledSecret(userId) { + const row = selectTotp.get(userId); + if (!row || row.enabled !== 1 || !row.secretEnc) return null; + return decryptSecret(row.secretEnc); + } + + function createChallenge(userId) { + deleteExpiredChallenges.run(now()); + const token = crypto.randomBytes(32).toString('hex'); + insertChallenge.run(hashToken(token), userId, now() + CHALLENGE_TTL_MS, now()); + return { + challengeToken: token, + expiresAt: now() + CHALLENGE_TTL_MS, + }; + } + + function consumeRecoveryCode(row, code) { + const normalized = normalizeRecoveryCode(code); + if (!/^[A-Z0-9]{12}$/.test(normalized)) return false; + const hashes = readRecoveryHashes(row.recoveryHashes); + const candidate = hashRecoveryCode(normalized); + const index = hashes.findIndex((h) => h === candidate); + if (index === -1) return false; + hashes.splice(index, 1); + updateRecoveryHashes.run(JSON.stringify(hashes), now(), row.userId); + return true; + } + + function verifyChallenge({ challengeToken, code, recoveryCode }) { + const tokenHash = hashToken(challengeToken); + const challenge = selectChallenge.get(tokenHash); + if (!challenge || challenge.expiresAt < now()) { + if (challenge) deleteChallenge.run(tokenHash); + throw mkErr('mfa_challenge_invalid'); + } + if (challenge.attempts >= MAX_CHALLENGE_ATTEMPTS) { + deleteChallenge.run(tokenHash); + throw mkErr('mfa_challenge_invalid'); + } + + const row = selectTotp.get(challenge.userId); + if (!row || row.enabled !== 1 || !row.secretEnc) { + deleteChallenge.run(tokenHash); + throw mkErr('mfa_challenge_invalid'); + } + + let ok = false; + let recoveryCodeUsed = false; + if (code) { + ok = verifyTotp(decryptSecret(row.secretEnc), code); + } else if (recoveryCode) { + ok = consumeRecoveryCode(row, recoveryCode); + recoveryCodeUsed = ok; + } + + if (!ok) { + bumpChallenge.run(tokenHash); + throw mkErr('invalid_totp_code'); + } + deleteChallenge.run(tokenHash); + return { userId: challenge.userId, recoveryCodeUsed }; + } + + function verifyUserCode(userId, code) { + const secret = getEnabledSecret(userId); + if (!secret) throw mkErr('totp_not_enabled'); + if (!verifyTotp(secret, code)) throw mkErr('invalid_totp_code'); + return true; + } + + function disableTotp(userId) { + disable.run(userId); + } + + return { + status, + beginSetup, + enableSetup, + getEnabledSecret, + createChallenge, + verifyChallenge, + verifyUserCode, + disable: disableTotp, + }; +} + +module.exports = { + createTotpRepo, + verifyTotp, + generateTotpCode, + normalizeRecoveryCode, + hashRecoveryCode, + CHALLENGE_TTL_MS, +}; diff --git a/routes/auth.js b/routes/auth.js index 1cac523..3edd3ee 100644 --- a/routes/auth.js +++ b/routes/auth.js @@ -17,6 +17,20 @@ const RegisterSchema = z.object({ const LoginSchema = RegisterSchema; +const TotpLoginSchema = z + .object({ + challengeToken: z.string().regex(/^[0-9a-fA-F]{64}$/), + code: z.string().min(1).max(32).optional(), + recoveryCode: z.string().min(1).max(64).optional(), + }) + .refine((v) => Boolean(v.code) !== Boolean(v.recoveryCode), { + message: 'Provide exactly one of code or recoveryCode', + }); + +const TotpCodeSchema = z.object({ + code: z.string().min(1).max(32), +}); + // PR 7 — password change now rotates the vault wrap atomically. // // The client re-derives the new vaultKey from (newPassword, email, @@ -91,6 +105,7 @@ function asyncHandler(fn) { function createAuthRouter({ users, sessions, + totp, pendingRegistrations, vaults, mailer, @@ -143,6 +158,27 @@ function createAuthRouter({ return `${verifyBase}/verify-email?token=${token}`; } + function userBody(user) { + return { + id: user.id, + email: user.email, + emailVerified: user.emailVerified, + notificationPrefs: user.notificationPrefs, + saltV: user.saltV, + totpEnabled: totp ? totp.status(user.id).enabled : false, + }; + } + + function issueLoginSession(req, res, user) { + const { token, expiresAt } = sessions.issue(user.id, { + userAgent: req.get('user-agent') || null, + ip: req.ip, + }); + sessionMw.setSessionCookie(res, token, expiresAt); + csrfMw.issueCookie(res, expiresAt); + return { expiresAt }; + } + // ------------------------------------------------------------------------- // POST /auth/register // @@ -401,17 +437,20 @@ function createAuthRouter({ }); return res.status(403).json({ error: 'email_not_verified' }); } + if (totp && totp.status(user.id).enabled) { + const { challengeToken, expiresAt } = totp.createChallenge(user.id); + return res.json({ + mfaRequired: true, + challengeToken, + expiresAt, + }); + } // sessions.issue / setSessionCookie / issueCookie may also throw on // transient DB failures. They ran unprotected before (Codex round-9 // P1 "Guard login session creation"); asyncHandler now forwards any // rejection to the error middleware instead of producing an // unhandled promise rejection. - const { token, expiresAt } = sessions.issue(user.id, { - userAgent: req.get('user-agent') || null, - ip: req.ip, - }); - sessionMw.setSessionCookie(res, token, expiresAt); - csrfMw.issueCookie(res, expiresAt); + const { expiresAt } = issueLoginSession(req, res, user); return res.json({ // saltV is delivered here (and on /auth/me) so the client has the // per-user vault salt in memory immediately after login, without a @@ -421,16 +460,39 @@ function createAuthRouter({ // Data Key inside the encrypted vault blob. Delivered on login // rather than gated behind /vault so an empty-vault first-write // ("create vault") does not need to round-trip for salt material. - user: { - id: user.id, - email: user.email, - emailVerified: user.emailVerified, - saltV: user.saltV, - }, + user: userBody(user), expiresAt, }); })); + router.post('/login/totp', limiters.login, asyncHandler(async (req, res) => { + if (!totp) return res.status(503).json({ error: 'server_misconfigured' }); + const parsed = TotpLoginSchema.safeParse(req.body); + if (!parsed.success) return badRequest(res, 'invalid_body'); + let out; + try { + out = totp.verifyChallenge(parsed.data); + } catch (err) { + if ( + err && + (err.code === 'mfa_challenge_invalid' || err.code === 'invalid_totp_code') + ) { + return res.status(401).json({ error: err.code }); + } + throw err; + } + const user = users.findById(out.userId); + if (!user || !user.emailVerified) { + return res.status(401).json({ error: 'mfa_challenge_invalid' }); + } + const { expiresAt } = issueLoginSession(req, res, user); + return res.json({ + user: userBody(user), + expiresAt, + recoveryCodeUsed: out.recoveryCodeUsed, + }); + })); + // ------------------------------------------------------------------------- // POST /auth/logout // ------------------------------------------------------------------------- @@ -465,13 +527,7 @@ function createAuthRouter({ // fields as /auth/login — otherwise a rehydrated client would // silently lose saltV and be unable to unlock the vault until // it re-logs-in. - user: { - id: req.user.id, - email: req.user.email, - emailVerified: req.user.emailVerified, - notificationPrefs: req.user.notificationPrefs, - saltV: req.user.saltV, - }, + user: userBody(req.user), }); }); @@ -530,6 +586,75 @@ function createAuthRouter({ } ); + router.get('/totp', sessionMw.requireAuth, (req, res) => { + if (!totp) return res.status(503).json({ error: 'server_misconfigured' }); + return res.json(totp.status(req.user.id)); + }); + + router.post( + '/totp/setup', + sessionMw.requireAuth, + csrfMw.require, + (req, res) => { + if (!totp) return res.status(503).json({ error: 'server_misconfigured' }); + const out = totp.beginSetup(req.user); + return res.json({ + secret: out.secret, + otpauthUrl: out.otpauthUrl, + }); + } + ); + + router.post( + '/totp/enable', + sessionMw.requireAuth, + csrfMw.require, + (req, res) => { + if (!totp) return res.status(503).json({ error: 'server_misconfigured' }); + const parsed = TotpCodeSchema.safeParse(req.body); + if (!parsed.success) return badRequest(res, 'invalid_body'); + try { + const out = totp.enableSetup(req.user.id, parsed.data.code); + return res.json({ + status: 'enabled', + recoveryCodes: out.recoveryCodes, + }); + } catch (err) { + if ( + err && + (err.code === 'totp_setup_not_started' || err.code === 'invalid_totp_code') + ) { + return res.status(400).json({ error: err.code }); + } + throw err; + } + } + ); + + router.post( + '/totp/disable', + sessionMw.requireAuth, + csrfMw.require, + (req, res) => { + if (!totp) return res.status(503).json({ error: 'server_misconfigured' }); + const parsed = TotpCodeSchema.safeParse(req.body); + if (!parsed.success) return badRequest(res, 'invalid_body'); + try { + totp.verifyUserCode(req.user.id, parsed.data.code); + totp.disable(req.user.id); + } catch (err) { + if ( + err && + (err.code === 'totp_not_enabled' || err.code === 'invalid_totp_code') + ) { + return res.status(400).json({ error: err.code }); + } + throw err; + } + return res.json({ status: 'disabled' }); + } + ); + // ------------------------------------------------------------------------- // POST /auth/change-password // Client re-derives both old & new authHash from old/new passwords and diff --git a/tests/auth.routes.test.js b/tests/auth.routes.test.js index 48e7e89..ee0d51e 100644 --- a/tests/auth.routes.test.js +++ b/tests/auth.routes.test.js @@ -1,5 +1,6 @@ const request = require('supertest'); const { buildTestApp } = require('./helpers/buildTestApp'); +const { generateTotpCode } = require('../lib/totp'); const SAMPLE_AUTH = 'a4f8b3c1d9e7f2a5b1c6d8e4f7a9b2c5d1e8f4a7b3c9d5e1f6a2b8c4d7e3f5a9'; @@ -329,6 +330,106 @@ describe('auth routes', () => { return { agent, csrf }; } + test('TOTP setup requires a valid authenticator code before enabling MFA', async () => { + const { agent, csrf } = await registerAndLogin(ctx); + + const setup = await agent + .post('/auth/totp/setup') + .set('X-CSRF-Token', csrf) + .send({}); + expect(setup.status).toBe(200); + expect(setup.body.secret).toEqual(expect.any(String)); + expect(setup.body.otpauthUrl).toMatch(/^otpauth:\/\/totp\//); + + const bad = await agent + .post('/auth/totp/enable') + .set('X-CSRF-Token', csrf) + .send({ code: '000000' }); + expect(bad.status).toBe(400); + expect(bad.body.error).toBe('invalid_totp_code'); + + const good = await agent + .post('/auth/totp/enable') + .set('X-CSRF-Token', csrf) + .send({ code: generateTotpCode(setup.body.secret) }); + expect(good.status).toBe(200); + expect(good.body.status).toBe('enabled'); + expect(good.body.recoveryCodes).toHaveLength(10); + + const me = await agent.get('/auth/me'); + expect(me.status).toBe(200); + expect(me.body.user.totpEnabled).toBe(true); + }); + + test('TOTP-enabled accounts require a second step before session cookies are issued', async () => { + const { agent, csrf } = await registerAndLogin(ctx); + const setup = await agent + .post('/auth/totp/setup') + .set('X-CSRF-Token', csrf) + .send({}); + await agent + .post('/auth/totp/enable') + .set('X-CSRF-Token', csrf) + .send({ code: generateTotpCode(setup.body.secret) }); + await agent.post('/auth/logout').set('X-CSRF-Token', csrf); + + const login = await request(ctx.app) + .post('/auth/login') + .send({ email: 'user@example.com', authHash: SAMPLE_AUTH }); + expect(login.status).toBe(200); + expect(login.body.mfaRequired).toBe(true); + expect(login.body.challengeToken).toMatch(/^[0-9a-f]{64}$/); + expect(extractCookies(login).sid).toBeUndefined(); + + const verify = await request(ctx.app) + .post('/auth/login/totp') + .send({ + challengeToken: login.body.challengeToken, + code: generateTotpCode(setup.body.secret), + }); + expect(verify.status).toBe(200); + expect(verify.body.user.email).toBe('user@example.com'); + expect(verify.body.user.totpEnabled).toBe(true); + expect(extractCookies(verify).sid).toMatch(/^[0-9a-f]{64}$/); + }); + + test('TOTP recovery codes are single use during login challenge verification', async () => { + const { agent, csrf } = await registerAndLogin(ctx); + const setup = await agent + .post('/auth/totp/setup') + .set('X-CSRF-Token', csrf) + .send({}); + const enabled = await agent + .post('/auth/totp/enable') + .set('X-CSRF-Token', csrf) + .send({ code: generateTotpCode(setup.body.secret) }); + await agent.post('/auth/logout').set('X-CSRF-Token', csrf); + + const login = await request(ctx.app) + .post('/auth/login') + .send({ email: 'user@example.com', authHash: SAMPLE_AUTH }); + const verify = await request(ctx.app) + .post('/auth/login/totp') + .send({ + challengeToken: login.body.challengeToken, + recoveryCode: enabled.body.recoveryCodes[0], + }); + expect(verify.status).toBe(200); + expect(verify.body.recoveryCodeUsed).toBe(true); + + const loginAgain = await request(ctx.app) + .post('/auth/login') + .send({ email: 'user@example.com', authHash: SAMPLE_AUTH }); + const reuse = await request(ctx.app) + .post('/auth/login/totp') + .send({ + challengeToken: loginAgain.body.challengeToken, + recoveryCode: enabled.body.recoveryCodes[0], + }); + expect(reuse.status).toBe(401); + expect(reuse.body.error).toBe('invalid_totp_code'); + }); + test('POST /auth/change-password (with vault): updates both auth AND vault atomically', async () => { const { agent, csrf } = await registerAndLogin(ctx); From adea6c75c2e1221f2541189f0e72e92382a4cfb6 Mon Sep 17 00:00:00 2001 From: jagdeep sidhu Date: Sat, 25 Apr 2026 07:34:09 -0700 Subject: [PATCH 2/8] fix(auth): add forward migration for TOTP tables Moves MFA table creation into a new numbered migration so existing databases receive the schema during upgrade instead of relying on an already-applied init migration. Made-with: Cursor --- db/migrations/001_init.sql | 26 -------------------------- db/migrations/002_totp_mfa.sql | 26 ++++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 26 deletions(-) create mode 100644 db/migrations/002_totp_mfa.sql diff --git a/db/migrations/001_init.sql b/db/migrations/001_init.sql index b639fd4..769e763 100644 --- a/db/migrations/001_init.sql +++ b/db/migrations/001_init.sql @@ -42,11 +42,6 @@ -- sessions -- Cookie-backed session rows. Also hashed at rest. -- --- user_totp, mfa_challenges --- Optional TOTP second factor. Shared secrets are encrypted with the --- server pepper before storage; recovery codes and login challenge --- tokens are hashed at rest. --- -- vaults -- 1:1 with users, created lazily on first PUT. No salt_v here — that -- lives on the user row (see above). @@ -146,27 +141,6 @@ CREATE TABLE sessions ( CREATE INDEX idx_sessions_user ON sessions(user_id); CREATE INDEX idx_sessions_expires ON sessions(expires_at); -CREATE TABLE user_totp ( - user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, - secret_enc TEXT, - pending_secret_enc TEXT, - recovery_hashes TEXT NOT NULL DEFAULT '[]', - enabled INTEGER NOT NULL DEFAULT 0, - created_at INTEGER NOT NULL, - updated_at INTEGER NOT NULL -); - -CREATE TABLE mfa_challenges ( - token_hash TEXT PRIMARY KEY, - user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, - expires_at INTEGER NOT NULL, - attempts INTEGER NOT NULL DEFAULT 0, - created_at INTEGER NOT NULL -); - -CREATE INDEX idx_mfa_challenges_user ON mfa_challenges(user_id); -CREATE INDEX idx_mfa_challenges_expires ON mfa_challenges(expires_at); - CREATE TABLE vaults ( user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, blob TEXT NOT NULL, diff --git a/db/migrations/002_totp_mfa.sql b/db/migrations/002_totp_mfa.sql new file mode 100644 index 0000000..67dc61a --- /dev/null +++ b/db/migrations/002_totp_mfa.sql @@ -0,0 +1,26 @@ +-- Migration 002: optional TOTP MFA. +-- +-- Adds account-scoped TOTP state and short-lived login challenges. Shared +-- secrets are encrypted by the application before storage; recovery codes and +-- login challenge tokens are hashed at rest. + +CREATE TABLE user_totp ( + user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, + secret_enc TEXT, + pending_secret_enc TEXT, + recovery_hashes TEXT NOT NULL DEFAULT '[]', + enabled INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL +); + +CREATE TABLE mfa_challenges ( + token_hash TEXT PRIMARY KEY, + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + expires_at INTEGER NOT NULL, + attempts INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL +); + +CREATE INDEX idx_mfa_challenges_user ON mfa_challenges(user_id); +CREATE INDEX idx_mfa_challenges_expires ON mfa_challenges(expires_at); From 0877361517516322c889fdd963cf944689d9c1ce Mon Sep 17 00:00:00 2001 From: jagdeep sidhu Date: Sat, 25 Apr 2026 07:43:14 -0700 Subject: [PATCH 3/8] fix(auth): scope TOTP login rate limiting by challenge Adds a dedicated MFA login limiter keyed by hashed challenge token plus IP so second-factor attempts do not collapse all shared-IP users into the password-login bucket. Made-with: Cursor --- lib/appFactory.js | 2 ++ middleware/rateLimit.js | 23 +++++++++++++++++++++++ middleware/rateLimit.test.js | 20 ++++++++++++++++++++ routes/auth.js | 2 +- tests/auth.routes.test.js | 2 +- tests/govProposals.routes.test.js | 2 ++ 6 files changed, 49 insertions(+), 2 deletions(-) diff --git a/lib/appFactory.js b/lib/appFactory.js index 0607366..1f236a0 100644 --- a/lib/appFactory.js +++ b/lib/appFactory.js @@ -177,6 +177,7 @@ function mountAuthAndVault( const limiters = disableRateLimit ? { login: rateLimiters.disabled(), + mfaLogin: rateLimiters.disabled(), register: rateLimiters.disabled(), verifyEmail: rateLimiters.disabled(), vote: rateLimiters.disabled(), @@ -184,6 +185,7 @@ function mountAuthAndVault( } : { login: rateLimiters.loginLimiter(), + mfaLogin: rateLimiters.mfaLoginLimiter(), register: rateLimiters.registerLimiter(), verifyEmail: rateLimiters.verifyEmailLimiter(), vote: rateLimiters.voteLimiter(), diff --git a/middleware/rateLimit.js b/middleware/rateLimit.js index 6619a6f..98a21d4 100644 --- a/middleware/rateLimit.js +++ b/middleware/rateLimit.js @@ -1,5 +1,6 @@ const rateLimit = require('express-rate-limit'); const { ipKeyGenerator } = require('express-rate-limit'); +const crypto = require('crypto'); const { normalizeEmail } = require('../lib/email'); const securityLog = require('../lib/securityLog'); @@ -43,6 +44,14 @@ function loginKey(req) { return `login|${ipBucket(req)}|${normalizeEmail(raw)}`; } +function mfaLoginKey(req) { + const raw = (req.body && req.body.challengeToken) || ''; + const tokenHash = /^[0-9a-fA-F]{64}$/.test(raw) + ? crypto.createHash('sha256').update(raw.toLowerCase(), 'utf8').digest('hex') + : ''; + return `login-totp|${ipBucket(req)}|${tokenHash}`; +} + function registerKey(req) { return `register|${ipBucket(req)}`; } @@ -98,6 +107,18 @@ function loginLimiter() { }); } +function mfaLoginLimiter() { + return rateLimit({ + windowMs: 15 * MINUTE, + max: 5, + standardHeaders: true, + legacyHeaders: false, + keyGenerator: mfaLoginKey, + message: { error: 'too_many_attempts' }, + handler: trippedHandler('login-totp'), + }); +} + function registerLimiter() { return rateLimit({ windowMs: 60 * MINUTE, @@ -162,6 +183,7 @@ function disabled() { module.exports = { loginLimiter, + mfaLoginLimiter, registerLimiter, verifyEmailLimiter, voteLimiter, @@ -169,6 +191,7 @@ module.exports = { disabled, // Exported for direct unit testing. loginKey, + mfaLoginKey, registerKey, verifyEmailKey, voteKey, diff --git a/middleware/rateLimit.test.js b/middleware/rateLimit.test.js index aadc84e..7e22a53 100644 --- a/middleware/rateLimit.test.js +++ b/middleware/rateLimit.test.js @@ -1,5 +1,6 @@ const { loginKey, + mfaLoginKey, registerKey, reconcileKey, voteKey, @@ -50,6 +51,25 @@ describe('rate-limit key generators', () => { }); }); + describe('mfaLoginKey', () => { + test('buckets by challenge token so shared IP users do not collide', () => { + const a = mfaLoginKey( + mk({ challengeToken: 'a'.repeat(64) }, '1.2.3.4') + ); + const b = mfaLoginKey( + mk({ challengeToken: 'b'.repeat(64) }, '1.2.3.4') + ); + expect(a).not.toBe(b); + expect(a).toMatch(/^login-totp\|1\.2\.3\.4\|[0-9a-f]{64}$/); + }); + + test('does not store the raw challenge token in the limiter key', () => { + const token = 'c'.repeat(64); + const key = mfaLoginKey(mk({ challengeToken: token }, '1.2.3.4')); + expect(key).not.toContain(token); + }); + }); + describe('registerKey', () => { test('is scoped per IPv4 only', () => { const a = registerKey(mk({ email: 'a@b.com' }, '1.2.3.4')); diff --git a/routes/auth.js b/routes/auth.js index 3edd3ee..8e8cbf0 100644 --- a/routes/auth.js +++ b/routes/auth.js @@ -465,7 +465,7 @@ function createAuthRouter({ }); })); - router.post('/login/totp', limiters.login, asyncHandler(async (req, res) => { + router.post('/login/totp', limiters.mfaLogin, asyncHandler(async (req, res) => { if (!totp) return res.status(503).json({ error: 'server_misconfigured' }); const parsed = TotpLoginSchema.safeParse(req.body); if (!parsed.success) return badRequest(res, 'invalid_body'); diff --git a/tests/auth.routes.test.js b/tests/auth.routes.test.js index ee0d51e..3b6c339 100644 --- a/tests/auth.routes.test.js +++ b/tests/auth.routes.test.js @@ -1393,7 +1393,7 @@ describe('createAuthRouter factory contract (Codex round-2 P3)', () => { }, sessionMw: { requireAuth: mw, parse: mw, setSessionCookie: noop, clearSessionCookie: noop }, csrfMw: { require: mw, parse: mw, issueCookie: noop, clearCookie: noop }, - limiters: { login: mw, register: mw, verifyEmail: mw, vote: mw }, + limiters: { login: mw, mfaLogin: mw, register: mw, verifyEmail: mw, vote: mw }, baseUrl: 'http://api.test', frontendUrl: 'http://app.test', scheduler: (fn) => fn(), diff --git a/tests/govProposals.routes.test.js b/tests/govProposals.routes.test.js index e29c589..232e08e 100644 --- a/tests/govProposals.routes.test.js +++ b/tests/govProposals.routes.test.js @@ -91,6 +91,7 @@ function buildApp({ csrfMw, limiters: { login: rateLimiters.disabled(), + mfaLogin: rateLimiters.disabled(), register: rateLimiters.disabled(), verifyEmail: rateLimiters.disabled(), vote: rateLimiters.disabled(), @@ -569,6 +570,7 @@ describe('drafts CRUD', () => { csrfMw, limiters: { login: rateLimiters.disabled(), + mfaLogin: rateLimiters.disabled(), register: rateLimiters.disabled(), verifyEmail: rateLimiters.disabled(), vote: rateLimiters.disabled(), From f0f7498c1098ee6f157e9e16f8ef4b7236a633be Mon Sep 17 00:00:00 2001 From: jagdeep sidhu Date: Sat, 25 Apr 2026 07:45:15 -0700 Subject: [PATCH 4/8] chore(auth): keep TOTP schema in v1 init Moves the MFA tables back into the pre-production v1 schema so the branch stays aligned with the repository rule that 001_init.sql remains the single source before launch. Made-with: Cursor --- db/migrations/001_init.sql | 26 ++++++++++++++++++++++++++ db/migrations/002_totp_mfa.sql | 26 -------------------------- 2 files changed, 26 insertions(+), 26 deletions(-) delete mode 100644 db/migrations/002_totp_mfa.sql diff --git a/db/migrations/001_init.sql b/db/migrations/001_init.sql index 769e763..b639fd4 100644 --- a/db/migrations/001_init.sql +++ b/db/migrations/001_init.sql @@ -42,6 +42,11 @@ -- sessions -- Cookie-backed session rows. Also hashed at rest. -- +-- user_totp, mfa_challenges +-- Optional TOTP second factor. Shared secrets are encrypted with the +-- server pepper before storage; recovery codes and login challenge +-- tokens are hashed at rest. +-- -- vaults -- 1:1 with users, created lazily on first PUT. No salt_v here — that -- lives on the user row (see above). @@ -141,6 +146,27 @@ CREATE TABLE sessions ( CREATE INDEX idx_sessions_user ON sessions(user_id); CREATE INDEX idx_sessions_expires ON sessions(expires_at); +CREATE TABLE user_totp ( + user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, + secret_enc TEXT, + pending_secret_enc TEXT, + recovery_hashes TEXT NOT NULL DEFAULT '[]', + enabled INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL +); + +CREATE TABLE mfa_challenges ( + token_hash TEXT PRIMARY KEY, + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + expires_at INTEGER NOT NULL, + attempts INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL +); + +CREATE INDEX idx_mfa_challenges_user ON mfa_challenges(user_id); +CREATE INDEX idx_mfa_challenges_expires ON mfa_challenges(expires_at); + CREATE TABLE vaults ( user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, blob TEXT NOT NULL, diff --git a/db/migrations/002_totp_mfa.sql b/db/migrations/002_totp_mfa.sql deleted file mode 100644 index 67dc61a..0000000 --- a/db/migrations/002_totp_mfa.sql +++ /dev/null @@ -1,26 +0,0 @@ --- Migration 002: optional TOTP MFA. --- --- Adds account-scoped TOTP state and short-lived login challenges. Shared --- secrets are encrypted by the application before storage; recovery codes and --- login challenge tokens are hashed at rest. - -CREATE TABLE user_totp ( - user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, - secret_enc TEXT, - pending_secret_enc TEXT, - recovery_hashes TEXT NOT NULL DEFAULT '[]', - enabled INTEGER NOT NULL DEFAULT 0, - created_at INTEGER NOT NULL, - updated_at INTEGER NOT NULL -); - -CREATE TABLE mfa_challenges ( - token_hash TEXT PRIMARY KEY, - user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, - expires_at INTEGER NOT NULL, - attempts INTEGER NOT NULL DEFAULT 0, - created_at INTEGER NOT NULL -); - -CREATE INDEX idx_mfa_challenges_user ON mfa_challenges(user_id); -CREATE INDEX idx_mfa_challenges_expires ON mfa_challenges(expires_at); From 1069e87178728c85891b1cc8fecf634ef6145c3e Mon Sep 17 00:00:00 2001 From: jagdeep sidhu Date: Sat, 25 Apr 2026 08:00:51 -0700 Subject: [PATCH 5/8] fix(auth): require password step-up for TOTP enrollment Blocks session-only MFA enrollment by requiring the current password-derived auth hash before issuing or enabling a TOTP setup secret. Made-with: Cursor --- routes/auth.js | 52 ++++++++++++++++++++++++++++++++++++++- tests/auth.routes.test.js | 40 ++++++++++++++++++++++++------ 2 files changed, 84 insertions(+), 8 deletions(-) diff --git a/routes/auth.js b/routes/auth.js index 8e8cbf0..ae8e67f 100644 --- a/routes/auth.js +++ b/routes/auth.js @@ -31,6 +31,14 @@ const TotpCodeSchema = z.object({ code: z.string().min(1).max(32), }); +const TotpSetupSchema = z.object({ + oldAuthHash: HEX_32_SCHEMA, +}); + +const TotpEnableSchema = TotpCodeSchema.extend({ + oldAuthHash: HEX_32_SCHEMA, +}); + // PR 7 — password change now rotates the vault wrap atomically. // // The client re-derives the new vaultKey from (newPassword, email, @@ -179,6 +187,26 @@ function createAuthRouter({ return { expiresAt }; } + function verifyPasswordStepUp(req, res, oldAuthHash, eventName) { + let confirmed; + try { + confirmed = users.verifyAuth(req.user.email, oldAuthHash); + } catch (err) { + if (err && err.code === 'kdf_config') { + securityLog.error(`${eventName}_kdf_config`, { req, error: err }); + res.status(503).json({ error: 'server_misconfigured' }); + return false; + } + throw err; + } + if (!confirmed) { + securityLog.warn(`${eventName}_invalid_password`, { req }); + res.status(401).json({ error: 'invalid_credentials' }); + return false; + } + return true; + } + // ------------------------------------------------------------------------- // POST /auth/register // @@ -597,6 +625,18 @@ function createAuthRouter({ csrfMw.require, (req, res) => { if (!totp) return res.status(503).json({ error: 'server_misconfigured' }); + const parsed = TotpSetupSchema.safeParse(req.body); + if (!parsed.success) return badRequest(res, 'invalid_body'); + if ( + !verifyPasswordStepUp( + req, + res, + parsed.data.oldAuthHash, + 'auth.totp_setup' + ) + ) { + return undefined; + } const out = totp.beginSetup(req.user); return res.json({ secret: out.secret, @@ -611,8 +651,18 @@ function createAuthRouter({ csrfMw.require, (req, res) => { if (!totp) return res.status(503).json({ error: 'server_misconfigured' }); - const parsed = TotpCodeSchema.safeParse(req.body); + const parsed = TotpEnableSchema.safeParse(req.body); if (!parsed.success) return badRequest(res, 'invalid_body'); + if ( + !verifyPasswordStepUp( + req, + res, + parsed.data.oldAuthHash, + 'auth.totp_enable' + ) + ) { + return undefined; + } try { const out = totp.enableSetup(req.user.id, parsed.data.code); return res.json({ diff --git a/tests/auth.routes.test.js b/tests/auth.routes.test.js index 3b6c339..324a55b 100644 --- a/tests/auth.routes.test.js +++ b/tests/auth.routes.test.js @@ -336,7 +336,7 @@ describe('auth routes', () => { const setup = await agent .post('/auth/totp/setup') .set('X-CSRF-Token', csrf) - .send({}); + .send({ oldAuthHash: SAMPLE_AUTH }); expect(setup.status).toBe(200); expect(setup.body.secret).toEqual(expect.any(String)); expect(setup.body.otpauthUrl).toMatch(/^otpauth:\/\/totp\//); @@ -344,14 +344,17 @@ describe('auth routes', () => { const bad = await agent .post('/auth/totp/enable') .set('X-CSRF-Token', csrf) - .send({ code: '000000' }); + .send({ code: '000000', oldAuthHash: SAMPLE_AUTH }); expect(bad.status).toBe(400); expect(bad.body.error).toBe('invalid_totp_code'); const good = await agent .post('/auth/totp/enable') .set('X-CSRF-Token', csrf) - .send({ code: generateTotpCode(setup.body.secret) }); + .send({ + code: generateTotpCode(setup.body.secret), + oldAuthHash: SAMPLE_AUTH, + }); expect(good.status).toBe(200); expect(good.body.status).toBe('enabled'); expect(good.body.recoveryCodes).toHaveLength(10); @@ -361,16 +364,36 @@ describe('auth routes', () => { expect(me.body.user.totpEnabled).toBe(true); }); + test('TOTP setup requires current password step-up auth', async () => { + const { agent, csrf } = await registerAndLogin(ctx); + + const missing = await agent + .post('/auth/totp/setup') + .set('X-CSRF-Token', csrf) + .send({}); + expect(missing.status).toBe(400); + + const wrong = await agent + .post('/auth/totp/setup') + .set('X-CSRF-Token', csrf) + .send({ oldAuthHash: 'deadbeef'.repeat(8) }); + expect(wrong.status).toBe(401); + expect(wrong.body.error).toBe('invalid_credentials'); + }); + test('TOTP-enabled accounts require a second step before session cookies are issued', async () => { const { agent, csrf } = await registerAndLogin(ctx); const setup = await agent .post('/auth/totp/setup') .set('X-CSRF-Token', csrf) - .send({}); + .send({ oldAuthHash: SAMPLE_AUTH }); await agent .post('/auth/totp/enable') .set('X-CSRF-Token', csrf) - .send({ code: generateTotpCode(setup.body.secret) }); + .send({ + code: generateTotpCode(setup.body.secret), + oldAuthHash: SAMPLE_AUTH, + }); await agent.post('/auth/logout').set('X-CSRF-Token', csrf); const login = await request(ctx.app) @@ -398,11 +421,14 @@ describe('auth routes', () => { const setup = await agent .post('/auth/totp/setup') .set('X-CSRF-Token', csrf) - .send({}); + .send({ oldAuthHash: SAMPLE_AUTH }); const enabled = await agent .post('/auth/totp/enable') .set('X-CSRF-Token', csrf) - .send({ code: generateTotpCode(setup.body.secret) }); + .send({ + code: generateTotpCode(setup.body.secret), + oldAuthHash: SAMPLE_AUTH, + }); await agent.post('/auth/logout').set('X-CSRF-Token', csrf); const login = await request(ctx.app) From bdb4304549d45348b27e56d4219ae29937afdc58 Mon Sep 17 00:00:00 2001 From: jagdeep sidhu Date: Sat, 25 Apr 2026 08:15:05 -0700 Subject: [PATCH 6/8] fix(auth): invalidate stale MFA login challenges Ensures each password login replaces prior MFA challenge tokens for the user so OTP attempt limits cannot be reset by minting parallel challenges. Made-with: Cursor --- lib/totp.js | 4 ++++ tests/auth.routes.test.js | 44 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/lib/totp.js b/lib/totp.js index 7d32d00..86a1935 100644 --- a/lib/totp.js +++ b/lib/totp.js @@ -212,6 +212,9 @@ function createTotpRepo(db, opts = {}) { const deleteChallenge = db.prepare( `DELETE FROM mfa_challenges WHERE token_hash = ?` ); + const deleteUserChallenges = db.prepare( + `DELETE FROM mfa_challenges WHERE user_id = ?` + ); const deleteExpiredChallenges = db.prepare( `DELETE FROM mfa_challenges WHERE expires_at < ?` ); @@ -259,6 +262,7 @@ function createTotpRepo(db, opts = {}) { function createChallenge(userId) { deleteExpiredChallenges.run(now()); + deleteUserChallenges.run(userId); const token = crypto.randomBytes(32).toString('hex'); insertChallenge.run(hashToken(token), userId, now() + CHALLENGE_TTL_MS, now()); return { diff --git a/tests/auth.routes.test.js b/tests/auth.routes.test.js index 324a55b..c1fb4f7 100644 --- a/tests/auth.routes.test.js +++ b/tests/auth.routes.test.js @@ -416,6 +416,50 @@ describe('auth routes', () => { expect(extractCookies(verify).sid).toMatch(/^[0-9a-f]{64}$/); }); + test('issuing a new TOTP challenge invalidates older challenge tokens', async () => { + const { agent, csrf } = await registerAndLogin(ctx); + const setup = await agent + .post('/auth/totp/setup') + .set('X-CSRF-Token', csrf) + .send({ oldAuthHash: SAMPLE_AUTH }); + await agent + .post('/auth/totp/enable') + .set('X-CSRF-Token', csrf) + .send({ + code: generateTotpCode(setup.body.secret), + oldAuthHash: SAMPLE_AUTH, + }); + await agent.post('/auth/logout').set('X-CSRF-Token', csrf); + + const first = await request(ctx.app) + .post('/auth/login') + .send({ email: 'user@example.com', authHash: SAMPLE_AUTH }); + const second = await request(ctx.app) + .post('/auth/login') + .send({ email: 'user@example.com', authHash: SAMPLE_AUTH }); + + expect(first.body.challengeToken).toMatch(/^[0-9a-f]{64}$/); + expect(second.body.challengeToken).toMatch(/^[0-9a-f]{64}$/); + expect(second.body.challengeToken).not.toBe(first.body.challengeToken); + + const stale = await request(ctx.app) + .post('/auth/login/totp') + .send({ + challengeToken: first.body.challengeToken, + code: generateTotpCode(setup.body.secret), + }); + expect(stale.status).toBe(401); + expect(stale.body.error).toBe('mfa_challenge_invalid'); + + const verify = await request(ctx.app) + .post('/auth/login/totp') + .send({ + challengeToken: second.body.challengeToken, + code: generateTotpCode(setup.body.secret), + }); + expect(verify.status).toBe(200); + }); + test('TOTP recovery codes are single use during login challenge verification', async () => { const { agent, csrf } = await registerAndLogin(ctx); const setup = await agent From 57747111dc06a07160d1a988c1adcf521c0a7135 Mon Sep 17 00:00:00 2001 From: jagdeep sidhu Date: Sat, 25 Apr 2026 08:30:33 -0700 Subject: [PATCH 7/8] fix(auth): harden MFA limiter and recovery consumption Keys TOTP login throttling by IP for endpoint abuse while relying on per-challenge attempt caps, and burns recovery codes only after session creation succeeds. Made-with: Cursor --- lib/totp.js | 23 +++++++++++++++++++++-- middleware/rateLimit.js | 12 +++++------- middleware/rateLimit.test.js | 14 +++++++------- routes/auth.js | 3 +++ 4 files changed, 36 insertions(+), 16 deletions(-) diff --git a/lib/totp.js b/lib/totp.js index 86a1935..4888c81 100644 --- a/lib/totp.js +++ b/lib/totp.js @@ -283,6 +283,20 @@ function createTotpRepo(db, opts = {}) { return true; } + function hasRecoveryCode(row, code) { + const normalized = normalizeRecoveryCode(code); + if (!/^[A-Z0-9]{12}$/.test(normalized)) return false; + const hashes = readRecoveryHashes(row.recoveryHashes); + const candidate = hashRecoveryCode(normalized); + return hashes.some((h) => h === candidate); + } + + function consumeUserRecoveryCode(userId, code) { + const row = selectTotp.get(userId); + if (!row || row.enabled !== 1 || !row.secretEnc) return false; + return consumeRecoveryCode(row, code); + } + function verifyChallenge({ challengeToken, code, recoveryCode }) { const tokenHash = hashToken(challengeToken); const challenge = selectChallenge.get(tokenHash); @@ -306,7 +320,7 @@ function createTotpRepo(db, opts = {}) { if (code) { ok = verifyTotp(decryptSecret(row.secretEnc), code); } else if (recoveryCode) { - ok = consumeRecoveryCode(row, recoveryCode); + ok = hasRecoveryCode(row, recoveryCode); recoveryCodeUsed = ok; } @@ -315,7 +329,11 @@ function createTotpRepo(db, opts = {}) { throw mkErr('invalid_totp_code'); } deleteChallenge.run(tokenHash); - return { userId: challenge.userId, recoveryCodeUsed }; + return { + userId: challenge.userId, + recoveryCodeUsed, + recoveryCode: recoveryCodeUsed ? recoveryCode : undefined, + }; } function verifyUserCode(userId, code) { @@ -336,6 +354,7 @@ function createTotpRepo(db, opts = {}) { getEnabledSecret, createChallenge, verifyChallenge, + consumeUserRecoveryCode, verifyUserCode, disable: disableTotp, }; diff --git a/middleware/rateLimit.js b/middleware/rateLimit.js index 98a21d4..ca4b2ff 100644 --- a/middleware/rateLimit.js +++ b/middleware/rateLimit.js @@ -1,6 +1,5 @@ const rateLimit = require('express-rate-limit'); const { ipKeyGenerator } = require('express-rate-limit'); -const crypto = require('crypto'); const { normalizeEmail } = require('../lib/email'); const securityLog = require('../lib/securityLog'); @@ -45,11 +44,7 @@ function loginKey(req) { } function mfaLoginKey(req) { - const raw = (req.body && req.body.challengeToken) || ''; - const tokenHash = /^[0-9a-fA-F]{64}$/.test(raw) - ? crypto.createHash('sha256').update(raw.toLowerCase(), 'utf8').digest('hex') - : ''; - return `login-totp|${ipBucket(req)}|${tokenHash}`; + return `login-totp|${ipBucket(req)}`; } function registerKey(req) { @@ -110,7 +105,10 @@ function loginLimiter() { function mfaLoginLimiter() { return rateLimit({ windowMs: 15 * MINUTE, - max: 5, + // Per-challenge OTP guessing is capped in the TOTP repository. This + // limiter is an endpoint DoS guard, so it must not key on the + // attacker-supplied challenge token. + max: 50, standardHeaders: true, legacyHeaders: false, keyGenerator: mfaLoginKey, diff --git a/middleware/rateLimit.test.js b/middleware/rateLimit.test.js index 7e22a53..c9d2aa3 100644 --- a/middleware/rateLimit.test.js +++ b/middleware/rateLimit.test.js @@ -52,21 +52,21 @@ describe('rate-limit key generators', () => { }); describe('mfaLoginKey', () => { - test('buckets by challenge token so shared IP users do not collide', () => { + test('ignores challenge token so invalid-token floods cannot rotate buckets', () => { const a = mfaLoginKey( mk({ challengeToken: 'a'.repeat(64) }, '1.2.3.4') ); const b = mfaLoginKey( mk({ challengeToken: 'b'.repeat(64) }, '1.2.3.4') ); - expect(a).not.toBe(b); - expect(a).toMatch(/^login-totp\|1\.2\.3\.4\|[0-9a-f]{64}$/); + expect(a).toBe(b); + expect(a).toBe('login-totp|1.2.3.4'); }); - test('does not store the raw challenge token in the limiter key', () => { - const token = 'c'.repeat(64); - const key = mfaLoginKey(mk({ challengeToken: token }, '1.2.3.4')); - expect(key).not.toContain(token); + test('distinct IPs produce distinct MFA endpoint buckets', () => { + const a = mfaLoginKey(mk({ challengeToken: 'c'.repeat(64) }, '1.2.3.4')); + const b = mfaLoginKey(mk({ challengeToken: 'c'.repeat(64) }, '5.6.7.8')); + expect(a).not.toBe(b); }); }); diff --git a/routes/auth.js b/routes/auth.js index ae8e67f..9e7f245 100644 --- a/routes/auth.js +++ b/routes/auth.js @@ -514,6 +514,9 @@ function createAuthRouter({ return res.status(401).json({ error: 'mfa_challenge_invalid' }); } const { expiresAt } = issueLoginSession(req, res, user); + if (out.recoveryCode) { + totp.consumeUserRecoveryCode(out.userId, out.recoveryCode); + } return res.json({ user: userBody(user), expiresAt, From 2f37f0d44a1468b8ec79bbd30d072fb042be3a0a Mon Sep 17 00:00:00 2001 From: jagdeep sidhu Date: Sat, 25 Apr 2026 08:41:28 -0700 Subject: [PATCH 8/8] fix(auth): consume recovery code before session issue Burns a verified recovery code after challenge/user validation but before issuing the authenticated session, so a failed consumption cannot leave a live session. Made-with: Cursor --- routes/auth.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/routes/auth.js b/routes/auth.js index 9e7f245..97ac945 100644 --- a/routes/auth.js +++ b/routes/auth.js @@ -513,10 +513,13 @@ function createAuthRouter({ if (!user || !user.emailVerified) { return res.status(401).json({ error: 'mfa_challenge_invalid' }); } - const { expiresAt } = issueLoginSession(req, res, user); if (out.recoveryCode) { - totp.consumeUserRecoveryCode(out.userId, out.recoveryCode); + const consumed = totp.consumeUserRecoveryCode(out.userId, out.recoveryCode); + if (!consumed) { + return res.status(401).json({ error: 'invalid_totp_code' }); + } } + const { expiresAt } = issueLoginSession(req, res, user); return res.json({ user: userBody(user), expiresAt,