diff --git a/.env.example b/.env.example index 38dbd72..ebfb1a1 100644 --- a/.env.example +++ b/.env.example @@ -16,6 +16,12 @@ FRONTEND_URL=http://localhost:5173 # Cookie domain (leave empty for localhost development) COOKIE_DOMAIN= +# Cookie name prefix — applied to every auth cookie (e.g. "myapp" → "myapp_access", +# "myapp_refresh"). In production, set this to something service-scoped (usually your +# service name) so cookies don't collide with other services on the same domain. +# Required in production: defaults of "", "app", or "api-template" are rejected. +COOKIE_PREFIX=app + # Logging LOG_LEVEL=INFO diff --git a/README.md b/README.md index 821e234..960fabd 100644 --- a/README.md +++ b/README.md @@ -162,11 +162,13 @@ All note endpoints require authentication. Users can only access their own notes Authentication uses httpOnly cookies with short-lived access tokens and rotating refresh tokens. -- **Access token**: 15-minute JWT stored in an `app_access` httpOnly cookie -- **Refresh token**: 7-day JWT stored in an `app_refresh` httpOnly cookie (scoped to `/auth/refresh`) +- **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 `/auth/refresh`) - **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) +> **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"`. + ### Role-Based Access Control Users have a `role` field (default: `user`). Roles are defined as a `StrEnum` in `app/auth/roles.py`: @@ -190,7 +192,7 @@ async def admin_only(user: User = Depends(require_role("admin"))): - **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 -- **Production config validation**: Rejects weak secrets and default database credentials at startup +- **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 ## Logging, Telemetry & Feature Flags @@ -369,9 +371,10 @@ api-template/ | `CORS_ORIGINS` | Required | Comma-separated allowed origins | (empty — dev uses localhost:5100-5199) | | `FRONTEND_URL` | Optional | Frontend URL for redirects | `http://localhost:5173` | | `COOKIE_DOMAIN` | Optional | Cookie domain (leave empty for localhost) | (empty) | +| `COOKIE_PREFIX` | Required | Prefix for auth cookie names (service-scoped) | `app` | | `LOG_LEVEL` | Optional | Logging level | `INFO` | | `OTEL_ENABLED` | Optional | Enable OpenTelemetry tracing | `false` | -| `OTEL_SERVICE_NAME` | Optional | Service name for traces | `api-template` | +| `OTEL_SERVICE_NAME` | Required | Service name for traces | `api-template` | | `OTEL_EXPORTER_ENDPOINT` | Optional | OTLP gRPC collector endpoint | `http://localhost:4317` | | `FEATURE_*` | Optional | Feature flags (e.g. `FEATURE_NEW_DASHBOARD=true`) | (none) | diff --git a/app/auth/backend.py b/app/auth/backend.py index 91d5e12..bb9ad21 100644 --- a/app/auth/backend.py +++ b/app/auth/backend.py @@ -5,7 +5,7 @@ ACCESS_TOKEN_LIFETIME = 900 # 15 minutes cookie_transport = CookieTransport( - cookie_name="app_access", + cookie_name=f"{settings.cookie_prefix}_access", cookie_max_age=ACCESS_TOKEN_LIFETIME, cookie_path="/", cookie_domain=settings.cookie_domain, diff --git a/app/auth/refresh.py b/app/auth/refresh.py index 6c20be1..b900d4f 100644 --- a/app/auth/refresh.py +++ b/app/auth/refresh.py @@ -1,6 +1,7 @@ import uuid from datetime import UTC, datetime, timedelta +import jwt from fastapi import Response from fastapi_users.jwt import decode_jwt, generate_jwt from sqlalchemy import select, update @@ -10,10 +11,27 @@ from app.models.refresh_token import RefreshToken REFRESH_TOKEN_LIFETIME = timedelta(days=7) -REFRESH_COOKIE_NAME = "app_refresh" +REFRESH_COOKIE_NAME = f"{settings.cookie_prefix}_refresh" REFRESH_AUDIENCE = ["app:refresh"] +def decode_refresh_token(token: str) -> dict | None: + """Decode a refresh-token JWT, returning None on any decode failure. + + Separates decode errors (invalid/expired/malformed → None) from downstream + DB errors (which should propagate as 500s, not be swallowed alongside decode + failures by a caller's broad except). + """ + try: + return decode_jwt( + token, + secret=settings.secret_key, + audience=REFRESH_AUDIENCE, + ) + except jwt.PyJWTError: + return None + + async def create_refresh_token( user_id: str, session: AsyncSession, @@ -50,13 +68,8 @@ async def validate_and_rotate_refresh_token( token_jwt: str, session: AsyncSession, ) -> tuple[str, str] | None: - try: - payload = decode_jwt( - token_jwt, - secret=settings.secret_key, - audience=REFRESH_AUDIENCE, - ) - except Exception: + payload = decode_refresh_token(token_jwt) + if payload is None: return None jti = payload.get("jti") diff --git a/app/config.py b/app/config.py index 9488291..f3e347e 100644 --- a/app/config.py +++ b/app/config.py @@ -28,6 +28,10 @@ class Settings(BaseSettings): # Cookie auth cookie_domain: str | None = None + # Prefix applied to every auth cookie (e.g. "app" → "app_access", "app_refresh"). + # Must be overridden in production to avoid collisions with other services on the + # same domain — see validate_production_settings below. + cookie_prefix: str = "app" # Logging log_level: str = "INFO" @@ -61,6 +65,16 @@ def validate_production_settings(self) -> "Settings": ) if "postgres:postgres@" in self.database_url: raise ValueError("Default database credentials must not be used in production") + weak_cookie_prefixes = {"", "app", "api-template"} + if self.cookie_prefix in weak_cookie_prefixes: + raise ValueError( + "COOKIE_PREFIX must be set to a service-scoped value in production " + "(e.g. your service name) to avoid cookie collisions across services " + "on the same domain" + ) + weak_otel_names = {"", "api-template"} + if self.otel_service_name in weak_otel_names: + raise ValueError("OTEL_SERVICE_NAME must be set to your service name in production") return self diff --git a/app/routers/auth_refresh.py b/app/routers/auth_refresh.py index 5b240ff..6338e8b 100644 --- a/app/routers/auth_refresh.py +++ b/app/routers/auth_refresh.py @@ -2,20 +2,19 @@ from fastapi import APIRouter, Cookie, Depends, Response from fastapi.responses import JSONResponse -from fastapi_users.jwt import decode_jwt from sqlalchemy import update from sqlalchemy.ext.asyncio import AsyncSession from app.auth.backend import cookie_transport, get_jwt_strategy from app.auth.refresh import ( - REFRESH_AUDIENCE, + REFRESH_COOKIE_NAME, clear_refresh_cookie, + decode_refresh_token, set_refresh_cookie, validate_and_rotate_refresh_token, ) from app.auth.security_logging import SecurityEvent, log_security_event from app.auth.users import get_user_db -from app.config import settings from app.database import get_async_session from app.models.refresh_token import RefreshToken @@ -24,13 +23,13 @@ @router.post("/refresh", status_code=204) async def refresh_access_token( - app_refresh: str | None = Cookie(None), + refresh_token: str | None = Cookie(None, alias=REFRESH_COOKIE_NAME), session: AsyncSession = Depends(get_async_session), ): - if not app_refresh: + if not refresh_token: return JSONResponse(status_code=401, content={"detail": "Missing refresh token"}) - result = await validate_and_rotate_refresh_token(app_refresh, session) + result = await validate_and_rotate_refresh_token(refresh_token, session) if result is None: log_security_event( @@ -58,7 +57,7 @@ async def refresh_access_token( response = Response(status_code=204) # Set access cookie response.set_cookie( - key="app_access", + key=cookie_transport.cookie_name, value=access_token, max_age=cookie_transport.cookie_max_age, path=cookie_transport.cookie_path, @@ -81,17 +80,17 @@ async def refresh_access_token( @router.post("/jwt/logout", status_code=204) async def logout( - app_refresh: str | None = Cookie(None), + refresh_token: str | None = Cookie(None, alias=REFRESH_COOKIE_NAME), session: AsyncSession = Depends(get_async_session), ): - # Revoke the refresh token family if a refresh cookie is present - if app_refresh: - try: - payload = decode_jwt( - app_refresh, - secret=settings.secret_key, - audience=REFRESH_AUDIENCE, - ) + # Revoke the refresh-token family if a valid refresh cookie is present. + # Decode failures → log and proceed (user-facing logout still succeeds). + # DB errors → propagate as 500, not silently swallowed as "decode failed". + if refresh_token: + payload = decode_refresh_token(refresh_token) + if payload is None: + log_security_event(SecurityEvent.LOGOUT, detail="refresh token decode failed") + else: family = payload.get("family") user_id = payload.get("sub") if family: @@ -106,8 +105,6 @@ async def logout( user_id=user_id, detail=f"revoked token family={family}", ) - except Exception: - log_security_event(SecurityEvent.LOGOUT, detail="refresh token decode failed") else: log_security_event(SecurityEvent.LOGOUT, detail="no refresh token cookie") diff --git a/tests/test_auth.py b/tests/test_auth.py new file mode 100644 index 0000000..3fbec13 --- /dev/null +++ b/tests/test_auth.py @@ -0,0 +1,143 @@ +import importlib +import os +from datetime import UTC, datetime, timedelta +from uuid import UUID, uuid4 + +from fastapi_users.jwt import generate_jwt +from httpx import AsyncClient +from sqlalchemy import select + +from app.auth.refresh import REFRESH_AUDIENCE, REFRESH_COOKIE_NAME, REFRESH_TOKEN_LIFETIME +from app.config import settings +from app.models.refresh_token import RefreshToken +from app.models.user import User + + +def _make_refresh_jwt(user_id: UUID, jti: UUID, family: str) -> str: + return generate_jwt( + { + "sub": str(user_id), + "jti": str(jti), + "family": family, + "aud": REFRESH_AUDIENCE, + }, + secret=settings.secret_key, + lifetime_seconds=int(REFRESH_TOKEN_LIFETIME.total_seconds()), + ) + + +class TestLogoutRevocation: + """Reproducer for the silent-failure bug: logout must revoke the token family.""" + + async def test_logout_revokes_token_family(self, client: AsyncClient, test_user: User, session): + session.add(test_user) + await session.commit() + + family = "family-under-test" + jti = uuid4() + db_token = RefreshToken( + id=jti, + user_id=test_user.id, + token_family=family, + is_revoked=False, + expires_at=datetime.now(UTC) + timedelta(days=7), + ) + session.add(db_token) + await session.commit() + + response = await client.post( + "/auth/jwt/logout", + cookies={REFRESH_COOKIE_NAME: _make_refresh_jwt(test_user.id, jti, family)}, + ) + assert response.status_code == 204 + + await session.refresh(db_token) + assert db_token.is_revoked is True + + async def test_logout_revokes_all_tokens_in_family( + self, client: AsyncClient, test_user: User, session + ): + """Multiple rotated tokens in one family: logout revokes every row.""" + session.add(test_user) + await session.commit() + + family = "shared-family" + tokens = [ + RefreshToken( + id=uuid4(), + user_id=test_user.id, + token_family=family, + is_revoked=False, + expires_at=datetime.now(UTC) + timedelta(days=7), + ) + for _ in range(3) + ] + for t in tokens: + session.add(t) + await session.commit() + + response = await client.post( + "/auth/jwt/logout", + cookies={REFRESH_COOKIE_NAME: _make_refresh_jwt(test_user.id, tokens[-1].id, family)}, + ) + assert response.status_code == 204 + + # Endpoint ran the UPDATE in its own session; expire this session's + # identity map so the re-query reflects the committed changes. + session.expire_all() + result = await session.execute( + select(RefreshToken).where(RefreshToken.token_family == family) + ) + rows = result.scalars().all() + assert len(rows) == 3 + assert all(row.is_revoked for row in rows) + + async def test_logout_with_invalid_token_returns_204(self, client: AsyncClient): + """Malformed cookie: logout still succeeds (user-facing) and does not crash.""" + response = await client.post( + "/auth/jwt/logout", + cookies={REFRESH_COOKIE_NAME: "not-a-real-jwt"}, + ) + assert response.status_code == 204 + + async def test_logout_without_cookie_returns_204(self, client: AsyncClient): + response = await client.post("/auth/jwt/logout") + assert response.status_code == 204 + + +class TestCookiePrefixWiring: + """COOKIE_PREFIX must flow through to every cookie surface.""" + + def test_cookie_names_compose_from_settings_prefix(self): + from app.auth.backend import cookie_transport + from app.auth.refresh import REFRESH_COOKIE_NAME as current_refresh_name + + assert cookie_transport.cookie_name == f"{settings.cookie_prefix}_access" + assert current_refresh_name == f"{settings.cookie_prefix}_refresh" + + def test_cookie_prefix_env_override_wires_through(self): + """Changing COOKIE_PREFIX changes the computed cookie names end-to-end. + + This is the test that distinguishes "reads from the setting" from + "happens to produce the right string because the default is 'app'". + """ + import app.auth.backend as backend_mod + import app.auth.refresh as refresh_mod + import app.config as config_mod + + original = os.environ.get("COOKIE_PREFIX") + os.environ["COOKIE_PREFIX"] = "verify_prefix" + try: + importlib.reload(config_mod) + importlib.reload(backend_mod) + importlib.reload(refresh_mod) + assert backend_mod.cookie_transport.cookie_name == "verify_prefix_access" + assert refresh_mod.REFRESH_COOKIE_NAME == "verify_prefix_refresh" + finally: + if original is None: + os.environ.pop("COOKIE_PREFIX", None) + else: + os.environ["COOKIE_PREFIX"] = original + importlib.reload(config_mod) + importlib.reload(backend_mod) + importlib.reload(refresh_mod)