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.
|
"""Rate limiting middleware for FastAPI.
|
||||||
|
|
||||||
Implements in-memory sliding window rate limiting per client IP.
|
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
|
import time
|
||||||
from collections import defaultdict
|
from collections import defaultdict
|
||||||
from typing import Callable
|
from typing import Callable
|
||||||
@@ -17,7 +19,12 @@ from src.backend.config import settings
|
|||||||
class RateLimitMiddleware(BaseHTTPMiddleware):
|
class RateLimitMiddleware(BaseHTTPMiddleware):
|
||||||
"""Middleware that enforces per-IP rate limits using a sliding window.
|
"""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.
|
- All other endpoints: limited to `rate_limit_general` req/min.
|
||||||
Returns HTTP 429 with Retry-After header when limit exceeded.
|
Returns HTTP 429 with Retry-After header when limit exceeded.
|
||||||
|
|
||||||
@@ -30,6 +37,14 @@ class RateLimitMiddleware(BaseHTTPMiddleware):
|
|||||||
LOGIN_PATH = "/api/auth/login"
|
LOGIN_PATH = "/api/auth/login"
|
||||||
WINDOW_SECONDS = 60
|
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:
|
def __init__(self, app) -> None:
|
||||||
super().__init__(app)
|
super().__init__(app)
|
||||||
# {ip: [timestamp, ...]} per bucket
|
# {ip: [timestamp, ...]} per bucket
|
||||||
@@ -70,6 +85,22 @@ class RateLimitMiddleware(BaseHTTPMiddleware):
|
|||||||
return real.strip()
|
return real.strip()
|
||||||
return request.client.host if request.client else "unknown"
|
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
|
@staticmethod
|
||||||
def _effective_limit(configured: int) -> int:
|
def _effective_limit(configured: int) -> int:
|
||||||
"""Divide a configured per-minute limit across worker processes.
|
"""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._login_requests, now)
|
||||||
self._evict_stale_ips(self._general_requests, now)
|
self._evict_stale_ips(self._general_requests, now)
|
||||||
|
|
||||||
# Check login-specific rate limit
|
# Check login-bucket rate limit (login itself, supervisor-credential
|
||||||
if path == self.LOGIN_PATH and request.method == "POST":
|
# paths, and the setup password)
|
||||||
|
if self._is_login_bucket_request(request.method, path):
|
||||||
allowed, retry_after = self._check_rate_limit(
|
allowed, retry_after = self._check_rate_limit(
|
||||||
self._login_requests, client_ip,
|
self._login_requests, client_ip,
|
||||||
self._effective_limit(settings.rate_limit_login), now,
|
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"
|
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:
|
class TestEffectiveLimit:
|
||||||
"""_effective_limit: dividing a configured limit across worker processes."""
|
"""_effective_limit: dividing a configured limit across worker processes."""
|
||||||
|
|
||||||
|
|||||||
@@ -124,6 +124,32 @@ class TestRateLimiting:
|
|||||||
)
|
)
|
||||||
assert resp.status_code == 429
|
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:
|
class TestPathTraversal:
|
||||||
"""Path traversal protection in file endpoints."""
|
"""Path traversal protection in file endpoints."""
|
||||||
|
|||||||
Reference in New Issue
Block a user