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

Manually merged

agent-pi merged 0 commits from feat/effort1-auth into feat/multiuser

2026-10-02 01:13:02 +00:00

agent-pi commented

2026-10-02 01:04:11 +00:00

Collaborator

Targets the collecting branch feat/multiuser (not main). Effort 1 of 3 of Phase 6.

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.

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.

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. 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`. ## 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. ## 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 manually merged commit b7c618a8f5 into feat/multiuser

2026-10-02 01:13:02 +00:00

agent-pi referenced this pull request from a commit

2026-10-02 01:45:17 +00:00

:repeat: Re-trigger CI after PR #63 push non-dispatch

agent-pi commented

2026-10-02 01:54:36 +00:00

Author

Collaborator

CI status note (agent)

The pull_request event dispatch has been wedged forge-side since the bdd5fab push — two pushes (the fix and a re-trigger empty commit) produced no run rows at all, while workflow_dispatch dispatches fine. Same class of outage as the earlier re-trigger commits (764, 760) in the history.

Validation in lieu of the PR status check: run 773 (workflow_dispatch on feat/effort1-auth @ ff1ee47, identical content to this PR head) is fully green:

checks ✅
gitleaks ✅
container ✅ (the job that failed pre-fix — the smoke test now fetches /login, the anonymous entry page since auth landed)
pr-review skipped (pull_request-only; WuMing reviewed the prior head with zero findings, and the only diff since is the workflow YAML)

When PR-event dispatch recovers, a push to this branch (even empty) should populate the native status check.

### CI status note (agent) The `pull_request` event dispatch has been wedged forge-side since the `bdd5fab` push — two pushes (the fix and a re-trigger empty commit) produced no run rows at all, while `workflow_dispatch` dispatches fine. Same class of outage as the earlier re-trigger commits (`764`, `760`) in the history. **Validation in lieu of the PR status check**: run [773](https://forge.marvin8.zone/marvin8/yunjin/actions/runs/773) (`workflow_dispatch` on `feat/effort1-auth` @ `ff1ee47`, identical content to this PR head) is fully green: - `checks` ✅ - `gitleaks` ✅ - `container` ✅ (the job that failed pre-fix — the smoke test now fetches `/login`, the anonymous entry page since auth landed) - `pr-review` skipped (pull_request-only; WuMing reviewed the prior head with zero findings, and the only diff since is the workflow YAML) When PR-event dispatch recovers, a push to this branch (even empty) should populate the native status check.

agent-pi commented

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

Author

Collaborator

Superseded — bogus merged record

This PR was created with head and base pointing at the same commit (my stacking error: the work was committed directly on feat/multiuser, then feat/effort1-auth was cut at that same tip). Zero-commit, zero-file diff — unreviewable.

Force-pushing the base back to its fork point (e47d317) made Forgejo record this PR as merged, which is false: no merge occurred, feat/multiuser never contained this work. The forge refuses to reopen a merged PR, so this record stays as a tombstone.

The real review happens in the successor PR. Same head (ff1ee47), base now genuinely empty.

### Superseded — bogus merged record This PR was created with head and base pointing at the same commit (my stacking error: the work was committed directly on `feat/multiuser`, then `feat/effort1-auth` was cut at that same tip). Zero-commit, zero-file diff — unreviewable. Force-pushing the base back to its fork point (`e47d317`) made Forgejo record this PR as **merged**, which is false: no merge occurred, `feat/multiuser` never contained this work. The forge refuses to reopen a merged PR, so this record stays as a tombstone. **The real review happens in the successor PR.** Same head (`ff1ee47`), base now genuinely empty.

agent-pi referenced this pull request

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

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