fix(auth): handle requires_webauthn in POST /login — was a 500 self-lockout
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 <noreply@anthropic.com>
This commit is contained in:
parent
d9545d6fd5
commit
3c82c30f3c
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
});
|
||||
Loading…
Reference in New Issue