From 3c82c30f3c7fad012cffe82cf2e0fe8136ab7697 Mon Sep 17 00:00:00 2001 From: null Date: Fri, 10 Jul 2026 16:29:08 -0500 Subject: [PATCH] =?UTF-8?q?fix(auth):=20handle=20requires=5Fwebauthn=20in?= =?UTF-8?q?=20POST=20/login=20=E2=80=94=20was=20a=20500=20self-lockout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit authService.login returns a requires_webauthn payload for webauthn-enabled users, but the login route only handled requires_totp and fell through to result.user.id (undefined) -> TypeError -> 500 on every attempt. Any user who enabled WebAuthn via the API (reachable today) was permanently locked out. - routes/auth.cts: return the two-step requires_webauthn payload (mirrors TOTP) - services/authService.cts: stale-flag guard — webauthn_enabled with no usable credentials falls through to password login instead of hard-failing - tests/authLoginWebauthnRoute.test.js: pins both paths + wrong-password 401 (QA-B1-01, lockout part) Co-Authored-By: Claude Fable 5 --- routes/auth.cts | 10 +++ services/authService.cts | 14 ++- tests/authLoginWebauthnRoute.test.js | 129 +++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 3 deletions(-) create mode 100644 tests/authLoginWebauthnRoute.test.js diff --git a/routes/auth.cts b/routes/auth.cts index 0bc32e0..64a825f 100644 --- a/routes/auth.cts +++ b/routes/auth.cts @@ -99,6 +99,16 @@ router.post( return res.json({ requires_totp: true, challenge_token: result.challenge_token }); } + // WebAuthn required — same two-step shape as TOTP; the session is only + // created after POST /webauthn/challenge verifies the key. + if (result.requires_webauthn) { + return res.json({ + requires_webauthn: true, + challenge_token: result.challenge_token, + webauthn_options: result.webauthn_options, + }); + } + logAudit({ user_id: result.user.id, action: 'login.success', diff --git a/services/authService.cts b/services/authService.cts index e2c3a87..7ed27f0 100644 --- a/services/authService.cts +++ b/services/authService.cts @@ -89,9 +89,17 @@ async function login(username: any, password: any) { createAuthenticationChallenge, createLoginChallenge, } = require('./webauthnService.cts'); - const { options, challengeId } = await createAuthenticationChallenge(getDb(), user.id); - const loginToken = createLoginChallenge(getDb(), user.id, challengeId); - return { requires_webauthn: true, challenge_token: loginToken, webauthn_options: options }; + try { + const { options, challengeId } = await createAuthenticationChallenge(getDb(), user.id); + const loginToken = createLoginChallenge(getDb(), user.id, challengeId); + return { requires_webauthn: true, challenge_token: loginToken, webauthn_options: options }; + } catch (err: any) { + // Stale flag (enabled but no usable credentials): a hard failure here would + // lock the account out entirely — fall through to password-only login instead. + log.warn( + `[auth] WebAuthn enabled for user ${user.id} but challenge creation failed (${err.message}); proceeding with password login`, + ); + } } // Clean up expired sessions for this user before creating new session diff --git a/tests/authLoginWebauthnRoute.test.js b/tests/authLoginWebauthnRoute.test.js new file mode 100644 index 0000000..dbbfd18 --- /dev/null +++ b/tests/authLoginWebauthnRoute.test.js @@ -0,0 +1,129 @@ +'use strict'; + +// QA-B1-01 — POST /login for a webauthn-enabled user must return the +// requires_webauthn two-step payload (not fall through and 500), and a user +// whose webauthn_enabled flag is stale (no credential rows) must still be able +// to log in with their password instead of being locked out. +const test = require('node:test'); +const assert = require('node:assert/strict'); +const os = require('node:os'); +const path = require('node:path'); +const fs = require('node:fs'); + +const dbPath = path.join(os.tmpdir(), `bill-tracker-webauthn-login-${process.pid}.sqlite`); +process.env.DB_PATH = dbPath; + +const { getDb, closeDb } = require('../db/database.cts'); +const { hashPassword } = require('../services/authService.cts'); +const router = require('../routes/auth.cts'); + +const PASSWORD = 'correct-horse-battery'; + +function handler(method, routePath) { + const layer = router.stack.find((l) => l.route?.path === routePath && l.route.methods[method]); + assert.ok(layer, `${method.toUpperCase()} ${routePath} exists`); + return layer.route.stack[layer.route.stack.length - 1].handle; +} + +function callLogin(body) { + const h = handler('post', '/login'); + return new Promise((resolve, reject) => { + const req = { + body, + ip: '127.0.0.1', + cookies: {}, + headers: {}, + secure: false, + get: () => 'node-test-agent', + }; + const cookies = []; + const res = { + statusCode: 200, + status(code) { + this.statusCode = code; + return this; + }, + cookie(name, value, opts) { + cookies.push({ name, value, opts }); + }, + json(data) { + resolve({ status: this.statusCode, body: data, cookies }); + }, + }; + Promise.resolve(h(req, res)).catch(reject); + }); +} + +let webauthnUserId; +let staleFlagUserId; + +test.before(async () => { + const db = getDb(); + const hash = await hashPassword(PASSWORD); + + const u1 = db + .prepare( + "INSERT INTO users (username, password_hash, role, webauthn_enabled) VALUES (?, ?, 'user', 1)", + ) + .run('webauthn-user', hash); + webauthnUserId = u1.lastInsertRowid; + db.prepare( + 'INSERT INTO webauthn_credentials (user_id, credential_id, public_key, sign_count, transports) VALUES (?, ?, ?, 0, ?)', + ).run(webauthnUserId, 'dGVzdC1jcmVkLWlk', 'dGVzdC1wdWJsaWMta2V5', JSON.stringify(['internal'])); + + const u2 = db + .prepare( + "INSERT INTO users (username, password_hash, role, webauthn_enabled) VALUES (?, ?, 'user', 1)", + ) + .run('stale-flag-user', hash); + staleFlagUserId = u2.lastInsertRowid; +}); + +test.after(() => { + closeDb(); + for (const suffix of ['', '-shm', '-wal']) { + try { + fs.unlinkSync(dbPath + suffix); + } catch {} + } +}); + +test('webauthn-enabled user gets the two-step payload, not a 500', async () => { + const { status, body, cookies } = await callLogin({ + username: 'webauthn-user', + password: PASSWORD, + }); + assert.equal(status, 200); + assert.equal(body.requires_webauthn, true); + assert.equal(typeof body.challenge_token, 'string'); + assert.ok(body.challenge_token.length > 0); + assert.ok(body.webauthn_options, 'webauthn_options present'); + assert.equal(typeof body.webauthn_options.challenge, 'string'); + assert.equal(cookies.length, 0, 'no session cookie before the key verifies'); + assert.equal(body.user, undefined, 'no user payload before the key verifies'); + + const challenge = getDb() + .prepare( + "SELECT id FROM webauthn_challenges WHERE user_id = ? AND challenge_type = 'authentication'", + ) + .get(webauthnUserId); + assert.ok(challenge, 'authentication challenge persisted'); +}); + +test('stale webauthn flag (no credentials) falls through to password login, not lockout', async () => { + const { status, body, cookies } = await callLogin({ + username: 'stale-flag-user', + password: PASSWORD, + }); + assert.equal(status, 200); + assert.equal(body.requires_webauthn, undefined); + assert.ok(body.user, 'password login succeeded despite the stale flag'); + assert.equal(body.user.id, staleFlagUserId); + assert.equal(cookies.length, 1, 'session cookie issued'); +}); + +test('wrong password still 401s for a webauthn-enabled user', async () => { + const { status, body } = await callLogin({ username: 'webauthn-user', password: 'nope-wrong' }); + assert.equal(status, 401); + assert.equal(body.code, 'AUTH_ERROR'); +});