fix(rate-limit): secchio login esteso a credenziali supervisore e setup (#8)
Il secchio "login" del rate limit valeva solo per /api/auth/login. Un
capoturno può ora essere attaccato a forza bruta anche via
/api/measurements/{id}/authorise e /api/production-runs/{id}/pause|
resume|close, senza mai toccare il bucket più permissivo "general".
_is_login_bucket_request confronta il path con un'espressione regolare,
non con ==, perché l'id in mezzo varia per richiesta.
Include anche la parte di #9 che riguarda questo stesso file: i path
sotto /api/setup/ entrano nello stesso secchio, indipendentemente dal
metodo (GET /api/setup/status accetta ora una password opzionale).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,8 +1,10 @@
|
||||
"""Rate limiting middleware for FastAPI.
|
||||
|
||||
Implements in-memory sliding window rate limiting per client IP.
|
||||
Configurable limits for login and general endpoints.
|
||||
Configurable limits for login and endpoints that carry credentials, and for
|
||||
everything else.
|
||||
"""
|
||||
import re
|
||||
import time
|
||||
from collections import defaultdict
|
||||
from typing import Callable
|
||||
@@ -17,7 +19,12 @@ from src.backend.config import settings
|
||||
class RateLimitMiddleware(BaseHTTPMiddleware):
|
||||
"""Middleware that enforces per-IP rate limits using a sliding window.
|
||||
|
||||
- Login endpoint (/api/auth/login): limited to `rate_limit_login` req/min.
|
||||
- Login bucket: `/api/auth/login`, the paths that carry a capoturno's
|
||||
credentials (`/api/measurements/*/authorise`,
|
||||
`/api/production-runs/{id}/pause|resume|close`), and everything under
|
||||
`/api/setup/` (the setup password). Limited to `rate_limit_login`
|
||||
req/min. Guessing a supervisor's or the setup password is the same
|
||||
attack as guessing a login password, just aimed at a different endpoint.
|
||||
- All other endpoints: limited to `rate_limit_general` req/min.
|
||||
Returns HTTP 429 with Retry-After header when limit exceeded.
|
||||
|
||||
@@ -30,6 +37,14 @@ class RateLimitMiddleware(BaseHTTPMiddleware):
|
||||
LOGIN_PATH = "/api/auth/login"
|
||||
WINDOW_SECONDS = 60
|
||||
|
||||
# Paths that carry a supervisor's (capoturno) credentials in the request
|
||||
# body. Matched by pattern, not equality, because the id in the middle
|
||||
# varies per request.
|
||||
_SUPERVISOR_CREDENTIAL_PATTERNS = (
|
||||
re.compile(r"^/api/measurements/\d+/authorise$"),
|
||||
re.compile(r"^/api/production-runs/\d+/(?:pause|resume|close)$"),
|
||||
)
|
||||
|
||||
def __init__(self, app) -> None:
|
||||
super().__init__(app)
|
||||
# {ip: [timestamp, ...]} per bucket
|
||||
@@ -70,6 +85,22 @@ class RateLimitMiddleware(BaseHTTPMiddleware):
|
||||
return real.strip()
|
||||
return request.client.host if request.client else "unknown"
|
||||
|
||||
@classmethod
|
||||
def _is_login_bucket_request(cls, method: str, path: str) -> bool:
|
||||
"""Whether this request counts against the login bucket rather than general.
|
||||
|
||||
`/api/setup/*` is checked regardless of method: `GET /api/setup/status`
|
||||
now accepts an optional password (see setup.py), so guessing it there
|
||||
is the same attack as guessing it on any of the POST endpoints.
|
||||
"""
|
||||
if path.startswith("/api/setup/"):
|
||||
return True
|
||||
if method != "POST":
|
||||
return False
|
||||
if path == cls.LOGIN_PATH:
|
||||
return True
|
||||
return any(p.match(path) for p in cls._SUPERVISOR_CREDENTIAL_PATTERNS)
|
||||
|
||||
@staticmethod
|
||||
def _effective_limit(configured: int) -> int:
|
||||
"""Divide a configured per-minute limit across worker processes.
|
||||
@@ -127,8 +158,9 @@ class RateLimitMiddleware(BaseHTTPMiddleware):
|
||||
self._evict_stale_ips(self._login_requests, now)
|
||||
self._evict_stale_ips(self._general_requests, now)
|
||||
|
||||
# Check login-specific rate limit
|
||||
if path == self.LOGIN_PATH and request.method == "POST":
|
||||
# Check login-bucket rate limit (login itself, supervisor-credential
|
||||
# paths, and the setup password)
|
||||
if self._is_login_bucket_request(request.method, path):
|
||||
allowed, retry_after = self._check_rate_limit(
|
||||
self._login_requests, client_ip,
|
||||
self._effective_limit(settings.rate_limit_login), now,
|
||||
|
||||
@@ -121,6 +121,44 @@ def test_trusted_proxy_count_zero_ignores_all_proxy_headers(monkeypatch):
|
||||
assert RateLimitMiddleware._client_ip(req) == "172.18.0.9"
|
||||
|
||||
|
||||
class TestLoginBucketPaths:
|
||||
"""_is_login_bucket_request: which paths share the login rate-limit bucket."""
|
||||
|
||||
def test_login_path_is_login_bucket(self):
|
||||
assert RateLimitMiddleware._is_login_bucket_request("POST", "/api/auth/login")
|
||||
|
||||
def test_login_path_get_is_not_login_bucket(self):
|
||||
assert not RateLimitMiddleware._is_login_bucket_request("GET", "/api/auth/login")
|
||||
|
||||
def test_measurement_authorise_is_login_bucket(self):
|
||||
assert RateLimitMiddleware._is_login_bucket_request(
|
||||
"POST", "/api/measurements/42/authorise"
|
||||
)
|
||||
|
||||
def test_production_pause_resume_close_are_login_bucket(self):
|
||||
for action in ("pause", "resume", "close"):
|
||||
assert RateLimitMiddleware._is_login_bucket_request(
|
||||
"POST", f"/api/production-runs/7/{action}"
|
||||
)
|
||||
|
||||
def test_production_cycle_is_not_login_bucket(self):
|
||||
"""Only the supervisor-gated actions, not every production-runs path."""
|
||||
assert not RateLimitMiddleware._is_login_bucket_request(
|
||||
"POST", "/api/production-runs/7/cycle"
|
||||
)
|
||||
|
||||
def test_setup_paths_are_login_bucket_any_method(self):
|
||||
assert RateLimitMiddleware._is_login_bucket_request("POST", "/api/setup/init-db")
|
||||
assert RateLimitMiddleware._is_login_bucket_request("GET", "/api/setup/status")
|
||||
|
||||
def test_bare_setup_page_is_not_login_bucket(self):
|
||||
"""The HTML setup page itself carries no password."""
|
||||
assert not RateLimitMiddleware._is_login_bucket_request("GET", "/api/setup")
|
||||
|
||||
def test_unrelated_path_is_not_login_bucket(self):
|
||||
assert not RateLimitMiddleware._is_login_bucket_request("POST", "/api/recipes")
|
||||
|
||||
|
||||
class TestEffectiveLimit:
|
||||
"""_effective_limit: dividing a configured limit across worker processes."""
|
||||
|
||||
|
||||
@@ -124,6 +124,32 @@ class TestRateLimiting:
|
||||
)
|
||||
assert resp.status_code == 429
|
||||
|
||||
async def test_production_run_pause_shares_login_bucket(self, client: AsyncClient):
|
||||
"""Repeated wrong-password POSTs to a supervisor-gated action are rate
|
||||
limited under the login bucket, not the (much higher) general one."""
|
||||
for i in range(5):
|
||||
resp = await client.post(
|
||||
f"/api/production-runs/{i + 1}/pause",
|
||||
json={
|
||||
"supervisor_username": "capoturno",
|
||||
"supervisor_password": "wrong",
|
||||
"note": None,
|
||||
},
|
||||
)
|
||||
# Unauthenticated (no X-API-Key) or run not found either way -
|
||||
# what matters here is that it is not yet rate limited.
|
||||
assert resp.status_code != 429
|
||||
|
||||
resp = await client.post(
|
||||
"/api/production-runs/999/pause",
|
||||
json={
|
||||
"supervisor_username": "capoturno",
|
||||
"supervisor_password": "wrong",
|
||||
"note": None,
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 429
|
||||
|
||||
|
||||
class TestPathTraversal:
|
||||
"""Path traversal protection in file endpoints."""
|
||||
|
||||
Reference in New Issue
Block a user