From 0a5eafd2f8c05324d26a3c70bfc49613f34d4130 Mon Sep 17 00:00:00 2001 From: lilleman Date: Thu, 18 Jun 2026 19:25:02 +0200 Subject: [PATCH] =?UTF-8?q?Tighten=20=C2=A75=20admin=20comments=20+=20READ?= =?UTF-8?q?ME=20(todo=20=C2=A75=20cleanup);=20compress=20the=20three=20nea?= =?UTF-8?q?r-identical=20admin=20module=20headers=20(drop=20restatement=20?= =?UTF-8?q?the=20README/code=20already=20carry),=20shorten=20the=20README?= =?UTF-8?q?=20Layout=20views/=20run-on=20+=20add=20the=20missing=20delete-?= =?UTF-8?q?confirm=20view.=20Also=20bank=20a=20pre-existing=20AGENTS.md=20?= =?UTF-8?q?tweak:=20skip=20the=20stability-reviewer=20for=20purely=20doc/c?= =?UTF-8?q?omment=20changes.=20Docs/comments-only=20=E2=80=94=20typecheck?= =?UTF-8?q?=20+=20244=20units=20green.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- AGENTS.md | 3 ++- README.md | 2 +- src/admin-groups.ts | 15 +++++++-------- src/admin-roles.ts | 17 ++++++++--------- src/admin-users.ts | 11 +++++------ todo.md | 2 +- 6 files changed, 24 insertions(+), 26 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 116cfda..bcc5000 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,4 +57,5 @@ docker compose -f compose.yml up --build -d # production exact by `.npmrc` (`save-exact=true`) + `npm ci`; the base image by tag (e.g. `node:24.16.0-alpine3.24`). - Run the stability reviewer agent after every implementation of something that can be like - a PR. That includes an implementation from the todo file that is pushed directly to master. \ No newline at end of file + a PR. That includes an implementation from the todo file that is pushed directly to master. + Skip this if the changes are purely documentation and/or comments. \ No newline at end of file diff --git a/README.md b/README.md index c036486..dfeb065 100644 --- a/README.md +++ b/README.md @@ -551,7 +551,7 @@ src/discovery.ts discoverPlugins(): scan plugins/, import + validate each pl src/router.ts matchRoute()/allowedMethods()/isAuthorized(): map method+path → plugin route, params, permission gate (§2) src/view-resolver.ts renderPluginView(): render plugins//views/.ejs; plugin views can include() core partials (§2) src/menu-config.ts loadMenuConfig()/defineMenu(): read config/menu.ts (central override + branding), validated at boot (§2) -views/ Core EJS templates (index = the app-shell People dashboard, admin/ = the Users list + create/edit form, the Groups list + create form + membership detail, and the Roles list + create form + assign/effective-access detail, auth = themed Kratos self-service page, 403/404/500, partials/ incl. app shell, nav tree, filter bar, data table, pagination, form field, auth card, alert, flow body, user-form/group-form/group-detail/role-form/role-detail bodies, menu/popover, theme switch, icon sprite) +views/ Core EJS templates: index (app-shell dashboard), admin/ (Users/Groups/Roles lists + create/edit/detail + delete-confirm), auth (themed Kratos flows), 403/404/500, partials/ (shell, nav tree, filter bar, data table, pagination, field, auth card, alert, flow + admin bodies, menu/popover, theme switch, icon sprite) public/ Static assets under /public/ (css/styles.css + auth.css, favicon, robots.txt) config/menu.ts Central menu override + branding (optional; defaults apply if absent) ory/ Ory service config (kratos/: identity schema, kratos.yml, oidc/ SSO claims mapper, tokenizer/ session→JWT claims mapper + dev signing JWKS; keto/: keto.yml + namespaces.keto.ts OPL — role/group/resource; hydra/hydra.yml: OAuth2 issuer + login/consent URLs) + storage init (postgres/init/init.sql: one DB per service) diff --git a/src/admin-groups.ts b/src/admin-groups.ts index 53b0700..060582c 100644 --- a/src/admin-groups.ts +++ b/src/admin-groups.ts @@ -1,11 +1,10 @@ -// Built-in Groups admin screen (todo §5): list / create / delete Keto groups and manage their -// membership. A group is a Keto subject set `Group:#members`; a membership tuple's subject is -// a user (`subject_id = user:`) or a nested group (`subject_set = Group:#members`). Writes -// go only to Keto (README "stateless"); there is no group store. Keto has no "create object" — a -// group exists exactly while it has ≥1 member, so creating one writes its first-member tuple and -// deleting one removes every member tuple. The pure builders turn tuples + the request URL into the -// building-block view models; `handleAdminGroups` is the imperative shell app.ts dispatches to — it -// gates (admin only), CSRF-guards every mutation, and maps each action to a RouteResult. +// Built-in Groups admin screen (todo §5): list / create / delete Keto groups and manage membership. +// A group is a Keto subject set `Group:#members`; a member is a user or a nested group (see +// parseSubject). Writes go only to Keto (README "stateless"). Keto has no "create object" — a group +// exists exactly while it has ≥1 member, so create writes its first-member tuple and delete removes +// every member tuple. Pure builders turn tuples + the request URL into view models; `handleAdminGroups` +// is the imperative shell app.ts dispatches to — gated admin-only, CSRF-guarded, mapping each action +// to a RouteResult. import { ADMIN_GROUPS_BASE, adminNav, buildConfirmModel, guardedForm, requireAdmin } from "./admin-nav.ts"; import type { FieldConfig } from "./admin-users.ts"; diff --git a/src/admin-roles.ts b/src/admin-roles.ts index 437c750..096c4f2 100644 --- a/src/admin-roles.ts +++ b/src/admin-roles.ts @@ -1,13 +1,12 @@ // Built-in Roles & permissions admin screen (todo §5): list / create / delete Keto roles and assign -// them to users and groups. A role is a Keto subject set `Role:#members` (OPL: members are -// users or groups, resolved transitively) — the source of truth for the JWT `roles` claim. It shares -// the user|group membership model of the Groups screen, so the pure helpers (parseSubject, member -// pickers, tuple paging) are reused from admin-groups. The one role-specific piece is the **effective -// access** view: `keto.expand(Role:#members)` returns the membership tree, which we flatten to -// the distinct set of users who hold the role directly or transitively via a group. Login resolves -// the same transitive membership into the JWT `roles` (login.ts readRoles), so this view matches what -// a user's token actually grants. Writes go only to Keto; Kratos is read only to label members. -// `handleAdminRoles` is the imperative shell app.ts dispatches to — gated admin-only, CSRF-guarded. +// them to users and groups. A role is a Keto subject set `Role:#members` (OPL: members are users +// or groups, resolved transitively) — the source of truth for the JWT `roles` claim. It shares the +// Groups screen's membership model, so the pure helpers (parseSubject, member pickers, tuple paging) +// are reused from admin-groups. The role-specific piece is the **effective access** view: +// `keto.expand(Role:#members)` flattened to the distinct users who hold the role directly or via +// a group — matching what login projects into the JWT (login.ts readRoles). Writes go only to Keto; +// Kratos is read only to label members. `handleAdminRoles` is the imperative shell app.ts dispatches +// to — gated admin-only, CSRF-guarded. import { ADMIN_PERMISSION, ADMIN_ROLES_BASE, adminNav, buildConfirmModel, guardedForm, requireAdmin } from "./admin-nav.ts"; import { diff --git a/src/admin-users.ts b/src/admin-users.ts index 0edc0ec..77a8d07 100644 --- a/src/admin-users.ts +++ b/src/admin-users.ts @@ -1,9 +1,8 @@ -// Built-in Users admin screen (todo §5): list Kratos identities (filter/sort/paginate) and -// create / edit / deactivate / delete / trigger-recovery them. Writes go only to Kratos via the -// admin client (README "stateless"); the app holds no user store. The pure builders here turn -// identities + the request URL into the building-block view models; `handleAdminUsers` is the -// imperative shell app.ts dispatches to — it gates (admin only), CSRF-guards every mutation, and -// maps each action to a RouteResult (render a page, or redirect after a write — PRG). +// Built-in Users admin screen (todo §5): list Kratos identities (filter/sort/paginate) + +// create/edit/deactivate/delete/trigger-recovery. Writes go only to Kratos via the admin client +// (README "stateless"). Pure builders turn identities + the request URL into building-block view +// models; `handleAdminUsers` is the imperative shell app.ts dispatches to — gated admin-only, +// CSRF-guarded, each action mapped to a RouteResult (render, or redirect after a write — PRG). import { ADMIN_USERS_BASE, adminNav, buildConfirmModel, guardedForm, requireAdmin } from "./admin-nav.ts"; import type { RequestContext, User } from "./context.ts"; diff --git a/todo.md b/todo.md index 196d17c..e86aba4 100644 --- a/todo.md +++ b/todo.md @@ -98,7 +98,7 @@ everything via Docker. - [x] Roles & permissions: Keto relations — assign roles to users/groups; "effective access" view via Keto expand. → `src/admin-roles.ts`: a role is a Keto subject set `Role:#members` (OPL: members are users or groups, resolved transitively — the source of truth the §4 login projects into the JWT). Same shape as the Groups screen, so the pure membership helpers are reused from `admin-groups.ts` (`parseSubject`, `isValidGroupName`, `memberView`, `groupsFromTuples`, and now-exported `pagedTuples`/`memberCandidates`/`safeDecode`). Routes (`handleAdminRoles`, dispatched by app.ts): `GET /admin/roles` (list — search/sort/paginate over one Keto scan), `GET|POST /admin/roles/new`+`/` (create = assign first member; rejects invalid/duplicate name), `GET /admin/roles/:name` (detail), `POST …/members` (assign a user/group) · `…/members/delete` (revoke) · `…/delete` (remove all member tuples). The one role-specific piece is **effective access**: `keto.expand(Role:#members, {maxDepth:50})` → `expandToEffectiveUsers` flattens the tree to the distinct users who hold the role directly *or transitively via a group* (the coarse JWT projection stays direct-only per the README's one-read-per-login design; this view is where group→role inheritance is surfaced). Writes go **only to Keto**; Kratos is read only to label members. Gated admin-only (anon→/login, non-admin→403) + CSRF-guarded, like Users/Groups. Added a "Roles" entry (`i-shield`) to the shared `admin-nav.ts`; new `.plain-list` CSS rule. Tests-first: `admin-roles.test.ts` (builders + expand-flatten matrix) + `app.test.ts` HTTP integration (gate/list/create/dup-reject/assign user&group/effective-access-via-expand/revoke/delete + CSRF + malformed-name→404). Stability-reviewer run as a local PR: APPROVE, no Critical/High; addressed its expand-depth nit (explicit `maxDepth`). 237→243 units + typecheck green. **Live boot-verify caught a real bug the tests missed:** Keto v26.2.0's expand nests the subject under `tuple` (`{type:"leaf",tuple:{subject_id}}`), not at the node top-level as the §4 `ExpandTree` type had guessed — fixed the type + walker + the (wrongly-shaped) fixtures, then re-verified live that a user reachable only through a group surfaces in effective access; torn down. Global-menu wiring is the next §5 item. - [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). -- [ ] 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 §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. ## 6. Hydra — OAuth2/OIDC provider (can ship after the rest)