diff --git a/app/main.py b/app/main.py index c6a6ad1..bae5678 100644 --- a/app/main.py +++ b/app/main.py @@ -79,19 +79,48 @@ async def get_current_user(user: User = Depends(current_active_user)): # OAuth endpoints use GET instead of POST _OAUTH_PATHS = {"/auth/google/authorize", "/auth/google/callback"} +# Global default for every other route, matching the per-client-IP keying of +# the auth limits. Enforced here in-house (limits via limiter._limiter) +# rather than through slowapi's SlowAPIMiddleware, whose default_limits +# enforcement FastAPI >=0.137 breaks for routes mounted via include_router +# (slowapi issue #281). +_DEFAULT_RATE_LIMIT: RateLimitItem = parse("300/minute") + +# Infrastructure paths that must never 429: probes, docs, and the security +# contact file. +_RATE_LIMIT_EXEMPT_PATHS = { + "/", + "/health", + "/docs", + "/openapi.json", + "/.well-known/security.txt", +} + @app.middleware("http") -async def rate_limit_auth(request: Request, call_next) -> Response: - """Apply rate limits to auth endpoints.""" - rate_limit = _AUTH_RATE_LIMITS.get(request.url.path) - is_oauth = request.url.path in _OAUTH_PATHS - if rate_limit and (request.method == "POST" or is_oauth): +async def rate_limit(request: Request, call_next) -> Response: + """Apply rate limits. + + Auth endpoints get their strict per-path limits; every other route gets + the global default except the exempt infrastructure paths. A request + that consumed an auth limit does not also consume the default bucket. + """ + path = request.url.path + auth_limit = _AUTH_RATE_LIMITS.get(path) + is_oauth = path in _OAUTH_PATHS + if auth_limit and (request.method == "POST" or is_oauth): + rate_limit_item: RateLimitItem | None = auth_limit + elif path in _RATE_LIMIT_EXEMPT_PATHS: + rate_limit_item = None + else: + rate_limit_item = _DEFAULT_RATE_LIMIT + if rate_limit_item is not None: key = get_remote_address(request) - if not limiter._limiter.hit(rate_limit, key): + if not limiter._limiter.hit(rate_limit_item, key): log_security_event( SecurityEvent.RATE_LIMIT_HIT, request=request, - detail=f"path={request.url.path}", + detail=f"path={path}", ) return JSONResponse( status_code=429, diff --git a/tests/conftest.py b/tests/conftest.py index 7e56b48..ac417c2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,6 +17,17 @@ async_session_maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) +@pytest.fixture(autouse=True) +def reset_rate_limiter(): + """Clear rate-limit buckets between tests so request counts don't leak + across tests (the limiter storage is process-global).""" + from app.main import limiter + + limiter._storage.reset() + yield + limiter._storage.reset() + + @pytest.fixture(autouse=True) async def setup_database(): """Create tables before each test, drop after.""" diff --git a/tests/test_rate_limiting.py b/tests/test_rate_limiting.py new file mode 100644 index 0000000..5fd3669 --- /dev/null +++ b/tests/test_rate_limiting.py @@ -0,0 +1,29 @@ +"""Global rate limiting behaviour. + +Every route outside the exempt infrastructure set gets the in-house +300/min default (auth endpoints keep their stricter per-path limits). +The default is enforced by our own middleware calling `limits` directly, +so it is immune to the FastAPI >=0.137 / slowapi issue #281 regression +that breaks SlowAPIMiddleware-based default_limits. +""" + +from httpx import AsyncClient + + +class TestGlobalRateLimit: + async def test_default_limit_applies_to_non_auth_routes(self, client: AsyncClient): + # /collections is an ordinary API route with no per-path limit, so + # it gets the 300/min default (matches _DEFAULT_RATE_LIMIT in + # app/main.py — bump both together if that changes). The limit is + # consumed regardless of auth outcome, so unauthenticated traffic + # cannot hammer the route either. + for _ in range(300): + assert (await client.get("/collections")).status_code != 429 + assert (await client.get("/collections")).status_code == 429 + + async def test_infrastructure_routes_are_exempt(self, client: AsyncClient): + # / and /health must never 429 — probes and uptime checks hit them + # far more often than any human client. + for _ in range(310): + assert (await client.get("/health")).status_code == 200 + assert (await client.get("/")).status_code == 200