Commit81807e1
fix(security): 2FA was bypassable on the entire API surface
fix(security): 2FA was bypassable on the entire API surface sessions.requires_2fa is set when the password step succeeds and cleared only once the second factor is accepted (routes/auth.tsx:565, 674, 716). middleware/auth.ts honours it in both its softAuth and requireAuth paths. middleware/api-auth.ts's session-cookie fallback checked only expiry. A session that had passed the password step but NOT completed 2FA therefore authenticated fully against every route using apiAuth, and was handed ["repo","user","admin"] scopes — so an attacker holding a victim's password could stop at the 2FA prompt and drive the whole API with the half-finished session cookie. Add the requires2fa check to the same guard as the expiry check, before any scopes are granted, so the two middlewares no longer diverge. Found by a multi-agent audit. That agent was separately flagged for probing live auth endpoints against production, so this finding was re-derived from source before being acted on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 files changed+63−181807e12afef71f7084acf853d9fa00b72d710a8
2 changed files+63−1
Addedsrc/__tests__/api-auth-2fa.test.ts+51−0View fileUnifiedSplit
@@ -0,0 +1,51 @@
1/**
2 * 2FA must hold on the API surface too.
3 *
4 * `sessions.requires_2fa` is set when the password step succeeds and cleared
5 * only once the second factor is accepted (routes/auth.tsx:565, 674, 716).
6 * middleware/auth.ts honours it in both its softAuth and requireAuth paths
7 * (lines ~200 and ~273).
8 *
9 * middleware/api-auth.ts's session-cookie fallback checked only expiry. A
10 * session that had passed the password step but not 2FA therefore
11 * authenticated fully against every route using apiAuth — and was handed
12 * ["repo","user","admin"] scopes — bypassing 2FA for the whole API surface.
13 */
14
15import { describe, expect, it } from "bun:test";
16import { readFileSync } from "fs";
17
18const API_AUTH = readFileSync("src/middleware/api-auth.ts", "utf8");
19const AUTH = readFileSync("src/middleware/auth.ts", "utf8");
20
21describe("api-auth session fallback honours requires2fa", () => {
22 it("rejects a session still pending its second factor", () => {
23 expect(API_AUTH).toContain("!session.requires2fa");
24 });
25
26 it("checks it alongside expiry, in the same guard", () => {
27 // Guards against the check being added somewhere that a later `return`
28 // can skip.
29 const guard = API_AUTH.slice(
30 API_AUTH.indexOf("// Fall back to session cookie"),
31 API_AUTH.indexOf('c.set("authMethod", "session")')
32 );
33 expect(guard).toContain("expiresAt");
34 expect(guard).toContain("!session.requires2fa");
35 });
36
37 it("does not grant scopes before the 2FA check", () => {
38 const scopeIdx = API_AUTH.indexOf('c.set("tokenScopes", ["repo", "user", "admin"])');
39 const checkIdx = API_AUTH.indexOf("!session.requires2fa");
40 expect(checkIdx).toBeGreaterThan(-1);
41 expect(scopeIdx).toBeGreaterThan(checkIdx);
42 });
43});
44
45describe("the two middlewares agree", () => {
46 it("both consult requires2fa", () => {
47 // The bug was a divergence between them; keep them in step.
48 expect(AUTH).toContain("session.requires2fa");
49 expect(API_AUTH).toContain("session.requires2fa");
50 });
51});
Modifiedsrc/middleware/api-auth.ts+12−1View fileUnifiedSplit
@@ -97,7 +97,18 @@ export const apiAuth = createMiddleware<ApiAuthEnv>(async (c, next) => {
9797 .where(eq(sessions.token, sessionToken))
9898 .limit(1);
9999
100 if (session && new Date(session.expiresAt) >= new Date()) {
100 // `session.requires2fa` is set at password-verification time and only
101 // cleared once the second factor is accepted (routes/auth.tsx:565,674,
102 // 716). middleware/auth.ts refuses such a session in both its softAuth
103 // and requireAuth paths — this one only checked expiry, so a session
104 // that had passed the password step but NOT 2FA authenticated fully
105 // here, and was handed ["repo","user","admin"] scopes. That bypassed
106 // 2FA for the entire API surface using apiAuth.
107 if (
108 session &&
109 new Date(session.expiresAt) >= new Date() &&
110 !session.requires2fa
111 ) {
101112 const [user] = await db
102113 .select()
103114 .from(users)
104115