diff --git a/backend/src/infrastructure/middleware.py b/backend/src/infrastructure/middleware.py index 49fb47bf..1746656f 100644 --- a/backend/src/infrastructure/middleware.py +++ b/backend/src/infrastructure/middleware.py @@ -7,24 +7,52 @@ # Two years, matching the HSTS preload-list requirement. HSTS_MAX_AGE_SECONDS = 63072000 +# Paths whose bytes are the same for every caller. SQLAdmin serves its own CSS and +# JS from ``/statics`` under the panel's mount point. +PUBLIC_CACHE_PREFIXES: tuple[str, ...] = ("/admin/statics/",) + class ClientCacheMiddleware(BaseHTTPMiddleware): - """Set Cache-Control headers. + """Set Cache-Control headers, withholding caching by default. + + Only the paths in ``public_prefixes`` are publicly cacheable. Everything else + is ``private, no-store``: the admin panel, the docs and the health endpoint + all answer with per-caller data or state, and a shared cache that kept one + response could hand it to the next visitor. API paths keep the longer + no-store wording they already had. A response that set its own + ``Cache-Control`` is left alone, so a route can opt in to caching. - API endpoints get no-cache (authenticated, dynamic data). - Static assets get public caching with the configured max_age. + Args: + app: The ASGI app to wrap. + max_age: Seconds a public response may be cached for. + public_prefixes: Path prefixes to mark publicly cacheable; pass your own + to add an application's static mount. """ - def __init__(self, app: ASGIApp, max_age: int = 60) -> None: + def __init__( + self, + app: ASGIApp, + max_age: int = 60, + public_prefixes: tuple[str, ...] = PUBLIC_CACHE_PREFIXES, + ) -> None: super().__init__(app) self.max_age: int = max_age + self.public_prefixes: tuple[str, ...] = public_prefixes async def dispatch(self, request: Request, call_next: RequestResponseEndpoint) -> Response: response: Response = await call_next(request) - if request.url.path.startswith("/api/"): + + if "cache-control" in response.headers: + return response + + path = request.url.path + if path.startswith("/api/"): response.headers["Cache-Control"] = "private, no-cache, no-store, must-revalidate" - else: + elif path.startswith(self.public_prefixes): response.headers["Cache-Control"] = f"public, max-age={self.max_age}" + else: + response.headers["Cache-Control"] = "private, no-store" + return response diff --git a/backend/src/modules/user/schemas.py b/backend/src/modules/user/schemas.py index 1ce1bd12..1d189653 100644 --- a/backend/src/modules/user/schemas.py +++ b/backend/src/modules/user/schemas.py @@ -52,11 +52,8 @@ class UserProfileRead(BaseModel): """ id: int - name: Annotated[str, Field(min_length=2, max_length=30, examples=["User Userson"])] - username: Annotated[ - str, - Field(min_length=2, max_length=20, pattern=r"^[a-z0-9]+$", examples=["userson"]), - ] + name: Annotated[str, Field(examples=["User Userson"])] + username: Annotated[str, Field(examples=["userson"])] profile_image_url: str tier_id: int | None = None @@ -80,15 +77,15 @@ class UserRead(BaseModel): class UserCreate(UserBase): - """Schema for creating a new user.""" + """Schema for creating a new user. + + Signing up never sets the OAuth identifiers or ``email_verified``: a self-declared + verified address would pre-claim it, and crudauth links a provider login to an + existing account by verified email. Those fields belong to + ``UserCreateInternal``, which only server-side code builds. + """ password: password_policy.body_field() # type: ignore[valid-type] - google_id: str | None = None - github_id: str | None = None - oauth_provider: str | None = None - email_verified: bool = False - oauth_created_at: datetime | None = None - oauth_updated_at: datetime | None = None model_config = ConfigDict(extra="forbid") diff --git a/backend/src/modules/user/service.py b/backend/src/modules/user/service.py index df12658c..5c2d7e5f 100644 --- a/backend/src/modules/user/service.py +++ b/backend/src/modules/user/service.py @@ -70,6 +70,10 @@ async def create(self, user: UserCreate, db: AsyncSession) -> dict[str, Any]: Passwords are automatically hashed using secure password hashing. Both email and username must be unique across the system. + Only the four fields above are read from the payload. The verification + flag and the OAuth identifiers decide which account a provider login + resolves to, so a password signup always leaves them at their defaults. + Example: ```python user_data = UserCreate( @@ -89,11 +93,12 @@ async def create(self, user: UserCreate, db: AsyncSession) -> dict[str, Any]: if username_exists: raise UserExistsError("Username already taken") - user_internal_dict = user.model_dump() - user_internal_dict["hashed_password"] = await get_password_hash_async(user_internal_dict["password"]) - del user_internal_dict["password"] - - user_internal = UserCreateInternal(**user_internal_dict) + user_internal = UserCreateInternal( + name=user.name, + username=user.username, + email=user.email, + hashed_password=await get_password_hash_async(user.password), + ) created_user = await crud_users.create(db=db, object=user_internal, schema_to_select=UserRead) if not created_user: raise PersistenceError("User row was not returned after insert") diff --git a/backend/tests/integration/api/v1/users/test_create.py b/backend/tests/integration/api/v1/users/test_create.py index fd9ce702..92787def 100644 --- a/backend/tests/integration/api/v1/users/test_create.py +++ b/backend/tests/integration/api/v1/users/test_create.py @@ -4,6 +4,7 @@ import pytest from faker import Faker from httpx import AsyncClient +from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from src.modules.user.models import User @@ -144,3 +145,39 @@ async def test_signup_names_every_missing_class_at_once(client: AsyncClient, db_ assert response.status_code == 422 requirements = {error["ctx"]["requirement"] for error in response.json()["detail"]} assert {"uppercase", "digit", "special"} <= requirements + + +@pytest.mark.parametrize( + "field, value", + [ + ("email_verified", True), + ("google_id", "attacker-google-sub"), + ("github_id", "attacker-github-id"), + ("oauth_provider", "google"), + ("oauth_created_at", "2026-01-01T00:00:00Z"), + ("oauth_updated_at", "2026-01-01T00:00:00Z"), + ], +) +async def test_signup_refuses_the_oauth_fields(client: AsyncClient, db_session: AsyncSession, field: str, value): + """Signing up as verified would pre-claim the address before its owner uses a provider login.""" + payload = {**generate_unique_user_data(), field: value} + + response = await client.post("/api/v1/users/", json=payload) + + assert response.status_code == 422 + stored = await db_session.scalar(select(User).where(User.email == payload["email"])) + assert stored is None + + +async def test_signup_leaves_the_address_unverified(client: AsyncClient, db_session: AsyncSession): + """Only a provider login or a verification email may mark an address verified.""" + payload = generate_unique_user_data() + + response = await client.post("/api/v1/users/", json=payload) + + assert response.status_code == 201 + stored = await db_session.scalar(select(User).where(User.email == payload["email"])) + assert stored is not None + assert stored.email_verified is False + assert stored.google_id is None + assert stored.oauth_provider is None diff --git a/backend/tests/integration/api/v1/users/test_read.py b/backend/tests/integration/api/v1/users/test_read.py index 7aa1c560..cdcdac2d 100644 --- a/backend/tests/integration/api/v1/users/test_read.py +++ b/backend/tests/integration/api/v1/users/test_read.py @@ -121,3 +121,29 @@ async def test_profile_lookup_never_carries_an_email(auth_client: AsyncClient, t assert data["username"] == test_user_2["username"] assert "email" not in data assert "is_superuser" not in data + + +@pytest.mark.parametrize( + "username", + [ + "with_underscore", + "a" * 25, + ], +) +async def test_a_username_signup_accepts_can_be_looked_up(client: AsyncClient, auth_client: AsyncClient, username: str): + """The profile response must describe what the column holds, not a stricter rule.""" + signup = await client.post( + "/api/v1/users/", + json={ + "name": "Lookup Target", + "username": username, + "email": f"{username}@example.com", + "password": "Password123!", + }, + ) + assert signup.status_code == 201 + + response = await auth_client.get(f"/api/v1/users/{username}") + + assert response.status_code == 200 + assert response.json()["username"] == username diff --git a/backend/tests/integration/auth/test_oauth.py b/backend/tests/integration/auth/test_oauth.py index 12ee871d..87af6eb1 100644 --- a/backend/tests/integration/auth/test_oauth.py +++ b/backend/tests/integration/auth/test_oauth.py @@ -203,3 +203,52 @@ async def test_the_auth_paths_keep_their_existing_contract(): "/api/v1/auth/oauth/callback/{provider}", ): assert path in paths + + +async def test_a_provider_login_claims_an_account_that_was_signed_up_for( + client: AsyncClient, db_session: AsyncSession, monkeypatch +): + """Signing up with someone's address must not survive that address proving itself. + + crudauth claims an unverified account when a verified provider login matches its + email: the password stops working, so whoever registered first can't keep access. + """ + signup = await client.post( + "/api/v1/users/", + json={ + "name": "Ada Lovelace", + "username": "adalovelace", + "email": "ada.lovelace@example.com", + "password": "Registered1!", + }, + ) + assert signup.status_code == 201 + + _stub_google( + monkeypatch, + { + "sub": "google-ada", + "email": "ada.lovelace@example.com", + "email_verified": True, + "name": "Ada Lovelace", + }, + ) + state = await _start(client) + callback = await client.get( + "/api/v1/auth/oauth/callback/google", + params={"code": "the-code", "state": state}, + follow_redirects=False, + ) + assert callback.status_code == 307 + client.cookies.clear() + + login = await client.post( + "/api/v1/auth/login", + data={"username": "adalovelace", "password": "Registered1!"}, + ) + + assert login.status_code == 401 + claimed = (await db_session.execute(select(User).where(User.email == "ada.lovelace@example.com"))).scalar_one() + await db_session.refresh(claimed) + assert claimed.google_id == "google-ada" + assert claimed.email_verified is True diff --git a/backend/tests/unit/infrastructure/test_middleware.py b/backend/tests/unit/infrastructure/test_middleware.py index 37fbc81b..627845d9 100644 --- a/backend/tests/unit/infrastructure/test_middleware.py +++ b/backend/tests/unit/infrastructure/test_middleware.py @@ -1,7 +1,7 @@ """Tests for middleware components.""" import pytest -from fastapi import FastAPI +from fastapi import FastAPI, Response from httpx import ASGITransport, AsyncClient from src.infrastructure.middleware import ClientCacheMiddleware, SecurityHeadersMiddleware @@ -28,6 +28,23 @@ async def api_route(): async def static_route(): return {"file": "logo"} + @app.get("/admin/statics/css/main.css") + async def admin_static_route(): + return {"file": "css"} + + @app.get("/admin/user/list") + async def admin_page_route(): + return {"page": "users"} + + @app.get("/health") + async def health_route(): + return {"status": "ok"} + + @app.get("/downloads/report.csv") + async def route_that_sets_its_own_header(response: Response): + response.headers["Cache-Control"] = "public, max-age=31536000, immutable" + return {"file": "report"} + return app @@ -45,15 +62,39 @@ async def test_api_paths_get_no_cache(): @pytest.mark.asyncio -async def test_static_paths_get_public_cache(): +async def test_admin_statics_get_public_cache(): + """The panel's own CSS and JS are the same bytes for everyone.""" app = _create_app_with_middleware(cache=True, max_age=120) async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: - resp = await client.get("/static/logo.png") + resp = await client.get("/admin/statics/css/main.css") assert resp.status_code == 200 assert resp.headers["cache-control"] == "public, max-age=120" +@pytest.mark.asyncio +@pytest.mark.parametrize("path", ["/admin/user/list", "/health", "/static/logo.png"]) +async def test_everything_else_is_private_and_unstored(path: str): + """An admin page holds another user's data; a shared cache must not serve it on.""" + app = _create_app_with_middleware(cache=True, max_age=120) + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + resp = await client.get(path) + + assert resp.status_code == 200 + assert resp.headers["cache-control"] == "private, no-store" + + +@pytest.mark.asyncio +async def test_a_header_the_route_set_is_left_alone(): + """A route that knows its response is cacheable keeps saying so.""" + app = _create_app_with_middleware(cache=True, max_age=120) + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + resp = await client.get("/downloads/report.csv") + + assert resp.status_code == 200 + assert resp.headers["cache-control"] == "public, max-age=31536000, immutable" + + # === SecurityHeadersMiddleware === diff --git a/backend/tests/unit/modules/user/test_service.py b/backend/tests/unit/modules/user/test_service.py index 5b9c629a..548466bb 100644 --- a/backend/tests/unit/modules/user/test_service.py +++ b/backend/tests/unit/modules/user/test_service.py @@ -3,7 +3,7 @@ import pytest from src.modules.common.exceptions import PermissionDeniedError -from src.modules.user.schemas import UserUpdate +from src.modules.user.schemas import UserCreate, UserUpdate from src.modules.user.service import UserService UPDATE = "user.update" @@ -95,3 +95,32 @@ def test_submitting_the_email_a_user_already_has_is_not_a_change(user_service: U frozenset({UPDATE}), frozenset(), ) + + +# ============================================================================= +# What signup is allowed to write +# ============================================================================= +class SignupWithServerFields(UserCreate): + """A signup payload that carries columns only the server may set.""" + + model_config = {"extra": "allow"} + + email_verified: bool = True + google_id: str | None = "smuggled-google-sub" + oauth_provider: str | None = "google" + + +async def test_signup_cannot_write_the_verification_or_oauth_columns(user_service: UserService, db_session): + """The public schema refuses these fields; creating the row must not trust them either.""" + created = await user_service.create( + SignupWithServerFields( + name="Smuggler", + username="smuggler", + email="smuggler@example.com", + password="Password123!", + ), + db_session, + ) + + assert created["email_verified"] is False + assert created["oauth_provider"] is None