Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,10 @@ MYBIBLI_COOKIE_SECURE=false
# Issue #173 — retain the dev seed users (`admin/admin`,
# `librarian/librarian`) that the migrations plant on a fresh install.
# Default off: on every boot, the seed gate in src/services/seed_gate.rs
# soft-deletes any seeded user whose password hash still matches the
# documented seed hash, making the first-launch wizard reachable.
# hard-deletes any seeded user whose password hash still matches the
# documented seed hash — plus the seeded session row, whose token is
# published — making the first-launch wizard reachable and leaving
# nothing recoverable from the Trash (issue #480).
# Set to `1` ONLY in development and the E2E test stack, where the
# integration test suite and Playwright login helpers depend on the
# seeded users existing. NEVER on a production deployment.
Expand Down
29 changes: 6 additions & 23 deletions .github/workflows/_gates.yml
Original file line number Diff line number Diff line change
Expand Up @@ -274,29 +274,12 @@ jobs:
-f tests/e2e/docker-compose.wizard.yml \
up -d --build --wait

- name: Wipe seeded users so the wizard predicate fires
# `sessions.user_id` carries an FK to `users.id` with no CASCADE
# (`fk_sessions_user`, story 1 initial schema), so a bare
# `DELETE FROM users` aborts with 23000. Drop the dependent
# session rows first; `admin_audit` cascades on user delete so
# we don't have to touch it explicitly.
run: |
docker compose \
-f tests/e2e/docker-compose.test.yml \
-f tests/e2e/docker-compose.wizard.yml \
exec -T db mariadb -uroot -proot_test mybibli_test \
-e "DELETE FROM sessions; DELETE FROM users;"

- name: Restart mybibli so it re-reads MYBIBLI_SKIP_SETUP="" and the empty users table
run: |
docker compose \
-f tests/e2e/docker-compose.test.yml \
-f tests/e2e/docker-compose.wizard.yml \
restart mybibli
# Boot is fast — give the listener a moment to bind before
# Playwright connects. Cargo's debug-build container starts in
# ~2 s on warm CI; the 5-s sleep here is generous.
sleep 5
# Issue #480 — no SQL wipe here on purpose. The wizard override
# unsets MYBIBLI_SEED_DEV_USERS, so the seed gate itself clears
# the seeded users and the published session row during the very
# boot that `--wait` above waited for. Wiping by hand would have
# hidden a broken gate; letting the gate do it makes this lane the
# end-to-end proof that a fresh production install starts clean.

- name: Install E2E deps
working-directory: tests/e2e
Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,6 @@ await expect(page.locator("h1")).toContainText(/Active loans|Prêts actifs/i);
- `tests/e2e/helpers/loans.ts` — `scanTitleAndVolume`, `createBorrower`, `createLoan`, `returnLoanFromLoansPage`. Canonical loan-flow helpers. `createLoan` submits via direct `page.request.post('/loans', ...)` instead of the HTMX form — the HTMX path proved racy under parallel load (story 5-1c) because `waitForURL(/\/loans/)` was a no-op when the form lives on /loans.
- `tests/e2e/helpers/scanner.ts` — `simulateScan` (scanner burst, 20 ms inter-key) and `simulateTyping` (human pace, 100 ms inter-key). Uses Playwright's native `{ delay }` option — do NOT re-roll `waitForTimeout` sequences.

**Session cookie format:** The `DEV_SESSION_COOKIE` value `"ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2"` is base64 of a development session token seeded by `migrations/20260329000002_seed_dev_user.sql`. Cookie name is `session` (NOT `session_token`).
**Session cookie format:** The `DEV_SESSION_COOKIE` value `"ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2"` is base64 of a development session token seeded by `migrations/20260329000002_seed_dev_user.sql`. Cookie name is `session` (NOT `session_token`). The row exists **only** where `MYBIBLI_SEED_DEV_USERS` is set: on any other boot the seed gate deletes it along with the seeded users (issue #480), so never write a test — or a runbook — that assumes the token resolves on a production-shaped stack.

**Known app quirks & deferred work** are tracked as GitHub Issues — filter by label `type:known-failure` for accepted-risk quirks, `type:code-review-finding` for deferred review findings, `type:change-request` for product/architectural change proposals. The `.github/ISSUE_TEMPLATE/` forms enforce structured fields for each type. GitHub Issues is the single source of truth — do not re-introduce a markdown tracking doc.
14 changes: 9 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,15 @@ The app listens on `http://localhost:8080`. The seed migrations create an admin
> ℹ️ **Seed users are now gated (issue #173, fixed in 1.1.0).**
> On a fresh install where `MYBIBLI_SEED_DEV_USERS` is unset, the
> seed migrations still apply but the gate in `src/services/seed_gate.rs`
> immediately soft-deletes any user whose hash still matches the
> documented seed value. The first-launch wizard at `/setup` is
> therefore reachable on every fresh production deployment. Set the
> env var to `1` only for local development and the E2E test stack —
> never in production.
> immediately **hard-deletes** any user whose hash still matches the
> documented seed value, along with the seeded session row whose token
> is equally public. The first-launch wizard at `/setup` is therefore
> reachable on every fresh production deployment, and nothing seeded is
> left in the Trash for a later Restore click to revive (issue #480 —
> before that fix the rows were only soft-deleted). An instance
> upgrading from v1.18.0 or earlier purges the leftovers on its first
> boot. Set the env var to `1` only for local development and the E2E
> test stack — never in production.

**Fresh-install wizard.** Story 8-8 introduced a first-launch wizard at `/setup` whose gate predicate is `(active_admin_count == 0) AND (settings.setup_completed_at IS NONE)`. Because the seed migrations create an admin before the gate is first evaluated, the wizard never triggers in practice on a default install — the password-rotation step above is the effective onboarding flow. The wizard can still be exercised by running `cargo run` against an empty DB with the seed migrations skipped. `MYBIBLI_SKIP_SETUP=1` (strict accept-set: `1` / `true` / `TRUE`) is the explicit bypass.

Expand Down
113 changes: 87 additions & 26 deletions src/services/seed_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,43 @@
//! Runs immediately after `sqlx::migrate!` in `main.rs`. When
//! [`MYBIBLI_SEED_DEV_USERS`](`crate::services::seed_gate`) is
//! **not** set (or set to anything other than `"1"` / `"true"` /
//! `"TRUE"`), soft-deletes any user whose `password_hash` still
//! matches the documented seed hash. The hash check protects
//! operators who have already rotated the seeded admin password —
//! their rotated row is left untouched.
//! `"TRUE"`), **hard-deletes** any user whose `password_hash` still
//! matches the documented seed hash, plus the seeded `sessions` row
//! whose token is equally public. The hash check protects operators
//! who have already rotated the seeded admin password — their
//! rotated row is left untouched.
//!
//! # Why hard-delete rather than soft-delete (issue #480)
//!
//! Until v1.18.0 the gate soft-deleted the seeded rows. That was
//! enough to make the wizard fire and to stop `admin/admin` from
//! logging in (both the login query and `find_resolved` require a
//! live user), but it left the seed `password_hash` recoverable for
//! the 30 days before `auto_purge` hard-deletes it — one
//! `UPDATE users SET deleted_at = NULL` away. The Trash panel's
//! Restore button is exactly that `UPDATE` in the UI: an
//! administrator who sees `admin` and `librarian` sitting in the
//! Trash and restores them, reasonably believing a predecessor
//! deleted them, gets back a live administrator whose password is
//! published in this repository's own `SECURITY.md`.
//!
//! Seed artefacts are not user data, so nothing in the Trash should
//! ever offer them back. The predicate deliberately does **not**
//! filter on `deleted_at`: an instance upgrading from v1.18.0 or
//! earlier carries rows the old gate soft-deleted, and the first
//! boot on this version purges them too.
//!
//! Net effect:
//!
//! * **Fresh production install** (default env): seed migrations
//! run → this gate fires → seeded users disappear → setup wizard
//! activates on the next request.
//! run → this gate fires → seeded users and the seeded session
//! row disappear → setup wizard activates on the next request.
//! * **Dev / E2E** (`MYBIBLI_SEED_DEV_USERS=1`): gate is a no-op →
//! `admin/admin` and `librarian/librarian` persist for the
//! integration and Playwright test suites.
//! * **Operator who has rotated their password**: hash no longer
//! matches → the rotated user row is left alone.
//! matches → the rotated user row is left alone. The seeded
//! session row still goes: its token is public whoever owns it.

use crate::db::DbPool;

Expand All @@ -59,6 +81,22 @@ const ADMIN_SEED_HASH: &str =
const LIBRARIAN_SEED_HASH: &str =
"$argon2id$v=19$m=19456,t=2,p=1$NfI9SYT0huhcqAanQWa9pw$mSEHLW8Wl8wlk504MRpzyS42JlcU9w2CXYVVFMFvbcU";

/// Session token planted by
/// `migrations/20260329000002_seed_dev_user.sql`. The value is
/// public — it appears in `CLAUDE.md`, in the E2E helpers and in the
/// git history — so the gate deletes the row outright rather than
/// relying on the two independent reasons it is currently inert
/// (the `users` join in `SessionModel::find_resolved`, and a
/// `last_activity` frozen at install time).
pub const DEV_SESSION_TOKEN: &str = "ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2";

/// Matches a seeded user row by username **and** untouched seed
/// hash. Bound twice at every call site (admin hash, then librarian
/// hash). No `deleted_at` filter — see the module docs on the
/// upgrade path from v1.18.0 and earlier.
const SEEDED_USER_PREDICATE: &str =
"(username = 'admin' AND password_hash = ?) OR (username = 'librarian' AND password_hash = ?)";

/// Pure parse for the `MYBIBLI_SEED_DEV_USERS` accept-set.
/// Strict: only the literal strings `1`, `true` and `TRUE` count
/// as "on". Anything else (including the empty string, `0`,
Expand All @@ -76,7 +114,7 @@ pub fn parse_seed_dev_users(raw: Option<&str>) -> bool {
/// site in `main.rs` short and lets integration tests target
/// [`apply_with`] without touching shared process state.
///
/// Returns the number of rows soft-deleted (0 when the env var
/// Returns the number of user rows hard-deleted (0 when the env var
/// opts back in, or when the operator already rotated the
/// passwords).
pub async fn apply(pool: &DbPool) -> Result<u64, sqlx::Error> {
Expand All @@ -98,29 +136,52 @@ pub async fn apply_with(pool: &DbPool, seed_enabled: bool) -> Result<u64, sqlx::
return Ok(0);
}

let result = sqlx::query(
"UPDATE users \
SET deleted_at = NOW(), version = version + 1 \
WHERE deleted_at IS NULL \
AND ( (username = 'admin' AND password_hash = ?) \
OR (username = 'librarian' AND password_hash = ?) )",
)
.bind(ADMIN_SEED_HASH)
.bind(LIBRARIAN_SEED_HASH)
.execute(pool)
.await?;

let removed = result.rows_affected();
if removed > 0 {
// Both deletes ride one transaction: a half-applied gate would
// leave a user row without its sessions, or worse, sessions
// pointing at a user the next statement was about to remove.
let mut tx = pool.begin().await?;

// `sessions.user_id` carries a bare FK to `users.id` — no
// CASCADE (`fk_sessions_user`, initial schema) — so the session
// rows must go first or the user DELETE aborts with 23000.
// `admin_audit.user_id` and `api_keys.created_by` are both
// ON DELETE SET NULL (#69 / #70), so those rows survive
// detached, which is what the audit trail wants.
let sessions_sql = format!(
"DELETE FROM sessions \
WHERE token = ? \
OR user_id IN (SELECT id FROM users WHERE {SEEDED_USER_PREDICATE})"
);
let sessions_removed = sqlx::query(&sessions_sql)
.bind(DEV_SESSION_TOKEN)
.bind(ADMIN_SEED_HASH)
.bind(LIBRARIAN_SEED_HASH)
.execute(&mut *tx)
.await?
.rows_affected();

let users_sql = format!("DELETE FROM users WHERE {SEEDED_USER_PREDICATE}");
let removed = sqlx::query(&users_sql)
.bind(ADMIN_SEED_HASH)
.bind(LIBRARIAN_SEED_HASH)
.execute(&mut *tx)
.await?
.rows_affected();

tx.commit().await?;

if removed > 0 || sessions_removed > 0 {
tracing::info!(
removed_count = removed,
"Issue #173 — dev seed gate removed seeded user(s). \
The setup wizard at /setup is now reachable."
sessions_removed = sessions_removed,
"Issue #173 / #480 — dev seed gate hard-deleted seeded user(s) \
and seeded session row(s). The setup wizard at /setup is now \
reachable."
);
} else {
tracing::debug!(
"Issue #173 — dev seed gate found no seeded users to remove \
(already rotated or never installed)."
"Issue #173 — dev seed gate found no seeded rows to remove \
(already rotated, already purged, or never installed)."
);
}

Expand Down
13 changes: 9 additions & 4 deletions tests/e2e/docker-compose.wizard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,19 @@
# -f tests/e2e/docker-compose.test.yml \
# -f tests/e2e/docker-compose.wizard.yml \
# up -d --build --wait
# # then: wipe `users` so the gate predicate evaluates to "active"
# # restart mybibli to pick up the empty `users` table
# # run `npx playwright test specs/journeys/setup-wizard.spec.ts`
# with MYBIBLI_SETUP_E2E=1
# # the seed gate empties `users` on boot, so the wizard predicate is
# # already "active" — then run:
# # MYBIBLI_SETUP_E2E=1 npx playwright test \
# # specs/journeys/setup-wizard.spec.ts
services:
mybibli:
environment:
MYBIBLI_SKIP_SETUP: ""
# Issue #480: unset here too, so the seed gate itself clears the
# seeded users and the published session row at boot. Before this,
# the CI lane wiped `users` and `sessions` with raw SQL, which
# made the wizard fire but tested nothing about the gate.
MYBIBLI_SEED_DEV_USERS: ""
# The base compose injects OMDB_API_KEY / TMDB_API_KEY for the
# seeded-stack E2E suite. `config::migrate_legacy_env_vars` writes
# them into `settings` on boot, so the wizard's resolver would see
Expand Down
55 changes: 43 additions & 12 deletions tests/e2e/specs/journeys/setup-wizard.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,20 @@
*
* Two ways to run it locally:
*
* # 1) Spawn the test stack fresh, drop the seed users, unset the
* # bypass, then run only this spec.
* docker compose -f tests/e2e/docker-compose.test.yml up -d --build
* docker compose -f tests/e2e/docker-compose.test.yml exec -T db \
* mariadb -uroot -proot_test mybibli_test \
* -e "DELETE FROM users;"
* docker compose -f tests/e2e/docker-compose.test.yml stop mybibli
* MYBIBLI_SKIP_SETUP="" \
* docker compose -f tests/e2e/docker-compose.test.yml up -d mybibli
* MYBIBLI_SETUP_E2E=1 npx playwright test specs/journeys/setup-wizard.spec.ts
* # 1) Spawn the test stack with the wizard override, which unsets
* # both MYBIBLI_SKIP_SETUP and MYBIBLI_SEED_DEV_USERS. The seed
* # gate then clears the seeded users at boot — no SQL wipe by
* # hand (issue #480), which is also what this spec now asserts.
* docker compose -f tests/e2e/docker-compose.test.yml \
* -f tests/e2e/docker-compose.wizard.yml up -d --build --wait
* cd tests/e2e && MYBIBLI_SETUP_E2E=1 \
* npx playwright test specs/journeys/setup-wizard.spec.ts
*
* # 2) Run cargo run locally against a clean DB on port 8080.
* MYBIBLI_SETUP_E2E=1 npx playwright test specs/journeys/setup-wizard.spec.ts
*
* The CI job that wires the wizard stack lives outside this PR — it
* is tracked as a follow-up issue surfaced in Epic 8 retrospective.
* The CI job that wires the wizard stack is `e2e-wizard` in
* `.github/workflows/_gates.yml`.
*/
import { test, expect } from "@playwright/test";
import AxeBuilder from "@axe-core/playwright";
Expand All @@ -38,6 +36,12 @@ const SETUP_E2E_GATED = process.env.MYBIBLI_SETUP_E2E !== "1";
// (this spec's lane) provides.
const WCAG_TAGS = ["wcag2a", "wcag2aa", "wcag22aa"];

// Issue #480 — the session token planted by
// `migrations/20260329000002_seed_dev_user.sql`. Published in
// CLAUDE.md, in the E2E helpers and in the git history, so a fresh
// production install must not carry the row.
const DEV_SESSION_TOKEN = "ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2ZGV2";

test.describe("Story 8-8 — First-launch setup wizard", () => {
test.skip(
SETUP_E2E_GATED,
Expand Down Expand Up @@ -100,6 +104,33 @@ test.describe("Story 8-8 — First-launch setup wizard", () => {
// /setup is dead — single-use property.
const resp = await page.request.get("/setup");
expect(resp.status()).toBe(404);

// Issue #480 — a fresh install must leave no seed artefact behind.
// This lane is the only one where the gate actually runs (every
// other spec pins MYBIBLI_SEED_DEV_USERS=1), so the two checks
// below live here.

// 1) Nothing restorable in the Trash. Until v1.18.0 the gate
// soft-deleted `admin` and `librarian`, which left them sitting
// in this panel behind a Restore button — one click from a live
// administrator whose password is published in SECURITY.md.
await page.goto("/admin?tab=trash");
await expect(page.locator("#admin-trash-panel")).toContainText(
/No deleted items|Aucun élément supprimé/i,
);

// 2) The seeded session token no longer authenticates: the gate
// deletes the row, so presenting the published cookie leaves the
// visitor anonymous and /admin bounces to the login page.
await page.context().addCookies([
{
name: "session",
value: DEV_SESSION_TOKEN,
url: baseURL ?? "http://localhost:8080",
},
]);
await page.goto("/admin");
await expect(page).toHaveURL(/\/login/);
});

// Note: the original "resume after browser close" test was removed
Expand Down
Loading