fix(auth): key the TOTP lockout on client_id + source IP

[Showdown76py]

Before: anyone who knew a client_id (it is not a secret: it shows up in authorize URLs, browser history, reverse-proxy logs) could send 5 wrong TOTP codes to POST /oauth/authorize, which needs nothing else and had no per-IP limit, and lock the owner out of the dashboard, /app/refresh, token minting, client_credentials and self-update for 5 minutes, again and again.

After: wrong codes lock out the address they come from, not the client. The owner signs in normally from anywhere else. A per-client cap still stops someone spreading guesses over many addresses.

How:

• New ratelimit.TotpLockout. 5 wrong codes from one IP lock that IP for that client; 20 wrong codes from any mix of IPs lock the client everywhere; 5-minute cooldown, pushed forward by each failure. Strikes from an expired cooldown no longer pile up (on main, 4 wrong codes one day and 1 the next locked the client). A success clears only the caller's own per-IP counter, so the owner signing in does not reset the per-client cap for a distributed guesser.
• The source IP (client_ip(), so trusted_proxies aware) is passed to every lockout check: /oauth/authorize, the passkey sign-in endpoints, /oauth/token and all dashboard 2FA paths (DashboardDeps.totp_* now take (client_id, ip)).
• POST /oauth/authorize gets the same per-IP limiter as /app/login (10 per minute). The limit message is shown on the authorize page (or in the JSON reply) with a Retry-After header.

Exercised on a real server

beaconmcp serve from main and from this branch, dashboard on, real clients and real TOTP codes, source addresses set through X-Forwarded-For from the trusted loopback proxy:

Scenario	main	this PR
Stranger sends 5 wrong codes to POST /oauth/authorize, then the owner logs into the dashboard from another IP with the right secret and code	429 "Too many attempts"	303, logged in
Same, owner asks /oauth/token (client_credentials)	400 "too many failed TOTP attempts"	200, token issued
Same, owner completes /oauth/authorize with the right code	429	200, ticket issued
Stranger's own IP with the right code	429	429 (still locked)
12 wrong codes from one IP spread over 3 client_ids (no client reaches 5)	every client already locked	10 × 401, then 429 rate limited
20 wrong codes spread over 5 IPs, then the owner	429	429 (per-client cap, by design)
HTML page, 11th POST from one IP	n/a	"Too many attempts. Retry in 60s." with Retry-After: 60

Unit tests: tests/test_totp_lockout.py (per-source vs per-client, expiry, no accumulation across cooldowns, what a success resets). Dashboard test fixtures updated to the two-argument callables.

Notes

• Behind a reverse proxy, trusted_proxies must list it (the example config and beaconmcp init already put 127.0.0.1 / ::1). Otherwise every visitor shares the proxy's address and the per-IP lock behaves like the old per-client one.
• A determined attacker with 4 or more addresses can still trigger the per-client cap. Raising it trades lockout resistance for brute-force resistance; 20 is the value from the audit plan.
• feat(auth): oauth refresh tokens and persisted oauth tokens #40 also edits oauth_token; whichever merges second needs a small rebase.

[Ailcope]

@Showdown76py keying the lockout on IP is a real step up, but the per-client cap brings back part of the problem it fixes:

1. The owner can still be locked out, passkeys included. /oauth/authorize only needs a client_id. 5 wrong codes from each of 4 IPs reach per_client=20 (ratelimit.py:148). That blocks TOTP, the dashboard, /oauth/token, and the OAuth passkey endpoints (__main__.py:1683, :1716 call totp_locked). The attacker can repeat it every 5 minutes, and IPv6 makes new IPs free.
2. Brute force gets 4x easier. A guesser now gets 20 codes per 5-minute window instead of 5, roughly a 1.7% chance per day of hitting a valid code.
3. Reverse proxy without trusted_proxies. Everyone shares the proxy's IP, so the per-IP lockout and the authorize limiter become one budget an attacker can spend for everyone. Nothing tells the operator about it.

Fix commit eaee3bf (pushing right after this comment):

• The OAuth passkey endpoints no longer check the TOTP lockout. A passkey can't be guessed, so it stays the owner's way in while someone trips the TOTP cap. The per-IP _authorize_passkey_guard limit still applies.
• per_client goes from 20 to 10. That's a compromise; feel free to pick another number.
• A one-time warning is logged when X-Forwarded-For arrives while server.trusted_proxies is empty.

Full suite: 521 passed, ruff clean.

---

Generated by Claude Code

[Ailcope]

Correction to my review: the push of eaee3bf was rejected by the repo ruleset ("Changes must be made through a pull request"), so here's the fix as a patch. To apply it, save the block below as fix.patch, then on claude/audit-fixes-ycn7ph-totp-lockout run git am fix.patch.

From eaee3bf59cc2d375054d03e3b0fe4c1afccafa36 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 11 Oct 2026 15:25:46 +0000
Subject: [PATCH] fix(auth): keep passkeys out of the TOTP lockout, lower the
 per-client cap

- The per-client cap can be tripped on purpose from a few IPs, which
  also blocked OAuth passkey sign-in. Passkeys can't be guessed, so the
  OAuth passkey endpoints no longer check the TOTP lockout (per-IP rate
  limiting still applies). The owner keeps a way in.
- per_client 20 -> 10: 20 gave a distributed guesser 4x the old budget.
- Warn once when X-Forwarded-For arrives with no trusted_proxies set:
  every caller then shares the proxy's IP for limits and the lockout.
---
 src/beaconmcp/__main__.py  | 39 ++++++++++++++++++++++++--------------
 src/beaconmcp/ratelimit.py |  6 ++++--
 tests/test_totp_lockout.py |  9 +++++++++
 3 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/src/beaconmcp/__main__.py b/src/beaconmcp/__main__.py
index 23a4c38..9cc7aa3 100644
--- a/src/beaconmcp/__main__.py
+++ b/src/beaconmcp/__main__.py
@@ -1179,15 +1179,33 @@ def _run_http(mcp, host: str, port: int):
     auth.register_token_store(token_store)
 
     # In-memory TOTP brute-force guard, keyed on (client_id, source IP): 5
-    # wrong codes from one IP lock that IP out for 5 minutes, and 20 from any
-    # mix of IPs lock the client. Keyed on client_id alone, anyone who knew a
-    # client_id could lock its owner out of every 2FA path.
-    totp_lockout = TotpLockout(per_source=5, per_client=20, lockout_seconds=300)
+    # wrong codes from one IP lock that IP out for 5 minutes, and 10 from any
+    # mix of IPs lock the client's TOTP paths. Keyed on client_id alone, anyone
+    # who knew a client_id could lock its owner out of every 2FA path. The
+    # per-client cap can still be tripped on purpose, so passkey sign-in (which
+    # can't be guessed) is not gated by it and stays open to the owner.
+    totp_lockout = TotpLockout(per_source=5, per_client=10, lockout_seconds=300)
     totp_locked = totp_lockout.locked
     totp_record_failure = totp_lockout.record_failure
     totp_record_success = totp_lockout.record_success
+    _proxy_warned = False
 
     def _ip(request: Request) -> str:
+        nonlocal _proxy_warned
+        if (
+            not _proxy_warned
+            and not config.server.trusted_proxies
+            and request.headers.get("x-forwarded-for")
+        ):
+            # Behind a proxy that isn't declared, every caller shares the
+            # proxy's address, and the per-IP TOTP and login limits become
+            # one budget an attacker can spend for everyone.
+            _proxy_warned = True
+            logging.getLogger("beaconmcp.auth").warning(
+                "Request carries X-Forwarded-For but server.trusted_proxies is "
+                "empty: all clients share the proxy's IP for rate limits and "
+                "the TOTP lockout. Declare the proxy in server.trusted_proxies."
+            )
         return client_ip(request, tuple(config.server.trusted_proxies))
 
     def _issuer(request: Request) -> str:
@@ -1680,11 +1698,9 @@ def _run_http(mcp, host: str, port: int):
             return JSONResponse(
                 {"ok": False, "error": "Unknown client."}, status_code=400,
             )
-        if totp_locked(client_id, _ip(request)):
-            return JSONResponse(
-                {"ok": False, "error": "Too many attempts. Try again in 5 minutes."},
-                status_code=429,
-            )
+        # No TOTP lockout check here: a passkey can't be guessed, and it is
+        # the owner's way in while someone trips the per-client TOTP cap.
+        # _authorize_passkey_guard already rate-limits per IP.
         try:
             options, state = passkey_service.authentication_options(
                 request, client_id=_passkey_owner(client_id),
@@ -1713,11 +1729,6 @@ def _run_http(mcp, host: str, port: int):
                 status_code=400,
             )
         client_id = normalized["client_id"]
-        if totp_locked(client_id, _ip(request)):
-            return JSONResponse(
-                {"ok": False, "error": "Too many attempts. Try again in 5 minutes."},
-                status_code=429,
-            )
         credential = body.get("credential")
         state = str(body.get("state") or "")
         if not isinstance(credential, dict) or not state:
diff --git a/src/beaconmcp/ratelimit.py b/src/beaconmcp/ratelimit.py
index 173a266..1943773 100644
--- a/src/beaconmcp/ratelimit.py
+++ b/src/beaconmcp/ratelimit.py
@@ -99,7 +99,9 @@ class TotpLockout:
     Strikes are counted per source instead: ``per_source`` wrong codes from
     one IP lock that IP out for that client. A second, higher per-client cap
     (``per_client`` wrong codes from any mix of IPs) still stops a guesser
-    who spreads attempts over many addresses.
+    who spreads attempts over many addresses. That cap can be tripped on
+    purpose from a few IPs, so callers must not use it to gate passkey
+    sign-in, which can't be guessed.
 
     Every failure pushes the cooldown of its counters forward; a counter
     whose cooldown has passed starts again from zero. A success clears the
@@ -111,7 +113,7 @@ class TotpLockout:
         self,
         *,
         per_source: int = 5,
-        per_client: int = 20,
+        per_client: int = 10,
         lockout_seconds: float = 300.0,
     ) -> None:
         self._per_source = per_source
diff --git a/tests/test_totp_lockout.py b/tests/test_totp_lockout.py
index 5b5a650..ed8964c 100644
--- a/tests/test_totp_lockout.py
+++ b/tests/test_totp_lockout.py
@@ -117,3 +117,12 @@ def test_unknown_ip_is_its_own_source(clock) -> None:
         guard.record_failure("c1")
     assert guard.locked("c1")
     assert not guard.locked("c1", OWNER)
+
+
+def test_default_per_client_cap_is_ten(clock) -> None:
+    # 10, not 20: a guesser spread over many IPs gets two lockouts' worth of
+    # tries per window, not four.
+    guard = TotpLockout()
+    for i in range(10):
+        guard.record_failure("c1", f"203.0.113.{i}")
+    assert guard.locked("c1", OWNER)
-- 
2.43.0

---

Generated by Claude Code

[Showdown76py]

@Ailcope agreed on all three. eaee3bf never reached this branch, so I applied the same changes in 0eb27dc:

• The OAuth passkey endpoints no longer check the TOTP lockout; _authorize_passkey_guard still rate-limits per IP. The dashboard passkey login already worked this way, so the two paths now match. docs/security.md said the opposite and is updated.
• per_client is now 10.
• A one-time warning is logged when X-Forwarded-For arrives while server.trusted_proxies is empty. The troubleshooting row explains both limits.

On a real server (trusted proxy, wrong codes spread over several X-Forwarded-For addresses), before: once 20 failures hit the cap, /oauth/passkey/options returned 429 for the owner. After: the cap trips at 10 and TOTP login gets 429, but the passkey endpoint still answers (here "no passkey registered", because none was enrolled). Without trusted_proxies, exactly one warning is logged. Full suite: 523 passed, ruff clean.

---

Generated by Claude Code

[Ailcope]

LGTM, 0eb27dc covers all three points. Tiny nit: the comment above TotpLockout(...) in __main__.py still says "20 from any mix of IPs".

---

Generated by Claude Code