Reverses a shipped, tested, merged decision (D4/PR #2) rather than building on it — see docs/DECISIONS.md D15 for the full record: what was rejected (Postgres as a second container; Postgres+PostGIS bundled inside the single container via a supervisor), what this costs (no database-level RLS, no PostGIS, procrastinate needs replacing — all stated as a concern before this was decided, and reaffirmed anyway, which is the user's call to make about their own instance). The one invariant-critical consequence: isolation between users now rests entirely on the repository-layer scope (db.py's `Scope.select()`), not two layers. CLAUDE.md's invariant #4 is revised accordingly. This is not a downgrade-and-hope — `Scope` is built so an unfiltered query against a user-owned table is structurally harder to write than a scoped one (there is no method on `Scope` that returns one), and tests/test_auth.py::test_scoped_session_blocks_cross_user_reads replaces the old RLS proof with the same empirical standard: it doesn't trust the query builder filters correctly because the code reads correctly, it registers two real users and checks. test_unscoped_session_can_see_every_user_when_misused is the deliberately alarming companion — it demonstrates exactly what a reviewer must now catch, since nothing else will. Six real, non-obvious SQLite behaviours found and fixed by actually running this against a real file, not assumed from docs: - Foreign keys, ON DELETE CASCADE included, are OFF by default per connection — deleting a user silently left orphaned sessions/api_tokens, no error either way. Fixed with PRAGMA foreign_keys=ON on every connect. - Transactions default to DEFERRED, which only takes a write lock on the first actual write — a real check-then-act race for invite redemption (two concurrent redemptions could both read used_count < max_uses as true before either commits). Fixed by disabling the driver's implicit BEGIN and issuing BEGIN IMMEDIATE ourselves — SQLAlchemy's own documented recipe for this, not improvised. - DateTime(timezone=True) does NOT round-trip tzinfo on SQLite — a tz-aware datetime goes in, a naive one comes back out, and every `expires_at < datetime.now(UTC)` comparison in auth/service.py then raises TypeError. Fixed once at the Base level with a UTCDateTime TypeDecorator rather than per-column. - Uuid(as_uuid=True) stores as 32-char hex with NO hyphens on SQLite, not str(uuid)'s hyphenated form. A test fixture that raw-inserted the hyphenated form left rows the ORM's own later UPDATE (via invite.used_count += 1's autoflush) could never match by primary key, updating zero rows and raising StaleDataError. Fixed by using .hex to match exactly what the ORM itself writes. - BEGIN IMMEDIATE applies to every transaction, reads included — a long-lived test fixture that autobegins a transaction via a bare read and never explicitly closes it holds SQLite's exclusive write lock for the rest of the test, and a later scoped_session() call fails with "database is locked". Not an app-code bug (every real session block closes cleanly on exit), but real enough to document since the next person writing a test against the db_auth fixture will hit it too. - Python's sqlite3 module deprecates its own implicit datetime adapter as of 3.12 — silent today, warns on every raw-SQL datetime bind. Only ever hit test fixture code (the ORM path never uses it, confirmed by running the ORM-only health test with warnings promoted to errors and it stayed clean); fixed there with an explicit .isoformat() rather than left for a future Python version to turn into a real failure. Also, since with_for_update() silently no-ops on SQLite (confirmed — SQLAlchemy emits no SQL for it, no error either) rather than actually locking anything: removed it from register()'s invite-redemption query and corrected the comment to attribute the concurrency guarantee to BEGIN IMMEDIATE, where it now actually lives. One PR, not several, for the same reason PR #2 was: the migration, the models, db.py, and the docs recording why are five views of one decision — splitting them wouldn't make review easier, just disconnected. 552 insertions / 548 deletions across 17 files, most of it necessarily touching what PR #2 shipped rather than net-new code. Deliberately deferred, not solved here: PostGIS's replacement for spatial storage, procrastinate's replacement for background jobs, and the EXCLUDE USING gist constraint's replacement for component_installs — none of those tables exist yet (Phase 1-2), so none of it is broken, and docs/DECISIONS.md D15 records exactly what each future phase needs to decide before it can be built. .gitea/workflows/deploy pipeline (PR #4, built for the old 3-container Postgres compose stack) was closed as superseded rather than merged; the single-container image build is follow-up work, not part of this change. Verified: ruff check, ruff format --check, and mypy --strict all clean. 13/13 pytest passing against a real SQLite file, including with DeprecationWarning promoted to an error (confirms the sqlite3 adapter deprecation fix actually holds, not just that it's quiet by default). Full alembic upgrade -> downgrade -1 -> upgrade cycle run clean. alembic check clean with no include_object filter needed at all now (SQLite starts with nothing but what our own migrations create — no PostGIS/TIGER noise to filter out in the first place). CI's exact migration command sequence reproduced locally end to end before touching the workflow file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
342 lines
14 KiB
Python
342 lines
14 KiB
Python
"""Auth endpoint tests, plus — the one that matters most — an empirical proof that the
|
|
repository-layer scope actually enforces isolation between users.
|
|
|
|
Per CLAUDE.md invariant #4: since D15 removed Postgres RLS, `Scope` in db.py is not one layer of
|
|
isolation among two — it's the only one. test_scoped_session_blocks_cross_user_reads below doesn't
|
|
trust that `Scope.select()` filters correctly because the code reads correctly; it proves it by
|
|
registering two real users and confirming a scoped read for user A never returns user B's row,
|
|
even when both exist in the same table.
|
|
"""
|
|
|
|
from datetime import UTC, datetime, timedelta
|
|
from uuid import UUID, uuid4
|
|
|
|
import httpx
|
|
import pytest
|
|
from sqlalchemy import select, text
|
|
from sqlalchemy.ext.asyncio import AsyncSession
|
|
|
|
from velodrome.auth.security import hash_invite_code, hash_password
|
|
from velodrome.db import scoped_session, unscoped_session
|
|
from velodrome.models import Invite, Session
|
|
|
|
_REGISTER_PASSWORD = "correct horse battery staple"
|
|
|
|
|
|
async def _seed_invite(db_auth: AsyncSession, *, code: str = "TESTCODE123") -> UUID:
|
|
"""Insert a usable invite directly, bypassing the API — this is fixture setup, not the
|
|
thing under test.
|
|
|
|
UUID values are passed as `.hex` (32 hex chars, no hyphens), not `str()` (36 chars, hyphenated)
|
|
or a raw UUID object — a `text()` query has no ORM-level column-type awareness, so this is
|
|
fixture setup working around two separate things confirmed empirically, not assumed:
|
|
(1) aiosqlite's driver has no built-in adapter for a Python `UUID` object at all (raises
|
|
"type 'UUID' is not supported"); (2) SQLAlchemy's `Uuid(as_uuid=True)` column type stores as
|
|
the 32-char hex form on SQLite, NOT the hyphenated `str()` form — inserting the hyphenated
|
|
form via raw SQL left rows the ORM's own later `UPDATE ... WHERE id = ?` (implicitly issued
|
|
by `invite.used_count += 1` in auth/service.py) could never match, silently updating zero
|
|
rows. `.hex` is what the ORM itself writes, so raw-SQL-inserted rows are indistinguishable
|
|
from ORM-inserted ones.
|
|
|
|
`creator_id` is a fixed sentinel, not a fresh `uuid4()` per call — confirmed the hard way: a
|
|
test calling this twice (two invite codes) with a fresh id each time but the same hardcoded
|
|
seed email hit the `ON CONFLICT DO NOTHING` on email on the second call, which silently
|
|
no-ops, leaving that call's fresh id never actually inserted — so its invite's `created_by`
|
|
pointed at a user row that was never created, and the FK constraint on the invites insert
|
|
failed. A stable id makes repeat calls genuinely idempotent instead of just quiet about it.
|
|
"""
|
|
# datetime binds use .isoformat() explicitly, not a raw datetime object — Python's sqlite3
|
|
# module has its own implicit datetime adapter, which is deprecated as of 3.12 and warns on
|
|
# every use (confirmed against this exact fixture); the ORM's own UTCDateTime type never hits
|
|
# this because SQLAlchemy's dialect handles the conversion itself rather than delegating to
|
|
# sqlite3's adapter, but a bare text() bind param has no such handling.
|
|
creator_id = UUID(int=0)
|
|
await db_auth.execute(
|
|
text(
|
|
"INSERT INTO users (id, email, display_name, password_hash, created_at) "
|
|
"VALUES (:id, 'seed@example.com', 'Seed', :ph, :now) ON CONFLICT DO NOTHING"
|
|
),
|
|
{"id": creator_id.hex, "ph": hash_password("unused"), "now": datetime.now(UTC).isoformat()},
|
|
)
|
|
await db_auth.execute(
|
|
text(
|
|
"INSERT INTO invites (id, code_hash, created_by, expires_at, max_uses, used_count) "
|
|
"VALUES (:id, :hash, :creator, :expires, 1, 0)"
|
|
),
|
|
{
|
|
"id": uuid4().hex,
|
|
"hash": hash_invite_code(code),
|
|
"creator": creator_id.hex,
|
|
"expires": (datetime.now(UTC) + timedelta(days=1)).isoformat(),
|
|
},
|
|
)
|
|
await db_auth.commit()
|
|
return creator_id
|
|
|
|
|
|
async def test_register_with_valid_invite_creates_session(
|
|
client: httpx.AsyncClient, db_auth: AsyncSession
|
|
) -> None:
|
|
await _seed_invite(db_auth)
|
|
resp = await client.post(
|
|
"/api/v1/auth/register",
|
|
json={
|
|
"email": "rider@example.com",
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "Rider",
|
|
"invite_code": "TESTCODE123",
|
|
},
|
|
)
|
|
assert resp.status_code == 201, resp.text
|
|
assert resp.json()["email"] == "rider@example.com"
|
|
assert "vd_session" in resp.cookies
|
|
|
|
|
|
async def test_register_rejects_unknown_invite_code(client: httpx.AsyncClient) -> None:
|
|
resp = await client.post(
|
|
"/api/v1/auth/register",
|
|
json={
|
|
"email": "rider@example.com",
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "Rider",
|
|
"invite_code": "no-such-code",
|
|
},
|
|
)
|
|
assert resp.status_code == 400
|
|
|
|
|
|
async def test_register_rejects_reused_invite(
|
|
client: httpx.AsyncClient, db_auth: AsyncSession
|
|
) -> None:
|
|
await _seed_invite(db_auth)
|
|
body = {
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "Rider",
|
|
"invite_code": "TESTCODE123",
|
|
}
|
|
first = await client.post("/api/v1/auth/register", json={**body, "email": "first@example.com"})
|
|
assert first.status_code == 201
|
|
|
|
second = await client.post(
|
|
"/api/v1/auth/register", json={**body, "email": "second@example.com"}
|
|
)
|
|
assert second.status_code == 400
|
|
|
|
|
|
async def test_login_with_correct_password_succeeds(
|
|
client: httpx.AsyncClient, db_auth: AsyncSession
|
|
) -> None:
|
|
await _seed_invite(db_auth)
|
|
await client.post(
|
|
"/api/v1/auth/register",
|
|
json={
|
|
"email": "rider@example.com",
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "Rider",
|
|
"invite_code": "TESTCODE123",
|
|
},
|
|
)
|
|
resp = await client.post(
|
|
"/api/v1/auth/login",
|
|
json={"email": "rider@example.com", "password": _REGISTER_PASSWORD},
|
|
)
|
|
assert resp.status_code == 200
|
|
assert "vd_session" in resp.cookies
|
|
|
|
|
|
async def test_login_with_wrong_password_fails(
|
|
client: httpx.AsyncClient, db_auth: AsyncSession
|
|
) -> None:
|
|
await _seed_invite(db_auth)
|
|
await client.post(
|
|
"/api/v1/auth/register",
|
|
json={
|
|
"email": "rider@example.com",
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "Rider",
|
|
"invite_code": "TESTCODE123",
|
|
},
|
|
)
|
|
resp = await client.post(
|
|
"/api/v1/auth/login",
|
|
json={"email": "rider@example.com", "password": "wrong password entirely"},
|
|
)
|
|
assert resp.status_code == 401
|
|
|
|
|
|
async def test_me_requires_authentication(client: httpx.AsyncClient) -> None:
|
|
resp = await client.get("/api/v1/auth/me")
|
|
assert resp.status_code == 401
|
|
|
|
|
|
async def test_me_returns_current_user_with_valid_session(
|
|
client: httpx.AsyncClient, db_auth: AsyncSession
|
|
) -> None:
|
|
await _seed_invite(db_auth)
|
|
await client.post(
|
|
"/api/v1/auth/register",
|
|
json={
|
|
"email": "rider@example.com",
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "Rider",
|
|
"invite_code": "TESTCODE123",
|
|
},
|
|
)
|
|
resp = await client.get("/api/v1/auth/me")
|
|
assert resp.status_code == 200
|
|
assert resp.json()["email"] == "rider@example.com"
|
|
|
|
|
|
async def test_logout_revokes_the_session(client: httpx.AsyncClient, db_auth: AsyncSession) -> None:
|
|
await _seed_invite(db_auth)
|
|
await client.post(
|
|
"/api/v1/auth/register",
|
|
json={
|
|
"email": "rider@example.com",
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "Rider",
|
|
"invite_code": "TESTCODE123",
|
|
},
|
|
)
|
|
logout_resp = await client.post(
|
|
"/api/v1/auth/logout", headers={"Origin": "http://localhost:5173"}
|
|
)
|
|
assert logout_resp.status_code == 204
|
|
|
|
me_resp = await client.get("/api/v1/auth/me")
|
|
assert me_resp.status_code == 401
|
|
|
|
|
|
async def test_logout_without_matching_origin_is_rejected(
|
|
client: httpx.AsyncClient, db_auth: AsyncSession
|
|
) -> None:
|
|
await _seed_invite(db_auth)
|
|
await client.post(
|
|
"/api/v1/auth/register",
|
|
json={
|
|
"email": "rider@example.com",
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "Rider",
|
|
"invite_code": "TESTCODE123",
|
|
},
|
|
)
|
|
resp = await client.post("/api/v1/auth/logout", headers={"Origin": "https://evil.example"})
|
|
assert resp.status_code == 403
|
|
|
|
|
|
async def test_scoped_session_blocks_cross_user_reads(
|
|
client: httpx.AsyncClient, db_auth: AsyncSession
|
|
) -> None:
|
|
"""The load-bearing test: prove the repository-layer scope actually enforces isolation, not
|
|
just that `Scope.select()` reads like it should.
|
|
|
|
Two users register (creating two session rows). We then open `scoped_session` as user A — the
|
|
exact code path every real request handler will use once Phase 1 adds user-owned domain
|
|
tables — and confirm `Scope.select(Session)` returns ONLY user A's row, even though both exist
|
|
in the same table. If this test ever passes with more than one row, the scope is not isolating
|
|
users and every other invariant in this codebase is resting on nothing (see CLAUDE.md
|
|
invariant #4 — there is no database-level backstop anymore; this IS the isolation boundary).
|
|
"""
|
|
await _seed_invite(db_auth, code="CODE-FOR-A")
|
|
await _seed_invite(db_auth, code="CODE-FOR-B")
|
|
|
|
resp_a = await client.post(
|
|
"/api/v1/auth/register",
|
|
json={
|
|
"email": "user-a@example.com",
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "User A",
|
|
"invite_code": "CODE-FOR-A",
|
|
},
|
|
)
|
|
user_a_id = UUID(resp_a.json()["id"])
|
|
|
|
resp_b = await client.post(
|
|
"/api/v1/auth/register",
|
|
json={
|
|
"email": "user-b@example.com",
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "User B",
|
|
"invite_code": "CODE-FOR-B",
|
|
},
|
|
)
|
|
user_b_id = UUID(resp_b.json()["id"])
|
|
assert user_a_id != user_b_id
|
|
|
|
# Sanity check first: BOTH sessions genuinely exist, via the pre-identity bootstrap path.
|
|
total = (await db_auth.execute(text("SELECT count(*) FROM sessions"))).scalar_one()
|
|
assert total == 2
|
|
# Close out the transaction this read just autobegan. db.py's BEGIN IMMEDIATE (see its
|
|
# docstring) takes SQLite's exclusive write lock the instant ANY transaction starts, read
|
|
# included — confirmed the hard way: without this commit, the db_auth fixture's session
|
|
# (which the pytest fixture keeps open for the whole test, not just this block) holds that
|
|
# lock indefinitely, and the scoped_session() calls below then fail with "database is
|
|
# locked" trying to acquire their own. Every write in this file already commits promptly;
|
|
# a read needs the same discipline once BEGIN IMMEDIATE is in play.
|
|
await db_auth.commit()
|
|
|
|
# Now the real test: Scope.select(), which is the only way application code is meant to
|
|
# query a user-owned table once identity is known.
|
|
async with scoped_session(user_a_id) as scope:
|
|
rows = (await scope.session.execute(scope.select(Session))).scalars().all()
|
|
|
|
assert len(rows) == 1, (
|
|
f"expected exactly 1 row (user A's own session), got {len(rows)} — "
|
|
"the repository-layer scope is not isolating users"
|
|
)
|
|
assert rows[0].user_id == user_a_id
|
|
|
|
# And the mirror image, as user B, proving this isn't a coincidence of row ordering.
|
|
async with scoped_session(user_b_id) as scope:
|
|
rows_b = (await scope.session.execute(scope.select(Session))).scalars().all()
|
|
assert len(rows_b) == 1
|
|
assert rows_b[0].user_id == user_b_id
|
|
|
|
|
|
async def test_scope_select_rejects_models_without_user_id() -> None:
|
|
"""`Scope.select()` refuses to build a query for a model that has no `user_id` column,
|
|
instead of silently returning an unfiltered (and therefore unsafe) query. `Invite` is a real
|
|
example, not a contrived one: it's scoped by `created_by`, not `user_id`."""
|
|
async with scoped_session(uuid4()) as scope:
|
|
with pytest.raises(TypeError, match="has no user_id column"):
|
|
scope.select(Invite)
|
|
|
|
|
|
async def test_unscoped_session_can_see_every_user_when_misused(
|
|
client: httpx.AsyncClient, db_auth: AsyncSession
|
|
) -> None:
|
|
"""The deliberately alarming counterpart to the isolation test above: `unscoped_session()`
|
|
plus a raw `select(Session)` returns EVERY user's rows, with no filtering at all — there is no
|
|
database-level backstop to catch this mistake anymore (docs/DECISIONS.md D15). This is exactly
|
|
why CLAUDE.md invariant #4 treats reaching for `unscoped_session()` on a user-owned table as a
|
|
review-blocking mistake, not a style preference: the code review IS the isolation boundary
|
|
for this specific failure mode, `Scope` is the boundary for the query-construction one.
|
|
"""
|
|
await _seed_invite(db_auth, code="CODE-FOR-A")
|
|
await _seed_invite(db_auth, code="CODE-FOR-B")
|
|
await client.post(
|
|
"/api/v1/auth/register",
|
|
json={
|
|
"email": "user-a@example.com",
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "User A",
|
|
"invite_code": "CODE-FOR-A",
|
|
},
|
|
)
|
|
await client.post(
|
|
"/api/v1/auth/register",
|
|
json={
|
|
"email": "user-b@example.com",
|
|
"password": _REGISTER_PASSWORD,
|
|
"display_name": "User B",
|
|
"invite_code": "CODE-FOR-B",
|
|
},
|
|
)
|
|
|
|
async with unscoped_session() as db:
|
|
rows = (await db.execute(select(Session))).scalars().all()
|
|
assert len(rows) == 2, (
|
|
"unscoped_session with a raw query sees every user's rows — by design, this is the "
|
|
"unsafe path Scope exists to replace"
|
|
)
|