Unify the §5 admin-test duplication (todo §5 test cleanup); the three admin-screen HTTP tests (Users/Groups/Roles) in app.test.ts repeated an identical ~13-line harness preamble (createApp+listen+url+CSRF token+admin cookie+get/post), an identical gate block, and a stateful in-memory KetoClient defined 3× (trivial stubKeto + two byte-identical inline fakes). Extracted adminHarness(t,opts)→{url,token,get,post}, assertAdminGate(url,get,path), and one fakeKeto(tuples?,over?) that subsumes stubKeto (login tests now fakeKeto([],…)) and both admin fakes (fakeKeto(tuples) / fakeKeto(tuples,{expand})); hoisted shared sameSet/matchesTuple up beside it. Per-module unit files keep their matrix pattern (no force-merge across modules; build*ListModel stays per-file). −30 net lines, zero coverage lost; typecheck + 244 units green.
This commit is contained in:
2
todo.md
2
todo.md
@@ -99,7 +99,7 @@ everything via Docker.
|
||||
- [x] Wire into the menu (admin section, permission-gated). → Extracted `adminSection(current?)` in `admin-nav.ts` as the single source of truth for the built-in screens' menu links: a permission-gated (`admin`) "Admin" header whose children are Users/Groups/Roles. Wired into the **global** dashboard menu (`dashboard.ts` appends `adminSection()`) so an admin sees the section on `/`; `composeNav`'s `filterByRoles` drops the whole gated header + subtree for a non-admin/anonymous (cosmetic — the routes themselves stay independently `GuardError(403)`-gated). The in-screen `adminNav()` now reuses the same `adminSection(current)` (Dashboard link + the active-marked section) so the two navs can't drift; narrowed `AdminScreen` to `groups|roles|users` (the home link was never `current`). Reuses existing sprite icons (no icon-guard change). Tests-first: `dashboard.test.ts` (admin→section present with the three hrefs; non-admin→absent) + `app.test.ts` HTTP integration (admin JWT→`/admin/users` link rendered, anonymous→absent). Default anonymous `/` render is byte-equivalent (section filtered out) so the visual E2E is unaffected. README Layout line updated. Stability-reviewer run as a local PR: APPROVE, no Critical/High/Medium. 242→244 units + typecheck green.
|
||||
- [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 all of `src/`/`views/`/`config/`/docs (weighted to the §5 admin screens). Architecture: **no Critical/High** (functional-core/imperative-shell genuinely honored, security primitives sound). Product: **2 Critical + 1 High**. **Fixed now (tests-first):** (1) Critical (product) — the Roles "Effective access" view showed group→role membership *transitively* but `login.ts` `readRoles` granted only **direct** memberships into the JWT, so a user holding a role *only via a group* was listed as having it yet gated as if not (two screens contradicting). Per the user's call, made `readRoles` transitive: enumerate the defined roles + Keto-`check` each (resolves group membership), so the JWT now matches the Effective-access view + the OPL model — at login/refresh only, never per request (README login section + `admin-roles.ts` header updated). (2) Critical (product) — no confirmation on destructive actions: added a server-rendered (zero-JS) confirm step (`views/admin/confirm.ejs` + `partials/confirm-body.ejs`, shared `buildConfirmModel`) — `GET /admin/{users,groups,roles}/:id/delete` renders an interstitial (Cancel + the real POST); each detail/edit Delete control is now a link to it. (3) High (product) — self-lockout: an admin can no longer delete or deactivate **their own** account, revoke **their own** (direct) admin grant, or delete the **admin role** outright (each → 400 + inline error). Covers the direct-grant paths (incl. the bootstrap-seeded admin, which holds a direct grant); admin held *only* via a group can still be self-revoked, so the robust "last effective admin won't drop" check is deferred to **§9** (stability-reviewer Medium). (4) MEDIUM (arch M1 pt.1) — extracted the gate+CSRF preamble copied verbatim across the 3 admin handlers into `admin-nav.ts` `requireAdmin`/`guardedForm` (one security-critical copy, can't drift). (5) MEDIUM (arch M4) — `shellUser` no longer blanks the email: name = email local part, full email beneath (matches `toUserView`). Tests-first throughout (extended the 3 admin HTTP tests + login/shell-context units); typecheck + 244 units + 8 visual E2E + the full-stack auth-refresh E2E green (the latter re-verifies live login→transitive `readRoles`→`roles:["admin"]`). **Deferred (reviewer-scoped, not the §5 checkpoint):** the host internal route-table (fold the admin if-ladder + Hydra into `matchRoute`/`isAuthorized`, arch M1 pt.2) → **§6** (the 2nd/3rd Hydra screen is the forcing function); admin list-model/template near-duplication across Users/Groups/Roles (arch M3) → the §5 comment/test-cleanup items below (lines 101–102); success-flash after writes + welcoming empty-list states + warn-on-dangling-group-references + >250-row truncation notice (product Medium) → §5 polish / §8 E2E; `safeUrl()` href helper (arch L1 — the recovery link is server-built, not exploitable today) → **§7** (first untrusted-URL flow); oversized-body→500 should be 413 (arch M2) + prod Ory-URL `https` enforcement (arch L3) + `§N`-in-comments / README Layout drift (arch L4) → **§9** (ops/security).
|
||||
- [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 §5 admin accretion. The §5 code was authored dense, so the wins are targeted: tightened the three near-identical module-header blocks (`admin-users`/`admin-groups`/`admin-roles`) — dropped per-file restatement the README/code already carry (subject-form detail → "see parseSubject", "no user/group store" → covered by README "stateless", the verbatim "it gates… CSRF-guards… maps each action to a RouteResult" boilerplate → "gated admin-only, CSRF-guarded"). README **Layout**: compressed the `views/` run-on (long admin/ + per-body-partial enumeration → grouped) and fixed an accuracy gap — it now lists the §5 delete-confirm view. Left intact: the EJS view config-doc headers (the only schema for untyped locals), the security-rationale comments, and the legitimate §9 forward-ref in `admin-roles.ts` (the deferred last-effective-admin check). Docs/comments-only (per AGENTS.md, no stability-reviewer needed); typecheck + 244 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.
|
||||
- [x] 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. → Pass over the §5 admin tests. The genuine §5-era duplication was all in `app.test.ts`: the three admin-screen HTTP tests (Users/Groups/Roles) each repeated an identical ~13-line harness preamble (createApp + listen + url + CSRF token + admin cookie + get/post), an identical 5-line gate block, and a stateful in-memory `KetoClient` defined 3× (the trivial `stubKeto` + two byte-identical inline fakes). Unified into shared helpers — `adminHarness(t, opts)` → `{url, token, get, post}`, `assertAdminGate(url, get, path)`, and one `fakeKeto(tuples?, over?)` that subsumes `stubKeto` (the login tests now use `fakeKeto([], …)`) and both inline admin fakes (`fakeKeto(tuples)` / `fakeKeto(tuples, { expand })`); hoisted the shared `sameSet`/`matchesTuple` up next to it. The per-module unit files (admin-users/groups/roles + the focused units) already follow the deliberate matrix pattern and the §3/§4 "don't force-merge across distinct modules" rule, so the near-identical `build*ListModel` tests stay per-file (each guards its own function; the source-side list-model dedup is the deferred arch-M3 item, not the test side). −30 net lines, zero coverage lost; typecheck + 244 units green.
|
||||
|
||||
## 6. Hydra — OAuth2/OIDC provider (can ship after the rest)
|
||||
- [ ] Login-challenge handler: authenticate via Kratos session, accept/reject.
|
||||
|
||||
Reference in New Issue
Block a user