diff --git a/config/menu.ts b/config/menu.ts index 09c0351..9f8d5e4 100644 --- a/config/menu.ts +++ b/config/menu.ts @@ -9,7 +9,7 @@ export default defineMenu({ branding: { name: "Plainpages", // app name shown in the sidebar sub: "Console", // optional subtitle under the name - // logo: "/public/logo.svg", // optional logo asset (rendered in the shell — next §2 item) + // logo: "/public/logo.svg", // optional logo asset (rendered in the sidebar brand) // theme: "auto", // default color theme: auto | light | dark }, diff --git a/e2e/full-flow.spec.ts b/e2e/full-flow.spec.ts index 48c5215..7d70488 100644 --- a/e2e/full-flow.spec.ts +++ b/e2e/full-flow.spec.ts @@ -2,9 +2,8 @@ import { type Browser, type Page, expect, test } from "@playwright/test"; import { randomUUID } from "node:crypto"; // Full browser E2E (todo §8): the real Playwright UI against the live stack via the same-origin -// gateway (compose.e2e-full.yml). Covers password + mocked-SSO login, menu filtering by role, the -// users/groups/roles admin CRUD, a permission-gated plugin page, and logout. The earlier full-stack -// suites drove flows over HTTP and deferred the browser-UI login here; this is that coverage. +// gateway (compose.e2e-full.yml) — the browser-UI login the earlier full-stack suites deferred here. +// Coverage is the test titles below, plus the standalone SSO test. // // Runs on a fresh stack (`down -v` after, like the other full-stack suites). The serial admin // journey and the standalone SSO test run in parallel (fullyParallel) but stay independent: each diff --git a/todo.md b/todo.md index ee95bfc..f60c726 100644 --- a/todo.md +++ b/todo.md @@ -121,7 +121,7 @@ everything via Docker. - [x] **Playwright full E2E**: login (password + mocked SSO), menu filtering by role, users/groups/permissions CRUD, a plugin page, logout. → New browser-UI suite `e2e/full-flow.spec.ts` (`compose.e2e-full.yml`) — the real Playwright UI the earlier full-stack suites deferred here ("browser-UI login is owned by §8"). The themed login form posts straight to Kratos' action and cookies are host-scoped, so a tiny **stdlib reverse proxy** (`e2e/proxy.mjs`) fronts web + Kratos on **one origin** (the browser's only host), exactly like a prod reverse proxy; `ory/kratos/e2e-proxy.yml` points Kratos' base_url + every self-service URL at it, and Kratos runs `--dev` so cookies aren't marked Secure over http (Kratos marks them Secure for a non-loopback host like the gateway). Coverage (6 tests, all green): **password login** (themed form → Kratos → `/auth/complete` → dashboard); **mocked SSO** (a stdlib **mock OIDC provider** `e2e/mock-oidc.mjs` — RS256-signed id_token, nonce-bound single-use codes — wired via `SELFSERVICE_METHODS_OIDC_*` env + the committed claims jsonnet; click the provider button → auto-approve → identity created → signed in); **menu filtering by role** (the admin sees the gated Admin section + the plugin nav; anon/SSO-user don't); **users/groups/roles CRUD** (create → list → delete a user via the confirm step; create a group + role, each with a first member since a Keto set needs ≥1); the **permission-gated plugin page** (`/scheduling/shifts` renders the mock upstream's shifts in the native shell); **logout** (sign-out ends the session → back to /login, admin nav gone). **Found + fixed a real bug the E2E surfaced:** the SSO submit button sits in the same `
` as the `required` email/password fields, so clicking it tripped HTML5 validation ("Please fill out this field") and never submitted — added `formnovalidate` to the SSO buttons (`views/partials/auth-card.ejs`), tests-first (`auth-card.test.ts` + `app.test.ts`); password login still validates (separate button). Stability-reviewer run as a local PR: **APPROVE, no Critical/High** — every dev/insecure knob (`--dev`, `SECURE_COOKIES=false`, the OIDC provider + mock + proxy) is confined to the e2e overlay, base/prod compose unaffected; addressed its top follow-up (documented the file's parallel-safety invariant). typecheck + **305 units** green; full-flow **6/6 green on a clean stack**, then `down -v` torn down. README E2E section + suite count updated. - [x] E2E harness: bring up the full compose stack, seed Keto roles + a test identity, **tear down after**. → Delivered by the §8 full-flow harness (`compose.e2e-full.yml`): one `run` brings up the whole stack (Postgres + Kratos + Keto + the one-command **bootstrap** + web + the same-origin gateway + the reference plugin's mock upstream + the mock OIDC provider), the bootstrap **seeds the demo admin identity in Kratos + its Keto roles** (`admin` + the plugin's declared `scheduling:read`/`write` tokens), the browser suite runs against that seeded identity, and `docker compose … down -v` **tears everything down** (run live, 6/6 green, torn down). The §4 auth-refresh + §6 oauth-login suites use the same full-stack-up / seed / tear-down pattern; this completes it for the browser-UI flows. - [x] Run the architecture and the product reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Ran both on the whole project (weighted to the §8 Testing & CI surfaces). **Fixed now (tests-first where code):** (1) **CRITICAL (arch)** — §8 is "Testing & **CI**" but the CI automation was missing: the gate was five hand-run commands with manual teardown. Added **`scripts/ci.sh`** — the whole gate in one reproducible command: a Playwright pin-lockstep check (M4) → typecheck → units (with a count-floor guard, L1) → the four E2E suites, each on its **own named fresh stack** with a guaranteed `down -v` and a non-zero exit on first failure (documented in README). **The gate immediately earned its keep:** it surfaced a latent bug — the auth-refresh suite's `web` inherited the §6 `web→hydra` dep, but the e2e overlays don't run Hydra with `--dev`, so Hydra refused its http issuer and never went healthy → the suite couldn't boot. Fixed by dropping Hydra from the auth suite's `web` deps (`!override`, mirroring the full suite — it never needed Hydra). (2) **Product 🔴 (doc correctness)** — the README **Status** note still claimed auth + Hydra handlers were "the roadmap"/unbuilt (materially false after §4/§6/§8); rewrote it to reflect built reality and dropped the now-false `_(planned)_` markers from the **Auth** + **MVP** headings + fixed their anchor links. (3) **Product 🟡** — the recovery flow existed but nothing linked to it from `/login` (an end-user lockout gap): added a login-only **"Forgot password?"** link (`flow-view.ts` `recoverHref` → `flow-body.ejs`, `.auth-aside` CSS), tests-first (flow-view unit + a full-flow E2E assertion). (4) **Product 🟡 (recurring §5/§6/§7)** — empty list tables rendered blank: added an empty-state row to `data-table.ejs` (overridable `emptyText`, colspan over all columns, only when the table has columns) + `.table-empty` CSS — fixes all five list surfaces at once; tests-first (data-table unit). (5) **M3 (docs)** — README Layout `e2e/` line + `e2e/package.json` description updated for the §8 full-flow suite + harness files. Stability-reviewer run as a local PR: **APPROVE (with nits), no Critical/High**; addressed both robustness nits (per-suite compose **project names** so a flaky teardown can't cross-contaminate; `|| true` on the pin/count greps so `set -e`+pipefail doesn't abort before the diagnostic) — and caught a bug they introduced (a `.` in the derived project name is invalid → sanitized with `tr`). **Corrected a reviewer error:** arch claimed the bootstrap service uses `restart: unless-stopped` (infinite crash-loop); it actually uses `restart: "on-failure:5"` (bounded retries for transient Ory-not-ready blips) — defensible, left as-is. typecheck + **306 units** green; **`scripts/ci.sh` green end-to-end** (visual 9 · auth 1 · oauth 2 · full 6, every stack torn down). **Deferred (reviewer-scoped):** the host **internal route-table** (fold the admin/oauth if-ladder in `app.ts` into one `{method,prefix,handler}` table, derive `RESERVED_PLUGIN_IDS`/`allowedMethods` — arch H1, widened by §6/§7) → **§9** (top structural item, raised urgency); **visual-parity (computed-style) checks for the admin + consent screens** (visual.spec only covers the dashboard — arch M2) → §9/polish; a **JWT key-rotation E2E** (L2) → ships with the §9 rotation-runbook item; `shifts.test.ts` deep `src/*` imports → the plugin-api barrel (L3) → the §8 test-cleanup item below; success-flash after writes (product 🟢) → deferred (stateless, known); a **CI-service workflow** (Actions YAML calling `scripts/ci.sh`) → needs the operator's CI platform + Docker-capable runners (the portable gate script is in place). **Then re-ran both reviewers to convergence (5 rounds, until both returned zero new actionable findings).** Rounds 1–4 fixed, tests-first: bounded every outbound Ory `fetch` with a timeout (`src/fetch-timeout.ts` + `ORY_TIMEOUT_SEC`, default 5; also the http JWKS fetch) so a hung Ory can't park a handler; **anonymous on a gated plugin route now 303→`/login`** (was a dead-end 403; signed-in-without-role still 403); an already-signed-in user is sent home from `/login`/`/registration`; the **`onRequest` short-circuit now sets the fresh CSRF cookie**; `admin-users` malformed `:id` → 404 (was 500); **`parseJwks` validates key shape** (fails loud at load); **removed the dead `COOKIE_SECRET`** (loaded + enforced + documented but never read); documented `HYDRA_ADMIN_URL`; the admin **recovery** UI now shows the **code** (links to the public `/recovery`) instead of the browser-unreachable admin-API link; reference-plugin breadcrumb-label + pagination/datetime notes; corrected the contract doc to not over-promise a post-login "retry". **Declined:** unconditional base-ctx chrome (would build the menu per request, regressing the lazy hot path). **Deferred → §9:** `return_to`-preservation so a deep-link login lands on the requested page (non-trivial: must route through `/auth/complete` to mint the JWT without colliding with the OAuth2-challenge `return_to`). Stability-reviewer on the cumulative diff: **APPROVE, no Critical/High** (addressed its Low nits). typecheck + **310 units** + the full `scripts/ci.sh` gate green throughout. -- [ ] Go over all comments in the code and the README and try to make it shorter and more information dense. Remove not strictly needed stuff. +- [x] Go over all comments in the code and the README and try to make it shorter and more information dense. Remove not strictly needed stuff. → Pass over the §8 accretion (`scripts/ci.sh`, the browser-E2E harness — `e2e/proxy.mjs`/`mock-oidc.mjs`/`full-flow.spec.ts` + the e2e compose/yml overlays — `src/fetch-timeout.ts`, and the §8-review comment additions across `app.ts`/`config.ts`/`jwks.ts`/`admin-users.ts`/`flow-view.ts`/`data-table.ejs`). All were authored dense — each comment carries a real non-obvious "why" (the gateway one-origin rationale, the `|| true`/project-name notes in ci.sh, the CSRF-cookie-on-short-circuit and already-signed-in caveats), so the wins are targeted (per the §6/§7 precedent): (1) fixed a **stale forward-ref** — `config/menu.ts`'s `logo` field said "(rendered in the shell — next §2 item)", but §2 wired branding into the shell long ago; now "(rendered in the sidebar brand)", matching its sibling field comments. (2) trimmed a **locally-duplicated restatement** — `full-flow.spec.ts`'s header re-listed its coverage (password/SSO login, menu-by-role, CRUD, plugin, logout) verbatim above the `test()` titles that already enumerate it *and* the README "Full browser flow" section; collapsed to "Coverage is the test titles below, plus the standalone SSO test", keeping the non-derivable framing (why the gateway, fresh-stack, the parallel-safety invariant). README §8 additions (the "full gate" section, the Full-browser-flow suite block, `ORY_TIMEOUT_SEC`) were written concise in §8 — untouched; the README **Status**/`_(planned)_`/Layout refresh stays §9 (line 133). Docs/comments-only (per AGENTS.md, no stability reviewer); typecheck + 310 units green. - [ ] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us. ## 9. Production, security, ops