diff --git a/README.md b/README.md index b4c6af1..0412043 100644 --- a/README.md +++ b/README.md @@ -167,9 +167,12 @@ Authentication uses httpOnly cookies with short-lived access tokens and rotating - **Access token**: 15-minute JWT stored in a `{COOKIE_PREFIX}_access` httpOnly cookie - **Refresh token**: 7-day JWT stored in a `{COOKIE_PREFIX}_refresh` httpOnly cookie (scoped to `/v1/auth/refresh` — the cookie's `path` tracks `API_V1_PREFIX` in `app/config.py`) - **Token rotation**: Each refresh issues a new token in the same family; reuse of an old token revokes the entire family (theft detection) -- **Rate limiting**: Login (5/min), registration (3/min), refresh (30/min) +- **Rate limiting**: 60/min per client IP globally (infrastructure routes exempt), with stricter per-endpoint limits on auth routes — login 5/min, registration 3/min, refresh 30/min. Limits are keyed by client IP, so behind a proxy/load balancer the app must run with uvicorn's `--proxy-headers` (already wired into `start.sh`). -> **Before first deploy**: set `COOKIE_PREFIX` to a service-scoped value (typically your service name, e.g. `myservice`). Browser cookies on the same domain are identified by name, so two services sharing a `.example.com` with the default prefix will overwrite each other's auth cookies. Production startup will refuse to boot with the template defaults `""`, `"app"`, or `"api-template"`. +> **Before first deploy**: +> +> - Set `COOKIE_PREFIX` to a service-scoped value (typically your service name, e.g. `myservice`). Browser cookies on the same domain are identified by name, so two services sharing a `.example.com` with the default prefix will overwrite each other's auth cookies. Production startup will refuse to boot with the template defaults `""`, `"app"`, or `"api-template"`. +> - Replace the placeholder `Contact:` in `SECURITY_TXT` (`app/main.py`) with a real security-disclosure address, and bump `Expires:` if it's close. ### Role-Based Access Control @@ -193,7 +196,8 @@ async def admin_only(user: User = Depends(require_role("admin"))): - **Cookie auth**: httpOnly, Secure (in production), SameSite - **CORS lockdown**: Explicit origins, methods, and headers (no wildcards in production) - **Security headers**: HSTS, X-Frame-Options DENY, X-Content-Type-Options nosniff, Referrer-Policy, Permissions-Policy -- **Rate limiting**: Per-endpoint limits on auth routes with security event logging +- **Rate limiting**: Global 60/min-per-IP default plus stricter per-endpoint limits on auth routes; rate-limit hits logged as security events +- **Security disclosure**: `/.well-known/security.txt` per [securitytxt.org](https://securitytxt.org/) (set your contact before deploying — see the note above) - **Production config validation**: Rejects weak secrets, default database credentials, unset cookie prefix, and default OTel service name at startup - **Security event logging**: Structured logs for login, logout, registration, token refresh, and rate limit events diff --git a/app/main.py b/app/main.py index a02e467..4a322ea 100644 --- a/app/main.py +++ b/app/main.py @@ -4,11 +4,12 @@ import structlog from fastapi import Depends, FastAPI, Request, Response from fastapi.middleware.cors import CORSMiddleware -from fastapi.responses import JSONResponse +from fastapi.responses import JSONResponse, PlainTextResponse from limits import RateLimitItem, parse from scalar_fastapi import get_scalar_api_reference from slowapi import Limiter, _rate_limit_exceeded_handler from slowapi.errors import RateLimitExceeded +from slowapi.middleware import SlowAPIMiddleware from slowapi.util import get_remote_address from app.auth import auth_backend, current_active_user, fastapi_users @@ -43,10 +44,18 @@ allow_headers=["Authorization", "Content-Type"], ) -# Rate limiting -limiter = Limiter(key_func=get_remote_address) +# Rate limiting. `default_limits` applies to every route that isn't decorated +# with its own `@limiter.limit(...)` or marked `@limiter.exempt`; SlowAPIMiddleware +# is what actually enforces it (without the middleware, only explicitly-decorated +# routes are limited). The stricter per-path auth limits in `rate_limit_auth` +# below stack on top of this default — an auth route is subject to both. +# `get_remote_address` reads `request.client.host`, which is the real client IP +# only if uvicorn runs with `--proxy-headers` behind a trusted proxy (see start.sh); +# without that, every request collapses into one bucket and the limit is useless. +limiter = Limiter(key_func=get_remote_address, default_limits=["60/minute"]) app.state.limiter = limiter app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) +app.add_middleware(SlowAPIMiddleware) MAX_REQUEST_BODY_SIZE = 1_048_576 # 1 MB @@ -177,7 +186,11 @@ async def request_logging_middleware(request: Request, call_next) -> Response: app.include_router(router, prefix=API_V1_PREFIX) +# Infrastructure routes — not part of the versioned API, and exempt from the +# global rate limit so health-check probes, doc/spec fetches, and automated +# scanners can't burn through the quota. @app.get("/docs", include_in_schema=False) +@limiter.exempt async def scalar_docs(): """Scalar API documentation.""" return get_scalar_api_reference( @@ -187,12 +200,31 @@ async def scalar_docs(): @app.get("/") +@limiter.exempt async def root(): """Health check endpoint.""" return {"status": "ok", "message": "API Template"} @app.get("/health") +@limiter.exempt async def health_check(): """Detailed health check.""" return {"status": "healthy"} + + +# Per https://securitytxt.org/ — security researchers and automated scanners look +# for this file to find a disclosure contact. Replace the contact before deploying +# (see the "before first deploy" note in the README) and bump Expires before the +# date below. +SECURITY_TXT = """\ +Contact: mailto:security@example.com +Expires: 2027-05-12T00:00:00.000Z +Preferred-Languages: en +""" + + +@app.get("/.well-known/security.txt", include_in_schema=False) +@limiter.exempt +async def security_txt() -> PlainTextResponse: + return PlainTextResponse(SECURITY_TXT) diff --git a/start.sh b/start.sh index 9fd160c..8eede91 100755 --- a/start.sh +++ b/start.sh @@ -4,5 +4,13 @@ set -e # Apply any pending database migrations uv run alembic upgrade head -# Start the application -exec uv run uvicorn app.main:app --host 0.0.0.0 --port "${PORT:-8000}" +# Start the application. +# --proxy-headers --forwarded-allow-ips='*' makes uvicorn trust the +# X-Forwarded-For / X-Forwarded-Proto headers set by the upstream proxy +# (Cloud Run, an ALB, nginx, ...), so request.client.host is the real client IP +# rather than the proxy's internal address. The rate limiter keys on that IP, so +# without this every request collapses into one bucket. '*' trusts any upstream — +# correct when the service is ALWAYS behind a proxy that overwrites these headers; +# if it can also be reached directly, restrict this to the proxy's IP range. +exec uv run uvicorn app.main:app --host 0.0.0.0 --port "${PORT:-8000}" \ + --proxy-headers --forwarded-allow-ips='*' diff --git a/tests/conftest.py b/tests/conftest.py index aa507c2..43be725 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,7 +7,7 @@ from app.auth import current_active_user from app.database import Base, get_async_session -from app.main import app +from app.main import app, limiter from app.models.user import User # Use SQLite for tests (faster, no external dependencies) @@ -27,6 +27,14 @@ async def setup_database(): await conn.run_sync(Base.metadata.drop_all) +@pytest.fixture(autouse=True) +def reset_rate_limiter(): + """Clear rate-limit buckets between tests so request counts don't leak across tests.""" + limiter._storage.reset() + yield + limiter._storage.reset() + + async def override_get_async_session() -> AsyncGenerator[AsyncSession, None]: async with async_session_maker() as session: yield session diff --git a/tests/test_app.py b/tests/test_app.py new file mode 100644 index 0000000..00468f4 --- /dev/null +++ b/tests/test_app.py @@ -0,0 +1,32 @@ +"""App-level / infrastructure route behaviour: security.txt and global rate limiting.""" + +from httpx import AsyncClient + + +class TestSecurityTxt: + async def test_served_as_plain_text_with_required_fields(self, client: AsyncClient): + response = await client.get("/.well-known/security.txt") + assert response.status_code == 200 + assert response.headers["content-type"].startswith("text/plain") + body = response.text + assert "Contact:" in body + assert "Expires:" in body + + +class TestGlobalRateLimit: + """The Limiter's `default_limits` (60/min) applies to every non-exempt route.""" + + async def test_default_limit_enforced_on_undecorated_route(self, auth_client: AsyncClient): + # /v1/notes has no explicit @limiter.limit, so it gets the 60/min default. + # (Matches `default_limits` in app/main.py — bump both together if changed.) + for _ in range(60): + assert (await auth_client.get("/v1/notes")).status_code == 200 + assert (await auth_client.get("/v1/notes")).status_code == 429 + + async def test_infrastructure_routes_are_exempt(self, client: AsyncClient): + # /health, /, /docs, /.well-known/security.txt are @limiter.exempt — many + # rapid hits (well past the 60/min default) never 429. + for _ in range(70): + assert (await client.get("/health")).status_code == 200 + assert (await client.get("/")).status_code == 200 + assert (await client.get("/.well-known/security.txt")).status_code == 200