BBergleandClaude Opus 5
e7392a5723
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>
109 lines
5.2 KiB
Python
109 lines
5.2 KiB
Python
"""Identity tables: users, invites, sessions, api_tokens.
|
|
|
|
See docs/PLAN.md "Auth" and "Schema > Tables > Identity" for the design rationale, and db.py's
|
|
module docstring for how isolation between users is enforced now that there's no RLS to do it at
|
|
the database level (docs/DECISIONS.md D15).
|
|
|
|
Types are deliberately dialect-generic (`sqlalchemy.Uuid`, `JSON`, plain `String` for the IP
|
|
column) rather than the `postgresql.*` variants Phase 0 originally used — this schema now targets
|
|
SQLite only, but there's no reason to hand-tie it to a Postgres-only type where a portable one
|
|
works identically. See models/base.py for why timestamps need a custom type at all.
|
|
"""
|
|
|
|
from datetime import UTC, datetime
|
|
from uuid import UUID
|
|
|
|
from sqlalchemy import JSON, ForeignKey, LargeBinary, String, Text, Uuid
|
|
from sqlalchemy.orm import Mapped, mapped_column, relationship
|
|
|
|
from velodrome.ids import new_id
|
|
from velodrome.models.base import Base
|
|
|
|
|
|
def _now_utc() -> datetime:
|
|
return datetime.now(UTC)
|
|
|
|
|
|
class User(Base):
|
|
__tablename__ = "users"
|
|
|
|
id: Mapped[UUID] = mapped_column(Uuid(as_uuid=True), primary_key=True, default=new_id)
|
|
email: Mapped[str] = mapped_column(String(320), unique=True, nullable=False)
|
|
display_name: Mapped[str] = mapped_column(String(200), nullable=False)
|
|
password_hash: Mapped[str] = mapped_column(Text, nullable=False)
|
|
role: Mapped[str] = mapped_column(String(20), nullable=False, default="member")
|
|
timezone: Mapped[str] = mapped_column(String(64), nullable=False, default="UTC")
|
|
# Display-only, per CLAUDE.md invariant #3 — storage is always SI, this never touches a query.
|
|
unit_system: Mapped[str] = mapped_column(String(10), nullable=False, default="imperial")
|
|
is_active: Mapped[bool] = mapped_column(nullable=False, default=True)
|
|
created_at: Mapped[datetime] = mapped_column(nullable=False, default=_now_utc)
|
|
|
|
sessions: Mapped[list["Session"]] = relationship(back_populates="user")
|
|
api_tokens: Mapped[list["ApiToken"]] = relationship(back_populates="user")
|
|
|
|
|
|
class Invite(Base):
|
|
__tablename__ = "invites"
|
|
|
|
id: Mapped[UUID] = mapped_column(Uuid(as_uuid=True), primary_key=True, default=new_id)
|
|
# sha256 digest of the invite code. The code itself is never stored anywhere — see
|
|
# auth/service.py. 32 bytes for sha256.
|
|
code_hash: Mapped[bytes] = mapped_column(LargeBinary(32), unique=True, nullable=False)
|
|
created_by: Mapped[UUID] = mapped_column(
|
|
Uuid(as_uuid=True), ForeignKey("users.id"), nullable=False
|
|
)
|
|
email: Mapped[str | None] = mapped_column(String(320), nullable=True)
|
|
role: Mapped[str] = mapped_column(String(20), nullable=False, default="member")
|
|
expires_at: Mapped[datetime] = mapped_column(nullable=False)
|
|
max_uses: Mapped[int] = mapped_column(nullable=False, default=1)
|
|
used_count: Mapped[int] = mapped_column(nullable=False, default=0)
|
|
revoked_at: Mapped[datetime | None] = mapped_column(nullable=True)
|
|
|
|
|
|
class Session(Base):
|
|
__tablename__ = "sessions"
|
|
|
|
id: Mapped[UUID] = mapped_column(Uuid(as_uuid=True), primary_key=True, default=new_id)
|
|
user_id: Mapped[UUID] = mapped_column(
|
|
Uuid(as_uuid=True),
|
|
ForeignKey("users.id", ondelete="CASCADE"),
|
|
nullable=False,
|
|
index=True,
|
|
)
|
|
# sha256 of the opaque bearer token. The token itself is returned to the client exactly once,
|
|
# at login, and never stored — see auth/security.py.
|
|
token_hash: Mapped[bytes] = mapped_column(LargeBinary(32), unique=True, nullable=False)
|
|
client: Mapped[str] = mapped_column(String(20), nullable=False, default="web")
|
|
user_agent: Mapped[str | None] = mapped_column(Text, nullable=True)
|
|
# Plain string, not a native INET type — SQLite has no such type, and app code never queries
|
|
# or indexes on structure within the address, only stores/displays it.
|
|
ip: Mapped[str | None] = mapped_column(String(45), nullable=True)
|
|
created_at: Mapped[datetime] = mapped_column(nullable=False, default=_now_utc)
|
|
last_seen_at: Mapped[datetime] = mapped_column(nullable=False, default=_now_utc)
|
|
expires_at: Mapped[datetime] = mapped_column(nullable=False)
|
|
revoked_at: Mapped[datetime | None] = mapped_column(nullable=True)
|
|
|
|
user: Mapped["User"] = relationship(back_populates="sessions")
|
|
|
|
|
|
class ApiToken(Base):
|
|
__tablename__ = "api_tokens"
|
|
|
|
id: Mapped[UUID] = mapped_column(Uuid(as_uuid=True), primary_key=True, default=new_id)
|
|
user_id: Mapped[UUID] = mapped_column(
|
|
Uuid(as_uuid=True),
|
|
ForeignKey("users.id", ondelete="CASCADE"),
|
|
nullable=False,
|
|
index=True,
|
|
)
|
|
name: Mapped[str] = mapped_column(String(200), nullable=False)
|
|
token_hash: Mapped[bytes] = mapped_column(LargeBinary(32), unique=True, nullable=False)
|
|
# JSON, not ARRAY(String) — SQLite has no array type. Stored as a JSON-encoded TEXT column;
|
|
# SQLAlchemy handles the (de)serialization transparently.
|
|
scopes: Mapped[list[str]] = mapped_column(JSON, nullable=False, default=list)
|
|
last_used_at: Mapped[datetime | None] = mapped_column(nullable=True)
|
|
expires_at: Mapped[datetime | None] = mapped_column(nullable=True)
|
|
revoked_at: Mapped[datetime | None] = mapped_column(nullable=True)
|
|
|
|
user: Mapped["User"] = relationship(back_populates="api_tokens")
|