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>
92 lines
3.8 KiB
Python
92 lines
3.8 KiB
Python
from fastapi import APIRouter, Depends, HTTPException, Request, Response, status
|
|
|
|
from velodrome.auth import service
|
|
from velodrome.auth.dependencies import get_current_user, require_same_origin_for_cookie_auth
|
|
from velodrome.auth.service import AuthenticatedSession
|
|
from velodrome.config import get_settings
|
|
from velodrome.schemas.auth import LoginRequest, RegisterRequest, UserOut
|
|
|
|
router = APIRouter(prefix="/auth", tags=["auth"])
|
|
|
|
|
|
def _set_session_cookie(response: Response, raw_token: str) -> None:
|
|
settings = get_settings()
|
|
# secure=True only in production, not merely "not development": a Secure cookie is never
|
|
# sent back by any client over plain HTTP, by spec. Gating on anything other than an
|
|
# explicit "production" (e.g. the previous `!= "development"`) breaks every non-dev
|
|
# environment served without TLS in front — including local testing and a real self-hosted
|
|
# deploy the operator is running temporarily without a reverse proxy — which fails silently
|
|
# as "login succeeds but every subsequent request looks unauthenticated."
|
|
response.set_cookie(
|
|
key=settings.session_cookie_name,
|
|
value=raw_token,
|
|
httponly=True,
|
|
secure=settings.environment == "production",
|
|
samesite="lax",
|
|
max_age=settings.session_ttl_days * 24 * 60 * 60,
|
|
path="/",
|
|
)
|
|
|
|
|
|
@router.post("/register", response_model=UserOut, status_code=status.HTTP_201_CREATED)
|
|
async def register(body: RegisterRequest, response: Response) -> UserOut:
|
|
try:
|
|
await service.register(
|
|
email=body.email,
|
|
password=body.password,
|
|
display_name=body.display_name,
|
|
invite_code=body.invite_code,
|
|
)
|
|
except service.InvalidInvite as exc:
|
|
raise HTTPException(status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc
|
|
except service.EmailAlreadyRegistered as exc:
|
|
raise HTTPException(status.HTTP_409_CONFLICT, detail=str(exc)) from exc
|
|
|
|
# A second call, not a shortcut through register()'s own result — this keeps session
|
|
# creation in exactly one place (service.login) rather than duplicating it, at the cost of
|
|
# one redundant password verification. See auth/service.py if that cost ever matters.
|
|
raw_token, session_info = await service.login(
|
|
email=body.email, password=body.password, client="web", user_agent=None, ip=None
|
|
)
|
|
_set_session_cookie(response, raw_token)
|
|
return UserOut(
|
|
id=session_info.user_id, email=session_info.email, display_name=session_info.display_name
|
|
)
|
|
|
|
|
|
@router.post("/login", response_model=UserOut)
|
|
async def login(body: LoginRequest, request: Request, response: Response) -> UserOut:
|
|
try:
|
|
raw_token, session_info = await service.login(
|
|
email=body.email,
|
|
password=body.password,
|
|
client="web",
|
|
user_agent=request.headers.get("user-agent"),
|
|
ip=request.client.host if request.client else None,
|
|
)
|
|
except service.InvalidCredentials as exc:
|
|
raise HTTPException(status.HTTP_401_UNAUTHORIZED, detail=str(exc)) from exc
|
|
|
|
_set_session_cookie(response, raw_token)
|
|
return UserOut(
|
|
id=session_info.user_id, email=session_info.email, display_name=session_info.display_name
|
|
)
|
|
|
|
|
|
@router.get("/me", response_model=UserOut)
|
|
async def me(current: AuthenticatedSession = Depends(get_current_user)) -> UserOut:
|
|
return UserOut(id=current.user_id, email=current.email, display_name=current.display_name)
|
|
|
|
|
|
@router.post(
|
|
"/logout",
|
|
status_code=status.HTTP_204_NO_CONTENT,
|
|
dependencies=[Depends(require_same_origin_for_cookie_auth)],
|
|
)
|
|
async def logout(request: Request, response: Response) -> None:
|
|
settings = get_settings()
|
|
raw_token = request.cookies.get(settings.session_cookie_name)
|
|
if raw_token:
|
|
await service.logout(raw_token)
|
|
response.delete_cookie(settings.session_cookie_name, path="/")
|