pdf-quiz-generator/backend/app/routers/uploads.py
Daniel e72cdd6716 feat: a versioned API, refresh tokens, and an end-to-end stack that found four bugs
**The API.** Every route now lives under `/api/v1`, with `/api/...` rewritten
onto it — one route, two spellings, so they cannot drift and the OpenAPI
document describes each endpoint once. Errors carry an `error` object with a
stable code, one human sentence and, for a validation failure, the fields that
were wrong; `detail` is untouched so nothing that reads it breaks. The whole
surface — 320 routes, their parameters and their status codes — is checked in
as `backend/tests/api-contract.json`, and a test fails on any difference,
naming the routes that moved. `docs/api.md` is the contract in prose.

**Refresh tokens**, so an app can stay signed in without keeping a password.
Rows rather than signatures: listable, withdrawable, stored as hashes, rotated
on every use. A spent token coming back ends the whole session, because a theft
and a replay look identical from the server and the safe reading is the unsafe
one. A browser is not given one — it has nowhere to put it and a person to ask.

**An end-to-end stack**: `docker-compose.test.yml` with its own Postgres and
Redis, `e2e/seed.py` for the smallest world the tests name, and Playwright with
five projects — desktop, iPhone, Pixel, iPad and a browserless API project.
Devices because every bug reported this week was a phone bug found by a person
looking at a screenshot; a desktop-only suite would have passed through all of
them. Forty tests, five clean runs.

It found four things in its first hour:

- **A fresh deploy could not start.** `create_all()` ran before
  `CREATE EXTENSION vector`, so any database that had never had pgvector
  installed died on the first table with a vector column. Invisible here
  because this one has had the extension for a year.
- **A figure in a published article was a 404 for everyone but an admin.**
  Media in the library is nobody's to read by default, and nothing made an
  exception for a drawing an article actually shows — so every illustration
  added this week was an empty box for every real user.
- **Every rate limit was one bucket for the whole site.** The backend saw
  nginx's address for every request, so ten bad passwords from anybody locked
  out everybody, and no log line could say who. nginx now takes the real
  address from the proxy and overwrites the header on the way in; uvicorn runs
  with --proxy-headers.
- **The reading page's breakpoints disagreed** — 1150px in the component,
  820px in the stylesheet. Between them the menu button claimed the contents
  drawer and then toggled a class on a rail that was still in the layout: the
  contents did not open and the site menu did not either. The button was dead
  on every tablet.

And two smaller ones: the login limiter counted successful sign-ins, so eleven
people behind one hospital NAT locked each other out — it is cleared by a
correct password now; and `/uploads/{path}` served GET and HEAD from one route
with one operation id, which makes every OpenAPI client generator refuse the
document.

The first admin's password is generated and printed once at first start when
`DEFAULT_ADMIN_PASSWORD` is blank, rather than the account not existing:
`docker compose logs backend | grep -A3 "FIRST ADMIN"`.

CI (`.forgejo/workflows/tests.yml`) runs the backend suite, the contract, the
frontend suite and the build on every push to dev, main or master, and the
end-to-end stack on those branches and on pull requests into them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
2026-09-13 01:23:38 +02:00

117 lines
6.1 KiB
Python

"""Native browser media authentication only; API authentication stays bearer-only."""
import mimetypes
from fastapi import APIRouter, Depends, HTTPException, Request
from fastapi.responses import FileResponse, Response
from sqlalchemy.orm import Session
from app.database import get_db
from app.services import storage_service, thumbnails
from app.utils.auth import get_current_user
from app.utils.upload_access import (
LEGACY_LMS_PREFIXES, local_upload_path, upload_file, references,
document_for_file, can_read_upload, card_deck_ids,
)
router = APIRouter()
# Two decorators rather than one `api_route` with both methods: that produced a
# single operation id for GET and HEAD, and a duplicate operation id makes
# every OpenAPI client generator refuse the document. HEAD answers the same
# way and is not worth a second entry in the contract.
@router.get("/uploads/{path:path}")
@router.head("/uploads/{path:path}", include_in_schema=False)
def read_upload(path: str, request: Request, attempt_id: int | None = None,
w: int | None = None, db: Session = Depends(get_db)):
"""An upload, optionally at one of two smaller widths.
`?w=256` and `?w=640` are the only sizes there are, and anything else is
refused rather than honoured — an endpoint that resizes to whatever the
query string asks for is a CPU sink anybody can point at. The authorisation
below is unchanged and runs first: a thumbnail of a file you may not read
is a file you may not read.
A derivative may be kept by the browser that fetched it; an original may
not. `private` in both cases — never a shared cache, because a shared cache
in front of access-controlled images is how one learner is served another's
private figure. What that leaves is the requester's own browser, which has
already been allowed to see the bytes, and which was re-fetching every
thumbnail on every page for no reason.
A derivative is safe to keep because it cannot change: `thumbs/256/<key>`
is made once from an immutable original and never rewritten. Losing access
to an image does not evict it from that one browser's cache for a week,
which is the honest cost, and a small one for a picture that browser was
entitled to draw yesterday.
"""
headers = {"Cache-Control": "private, no-store", "Vary": "Cookie, Authorization", "X-Content-Type-Options": "nosniff"}
if w is not None and w not in thumbnails.WIDTHS:
raise HTTPException(400, f"Width must be one of {', '.join(map(str, thumbnails.WIDTHS))}",
headers=headers)
try:
return _read_upload(path, request, attempt_id, db, headers, w)
except HTTPException as exc:
exc.headers = {**(exc.headers or {}), **headers}
raise
#: A week in the requester's own browser. `immutable` so a reload does not
#: revalidate it: the derivative is made once from an original that never
#: changes, so there is nothing for a conditional request to discover.
DERIVATIVE_CACHE = "private, max-age=604800, immutable"
def _read_upload(path, request, attempt_id, db, headers, width=None):
try:
path = local_upload_path(path)
target = upload_file(path)
except HTTPException:
raise HTTPException(404, "File not found")
questions = references(db, path)
cards = card_deck_ids(db, path)
# Default private also prevents orphaned question/card files becoming anonymous.
protected = (not path.startswith(LEGACY_LMS_PREFIXES) or document_for_file(db, path)
or questions or cards)
if protected:
authorization = request.headers.get("authorization", "")
token = authorization[7:] if authorization.lower().startswith("bearer ") else request.cookies.get("pedshub_media", "")
user = get_current_user(token, db)
if not can_read_upload(db, path, user, questions, cards, attempt_id):
raise HTTPException(404, "File not found")
# Known legacy LMS directories retain their policy; this is not a whole-LMS audit.
if target.suffix.lower() == ".svg":
headers["Content-Security-Policy"] = "sandbox; default-src 'none'; style-src 'unsafe-inline'"
# Asked for small, and small exists: the derivative is made on the first
# ask and stored beside the original. `None` back means there is nothing
# smaller worth serving — not an image, or already narrower than asked —
# so the original goes out, which is what the caller wanted anyway.
# A width asks for a smaller copy. A format no browser draws asks for one
# too, whatever size it was requested at: 21 stem figures here are JPEG
# 2000, which Chrome dropped in 2015 and Firefox never had, and which the
# slim base image does not even have a MIME type for — so they were going
# out as `application/octet-stream` under `nosniff` and rendering nowhere.
# The bytes are fine; Pillow reads them. Only the delivery had to change.
if width or thumbnails.needs_converting(path):
small = thumbnails.get(path, width)
if small is not None:
cached = {**headers, "Cache-Control": DERIVATIVE_CACHE}
if request.method == "HEAD":
return Response(status_code=200, media_type="image/webp",
headers={**cached, "Content-Length": str(len(small))})
return Response(content=small, media_type="image/webp", headers=cached)
# Serving must go through the storage service, or object storage would be
# write-only: the bytes would be in the bucket and still read from disk.
# The path is already authorised and confined to the upload root above.
if target.is_file():
return FileResponse(target, headers=headers)
data = storage_service.s3_object(path)
if data is None:
raise HTTPException(404, "File not found")
media_type = mimetypes.guess_type(path)[0] or "application/octet-stream"
if request.method == "HEAD":
return Response(status_code=200, headers={**headers, "Content-Length": str(len(data))},
media_type=media_type)
return Response(content=data, media_type=media_type, headers=headers)