Skip to content

Commit 3eff43d

Browse files
Merge pull request #118 from CodeForPhilly/feat/login-migration-phase-a
feat(auth): three-algorithm password verifier + rehash infrastructure
2 parents 50a6b94 + 8922b4b commit 3eff43d

11 files changed

Lines changed: 610 additions & 39 deletions

File tree

apps/api/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
"@fastify/static": "^9.1.3",
3030
"@fastify/swagger": "^9.7.0",
3131
"@fastify/swagger-ui": "^5.2.6",
32+
"argon2": "^0.44.0",
3233
"bcryptjs": "^3.0.3",
3334
"better-sqlite3": "^12.10.0",
3435
"fastify": "^5.8.5",

apps/api/src/auth/argon2-params.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* Argon2id parameter source-of-truth.
3+
*
4+
* Bumping these values is a deliberate spec event per
5+
* specs/behaviors/password-hash-rotation.md — every subsequent
6+
* successful login rehashes credentials to the new floor, and the
7+
* corpus drifts naturally without retroactive backfill.
8+
*
9+
* Starting values produce ~50 ms hashes on the production pod's CPU
10+
* profile (Linode amd64). Within budget for POST /api/auth/login
11+
* latency.
12+
*/
13+
import argon2 from 'argon2';
14+
15+
export const ARGON2_PARAMS = {
16+
type: argon2.argon2id,
17+
memoryCost: 19_456, // 19 MiB
18+
timeCost: 2, // iterations
19+
parallelism: 1,
20+
} as const;
Lines changed: 128 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,139 @@
11
/**
2-
* Legacy laddr password verification.
2+
* Legacy password verification + rehash-on-login.
33
*
4-
* Dispatches by hash format prefix so we can support whatever the laddr import
5-
* lands on. v1 supports bcrypt (`$2a$`, `$2b$`, `$2y$`). Other formats throw
6-
* UnknownHashFormatError so callers can surface a uniform "credentials invalid"
7-
* response (rather than leaking algorithm details) while still logging the
8-
* mismatch internally.
4+
* Per specs/behaviors/password-hash-rotation.md. Dispatches by hash
5+
* format (SHA-1 unsalted, bcrypt, or argon2id), verifies in constant
6+
* time, and reports `needsRehash` so callers can rotate the
7+
* credential to argon2id on successful login. The corpus drifts
8+
* toward argon2id without forcing a password reset.
9+
*
10+
* Failure modes — wrong password, missing credential, unknown
11+
* format, internal error — collapse to `{ valid: false }` so callers
12+
* can return a uniform `invalid_credentials` response without
13+
* leaking algorithm or user-existence signal.
914
*/
15+
import { createHash, timingSafeEqual } from 'node:crypto';
16+
17+
import argon2 from 'argon2';
1018
import bcrypt from 'bcryptjs';
1119

12-
export class UnknownHashFormatError extends Error {
13-
constructor(prefix: string) {
14-
super(`Unknown legacy password hash format: ${prefix}`);
15-
this.name = 'UnknownHashFormatError';
20+
import { ARGON2_PARAMS } from './argon2-params.js';
21+
22+
const BCRYPT_PREFIXES = ['$2a$', '$2b$', '$2y$'];
23+
const ARGON2ID_PREFIX = '$argon2id$';
24+
const SHA1_HEX_RE = /^[0-9a-f]{40}$/;
25+
26+
export type VerifyResult =
27+
| { valid: true; needsRehash: boolean }
28+
| { valid: false };
29+
30+
/**
31+
* Verify a plaintext password against a stored hash.
32+
*
33+
* Returns `{ valid: true, needsRehash }` on a successful match; the
34+
* caller is responsible for rotating to argon2id when `needsRehash`
35+
* is true.
36+
*
37+
* Any failure path — wrong password, unrecognized format, internal
38+
* error during verify — returns `{ valid: false }` without leaking
39+
* the cause. Internal errors are not re-thrown.
40+
*/
41+
export async function verifyLegacyPassword(
42+
password: string,
43+
hash: string,
44+
): Promise<VerifyResult> {
45+
try {
46+
if (hash.startsWith(ARGON2ID_PREFIX)) {
47+
const ok = await argon2.verify(hash, password);
48+
if (!ok) return { valid: false };
49+
// Argon2's encoded format embeds the params; if they don't match
50+
// the current floor, the credential is correct but stale —
51+
// rotate.
52+
return { valid: true, needsRehash: argonNeedsRehash(hash) };
53+
}
54+
55+
if (BCRYPT_PREFIXES.some((p) => hash.startsWith(p))) {
56+
const ok = await bcrypt.compare(password, hash);
57+
if (!ok) return { valid: false };
58+
// Spec unifies on argon2id; every bcrypt source rotates on login.
59+
// (Laddr is SHA-1, not bcrypt; the bcrypt branch is defensive
60+
// fallback for any future bcrypt-imported credentials.)
61+
return { valid: true, needsRehash: true };
62+
}
63+
64+
if (SHA1_HEX_RE.test(hash)) {
65+
const computedHex = createHash('sha1').update(password).digest('hex');
66+
// Length-check before timingSafeEqual — equal lengths are
67+
// guaranteed by the regex on `hash` plus sha1's fixed 40-char
68+
// output, but defense in depth: timingSafeEqual throws
69+
// synchronously on length mismatch, which would itself be a
70+
// timing oracle if the throw vs. compare paths differ.
71+
if (computedHex.length !== hash.length) return { valid: false };
72+
const ok = timingSafeEqual(
73+
Buffer.from(computedHex, 'hex'),
74+
Buffer.from(hash, 'hex'),
75+
);
76+
if (!ok) return { valid: false };
77+
return { valid: true, needsRehash: true };
78+
}
79+
80+
// Unknown format. No throw — uniform invalid response.
81+
return { valid: false };
82+
} catch {
83+
// Library error, malformed hash that slipped past the regex,
84+
// anything else — collapse to invalid. Never leak via different
85+
// outcomes.
86+
return { valid: false };
1687
}
1788
}
1889

19-
const BCRYPT_PREFIXES = ['$2a$', '$2b$', '$2y$'];
90+
/**
91+
* Hash a plaintext password to argon2id with current params. Used
92+
* after a successful verify when `needsRehash` is true, and by the
93+
* password-reset confirm flow (phase C).
94+
*/
95+
export async function rehashPassword(password: string): Promise<string> {
96+
return argon2.hash(password, ARGON2_PARAMS);
97+
}
98+
99+
/**
100+
* Returns true when the supplied argon2id-encoded hash uses
101+
* parameters different from the current floor (`ARGON2_PARAMS`).
102+
*/
103+
function argonNeedsRehash(hash: string): boolean {
104+
try {
105+
return argon2.needsRehash(hash, ARGON2_PARAMS);
106+
} catch {
107+
// If parsing the encoded hash fails, conservatively rotate.
108+
return true;
109+
}
110+
}
20111

21-
export async function verifyLaddrPassword(password: string, hash: string): Promise<boolean> {
22-
if (BCRYPT_PREFIXES.some((p) => hash.startsWith(p))) {
23-
return bcrypt.compare(password, hash);
112+
/**
113+
* Pre-computed argon2id hash of a fixed sentinel plaintext. Computed
114+
* lazily on first `dummyVerify` so we don't block module load.
115+
*/
116+
let dummyHashPromise: Promise<string> | null = null;
117+
function ensureDummyHash(): Promise<string> {
118+
if (!dummyHashPromise) {
119+
dummyHashPromise = rehashPassword('this-string-is-never-a-real-password');
120+
}
121+
return dummyHashPromise;
122+
}
123+
124+
/**
125+
* Run an argon2 verify against a fixed sentinel hash. Always returns
126+
* `{ valid: false }`. Callers invoke this when the user or credential
127+
* lookup misses so the overall response timing matches the success
128+
* path — per specs/behaviors/password-hash-rotation.md
129+
* § anti-enumeration timing.
130+
*/
131+
export async function dummyVerify(): Promise<VerifyResult> {
132+
try {
133+
const dummy = await ensureDummyHash();
134+
await argon2.verify(dummy, 'this-string-also-never-a-real-password');
135+
} catch {
136+
// Outcome doesn't matter — purpose is timing, not correctness.
24137
}
25-
throw new UnknownHashFormatError(hash.slice(0, Math.min(4, hash.length)));
138+
return { valid: false };
26139
}

apps/api/src/routes/account-claim.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -266,14 +266,10 @@ export async function accountClaimRoutes(fastify: FastifyInstance): Promise<void
266266

267267
const outcome = result.value;
268268
if (!outcome.ok) {
269-
if (outcome.reason === 'unknown_format') {
270-
// Internal log — uniform user-facing response per anti-enumeration.
271-
request.log.warn(
272-
{ slug, reason: 'unknown_format' },
273-
'legacy password hash format unknown',
274-
);
275-
}
276-
// Uniform 401 for any failure
269+
// Uniform 401 for any failure — verifier collapses
270+
// wrong-password / unknown-format / internal-error into a
271+
// single `wrong_password` reason per
272+
// specs/behaviors/password-hash-rotation.md.
277273
throw new UnauthenticatedError(
278274
'Invalid credentials',
279275
'claim_credentials_invalid',

apps/api/src/services/account-claim.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import type {
3838
ClaimPendingClaims,
3939
GhIdentitySnapshot,
4040
} from '../auth/jwt.js';
41-
import { verifyLaddrPassword, UnknownHashFormatError } from '../auth/legacy-password.js';
41+
import { verifyLegacyPassword } from '../auth/legacy-password.js';
4242
import { GitHubAccountService } from './github-account.js';
4343
import type { ResolvedGitHubIdentity } from '../auth/github-client.js';
4444

@@ -253,7 +253,7 @@ export class AccountClaimService {
253253
password: string,
254254
): Promise<
255255
| { ok: true; result: AutoClaimResult }
256-
| { ok: false; code: 'invalid'; reason: 'no_slug' | 'already_claimed' | 'no_credential' | 'wrong_password' | 'unknown_format' }
256+
| { ok: false; code: 'invalid'; reason: 'no_slug' | 'already_claimed' | 'no_credential' | 'wrong_password' }
257257
> {
258258
const personId = this.#state.personIdBySlug.get(slug.toLowerCase());
259259
if (!personId) {
@@ -272,16 +272,13 @@ export class AccountClaimService {
272272
return { ok: false, code: 'invalid', reason: 'no_credential' };
273273
}
274274

275-
let matched: boolean;
276-
try {
277-
matched = await verifyLaddrPassword(password, cred.passwordHash);
278-
} catch (err) {
279-
if (err instanceof UnknownHashFormatError) {
280-
return { ok: false, code: 'invalid', reason: 'unknown_format' };
281-
}
282-
throw err;
283-
}
284-
if (!matched) {
275+
// verifyLegacyPassword collapses wrong-password, unknown-format,
276+
// and internal errors into a uniform `{ valid: false }` per
277+
// specs/behaviors/password-hash-rotation.md — the byPassword path
278+
// surfaces this as `wrong_password` and the route translates to
279+
// the uniform `claim_credentials_invalid` user-facing response.
280+
const verifyResult = await verifyLegacyPassword(password, cred.passwordHash);
281+
if (!verifyResult.valid) {
285282
return { ok: false, code: 'invalid', reason: 'wrong_password' };
286283
}
287284

apps/api/tests/account-claim.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,7 @@ describe('POST /api/account-claim/by-password', () => {
371371
let app: FastifyInstance;
372372
const candidateId = '01951a3c-0000-7000-8000-0000ddddddd1';
373373
const linkedId = '01951a3c-0000-7000-8000-0000ddddddd2';
374+
const sha1CandidateId = '01951a3c-0000-7000-8000-0000ddddddd3';
374375
const correctPassword = 'hunter2-correct';
375376

376377
beforeAll(async () => {
@@ -389,6 +390,15 @@ describe('POST /api/account-claim/by-password', () => {
389390
});
390391
await seedPrivateProfile(privateStore.path, linkedId, 'linked@example.com');
391392
await seedLegacyPassword(privateStore.path, linkedId, hash);
393+
// Legacy-laddr candidate with an unsalted SHA-1 hash (the actual
394+
// production format per emergence-skeleton User.class.php:33). The
395+
// new verifier accepts this path; pre-rewrite the verifier only
396+
// knew bcrypt.
397+
await seedPerson(dataRepo.path, 'sha1-user', sha1CandidateId);
398+
await seedPrivateProfile(privateStore.path, sha1CandidateId, 'sha1-user@example.com');
399+
const { createHash } = await import('node:crypto');
400+
const sha1Hash = createHash('sha1').update(correctPassword).digest('hex');
401+
await seedLegacyPassword(privateStore.path, sha1CandidateId, sha1Hash);
392402

393403
app = await buildTestApp(dataRepo.path, privateStore.path);
394404
}, 60_000);
@@ -417,6 +427,27 @@ describe('POST /api/account-claim/by-password', () => {
417427
expect(cred).toBeNull();
418428
});
419429

430+
it('claims on correct password against a SHA-1 hash (legacy laddr format)', async () => {
431+
const token = await mintClaim('3010', 'gh-sha1-user', ['gh-sha1-fresh@example.com'], []);
432+
const res = await app.inject({
433+
method: 'POST',
434+
url: '/api/account-claim/by-password',
435+
remoteAddress: nextTestIp(),
436+
cookies: { cfp_claim: token },
437+
payload: { slug: 'sha1-user', password: correctPassword },
438+
});
439+
expect(res.statusCode).toBe(200);
440+
441+
const person = app.inMemoryState.people.get(sha1CandidateId);
442+
expect(person?.githubUserId).toBe(3010);
443+
444+
// Credential deleted on successful claim (per byPassword semantics —
445+
// the claim path still removes the credential since the user is now
446+
// GitHub-linked. Rehash-on-keep is a phase-B concern.)
447+
const cred = await app.store.private.getLegacyPassword(sha1CandidateId);
448+
expect(cred).toBeNull();
449+
});
450+
420451
it('returns uniform 401 claim_credentials_invalid for unknown slug', async () => {
421452
const token = await mintClaim('3002', 'gh-nothing-user', ['none@example.com'], []);
422453
const res = await app.inject({

0 commit comments

Comments
 (0)