fix: 2FA single-use consume is DB-atomic; login lockout escalates 15/30/60m with uniform 401

F5.5 brute-force hardening:
- internal/twofa.Check consume is now a conditional UPDATE (WHERE id AND
  two_factor_pending_code_hash) reporting rows affected: two concurrent
  verifications of the same code on different instances both match the digest,
  but only the first conditional UPDATE can affect a row — the loser sees 0
  rows and fails MissingOrExpired, so one code authorizes exactly ONE operation
  across instances (the per-user mutex only serialized within one process).
- /login lockout is now indistinguishable from a wrong password: a locked
  account returns the same uniform 401 'invalid credentials' and burns the same
  constant-time bcrypt compare (via the shared semaphore), removing the
  account-existence oracle and lockout-probing signal of the old 429.
- Lockout tiers escalate 15m (5+) / 30m (7+) / 60m (10+): an attacker who keeps
  guessing past each unlock makes the lock LONGER, raising the repeat-DoS
  effort while the response stays uniform.

backend/handlers/auth/auth_test.go

@@ -1853,7 +1853,11 @@ func TestValidateUKPhoneNumber_RejectsMixedInjectionPayloads(t *testing.T) {
 // =============================================================================
 
 // TestLogin_AccountLockout_After5Failures verifies that after 5 failed login
-// attempts, the account is locked and the next login attempt returns HTTP 429.
+// attempts, the account is locked and the next login attempt returns HTTP 401 —
+// INDISTINGUISHABLE from a wrong password (F5.5): the old 429 lockout response
+// revealed account existence and lockout state, so an attacker could probe
+// whether their lockout DoS of a victim account was in effect. The DB-side
+// lockout state (failed_attempts >= 5, locked_until set) is unchanged.
 func TestLogin_AccountLockout_After5Failures(t *testing.T) {
 	ctx, tx := resetTestData(t)
 
@@ -1882,8 +1886,8 @@ func TestLogin_AccountLockout_After5Failures(t *testing.T) {
 		Password: "wrongpassword",
 	}
 	w := testutils.MakeRequestNoAuth(handler, "POST", "/api/login", body, ctx)
-	if w.Code != http.StatusTooManyRequests {
+	if w.Code != http.StatusUnauthorized {
-		t.Errorf("expected 429 after 5 failures, got %d. body: %s", w.Code, w.Body.String())
+		t.Errorf("expected 401 on a locked account (F5.5 — indistinguishable from a wrong password), got %d. body: %s", w.Code, w.Body.String())
 	}
 
 	var failedAttempts int
@@ -1945,11 +1949,12 @@ func TestLogin_AccountLockout_ResetsOnSuccess(t *testing.T) {
 	}
 }
 
-// TestLogin_AccountLockout_CappedAt30Min verifies the LOW-6 fix: the lockout
+// TestLogin_AccountLockout_CappedAt60Min verifies the LOW-6 follow-up: the
-// ceiling never exceeds 30 minutes no matter how many failed attempts pile up
+// escalating lockout ceiling never exceeds 60 minutes no matter how many failed
-// (previously 20+ failures locked the account for 2 hours — a persistent,
+// attempts pile up (the 15/30/60-minute tiers replaced the old flat 30-minute
+// cap; the earliest pre-cap lockout locked 20+ failures for 2 hours — a
 // repeatedly-extendable DoS window for a guessing attacker).
-func TestLogin_AccountLockout_CappedAt30Min(t *testing.T) {
+func TestLogin_AccountLockout_CappedAt60Min(t *testing.T) {
 	ctx, tx := resetTestData(t)
 
 	handler := http.HandlerFunc(LoginHandler)
@@ -1961,7 +1966,8 @@ func TestLogin_AccountLockout_CappedAt30Min(t *testing.T) {
 	defer fixtures.DeleteUser(tx, userID)
 	defer tx.Exec(ctx, "UPDATE users SET failed_attempts = 0, locked_until = NULL WHERE id = $1", userID)
 
-	// Simulate 20 prior failures — the old CASE locked for 2 hours here.
+	// Simulate 20 prior failures — the escalation CASE (15/30/60min tiers)
+	// caps the lock here instead of growing without bound.
 	_, err = tx.Exec(ctx, "UPDATE users SET failed_attempts = 20 WHERE id = $1", userID)
 	if err != nil {
 		t.Fatalf("failed to seed failed_attempts: %v", err)
@@ -1976,10 +1982,11 @@ func TestLogin_AccountLockout_CappedAt30Min(t *testing.T) {
 	if w.Code != http.StatusUnauthorized {
 		t.Fatalf("expected 401 on the lock-setting attempt, got %d. body: %s", w.Code, w.Body.String())
 	}
-	// Second attempt hits the active lock → 429.
+	// Second attempt hits the active lock → 401 (F5.5: indistinguishable from
+	// a wrong password; the old 429 leaked the lockout state).
 	w = testutils.MakeRequestNoAuth(handler, "POST", "/api/login", body, ctx)
-	if w.Code != http.StatusTooManyRequests {
+	if w.Code != http.StatusUnauthorized {
-		t.Fatalf("expected 429, got %d. body: %s", w.Code, w.Body.String())
+		t.Fatalf("expected 401 on the active lock, got %d. body: %s", w.Code, w.Body.String())
 	}
 
 	var lockedUntil *time.Time
@@ -1991,8 +1998,8 @@ func TestLogin_AccountLockout_CappedAt30Min(t *testing.T) {
 	if lockedUntil == nil {
 		t.Fatal("expected locked_until to be set")
 	}
-	if until := lockedUntil.Sub(clock.Now()); until > 30*time.Minute {
+	if until := lockedUntil.Sub(clock.Now()); until > 60*time.Minute {
-		t.Errorf("lockout must never exceed the 30-minute ceiling, got %v", until)
+		t.Errorf("lockout must never exceed the 60-minute ceiling, got %v", until)
 	}
 }
 

backend/handlers/auth/local.go

@@ -480,13 +480,31 @@ func LoginHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Check if account is locked
+	// Check if account is locked (F5.5): the response MUST be indistinguishable
+	// from a generic invalid-credentials failure — same status, same body, and
+	// the same constant-time bcrypt work — so an attacker can never tell
+	// "locked" from "wrong password". A distinguishable lockout (the old 429
+	// "account is temporarily locked") is an account-existence oracle AND a
+	// lockout-probing signal: an attacker burning 5 wrong passwords to DoS a
+	// victim could then watch the victim's lockout state flip. The lockout
+	// itself is inherent to the 5-attempt progressive policy; hiding the state
+	// is what removes the oracle. The audit line stays server-side only.
+	// A locked-out user's recovery is the backend-only password-reset flow (see
+	// the success-path TODO below) or an operator clearing the columns at the DB.
 	var failedAttempts int
 	var lockedUntil *time.Time
 	err = db.Conn.QueryRow(r.Context(), `SELECT failed_attempts, locked_until FROM users WHERE id = $1`, userID).Scan(&failedAttempts, &lockedUntil)
 	if err == nil && lockedUntil != nil && clock.Now().Before(*lockedUntil) {
-		http.Error(w, "account is temporarily locked. try again later.", http.StatusTooManyRequests)
+		// Burn the same constant-time bcrypt compare a real login would, so
+		// the locked path's latency cannot distinguish it either. The compare
+		// result is discarded: a locked account stays locked (and the result
+		// could match if the attacker guessed the password — still no login).
+		if release, ok := acquireBcryptSlot(); ok {
+			_ = bcrypt.CompareHashAndPassword([]byte(passwordHash), []byte(req.Password))
+			release()
+		}
 		log.Printf("LOGIN_AUDIT: locked account attempt - user=%s ip=%s", userID, middleware.GetClientIP(r.Context()))
+		http.Error(w, "invalid credentials", http.StatusUnauthorized)
 		return
 	}
 
@@ -554,6 +572,7 @@ func LoginHandler(w http.ResponseWriter, r *http.Request) {
 			SET failed_attempts = failed_attempts + 1,
 			    locked_until = CASE 
 				    WHEN failed_attempts + 1 >= 5 THEN NOW() + (CASE 
+					    WHEN failed_attempts + 1 >= 10 THEN INTERVAL '60 minutes'
 					    WHEN failed_attempts + 1 >= 7 THEN INTERVAL '30 minutes'
 					    ELSE INTERVAL '15 minutes'
 				    END)
@@ -588,14 +607,19 @@ func LoginHandler(w http.ResponseWriter, r *http.Request) {
 	// LOW 6 documented gap (finding 6): there is NO password-reset UI — the
 	// backend-only reset flow (GenerateVerificationCodeHandler/VerifyCodeHandler)
 	// has no frontend link, so a user locked out by a guessing attacker has no
-	// self-service recovery until locked_until lapses (capped at 30 minutes —
+	// self-service recovery until locked_until lapses (15min at 5+ failures,
-	// see the failure path above); the operator can only intervene at the DB.
+	// 30min at 7+, 60min at 10+ — see the failure path above); the operator can
-	// The lockout counter stays keyed per-user (not per-(user,IP)) because this
+	// only intervene at the DB. The escalating ceiling is the repeat-DoS
-	// codebase deliberately rejects IP-in-the-key for account-level budgets (see
+	// mitigation: an attacker who keeps guessing past each unlock makes the lock
-	// the 2FA limiter note in main.go, B8): a client that rotates its source IP
+	// LONGER (up to 60 minutes) instead of merely sustaining the 15-minute tier,
-	// would mint a fresh bucket per IP and collapse the per-account budget. The
+	// raising the effort-per-DoS ratio while the response stays the uniform 401
-	// 30-minute ceiling is the bounded-DoS compromise; successful 2FA verifies
+	// (never distinguishable from a wrong password). The lockout counter stays
-	// also clear the lockout (internal/twofa.Check).
+	// keyed per-user (not per-(user,IP)) because this codebase deliberately
+	// rejects IP-in-the-key for account-level budgets (see the 2FA limiter note
+	// in main.go, B8): a client that rotates its source IP would mint a fresh
+	// bucket per IP and collapse the per-account budget. The 60-minute ceiling
+	// is the bounded-DoS compromise; successful 2FA verifies also clear the
+	// lockout (internal/twofa.Check).
 	tx, err := db.Conn.Begin(r.Context())
 	if err != nil {
 		log.Printf("Failed to begin transaction: %v", err)

backend/handlers/auth/lockout_test.go

@@ -0,0 +1,111 @@
+//go:build test
+
+package auth
+
+// F5.5 regression: a locked account must respond EXACTLY like a generic
+// invalid-credentials failure — same status, same body — so an attacker can
+// never distinguish "locked" from "wrong password". The old distinguishable
+// 429 lockout was an account-existence oracle and a lockout-probing signal.
+
+import (
+	"net/http"
+	"testing"
+
+	"crussell/testutils"
+	"crussell/testutils/fixtures"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestLogin_LockedAccount_IndistinguishableFromWrongPassword pins the F5.5
+// fix: a login attempt against a locked account returns the SAME status and
+// byte-identical body as a wrong-password attempt against a normal account.
+// Before the fix the locked account answered 429 "account is temporarily
+// locked..." — distinguishable from the 401 "invalid credentials" a wrong
+// password returns, revealing account existence and lockout state.
+func TestLogin_LockedAccount_IndistinguishableFromWrongPassword(t *testing.T) {
+	ctx, tx := testutils.SetupTestTx(t)
+	handler := http.HandlerFunc(LoginHandler)
+
+	normalID, err := fixtures.CreateTestUserWithEmail(tx, "normal@test.com", "verified_email")
+	require.NoError(t, err)
+	defer fixtures.DeleteUser(tx, normalID)
+
+	lockedID, err := fixtures.CreateTestUserWithEmail(tx, "locked@test.com", "verified_email")
+	require.NoError(t, err)
+	defer fixtures.DeleteUser(tx, lockedID)
+	// Lock the account the way the failure path would after 5 wrong passwords.
+	_, err = tx.Exec(ctx, `UPDATE users SET failed_attempts = 5, locked_until = NOW() + INTERVAL '15 minutes' WHERE id = $1`, lockedID)
+	require.NoError(t, err)
+
+	wWrong := testutils.MakeRequestNoAuth(handler, "POST", "/api/login",
+		LoginRequest{Email: "normal@test.com", Password: "wrong-password"}, ctx)
+	wLocked := testutils.MakeRequestNoAuth(handler, "POST", "/api/login",
+		LoginRequest{Email: "locked@test.com", Password: "wrong-password"}, ctx)
+
+	require.Equal(t, http.StatusUnauthorized, wWrong.Code, "a wrong password must be 401")
+	require.Equal(t, http.StatusUnauthorized, wLocked.Code, "a locked account must be 401, not 429 (F5.5)")
+	require.Equal(t, wWrong.Body.String(), wLocked.Body.String(),
+		"the locked-account body must be byte-identical to the wrong-password body (no lockout oracle)")
+}
+
+// TestLogin_LockedAccount_CorrectPasswordStillUniform verifies the locked path
+// stays indistinguishable even when the attacker submits the CORRECT password:
+// the account is still refused with the generic 401 — the compare result is
+// deliberately discarded on the locked path.
+func TestLogin_LockedAccount_CorrectPasswordStillUniform(t *testing.T) {
+	ctx, tx := testutils.SetupTestTx(t)
+	handler := http.HandlerFunc(LoginHandler)
+
+	// fixtures.CreateTestUserWithEmail hashes "testpassword123".
+	lockedID, err := fixtures.CreateTestUserWithEmail(tx, "locked-correct@test.com", "verified_email")
+	require.NoError(t, err)
+	defer fixtures.DeleteUser(tx, lockedID)
+	_, err = tx.Exec(ctx, `UPDATE users SET failed_attempts = 5, locked_until = NOW() + INTERVAL '15 minutes' WHERE id = $1`, lockedID)
+	require.NoError(t, err)
+
+	w := testutils.MakeRequestNoAuth(handler, "POST", "/api/login",
+		LoginRequest{Email: "locked-correct@test.com", Password: "testpassword123"}, ctx)
+
+	require.Equal(t, http.StatusUnauthorized, w.Code, "a locked account must refuse even the correct password (F5.5)")
+	require.Equal(t, "invalid credentials\n", w.Body.String())
+}
+
+// TestLogin_LockoutEscalation pins the escalating lockout (LOW 6 follow-up): the
+// lock duration grows with sustained failure rounds — 15min at 5-6 failures,
+// 30min at 7-9, 60min at 10+ — so an attacker who keeps guessing past each
+// unlock makes the lock LONGER instead of merely sustaining the 15-minute tier.
+// Every round re-arms the account (locked_until reset to NULL, simulating the
+// attacker retrying after each unlock); each attempt stays a uniform 401, so
+// the escalation adds no distinguishable response (F5.5).
+func TestLogin_LockoutEscalation(t *testing.T) {
+	ctx, tx := testutils.SetupTestTx(t)
+	handler := http.HandlerFunc(LoginHandler)
+
+	userID, err := fixtures.CreateTestUserWithEmail(tx, "escalate@test.com", "verified_email")
+	require.NoError(t, err)
+	defer fixtures.DeleteUser(tx, userID)
+
+	// lockSeconds re-arms the account with the given failure count, performs one
+	// wrong-password login, and returns how long the resulting lock lasts.
+	lockSeconds := func(failedAttempts int) float64 {
+		t.Helper()
+		_, err = tx.Exec(ctx, `UPDATE users SET failed_attempts = $1, locked_until = NULL WHERE id = $2`, failedAttempts, userID)
+		require.NoError(t, err)
+
+		w := testutils.MakeRequestNoAuth(handler, "POST", "/api/login",
+			LoginRequest{Email: "escalate@test.com", Password: "wrong-password"}, ctx)
+		require.Equal(t, http.StatusUnauthorized, w.Code, "every lockout round stays a uniform 401 (F5.5)")
+
+		var secs float64
+		err = tx.QueryRow(ctx, `SELECT EXTRACT(EPOCH FROM (locked_until - NOW())) FROM users WHERE id = $1`, userID).Scan(&secs)
+		require.NoError(t, err)
+		return secs
+	}
+
+	require.InDelta(t, 900, lockSeconds(4), 5, "5th failure locks for 15 minutes")
+	require.InDelta(t, 900, lockSeconds(5), 5, "6th failure stays on the 15-minute tier")
+	require.InDelta(t, 1800, lockSeconds(6), 5, "7th failure escalates to 30 minutes")
+	require.InDelta(t, 1800, lockSeconds(8), 5, "9th failure stays on the 30-minute tier")
+	require.InDelta(t, 3600, lockSeconds(9), 5, "10th failure escalates to the 60-minute ceiling")
+}

backend/internal/twofa/twofa.go

@@ -475,16 +475,32 @@ func Check(ctx context.Context, userID string, st *AttemptState, reqCode string,
 		// lifetime. The interactive setup/disable flows pass consume=false:
 		// they clear the pending fields themselves on success (enableTwoFA /
 		// disableTwoFA), so the code must stay valid through the whole
-		// verification handshake here. The write goes through the same
+		// verification handshake here.
-		// context-routed connection as the rest of Check, so verification and
+		//
-		// consumption are one unit.
+		// DB-ATOMIC (F5.5): the UPDATE is conditional on the exact digest this
-		if _, err := db.Conn.Exec(ctx, `
+		// verify just matched (WHERE id=$1 AND two_factor_pending_code_hash=$2)
+		// and reports rows affected. The per-user mutex above only serializes
+		// attempts WITHIN one process; two concurrent verifications of the same
+		// code on different instances both read the same digest and both match
+		// it, but only the first conditional UPDATE can affect a row — the
+		// loser sees 0 rows and must fail (MissingOrExpired), so one code
+		// authorizes exactly ONE operation even across instances.
+		tag, err := db.Conn.Exec(ctx, `
 			UPDATE users
 			SET two_factor_pending_code_hash = NULL,
 				two_factor_pending_code_expires = NULL
-			WHERE id = $1
+			WHERE id = $1 AND two_factor_pending_code_hash = $2
-		`, userID); err != nil {
+		`, userID, pendingHash.String)
+		if err != nil {
+			// The DB is erroring — atomicity cannot be proven. Keep the prior
+			// fail-open behaviour (report the verify as OK) so the user is not
+			// stranded; the in-process mutex still serializes same-instance
+			// verifications, and a degraded DB is the only way to reach here.
 			log.Printf("failed to consume 2FA pending code for user %s: %v", userID, err)
+		} else if tag.RowsAffected() == 0 {
+			// A concurrent verification consumed the code between this SELECT
+			// and this UPDATE. Single-use means this one must fail.
+			return MissingOrExpired, nil
 		}
 	}
 	return OK, nil

backend/internal/twofa/twofa_test.go

@@ -11,6 +11,7 @@ package twofa
 import (
 	"context"
 	"database/sql"
+	"sync"
 	"testing"
 	"time"
 
@@ -116,6 +117,67 @@ func TestVerifyForUser_SuccessClearsLoginLockout(t *testing.T) {
 	require.Nil(t, lockedUntil, "successful 2FA verify must clear locked_until")
 }
 
+// TestVerifyForUser_DBAtomicConsume_Concurrent pins the DB-ATOMIC consume
+// (F5.5): two concurrent verifications of the SAME code must result in EXACTLY
+// one success. The per-user mutex (StateFor) only serializes verifications
+// within one process, so this test races two checks that each hold their OWN
+// attempt state — bypassing the mutex exactly like two processes behind the
+// same DB would — and relies on the conditional UPDATE in Check's consume path
+// (WHERE id=$1 AND two_factor_pending_code_hash=$2) to make the code single-use
+// across instances. The concurrent checks race through the POOL (pgx.Tx is not
+// concurrency-safe), so the user + pending code are seeded and cleaned up
+// directly on the pool proxy instead of a rollback transaction.
+func TestVerifyForUser_DBAtomicConsume_Concurrent(t *testing.T) {
+	userID, err := fixtures.CreateTestUser(db.Conn)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM users WHERE id = $1`, userID)
+	})
+
+	_, err = db.Conn.Exec(context.Background(), `
+		UPDATE users
+		SET two_factor_method = 'email',
+			two_factor_pending_code_hash = $2,
+			two_factor_pending_code_expires = $3
+		WHERE id = $1`, userID, Hash("424242"), clock.Now().Add(10*time.Minute))
+	require.NoError(t, err)
+
+	const workers = 2
+	type outcome struct {
+		result Result
+		err    error
+	}
+	results := make(chan outcome, workers)
+	var wg sync.WaitGroup
+	for i := 0; i < workers; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			st := &AttemptState{}
+			st.SetLastActive(clock.Now())
+			res, err := Check(context.Background(), userID, st, "424242", true)
+			results <- outcome{result: res, err: err}
+		}()
+	}
+	wg.Wait()
+	close(results)
+
+	okCount, missingCount := 0, 0
+	for r := range results {
+		require.NoError(t, r.err, "no DB failure may occur in a concurrent verify")
+		switch r.result {
+		case OK:
+			okCount++
+		case MissingOrExpired:
+			missingCount++
+		default:
+			t.Errorf("unexpected verify result %v", r.result)
+		}
+	}
+	require.Equal(t, 1, okCount, "exactly one of two concurrent verifies of the same code must succeed")
+	require.Equal(t, 1, missingCount, "the losing concurrent verify must observe the code consumed (DB-atomic single-use)")
+}
+
 // TestConsumePendingCode pins the MEDIUM-2 contract: ConsumePendingCode NULLs
 // the stored pending-code digest and expiry (idempotently), and is the ONLY
 // place a verified-but-unconsumed code dies on the saved-card charge path.