Phase 19: Local Auth (No-OIDC Mode) #23
@@ -18,7 +18,31 @@
|
||||
* Max length: ~83 chars — fits in varchar(256) password_hash column
|
||||
*/
|
||||
|
||||
import { scryptSync, randomBytes, timingSafeEqual } from 'node:crypto';
|
||||
import { scrypt, randomBytes, timingSafeEqual } from 'node:crypto';
|
||||
import type { BinaryLike, ScryptOptions } from 'node:crypto';
|
||||
|
||||
// WR-03: use the ASYNC scrypt (libuv threadpool) so password hashing does NOT block the
|
||||
// single Node event loop. scryptSync ran on the main thread, so a burst of unauthenticated
|
||||
// POST /local/login requests (each running scrypt N=16384, ~tens of ms of CPU, including the
|
||||
// always-run dummy-hash path) could pin the loop and stall ALL other API traffic — a cheap
|
||||
// unauthenticated DoS. This async wrapper offloads the CPU to the threadpool, preserving the
|
||||
// timing-defense property while keeping the loop responsive.
|
||||
//
|
||||
// A hand-rolled Promise wrapper is used (rather than promisify) because promisify's typings
|
||||
// do not cover the options-carrying scrypt overload (N/r/p).
|
||||
function scryptAsync(
|
||||
password: BinaryLike,
|
||||
salt: BinaryLike,
|
||||
keylen: number,
|
||||
options: ScryptOptions,
|
||||
): Promise<Buffer> {
|
||||
return new Promise((resolve, reject) => {
|
||||
scrypt(password, salt, keylen, options, (err, derivedKey) => {
|
||||
if (err) reject(err);
|
||||
else resolve(derivedKey);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
// OWASP-compatible scrypt parameters for password hashing
|
||||
const SCRYPT_N = 16384; // CPU/memory cost factor (2^14)
|
||||
@@ -36,12 +60,15 @@ const KEY_LEN = 32; // 256-bit derived key output
|
||||
* the hash without relying on hardcoded constants — supports future parameter
|
||||
* migration without a DB schema change.
|
||||
*
|
||||
* NOTE: scryptSync blocks the event loop. For a 2-person household with
|
||||
* infrequent logins this is acceptable. Use promisify(scrypt) if async is needed.
|
||||
* WR-03: async — scrypt runs on the libuv threadpool, not the event loop.
|
||||
*/
|
||||
export function hashPassword(password: string): string {
|
||||
export async function hashPassword(password: string): Promise<string> {
|
||||
const salt = randomBytes(16);
|
||||
const hash = scryptSync(password, salt, KEY_LEN, { N: SCRYPT_N, r: SCRYPT_R, p: SCRYPT_P });
|
||||
const hash = await scryptAsync(password, salt, KEY_LEN, {
|
||||
N: SCRYPT_N,
|
||||
r: SCRYPT_R,
|
||||
p: SCRYPT_P,
|
||||
});
|
||||
return [
|
||||
'scrypt',
|
||||
SCRYPT_N,
|
||||
@@ -66,9 +93,12 @@ export function hashPassword(password: string): string {
|
||||
* scrypt parameter error — safe to call with untrusted input.
|
||||
* - Never logs the candidate password.
|
||||
*
|
||||
* @returns true if candidate matches the stored hash; false otherwise (incl. errors)
|
||||
* WR-03: async — scrypt runs on the libuv threadpool, not the event loop.
|
||||
*
|
||||
* @returns Promise<true> if candidate matches the stored hash; Promise<false> otherwise
|
||||
* (including all parse/format/crypto errors — never rejects).
|
||||
*/
|
||||
export function verifyPassword(storedEncoded: string, candidate: string): boolean {
|
||||
export async function verifyPassword(storedEncoded: string, candidate: string): Promise<boolean> {
|
||||
try {
|
||||
const parts = storedEncoded.split('$');
|
||||
if (parts.length !== 6) return false;
|
||||
@@ -76,7 +106,7 @@ export function verifyPassword(storedEncoded: string, candidate: string): boolea
|
||||
const salt = Buffer.from(saltB64, 'base64url');
|
||||
const storedHash = Buffer.from(hashB64, 'base64url');
|
||||
if (salt.length === 0 || storedHash.length === 0) return false;
|
||||
const candidateHash = scryptSync(candidate, salt, storedHash.length, {
|
||||
const candidateHash = await scryptAsync(candidate, salt, storedHash.length, {
|
||||
N: Number(n),
|
||||
r: Number(r),
|
||||
p: Number(p),
|
||||
|
||||
@@ -141,6 +141,10 @@ adminRouter.post(
|
||||
COLOR_PALETTE.find((c) => !usedColors.has(c)) ??
|
||||
COLOR_PALETTE[usedColors.size % COLOR_PALETTE.length];
|
||||
|
||||
// WR-03: hash the initial password BEFORE opening the transaction so the (now async,
|
||||
// threadpool) scrypt work does not hold the DB transaction open for its duration.
|
||||
const initialPasswordHash = await hashPassword(initialPassword);
|
||||
|
||||
try {
|
||||
let newUserId: number;
|
||||
|
||||
@@ -163,7 +167,7 @@ adminRouter.post(
|
||||
await tx.insert(localCredentials).values({
|
||||
userId: newUserId,
|
||||
username,
|
||||
passwordHash: hashPassword(initialPassword),
|
||||
passwordHash: initialPasswordHash,
|
||||
});
|
||||
});
|
||||
|
||||
@@ -236,7 +240,7 @@ adminRouter.post(
|
||||
try {
|
||||
await db
|
||||
.update(localCredentials)
|
||||
.set({ passwordHash: hashPassword(newPassword) })
|
||||
.set({ passwordHash: await hashPassword(newPassword) })
|
||||
.where(eq(localCredentials.userId, targetId));
|
||||
|
||||
// CR-04: an admin password reset must immediately clear any rate-limit / lockout
|
||||
|
||||
@@ -99,8 +99,10 @@ export function resetLoginAttempts(username: string): void {
|
||||
|
||||
// Pre-computed dummy hash used to run verifyPassword on unknown-username paths
|
||||
// (timing defense — prevents timing-oracle username enumeration, RESEARCH Pitfall 2).
|
||||
// Computed once at module load time; the actual value is never used for auth.
|
||||
const DUMMY_HASH = hashPassword('dummy-constant-time-filler-xyzzy');
|
||||
// WR-03: hashPassword is now async. Kick off the computation once at module load and keep
|
||||
// the PROMISE; the login handler awaits it. The value is never used for auth — only to make
|
||||
// the unknown-username path perform the same scrypt work as the known-username path.
|
||||
const dummyHashPromise: Promise<string> = hashPassword('dummy-constant-time-filler-xyzzy');
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// POST /local/login → POST /api/auth/local/login
|
||||
@@ -160,9 +162,10 @@ localAuthRouter.post('/local/login', zValidator('json', loginSchema, noEchoHook)
|
||||
// ALWAYS run verifyPassword — even for unknown usernames — to prevent timing-oracle
|
||||
// username enumeration (T-19-12 / RESEARCH Pitfall 2). Use a pre-computed dummy hash
|
||||
// so the scrypt work is always performed regardless of whether username was found.
|
||||
// WR-03: verifyPassword is async (threadpool scrypt) — await it.
|
||||
const valid = cred
|
||||
? verifyPassword(cred.passwordHash, password)
|
||||
: verifyPassword(DUMMY_HASH, password);
|
||||
? await verifyPassword(cred.passwordHash, password)
|
||||
: await verifyPassword(await dummyHashPromise, password);
|
||||
|
||||
if (!valid || !cred) {
|
||||
// Increment failure counter (keyed on username)
|
||||
|
||||
@@ -255,8 +255,8 @@ meRouter.post(
|
||||
return c.json({ error: 'No local credential found' }, 404);
|
||||
}
|
||||
|
||||
// T-19-07: verify current password before any update
|
||||
const isCorrect = verifyPassword(credRow.passwordHash, currentPassword);
|
||||
// T-19-07: verify current password before any update (WR-03: async scrypt)
|
||||
const isCorrect = await verifyPassword(credRow.passwordHash, currentPassword);
|
||||
if (!isCorrect) {
|
||||
// CR-03: return 403 (NOT 401) for a wrong current password. The PWA's global
|
||||
// MutationCache treats any 401 as "session expired" and arms the re-auth
|
||||
@@ -269,7 +269,7 @@ meRouter.post(
|
||||
try {
|
||||
await db
|
||||
.update(localCredentials)
|
||||
.set({ passwordHash: hashPassword(newPassword) })
|
||||
.set({ passwordHash: await hashPassword(newPassword) })
|
||||
.where(eq(localCredentials.userId, currentUserId));
|
||||
|
||||
return c.json({ ok: true }, 200);
|
||||
|
||||
@@ -4,6 +4,9 @@
|
||||
* Uses node:crypto scrypt under the hood; no external dependencies.
|
||||
* All tests run without MariaDB or any external service.
|
||||
*
|
||||
* WR-03: hashPassword / verifyPassword are now async (promisify(scrypt), threadpool) —
|
||||
* all assertions await them.
|
||||
*
|
||||
* Test suite (TDD RED → GREEN — Plan 19-01 Task 1):
|
||||
* Test 1: correct password verifies true
|
||||
* Test 2: wrong password verifies false
|
||||
@@ -16,33 +19,32 @@ import { describe, it, expect } from 'vitest';
|
||||
import { hashPassword, verifyPassword } from '../../src/auth/localCredentials.js';
|
||||
|
||||
describe('hashPassword / verifyPassword', () => {
|
||||
it('Test 1: verifyPassword(hashPassword(pw), pw) === true (round-trip)', () => {
|
||||
const encoded = hashPassword('hunter2');
|
||||
const result = verifyPassword(encoded, 'hunter2');
|
||||
it('Test 1: verifyPassword(hashPassword(pw), pw) === true (round-trip)', async () => {
|
||||
const encoded = await hashPassword('hunter2');
|
||||
const result = await verifyPassword(encoded, 'hunter2');
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('Test 2: verifyPassword(hashPassword(pw), wrong) === false', () => {
|
||||
const encoded = hashPassword('hunter2');
|
||||
const result = verifyPassword(encoded, 'wrong-password');
|
||||
it('Test 2: verifyPassword(hashPassword(pw), wrong) === false', async () => {
|
||||
const encoded = await hashPassword('hunter2');
|
||||
const result = await verifyPassword(encoded, 'wrong-password');
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it('Test 3: two hashPassword calls on same input produce different encoded strings (unique salt)', () => {
|
||||
const encoded1 = hashPassword('x');
|
||||
const encoded2 = hashPassword('x');
|
||||
it('Test 3: two hashPassword calls on same input produce different encoded strings (unique salt)', async () => {
|
||||
const encoded1 = await hashPassword('x');
|
||||
const encoded2 = await hashPassword('x');
|
||||
expect(encoded1).not.toBe(encoded2);
|
||||
});
|
||||
|
||||
it('Test 4: verifyPassword returns false (never throws) on a malformed stored hash', () => {
|
||||
expect(() => verifyPassword('not-a-valid-hash', 'x')).not.toThrow();
|
||||
expect(verifyPassword('not-a-valid-hash', 'x')).toBe(false);
|
||||
expect(verifyPassword('', 'x')).toBe(false);
|
||||
expect(verifyPassword('scrypt$bad$data', 'x')).toBe(false);
|
||||
it('Test 4: verifyPassword returns false (never throws) on a malformed stored hash', async () => {
|
||||
await expect(verifyPassword('not-a-valid-hash', 'x')).resolves.toBe(false);
|
||||
await expect(verifyPassword('', 'x')).resolves.toBe(false);
|
||||
await expect(verifyPassword('scrypt$bad$data', 'x')).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it('Test 5: encoded string has scrypt$N$r$p$salt$hash shape (6 $-delimited segments)', () => {
|
||||
const encoded = hashPassword('testpassword');
|
||||
it('Test 5: encoded string has scrypt$N$r$p$salt$hash shape (6 $-delimited segments)', async () => {
|
||||
const encoded = await hashPassword('testpassword');
|
||||
const segments = encoded.split('$');
|
||||
expect(segments).toHaveLength(6);
|
||||
expect(segments[0]).toBe('scrypt');
|
||||
|
||||
@@ -890,7 +890,7 @@ describe('POST /api/admin/members', () => {
|
||||
.where(eq(localCredentials.userId, body.id))
|
||||
.limit(1);
|
||||
expect(credRow).toBeDefined();
|
||||
expect(verifyPassword(credRow.passwordHash, initialPassword)).toBe(true);
|
||||
expect(await verifyPassword(credRow.passwordHash, initialPassword)).toBe(true);
|
||||
});
|
||||
|
||||
it('Test 2: duplicate username returns 409 — transaction rolls back (no orphaned users row)', async () => {
|
||||
@@ -967,8 +967,8 @@ describe('POST /api/admin/members', () => {
|
||||
.where(eq(localCredentials.userId, newMemberId))
|
||||
.limit(1);
|
||||
expect(credRow).toBeDefined();
|
||||
expect(verifyPassword(credRow.passwordHash, newPassword)).toBe(true);
|
||||
expect(verifyPassword(credRow.passwordHash, 'old-password-123')).toBe(false);
|
||||
expect(await verifyPassword(credRow.passwordHash, newPassword)).toBe(true);
|
||||
expect(await verifyPassword(credRow.passwordHash, 'old-password-123')).toBe(false);
|
||||
});
|
||||
|
||||
it('Test 4: non-admin gets 403 on POST /members and POST /members/:id/password', async () => {
|
||||
|
||||
@@ -164,7 +164,7 @@ async function getLocalCredentials() {
|
||||
describe('POST /api/auth/local/login', () => {
|
||||
it('Test 1: valid username+password → 200 { ok:true } and issueLocalSessionCookie called', async () => {
|
||||
const { hashPassword } = await getLocalCredentials();
|
||||
const hash = hashPassword('correcthorse');
|
||||
const hash = await hashPassword('correcthorse');
|
||||
mockCredRow = { userId: 5, passwordHash: hash };
|
||||
|
||||
const app = await getApp();
|
||||
@@ -179,7 +179,7 @@ describe('POST /api/auth/local/login', () => {
|
||||
|
||||
it('Test 2: wrong password → 401 { error: "Invalid credentials" }', async () => {
|
||||
const { hashPassword } = await getLocalCredentials();
|
||||
const hash = hashPassword('correcthorse');
|
||||
const hash = await hashPassword('correcthorse');
|
||||
mockCredRow = { userId: 5, passwordHash: hash };
|
||||
|
||||
const app = await getApp();
|
||||
|
||||
@@ -274,7 +274,7 @@ describe('POST /api/me/password — self-change password (AUTH-LOCAL-09)', () =>
|
||||
|
||||
const oldPassword = 'old-password-correct-123';
|
||||
const newPassword = 'new-password-secure-456';
|
||||
const storedHash = hashPassword(oldPassword);
|
||||
const storedHash = await hashPassword(oldPassword);
|
||||
let updatedHash: string | null = null;
|
||||
|
||||
// Mock sequence: resolveUserId (devBypass sets user), then:
|
||||
@@ -326,15 +326,15 @@ describe('POST /api/me/password — self-change password (AUTH-LOCAL-09)', () =>
|
||||
// The updatedHash must verify the new password
|
||||
expect(updatedHash).not.toBeNull();
|
||||
const { verifyPassword } = await import('../../src/auth/localCredentials.js');
|
||||
expect(verifyPassword(updatedHash!, newPassword)).toBe(true);
|
||||
expect(verifyPassword(updatedHash!, oldPassword)).toBe(false);
|
||||
expect(await verifyPassword(updatedHash!, newPassword)).toBe(true);
|
||||
expect(await verifyPassword(updatedHash!, oldPassword)).toBe(false);
|
||||
});
|
||||
|
||||
it('Test 2: wrong currentPassword → 403 and update is NOT called', async () => {
|
||||
const { db } = await import('../../src/db/client.js');
|
||||
|
||||
const realPassword = 'real-password-correct-789';
|
||||
const storedHash = hashPassword(realPassword);
|
||||
const storedHash = await hashPassword(realPassword);
|
||||
let updateWasCalled = false;
|
||||
|
||||
let callCount = 0;
|
||||
|
||||
Reference in New Issue
Block a user