BBergleandClaude Opus 5
ddea750792
Phase 0's API half: a working FastAPI app with register/login/me/logout, backed by a Postgres schema where row-level security is real and independently proven, not just declared. The core design decision, and the reason this lands as one PR instead of several: request-scoped queries run as `velodrome_app` (NOBYPASSRLS), but looking up identity in the first place — login by email, a session by its token hash — has to happen *before* app.user_id can be set, so those specific lookups run as a second role, `velodrome_auth` (BYPASSRLS), used nowhere else in the codebase. See velodrome/db.py's module docstring and apps/api/README.md for the full rationale. This is genuinely one reviewable unit: the migration, the models, and the auth service only make sense evaluated together, since they're three views of the same invariant. tests/test_auth.py::test_rls_blocks_cross_user_session_reads is the test worth reading first — it doesn't trust the RLS policy SQL because it reads correctly, it proves isolation by registering two users and confirming a scoped read of `sessions` for user A returns exactly one row, never two. Bugs found and fixed while actually running this against real Postgres (everything below was verified against a live postgis/postgis:16-3.4 container and a built Docker image, not just read for correctness): - CREATE ROLE's PASSWORD clause is DDL, not DML — it doesn't accept bind parameters (`PASSWORD $1` is a syntax error). Fixed with dollar-quoting. - Postgres roles are cluster-wide, not per-database — a second database in the same cluster hit "role already exists" on a plain CREATE ROLE. Fixed with a DO block catching duplicate_object. - A bare `Mapped[datetime]` on the ORM models infers a naive timestamp, silently disagreeing with the migration's correct `DateTime(timezone=True)` — asyncpg rejected the mismatch at insert time. Fixed once, at the declarative Base level via type_annotation_map, rather than per-column. - The session cookie's `secure` flag was gated on `!= "development"`, so anything else — including local testing and a real deploy running temporarily without TLS in front — got a Secure cookie no HTTP client will ever send back, breaking every authenticated request after login with no visible error. Gated on `== "production"` instead. - `alembic check` initially flagged every PostGIS/TIGER-installed table (dozens of them) as drift, because they're not in our metadata. A schema-based denylist doesn't work — reflected foreign tables come back with schema=None regardless of their real schema. Fixed with an allowlist keyed on target_metadata.tables instead, which is also more robust against future PostGIS versions adding more tables. - The migration itself was missing `nullable=False` on three timestamp columns that the ORM model assumed were never null — a genuine model/migration drift that alembic check caught once the PostGIS noise above was filtered out. Fixed in 0001 directly, since it's never shipped. - Two indexes the migration creates explicitly weren't declared on the ORM models, causing the same kind of drift. Added index=True to match. Deliberately deferred, not forgotten: per-IP/per-account login rate limiting (docs/PLAN.md mentions it; Phase 0's bar is a working skeleton, and this needs its own design pass) and the procrastinate job runner / worker container (nothing to run yet — arrives with the ingestion pipeline). ci.yml updated to match: the api and migrations jobs now provision the same two runtime roles this code actually needs, replacing the single placeholder DATABASE_URL from before any code existed. Verified: ruff check, ruff format --check, and mypy --strict all clean. 12/12 pytest passing against a real Postgres. Full alembic upgrade -> downgrade -1 -> upgrade cycle run twice (once standalone, once inside a two-database cluster to specifically catch the role-collision bug). alembic check clean. Docker image builds and serves real traffic — register and an authenticated GET /me both exercised against the actual built container, not just the test suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
57 lines
1.5 KiB
TOML
57 lines
1.5 KiB
TOML
[project]
|
|
name = "velodrome"
|
|
version = "0.1.0"
|
|
description = "Self-hosted cycling app API"
|
|
requires-python = ">=3.12"
|
|
dependencies = [
|
|
"fastapi>=0.115",
|
|
"uvicorn[standard]>=0.32",
|
|
"sqlalchemy[asyncio]>=2.0.35",
|
|
"asyncpg>=0.30",
|
|
"alembic>=1.13",
|
|
"pydantic[email]>=2.9",
|
|
"pydantic-settings>=2.6",
|
|
"argon2-cffi>=23.1",
|
|
"uuid6>=2024.7.10",
|
|
]
|
|
|
|
[project.optional-dependencies]
|
|
dev = [
|
|
"ruff>=0.7",
|
|
"mypy>=1.13",
|
|
"pytest>=8.3",
|
|
"pytest-asyncio>=0.24",
|
|
"httpx>=0.27",
|
|
]
|
|
|
|
[build-system]
|
|
requires = ["hatchling"]
|
|
build-backend = "hatchling.build"
|
|
|
|
[tool.hatch.build.targets.wheel]
|
|
packages = ["velodrome"]
|
|
|
|
[tool.ruff]
|
|
line-length = 100
|
|
target-version = "py312"
|
|
|
|
[tool.ruff.lint]
|
|
select = ["E", "F", "I", "UP", "B", "ASYNC"]
|
|
|
|
[tool.ruff.lint.flake8-bugbear]
|
|
# FastAPI's dependency-injection idiom is Depends(...) as a default argument value — that IS
|
|
# the framework's intended pattern (see https://fastapi.tiangolo.com/tutorial/dependencies/),
|
|
# not the mutable-default-argument bug B008 exists to catch. Same reasoning for Query/Body/
|
|
# Cookie/Header — declared now even though only Depends is used yet, so nobody hits this same
|
|
# false positive when the others show up in a later phase.
|
|
extend-immutable-calls = ["fastapi.Depends", "fastapi.Query", "fastapi.Body", "fastapi.Cookie", "fastapi.Header"]
|
|
|
|
[tool.mypy]
|
|
python_version = "3.12"
|
|
strict = true
|
|
plugins = ["pydantic.mypy"]
|
|
|
|
[tool.pytest.ini_options]
|
|
asyncio_mode = "auto"
|
|
testpaths = ["tests"]
|