Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions backend/src/infrastructure/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
21 changes: 9 additions & 12 deletions backend/src/modules/user/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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")

Expand Down
15 changes: 10 additions & 5 deletions backend/src/modules/user/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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")
Expand Down
37 changes: 37 additions & 0 deletions backend/tests/integration/api/v1/users/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
26 changes: 26 additions & 0 deletions backend/tests/integration/api/v1/users/test_read.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
49 changes: 49 additions & 0 deletions backend/tests/integration/auth/test_oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
47 changes: 44 additions & 3 deletions backend/tests/unit/infrastructure/test_middleware.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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


Expand All @@ -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 ===


Expand Down
31 changes: 30 additions & 1 deletion backend/tests/unit/modules/user/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Loading