#30: stop enrolment from killing the auth service, and from enabling MFA without recovery codes

Confirming a second factor returned "Upstream service is unreachable" - the
Next proxy's message for a backend it cannot reach. The backend was not
unreachable, it was dead: the process had just been OOM-killed.

generateRecoveryCodes hashed all ten codes with Promise.all. Argon2id here is
configured for 64 MiB per hash, so ten concurrently reserve 640 MiB against a
container limited to 512 MiB. Hashing is now sequential, peaking at 64 MiB and
taking a few seconds - the right trade for something that happens once per
enrolment. A timing-based test covers it, against the real hash rather than a
mock, because the cost being guarded is memory held concurrently and only the
real implementation reserves it.

The worse half is what the crash left behind. The enrolment row was marked
CONFIRMED before the codes were generated, so the account came out of this with
a second factor enabled, no recovery codes, and nobody having seen any: locked
behind an authenticator entry with no way back. That is how an account ends up
"assuming" it has a factor its owner cannot use.

The order is now verify, store the codes, then confirm. Anything that fails in
between leaves the enrolment pending and retryable, which is the recoverable
end of that failure.

Both bugs sat behind the one fixed in #28 - the code check failed first, so the
crash was never reached.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

services/auth/src/auth.test.ts

@@ -212,3 +212,34 @@ describe("TOTP code parsing", () => {
     }
   });
 });
+
+describe("recovery code generation stays inside the container's memory", () => {
+  // Argon2id here is configured for 64 MiB per hash. Hashing the ten codes in
+  // parallel reserved 640 MiB, over the auth container's 512 MiB limit, and the
+  // process was OOM-killed in the middle of enrolment - which reached the
+  // browser as "upstream service is unreachable" and left the account with a
+  // confirmed second factor and no recovery codes.
+  //
+  // Timed against the real hash rather than a mock: the cost here is memory
+  // held concurrently, and only the real implementation reserves it. Four codes
+  // sequentially must take roughly four times one hash; in parallel they would
+  // take about one.
+  it("hashes the codes one at a time, not all at once", { timeout: 60000 }, async () => {
+    const { hashPassword } = await import("./crypto.js");
+    const { generateRecoveryCodes } = await import("./mfaHelpers.js");
+
+    const singleStart = Date.now();
+    await hashPassword("baseline");
+    const single = Date.now() - singleStart;
+
+    const batchStart = Date.now();
+    const out = await generateRecoveryCodes(4);
+    const batch = Date.now() - batchStart;
+
+    expect(out.rawCodes).toHaveLength(4);
+    expect(out.hashedCodes).toHaveLength(4);
+    expect(out.hashedCodes.every((h) => h.startsWith("$argon2id$"))).toBe(true);
+    // Generous lower bound: parallel would land near 1x, sequential near 4x.
+    expect(batch).toBeGreaterThan(single * 2.5);
+  });
+});

services/auth/src/mfaHelpers.ts

@@ -10,7 +10,19 @@ export async function generateRecoveryCodes(count = 10): Promise<{ rawCodes: str
   for (let i = 0; i < count; i++) {
     rawCodes.push(crypto.randomBytes(6).toString("hex").substring(0, 10).toUpperCase());
   }
-  const hashedCodes = await Promise.all(rawCodes.map((c) => hashPassword(c)));
+  // Hashed one at a time, deliberately. Argon2id is configured for 64 MiB per
+  // hash, so ten of them in parallel reserve 640 MiB - more than the auth
+  // container is allowed - and the process was OOM-killed partway through
+  // enrolment. From the browser that surfaced as "upstream service is
+  // unreachable"; in the database it left an account with a confirmed second
+  // factor and no recovery codes, because the row is written before this runs.
+  //
+  // Sequentially it peaks at 64 MiB and takes a few seconds, which is the
+  // right trade for an operation that happens once per enrolment.
+  const hashedCodes: string[] = [];
+  for (const code of rawCodes) {
+    hashedCodes.push(await hashPassword(code));
+  }
   return { rawCodes, hashedCodes };
 }
 

services/auth/src/repository.ts

@@ -201,19 +201,34 @@ async function loadTotp(mailboxId: string, status: "PENDING" | "CONFIRMED"): Pro
 }
 
 /** Confirms enrollment by proving one code against the stored secret. */
-export async function confirmTotpEnrollment(mailboxId: string, code: string): Promise<boolean> {
+/**
+ * Checks a code against the pending enrolment without committing anything.
+ *
+ * Split from the commit so the caller can store the recovery codes first.
+ * Confirming and then generating them meant a failure in between - which
+ * happened, on memory - left an account with a second factor enabled and no
+ * recovery codes, and the person enrolling never saw the codes they are told
+ * to save. Leaving the enrolment pending is the recoverable end of that.
+ */
+export async function verifyPendingTotp(
+  mailboxId: string,
+  code: string,
+): Promise<{ id: string; step: number } | null> {
   const row = await loadTotp(mailboxId, "PENDING");
-  if (!row) return false;
+  if (!row) return null;
 
   const secret = decryptSecret(row.secret_enc, row.secret_nonce, row.key_version);
   const result = verifyTotpCode(secret, code, row.last_used_step === null ? null : Number(row.last_used_step));
-  if (!result.valid) return false;
+  if (!result.valid) return null;
+  return { id: row.id, step: result.step };
+}
 
+/** Marks a verified enrolment confirmed. */
+export async function markTotpConfirmed(id: string, step: number): Promise<void> {
   await query(
     `UPDATE mfa_totp SET status = 'CONFIRMED', confirmed_at = NOW(), last_used_step = $2 WHERE id = $1`,
-    [row.id, result.step],
+    [id, step],
   );
-  return true;
 }
 
 /**

services/auth/src/router.ts

@@ -422,11 +422,18 @@ async function totpEnroll(
 async function totpConfirm(req: IncomingMessage, res: ServerResponse, session: SessionTokenPayload): Promise<void> {
   const body = await parseJsonBody(req);
   const code = typeof body?.code === "string" ? body.code : "";
-  if (!(await repo.confirmTotpEnrollment(session.sub, code))) {
+  const verified = await repo.verifyPendingTotp(session.sub, code);
+  if (!verified) {
     return sendJson(res, 400, { error: "INVALID_TOTP_CODE", message: "TOTP verification failed" });
   }
+
+  // The codes are generated and stored before the factor is marked confirmed.
+  // The other order enables the factor first, so anything that goes wrong here
+  // leaves an account locked behind a second factor with no way back in.
   const recovery = await generateRecoveryCodes(10);
   await repo.storeRecoveryCodes(session.sub, recovery.hashedCodes);
+  await repo.markTotpConfirmed(verified.id, verified.step);
+
   // The raw codes are shown exactly once; only their hashes are kept.
   sendJson(res, 200, { confirmed: true, recovery_codes: recovery.rawCodes });
 }