COLLECTING BRANCH Auth, sessions, and CSRF protection (Effort 1) #64

Manually merged

marvin8 merged 12 commits from feat/effort1-auth into feat/multiuser

2026-10-02 19:56:47 +00:00

agent-pi commented

2026-10-02 10:58:39 +00:00

Collaborator

Targets the collecting branch feat/multiuser (not main). Effort 1 of 3 of Phase 6. Successor to #63 (bogus auto-merged tombstone — stacking error on my part, explained there).

Refs #25

What this does

Adds authentication, sessions, and CSRF protection — the door lock for the reader:

Migration (20261002_add_user_credentials): users.password_hash (nullable) and users.sessions_valid_after (nullable, session-revocation stamp). Idempotent guarded DDL, replay-safe.
services/auth.py: werkzeug scrypt hashing (hash_password/verify_password), setup_required, is_session_valid (constant-work verify; >= boundary semantics).
web/auth.py: env config (YUNJIN_SESSION_LIFETIME_DAYS default 30, YUNJIN_SESSION_COOKIE_SECURE, SESSION_COOKIE_SAMESITE=Lax), current_user/require_current_user, CSRF primitives (secrets.token_hex(32) per session, hmac.compare_digest), and one before_request guard: exempt endpoints → setup gate → login gate → revocation → CSRF on POST/PUT/PATCH/DELETE.
auth blueprint: first-run /setup (reachable only while no user has a credential; self-disables), /login (full session rotation, permanent 30-day session, open-redirect-safe next), /logout. Constant-work failed-login handling with equalized 0.5 s delay on both wrong-password and unknown-user paths (CWE-204).
CSRF on all 20 POST forms (17 app forms + nav logout + login + setup) via a Jinja macro; failures render a 400 error page.
Every hard-coded user_id = 1 replaced with the session-derived user (reader, settings, ignore rules, display-settings injection).
yunjin set-password <username> CLI: getpass-prompted, migrates the DB first, revokes existing sessions — the standing bootstrap/reset tool.
Docs: docs/cli.md, docs/config.md. Container smoke test fetches /login (the anonymous entry page since auth landed).

Upgrade notes for existing deployments

After upgrade, the first browser request redirects to /setup — set the password for the seeded default user there (or uv run yunjin set-password default via docker exec). Existing read state is untouched. Set YUNJIN_SESSION_COOKIE_SECURE=true when behind the HTTPS reverse proxy.

Evidence

Full suite: 489 passed (was 434). uv run nox all sessions green (ty, ruff, complexipy, pysentry, pytest 3.12/3.13/3.14, deptry, zaojun).
Eight task-scoped reviews + final whole-branch review (fresh reviewer), one fix wave (b7c618a), all findings verified addressed by scoped re-review.
CI on this exact content: run 773 — checks ✅ gitleaks ✅ container ✅ (dispatched via workflow_dispatch because the forge's pull_request event dispatch is currently wedged; see #63 comment).

Known deferred minors (tracked for Effort 2/3 or final triage)

Pre-existing sqlite3 datetime-adapter DeprecationWarning in db/ INSERTs (predates branch)
password_hash is NULL login path still redirects to /setup without delay (bootstrap-only window)
No regression test for set-password fresh-DB bootstrap (manual repro only)
csrf_token is a callable Jinja global (comment added at the call site)

Targets the collecting branch `feat/multiuser` (not `main`). Effort 1 of 3 of Phase 6. Successor to #63 (bogus auto-merged tombstone — stacking error on my part, explained there). Refs #25 ## What this does Adds authentication, sessions, and CSRF protection — the door lock for the reader: - **Migration** (`20261002_add_user_credentials`): `users.password_hash` (nullable) and `users.sessions_valid_after` (nullable, session-revocation stamp). Idempotent guarded DDL, replay-safe. - **`services/auth.py`**: werkzeug scrypt hashing (`hash_password`/`verify_password`), `setup_required`, `is_session_valid` (constant-work verify; `>=` boundary semantics). - **`web/auth.py`**: env config (`YUNJIN_SESSION_LIFETIME_DAYS` default 30, `YUNJIN_SESSION_COOKIE_SECURE`, `SESSION_COOKIE_SAMESITE=Lax`), `current_user`/`require_current_user`, CSRF primitives (`secrets.token_hex(32)` per session, `hmac.compare_digest`), and one `before_request` guard: exempt endpoints → setup gate → login gate → revocation → CSRF on POST/PUT/PATCH/DELETE. - **`auth` blueprint**: first-run `/setup` (reachable only while *no* user has a credential; self-disables), `/login` (full session rotation, permanent 30-day session, open-redirect-safe `next`), `/logout`. Constant-work failed-login handling with equalized 0.5 s delay on both wrong-password and unknown-user paths (CWE-204). - **CSRF** on all 20 POST forms (17 app forms + nav logout + login + setup) via a Jinja macro; failures render a 400 error page. - **Every hard-coded `user_id = 1` replaced** with the session-derived user (reader, settings, ignore rules, display-settings injection). - **`yunjin set-password <username>`** CLI: getpass-prompted, migrates the DB first, revokes existing sessions — the standing bootstrap/reset tool. - Docs: `docs/cli.md`, `docs/config.md`. Container smoke test fetches `/login` (the anonymous entry page since auth landed). ## Upgrade notes for existing deployments After upgrade, the first browser request redirects to `/setup` — set the password for the seeded `default` user there (or `uv run yunjin set-password default` via docker exec). Existing read state is untouched. Set `YUNJIN_SESSION_COOKIE_SECURE=true` when behind the HTTPS reverse proxy. ## Evidence - Full suite: 489 passed (was 434). `uv run nox` all sessions green (ty, ruff, complexipy, pysentry, pytest 3.12/3.13/3.14, deptry, zaojun). - Eight task-scoped reviews + final whole-branch review (fresh reviewer), one fix wave (`b7c618a`), all findings verified addressed by scoped re-review. - CI on this exact content: run [773](https://forge.marvin8.zone/marvin8/yunjin/actions/runs/773) — `checks` ✅ `gitleaks` ✅ `container` ✅ (dispatched via `workflow_dispatch` because the forge's `pull_request` event dispatch is currently wedged; see #63 comment). ## Known deferred minors (tracked for Effort 2/3 or final triage) - Pre-existing sqlite3 datetime-adapter DeprecationWarning in `db/` INSERTs (predates branch) - `password_hash is NULL` login path still redirects to `/setup` without delay (bootstrap-only window) - No regression test for `set-password` fresh-DB bootstrap (manual repro only) - `csrf_token` is a callable Jinja global (comment added at the call site)

agent-pi added 12 commits

2026-10-02 10:58:39 +00:00

✨ Add user credential columns migration 6dcd63fc1c