Rate-limit failed authentication attempts per client IP#69
Merged
Conversation
Adds app/ratelimit.py: a fixed-window, per-IP failure counter applied as middleware over the four auth surfaces (REST /api/v1, MCP /mcp, OAuth consent POST /oauth/authorize, /oauth/token). Only failed attempts count; an over-limit IP gets 429 + Retry-After until the window expires. New auth_failures_total and rate_limited_requests_total metrics provide a brute-force signal. Limits and X-Forwarded-For trust are configurable via Settings; a *_FAILURES value of 0 disables a surface's limiter. Closes #66 https://claude.ai/code/session_01H2Dbh6kD8bseWZZEf7kGhx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #66
What
Adds per-IP, fixed-window rate limiting of failed authentication attempts across all four auth surfaces:
/api/v1/.../mcpPOST /oauth/authorizePOST /oauth/tokenAn IP over the limit gets
429+Retry-Afteron that surface (even with valid credentials) until the window expires. Successful auth never counts./healthz,/metrics, the.well-knownmetadata, and/oauth/registerare never limited. Surfaces are limited independently.How
app/ratelimit.py: a ~60-line locked fixed-window counter (no new dependencies),client_ip()honoring the firstX-Forwarded-Forhop (configurable viaTRUST_FORWARDED_FORfor proxy-less deployments), and a single middleware that classifies the surface and counts failed-auth response statuses. The middleware approach is what makes MCP coverage possible — the FastMCP token verifier never sees the request, so per-IP accounting has to happen at the HTTP layer.log_requestsso rate-limited 429s still get a request log line and latency metric.auth_failures_total{surface}andrate_limited_requests_total{surface}give a brute-force signal independent of the limiter; each 429 and each auth failure also emits a structlog warning with the IP.Settings(RATE_LIMIT_*); a*_FAILURESvalue of0disables a surface's limiter. State is in-process and per uvicorn worker by design (effective limit ≈ 2× configured with the default--workers 2) — documented in the user guide.Tests
31 new tests in
tests/test_ratelimit.py: limiter unit behavior (window expiry, key independence, prune cap, disable switch),client_iptrust handling, surface classification, middleware behavior on a stub app for every surface, and end-to-end lockout through the real app (bad bearer tokens → 429,/healthzunaffected). An autouse conftest fixture resets limiter state between tests. Full suite: 198 passed,ruffclean.Review
Adversarial review (3 finder angles + verification) found no correctness bugs; flagged tradeoffs (shared-NAT lockout, status-code-based failure detection) are deliberate and documented in the guides.
https://claude.ai/code/session_01H2Dbh6kD8bseWZZEf7kGhx
Generated by Claude Code