diff --git a/src/backend/api/middleware/rate_limit.py b/src/backend/api/middleware/rate_limit.py index ccd1576..22c20f0 100644 --- a/src/backend/api/middleware/rate_limit.py +++ b/src/backend/api/middleware/rate_limit.py @@ -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, diff --git a/src/backend/tests/test_rate_limit_proxy.py b/src/backend/tests/test_rate_limit_proxy.py index a81a031..da9bf08 100644 --- a/src/backend/tests/test_rate_limit_proxy.py +++ b/src/backend/tests/test_rate_limit_proxy.py @@ -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.""" diff --git a/src/backend/tests/test_security.py b/src/backend/tests/test_security.py index ca0414c..b68b00b 100644 --- a/src/backend/tests/test_security.py +++ b/src/backend/tests/test_security.py @@ -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."""