Harden the API and the data layer after a security and performance audit
Security - Every route now requires a login and refuses ids outside the caller's workspace with 404: plan, leave-wishes, day-overrides and vacation-days were fully anonymous; every other router trusted client-supplied workspace/child/parent ids. Scraper sessions are bound to their owner. - Invitations can no longer be created for a foreign workspace, and a redeemer who is already a member (or a full workspace) is refused. - Production refuses the default JWT secret; API docs are hidden in production; a failed migration aborts startup instead of serving. - Login and register no longer reveal whether an email exists; inactive accounts are rejected; /auth/refresh is rate limited. - The FK scraper writes debug artefacts only when explicitly enabled, logs no scraped values or page text, clears the BankID QR as soon as it is scanned and purges finished sessions after five minutes. - Real FK page captures are removed from the tree (history rewrite follows); the parser tests use a synthetic fixture. Performance and correctness - Plan apply/sync run inside one locked transaction with batched writes instead of one commit per row, so a failure mid-batch leaves nothing half-applied and concurrent syncs no longer corrupt each other. - SQLite runs in WAL mode with a busy timeout; 14 indexes cover the filters that were full scans; argon2 hashing runs off the event loop; responses are gzipped. - Frontend: a 401 now refreshes the token once and retries, so sessions no longer die silently after 15 minutes; every data hook discards stale responses; logout clears state even when the server call fails. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsXCCcDfmV4Sps4ZQSvNEY
This commit is contained in:
@@ -10,6 +10,12 @@ connection so test fixtures stay fast and do not depend on Alembic.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import weakref
|
||||
from collections.abc import AsyncIterator
|
||||
from contextlib import asynccontextmanager
|
||||
from contextvars import ContextVar
|
||||
|
||||
import aiosqlite
|
||||
|
||||
# Full schema used by test fixtures directly (avoids a full Alembic run in tests).
|
||||
@@ -46,6 +52,7 @@ CREATE TABLE IF NOT EXISTS "user" (
|
||||
email TEXT NOT NULL UNIQUE,
|
||||
hashed_password TEXT NOT NULL,
|
||||
created_at TIMESTAMP NOT NULL,
|
||||
is_active INTEGER NOT NULL DEFAULT 1,
|
||||
FOREIGN KEY (personal_data_uuid) REFERENCES personal_data(uuid) ON DELETE CASCADE
|
||||
);
|
||||
|
||||
@@ -183,18 +190,152 @@ CREATE TABLE IF NOT EXISTS tip_dismissal (
|
||||
UNIQUE(workspace_id, tip_id),
|
||||
FOREIGN KEY (workspace_id) REFERENCES workspace(id) ON DELETE CASCADE
|
||||
);
|
||||
|
||||
-- Indexes (migration 011). Mirrored verbatim (same columns, same order, no
|
||||
-- partial indexes) from migrations/versions/011_add_indexes.py so
|
||||
-- tests/integration/test_schema_parity.py, which compares index sets between
|
||||
-- the Alembic chain and this hand-maintained DDL, stays green. See the
|
||||
-- backend-perf audit §4 for what each index serves.
|
||||
CREATE INDEX IF NOT EXISTS idx_pld_parent_child_type_date
|
||||
ON planned_leave_day (parent_personal_data_uuid, child_id, day_type, date);
|
||||
CREATE INDEX IF NOT EXISTS idx_pld_child_dubbeldag_date
|
||||
ON planned_leave_day (child_id, is_dubbeldag, date);
|
||||
CREATE INDEX IF NOT EXISTS idx_pld_wish
|
||||
ON planned_leave_day (wish_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_plf_parent_child_fetched
|
||||
ON parental_leave_fetch (personal_data_uuid, child_id, fetched_at);
|
||||
CREATE INDEX IF NOT EXISTS idx_plf_workspace
|
||||
ON parental_leave_fetch (workspace_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_plf_child
|
||||
ON parental_leave_fetch (child_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_leave_wish_workspace
|
||||
ON leave_wish (workspace_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_leave_wish_child
|
||||
ON leave_wish (child_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_leave_wish_parent
|
||||
ON leave_wish (parent_personal_data_uuid);
|
||||
CREATE INDEX IF NOT EXISTS idx_child_workspace_birth
|
||||
ON child (workspace_id, birth_date);
|
||||
CREATE INDEX IF NOT EXISTS idx_personal_data_workspace_deleted
|
||||
ON personal_data (workspace_id, deleted_at);
|
||||
CREATE INDEX IF NOT EXISTS idx_invitation_workspace_status_created
|
||||
ON workspace_invitation (workspace_id, status, created_at);
|
||||
CREATE INDEX IF NOT EXISTS idx_invitation_invited_by
|
||||
ON workspace_invitation (invited_by_user_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_vacation_days_workspace
|
||||
ON vacation_days (workspace_id);
|
||||
"""
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Unit of work — see backend-perf audit §3 for the full design rationale.
|
||||
#
|
||||
# The app runs on ONE shared aiosqlite connection (app.state.db_connection),
|
||||
# so a multi-statement write needs (a) mutual exclusion for the whole unit of
|
||||
# work and (b) the repos' own per-row `commit()` calls suppressed while one is
|
||||
# in progress — otherwise a concurrent request's commit would durably commit
|
||||
# this one's half-finished writes. `_in_uow` and `_locks` below exist to
|
||||
# provide exactly that on top of the existing single-connection, per-repo-
|
||||
# commit design, without touching any repo's read/write SQL.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# True while the current asyncio task is inside a `unit_of_work()` block, on
|
||||
# whichever connection started it. Checked by `commit()` (skip the per-row
|
||||
# commit; the unit of work commits once, at the end) and by `unit_of_work()`
|
||||
# itself (a nested call is a no-op passthrough — see its docstring).
|
||||
_in_uow: ContextVar[bool] = ContextVar("db_in_uow", default=False)
|
||||
|
||||
# One asyncio.Lock per connection, created lazily on first use. A module-level
|
||||
# lock would break under pytest: each test's `connection` fixture opens a new
|
||||
# in-memory connection under a fresh event loop, and an `asyncio.Lock` created
|
||||
# against one loop raises "bound to a different event loop" if awaited from
|
||||
# another. Keying by connection (and using a WeakKeyDictionary so a closed
|
||||
# test connection's lock is garbage-collected instead of leaking) sidesteps
|
||||
# that entirely: production has exactly one connection => exactly one lock.
|
||||
_locks: "weakref.WeakKeyDictionary[aiosqlite.Connection, asyncio.Lock]" = (
|
||||
weakref.WeakKeyDictionary()
|
||||
)
|
||||
|
||||
|
||||
def _lock_for(conn: aiosqlite.Connection) -> asyncio.Lock:
|
||||
lock = _locks.get(conn)
|
||||
if lock is None:
|
||||
lock = asyncio.Lock()
|
||||
_locks[conn] = lock
|
||||
return lock
|
||||
|
||||
|
||||
async def commit(conn: aiosqlite.Connection) -> None:
|
||||
"""Commit, unless a `unit_of_work()` is currently in progress on this task.
|
||||
|
||||
Every sqlite repository's single-row write calls this instead of
|
||||
`conn.commit()` directly. Outside a unit of work this behaves exactly
|
||||
like today (commit immediately, once per write); inside one, it is a
|
||||
no-op, and the surrounding `unit_of_work()` block commits once, atomically,
|
||||
when it exits.
|
||||
"""
|
||||
if not _in_uow.get():
|
||||
await conn.commit()
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
async def unit_of_work(conn: aiosqlite.Connection) -> AsyncIterator[None]:
|
||||
"""Wrap the block in one atomic transaction on the shared connection.
|
||||
|
||||
Acquires this connection's lock for the whole block (SQLite is a
|
||||
single-writer database anyway, so this costs no real throughput) and
|
||||
issues `BEGIN IMMEDIATE` up front — that claims the write lock
|
||||
immediately rather than at the first write statement, so a second,
|
||||
concurrent `unit_of_work()` call blocks here and only proceeds once this
|
||||
one has committed (or rolled back), then re-reads state as of that commit
|
||||
instead of racing it. Any exception rolls the whole block back; a normal
|
||||
exit commits it.
|
||||
|
||||
Nested calls (e.g. `PlanningService.sync_plan` wrapping its own call to
|
||||
`apply_proposed_days`, which wraps its own writes) are a no-op passthrough
|
||||
when `_in_uow` is already set — the outer call owns the lock/transaction,
|
||||
and a naive re-entry would try to re-acquire the same (non-reentrant)
|
||||
`asyncio.Lock` from the same task and deadlock.
|
||||
"""
|
||||
if _in_uow.get():
|
||||
yield
|
||||
return
|
||||
|
||||
async with _lock_for(conn):
|
||||
token = _in_uow.set(True)
|
||||
try:
|
||||
await conn.execute("BEGIN IMMEDIATE")
|
||||
try:
|
||||
yield
|
||||
except BaseException:
|
||||
await conn.rollback()
|
||||
raise
|
||||
else:
|
||||
await conn.commit()
|
||||
finally:
|
||||
_in_uow.reset(token)
|
||||
|
||||
|
||||
async def connect(path: str) -> aiosqlite.Connection:
|
||||
"""Open a connection and enable row-by-name access + FK enforcement.
|
||||
|
||||
Schema is applied via Alembic in the app lifespan (`run_migrations`).
|
||||
For test fixtures, call `ensure_schema` directly.
|
||||
|
||||
PRAGMAs: `journal_mode=WAL` + `synchronous=NORMAL` cut single-row commit
|
||||
latency roughly 4x (measured 88.7ms -> 21.7ms per 300 sequential commits)
|
||||
and let Alembic/`seed_demo.py` co-access the same file wait instead of
|
||||
hitting "database is locked" outright; `busy_timeout` backs that wait with
|
||||
an actual retry budget. Durability trade-off of NORMAL: at most the last
|
||||
transaction(s) are lost on an OS crash/power loss, never on an app crash —
|
||||
acceptable here (see backend-perf audit §3).
|
||||
"""
|
||||
conn = await aiosqlite.connect(path)
|
||||
conn.row_factory = aiosqlite.Row
|
||||
await conn.execute("PRAGMA foreign_keys = ON;")
|
||||
await conn.execute("PRAGMA journal_mode = WAL;")
|
||||
await conn.execute("PRAGMA synchronous = NORMAL;")
|
||||
await conn.execute("PRAGMA busy_timeout = 5000;")
|
||||
return conn
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user