From b77a4875157ed93037c44db8ad97e8f1f0343eaf Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Thu, 13 Aug 2026 22:52:31 +0200 Subject: [PATCH 1/7] feat: pending group_role domains, security-level refs, and an audit that can see past OpenAPI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues that all trace back to one habit: writing down "the API cannot do this" when what was actually checked was narrower than that. #106 — a `ct.groupRole` whose group is declared in the same config could not plan on a host where that group did not exist yet, so one config planned clean on prod and exited 1 on dev. The information needed was already available at apply time: the resource tier runs first, so the group exists before permissions are applied. The domain now resolves to a PendingRef and is completed inside applyPermissionPlan by reading the freshly created group's own /groups/{id}/roles and matching the declared role name. Pending is opt-in per call site, because this is the one pending ref reresolvePendingValue cannot finish from state alone — everywhere else a same-run group keeps the old fail-fast message. A role the created group does not have is still a hard error listing the available ones. #110 — "security-level ids are universal, so a numeric literal is portable" was an assumption. cc_securitylevel is an admin-editable master-data table with an auto-increment id, so 1/2/3 line up by convention, not by construction. Adds `{ securityLevel: "" }`, resolved through GET /securitylevels (a new `ct get security-levels` lists them), keeps numerics working, and retires the "numeric-universal dimension" exemption in the coverage report. #111 — the manual-surface re-audit only grepped the OpenAPI spec, so it could never discover the legacy master-data endpoint the admin UI writes through — and concluded "permanently", "never", "not resources at all" on that basis. The procedure now requires a REST probe AND a getMasterData registry probe, plus a UI capture where neither settles it, and says to record which probes ran. The three buckets are relabelled so a capability fact, a not-yet-implemented item and a product decision stop reading alike, and the absolutes that were unsupported are down-scoped in the runbook, api-coverage, the handbook and the code comments. Bereiche (#108) and the generic master-data driver (#109) are not implemented here: both need live probing and a design decision about REST-vs-master-data authority per table. Claude-Session: https://claude.ai/code/session_016TacHkv3oVczwXGHBwckq2 --- docs/README.md | 12 +- docs/api-coverage.md | 11 +- docs/handbuch/field-definitions.md | 30 +++- docs/handbuch/permissions.md | 132 +++++++++----- docs/runbook-manual-surface.md | 131 +++++++++----- src/commands/get.ts | 5 + src/coverage/report.ts | 17 +- src/permissions/adopt.ts | 3 +- src/permissions/apply.ts | 40 ++++- src/permissions/plan.ts | 29 ++-- src/permissions/scope.ts | 24 +-- src/permissions/types.ts | 16 +- src/resolve/refs.ts | 68 ++++++-- src/resolve/resolver.ts | 218 ++++++++++++++++++------ tests/permission-adopt.test.ts | 19 ++- tests/permission-pending-domain.test.ts | 149 +++++++++++++--- tests/permission-scope-refs.test.ts | 66 ++++++- tests/portable-refs.test.ts | 12 +- 18 files changed, 734 insertions(+), 248 deletions(-) diff --git a/docs/README.md b/docs/README.md index 0609890..6d448f0 100644 --- a/docs/README.md +++ b/docs/README.md @@ -48,9 +48,9 @@ A page with no code behaviour to track declares `sources: []` plus a ## Everything else — developer- and operator-facing, unpublished -| Page | About | -| -------------------------------------------------------- | --------------------------------------------------------------------- | -| [`api-coverage.md`](api-coverage.md) | Which ChurchTools endpoints support which CRUD verbs | -| [`group-field-decisions.md`](group-field-decisions.md) | Which group fields are managed vs. left to the CT UI, and why | -| [`runbook-manual-surface.md`](runbook-manual-surface.md) | What `ct` cannot automate — API gaps and the manual steps around them | -| `superpowers/` | Historical implementation plans; kept as a record, never published | +| Page | About | +| -------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | +| [`api-coverage.md`](api-coverage.md) | Which ChurchTools endpoints support which CRUD verbs | +| [`group-field-decisions.md`](group-field-decisions.md) | Which group fields are managed vs. left to the CT UI, and why | +| [`runbook-manual-surface.md`](runbook-manual-surface.md) | What `ct` cannot automate today — where the write path is missing, and the manual steps around it | +| `superpowers/` | Historical implementation plans; kept as a record, never published | diff --git a/docs/api-coverage.md b/docs/api-coverage.md index 8d98495..3fcfede 100644 --- a/docs/api-coverage.md +++ b/docs/api-coverage.md @@ -30,7 +30,7 @@ Methods marked only if they actually exist on the matched path. "Update" = PUT o | 11 | `ct_meeting_point` | — none — | ❌ | ❌ | ❌ | ❌ | **Not in API → fully manual.** Zero matches for `treffpunkt`/`meetingpoint`/`meeting point` anywhere in the spec. Closest neighbours are _meeting templates_ (`/group/meetingtemplates`, full CRUD) and _group meetings_ (`/groups/{groupId}/meetings`, CRUD) — different concepts; confirm with product whether "meeting point" was meant to be one of those. | | 12 | `ct_relationship_type` | `/person/relationshiptypes`, `/person/relationshiptypes/{id}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** | | 13 | `ct_person_status` | `/statuses`, `/statuses/{id}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD, live-verified 2026-08-13** (eqrm prod, CT 3.135.2, read from the instance OpenAPI spec): `/statuses` → GET, POST; `/statuses/{id}` → GET, PUT, DELETE. `POST` requires `name`, `shorty`, `isMember`; `PUT` requires ALL of `name`, `shorty`, `isMember`, `isSearchable`, `sortKey`, `securityLevelId` — uniquely strict among managed types (every other managed PUT declares no required fields), and since PUT is a full replace the registry manages all six rather than a subset. The person/community **Status** master data (tag `Status`): "0 - First", "3 - Group Active", …, the domain a `ct.status` permission grant hangs off. Adoptable since #96, which is what makes a config using that domain self-sufficient across hosts. Do NOT conflate with `ct_group_status` (row 8, no endpoint at all) or `/group/memberstatus` (member statuses, string ids). Master data — never a person record; the people guard is unaffected. | -| 14 | `ct_department` | `/departments` | list ✅ / by-id ❌ | ❌ | ❌ | ❌ | **Read-only — a ref catalog, not a managed resource.** Live-probed 2026-08-13 (eqrm prod, CT 3.135.2): `GET /departments` returns `[{id, name, nameTranslated, sortKey, shorty}]`; no `POST`/`PUT`/`DELETE` on `/departments` exists in the spec, and there is no `/departments/{id}` path at all. Bereiche are the `cdb_bereich` permission scope dimension (`churchdb:view alldata`), so `ct` resolves them BY NAME for a `scope: [{ department: "…" }]` reference (#98) and surfaces them via `ct get departments` — but they can never be declared, adopted or created. | +| 14 | `ct_department` | `/departments` | list ✅ / by-id ❌ | ❌ | ❌ | ❌ | **No REST write path — a ref catalog here, not a managed resource.** Live-probed 2026-08-13 (eqrm prod, CT 3.135.2): `GET /departments` returns `[{id, name, nameTranslated, sortKey, shorty}]`; no `POST`/`PUT`/`DELETE` on `/departments` exists in the spec, and there is no `/departments/{id}` path at all. Bereiche are the `cdb_bereich` permission scope dimension (`churchdb:view alldata`), so `ct` resolves them BY NAME for a `scope: [{ department: "…" }]` reference (#98) and surfaces them via `ct get departments` — but `ct` cannot declare, adopt or create one today. **Not a ChurchTools limitation:** the admin UI creates Bereiche through the legacy `POST /index.php?q=churchdb/ajax` `func=saveMasterData` interface (captured 2026-08-13), which appears in no OpenAPI spec — see #108/#109, and #111 for why an OpenAPI-only audit could never have found it. | > **Create-time required fields (#73).** The "POST ✅" marks above were spec-derived, not live-exercised for > create. CT's POST validators require fields the tool does not manage for diffing: `group-type` needs @@ -85,10 +85,11 @@ API-client libraries (5pm-HDH `churchtools-api` @ CT 3.104, bensteUEM `src/api/schema.d.ts` is git-ignored and was not available offline. Re-verify per the runbook's re-audit procedure once the schema is regenerated. -| Resource | Matched path(s) | GET | POST/PUT/PATCH/DELETE | Verdict | -| ------------------------------------ | ----------------------------- | ------------------ | --------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| Person master-data model | `/person/masterdata` | ✅ (single object) | ❌ | **Read-only.** Versionable master-data model incl. the `securityLevels` enumeration. No write endpoint (edited in the CT master-data admin UI). | -| Data-field definitions (Datenfelder) | `/dbfields`, `/dbfields/{id}` | list ✅ / by-id ✅ | ❌ | **Read-only.** Unified person + group field definitions, discriminated by `fieldCategory`. Mutation only via legacy churchdb AJAX (`db_insert/update/deletefields`), not REST. | +| Resource | Matched path(s) | GET | POST/PUT/PATCH/DELETE | Verdict | +| ------------------------------------ | ----------------------------- | ------------------ | --------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Person master-data model | `/person/masterdata` | ✅ (single object) | ❌ | **Read-only.** Versionable master-data model incl. the `securityLevels` enumeration. No write endpoint (edited in the CT master-data admin UI). | +| Data-field definitions (Datenfelder) | `/dbfields`, `/dbfields/{id}` | list ✅ / by-id ✅ | ❌ | **Read-only.** Unified person + group field definitions, discriminated by `fieldCategory`. Mutation only via legacy churchdb AJAX (`db_insert/update/deletefields`), not REST. | +| Security levels | `/securitylevels` | list ✅ | ❌ (no REST write) | **Read here, writable elsewhere.** Flat `[{id, name, sortKey}]` — live-verified 2026-08-13 (eqrm prod, CT 3.135.2). The `cc_securitylevel` permission scope dimension, resolvable by name since #110. The ids are NOT protocol constants: `cc_securitylevel` is an editable master-data table, writable through the legacy `getMasterData`/`saveMasterData` endpoint (#109/#110). | See [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md) for the full writability decision, evidence, and the schema/values boundary. diff --git a/docs/handbuch/field-definitions.md b/docs/handbuch/field-definitions.md index a41a6a5..e45aeec 100644 --- a/docs/handbuch/field-definitions.md +++ b/docs/handbuch/field-definitions.md @@ -3,7 +3,7 @@ title: Field definitions & security levels sources: - src/commands/get.ts - src/api/ctClient.ts -sources_hash: 16a6283b7ce1d982 +sources_hash: 93ba678d9f747e3c reviewed: 2026-08-13 --- @@ -40,6 +40,7 @@ _definitions only_; none of them read or write a per-record field value. ```bash ct get person-masterdata # the person master-data model incl. the security-level enumeration +ct get security-levels # just the security levels, as a flat list ct get data-fields # all data-field definitions (person + group), one row per field ``` @@ -47,9 +48,13 @@ ct get data-fields # all data-field definitions (person + group), one ro paged list) — the versionable person master-data model. It carries the security-level enumeration (`securityLevels`), which is the model the churchdb permission scopes reference (`cc_securitylevel`; see [`permissions.md`](permissions.md)). - This is where **security levels** are surfaced — ChurchTools has no dedicated - standalone security-levels REST resource; the Persons/Groups master-data module - is where they are defined, and `/person/masterdata` is its download. +- `ct get security-levels` → `GET /securitylevels` (#110). The same levels as a + flat `[{id, name, sortKey}]` list — the catalog a + `scope: [{ securityLevel: "" }]` reference resolves against. (An + earlier version of this page said no standalone security-levels resource + existed; it does, live-verified 2026-08-13 on CT 3.135.2.) Reading it per host + is also how you check whether the usual ids `1..4` really hold there — they are + editable master-data rows, not protocol constants. - `ct get data-fields` → `GET /dbfields` (auto-paginated). The **unified** data-field definition catalog. Person master-data fields **and** group custom fields live in the same list, discriminated per-row by `fieldCategory` @@ -100,13 +105,20 @@ out of bounds. If a future CT release adds REST write endpoints on `/dbfields` resource then — add a `src/resources/registry.ts` entry + DSL + tests — and move the row out of the read-only section here and in the runbook. +Note that "never write the legacy surface" is a **decision**, not a capability +limit, and [#109](https://github.com/eqrm/ct-cli/issues/109) reopens it for the +master-data registry specifically (a self-describing set of editable tables the +admin UI writes through `saveMasterData`). If that lands, this paragraph needs +re-deciding rather than merely re-reading. + ## Endpoint reference -| Purpose | Path | Methods (this CT) | `ct` surface | -| ------------------------------------------ | ------------------------------------------------------------------------------ | ----------------- | -------------------------- | -| Person master-data model + security levels | `/person/masterdata` | GET (read-only) | `ct get person-masterdata` | -| Data-field definitions (person + group) | `/dbfields`, `/dbfields/{id}` | GET (read-only) | `ct get data-fields` | -| Field-definition **mutation** | legacy `churchdb` AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) | non-REST | **not managed — manual** | +| Purpose | Path | Methods (this CT) | `ct` surface | +| ------------------------------------------ | ------------------------------------------------------------------------------ | ------------------- | -------------------------- | +| Person master-data model + security levels | `/person/masterdata` | GET (read-only) | `ct get person-masterdata` | +| Security levels, standalone | `/securitylevels` | GET (no REST write) | `ct get security-levels` | +| Data-field definitions (person + group) | `/dbfields`, `/dbfields/{id}` | GET (read-only) | `ct get data-fields` | +| Field-definition **mutation** | legacy `churchdb` AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) | non-REST | **not managed — manual** | All paths verified against public CT client libraries + CT Academy docs, **not** against this repo's (git-ignored, ungenerated) `src/api/schema.d.ts`. diff --git a/docs/handbuch/permissions.md b/docs/handbuch/permissions.md index fe0c962..6de9ae0 100644 --- a/docs/handbuch/permissions.md +++ b/docs/handbuch/permissions.md @@ -5,7 +5,7 @@ sources: - src/resolve/resolver.ts - src/resolve/refs.ts - src/config/context.ts -sources_hash: 050a604ccb2391c8 +sources_hash: e3b937247ebebc2a reviewed: 2026-08-13 --- @@ -57,9 +57,10 @@ reference or a numeric `id`: - `ct.groupRole` — `group: "", role: ""` resolves the (group, role) pair to its pairing domainId per host (#25), or `id: ` targets one directly. The group must be **managed** (declared via `ct.group` - or adopted into state) and already created — a same-run group is rejected - (its pairing id is only known once it exists; pass a numeric `id` there). - Declaring both a logical form and a numeric `id` is a conflict and throws. + or adopted into state); it need not exist on the host yet — a group declared + in the same config plans as a pending domain and is granted later in that same + `ct apply`, once it exists (#106). Declaring both a logical form and a numeric + `id` is a conflict and throws. See "domainId semantics" for how the pairing id is resolved. - `ct.status` — `personStatus: ""` resolves against the live `/statuses` catalog per host, or `id: ` targets one directly. @@ -255,14 +256,27 @@ handled as a **pending domain** rather than aborting the plan: genuine unresolvable) is reserved for references that resolve to **nothing**: a key absent from the config, state, and the live catalog (a typo). -**`group_role` is deliberately NOT symmetric here.** A `group_role` domain id is +**`group_role` behaves the same way since #106.** A `group_role` domain id is the (group, role) **pairing** id, which only exists on -`GET /groups/{groupId}/roles` — re-resolving it needs a _live fetch_ after the -group exists, not just a post-execute state lookup. So a `group_role` domain -referencing a same-run-created **group** still fails fast with its own -actionable message ("apply the group first, or pass a numeric id"). Its harder -deferral is out of scope for #69 (which targets the #23 `group_type_role` -scenario). +`GET /groups/{groupId}/roles` — so completing it needs a _live fetch_ after the +group exists, not just a post-execute state lookup. `ct apply` does exactly +that: `executePlan` creates the group, then permission reconciliation reads the +new group's own role list, matches the declared `role` name, and grants on the +resulting pairing id — one run, no second merge. + +- `ct plan` renders the domain as + `, role=) (created this apply)>`, the same + pending marker every other deferred reference uses. +- A `role` name the created group turns out not to have is still a **hard + error**, listing the roles it does have — the same message you would get at + plan time on a host where the group already exists. +- Nothing changes on a host where the group already exists: the domain resolves + to a concrete pairing id at plan time, exactly as before. + +Until #106 this was a hard error, and that made a config **non-portable by +construction**: declaring a group and a grant on its own role planned clean on +prod (where the group existed) and exited 1 on dev (where it did not), so one +logical change had to be split across two merges. ## Scope resolution @@ -321,12 +335,13 @@ ct.groupRole({ `{ campus: "koblenz" }` is sugar for `ref.campus("koblenz")` — the same `Ref` the rest of the DSL uses — so both spellings are interchangeable. -| `scopeField` | Reference form | Resolved against | -| ---------------- | ----------------------------------------- | -------------------------------------------------- | -| `cdb_gruppe` | `{ group: "" }` (or the bare string) | managed groups | -| `cdb_station` | `{ campus: "" }` | managed campuses, then `GET /campuses` | -| `cdb_gruppentyp` | `{ groupType: "" }` | managed group types, then `GET /group/grouptypes` | -| `cdb_bereich` | `{ department: "" }` | `GET /departments` **only** — read-only, see below | +| `scopeField` | Reference form | Resolved against | +| ------------------ | ----------------------------------------- | ------------------------------------------------- | +| `cdb_gruppe` | `{ group: "" }` (or the bare string) | managed groups | +| `cdb_station` | `{ campus: "" }` | managed campuses, then `GET /campuses` | +| `cdb_gruppentyp` | `{ groupType: "" }` | managed group types, then `GET /group/grouptypes` | +| `cdb_bereich` | `{ department: "" }` | `GET /departments` **only** — see below | +| `cc_securitylevel` | `{ securityLevel: "" }` | `GET /securitylevels` **only** — see below (#110) | **Why this matters:** campus ids are host-specific — Mainz is `0` on eqrm prod and `6` on eqrm dev. A campus-scoped grant written as a numeric literal is @@ -347,37 +362,58 @@ Three things are hard errors at **plan** time, never a guessed `dataId`: - a reference whose dimension does not match the right's `scopeField` — e.g. `{ groupType: … }` on `churchdb:view station` (a `cdb_station` right); -- a reference on a dimension that has **no** logical form (`cc_securitylevel`, - `ccm_data_category`, `oauth_client`, … — values that are not resources at - all) — the message points at the numeric escape hatch; +- a reference on a dimension that has **no** logical form yet + (`cdb_comment_viewer`, `ccm_data_category`, `oauth_client`, … — values `ct` + has no host-independent way to name today) — the message points at the numeric + escape hatch; - a **bare string** on a non-group dimension. A string always means "managed group", so on e.g. a `cdb_station` right it would either fail confusingly or — worse — match an unrelated group that happens to carry that key. -### Departments are referenceable, never declarable +### Departments and security levels: referenceable, not declarable -`cdb_bereich` (Bereiche) is the one dimension with a **read-only** catalog. -Live-probed against the instance's own OpenAPI spec (eqrm prod, CT 3.135.2, -2026-08-13): `GET /departments` exists, and **no** `POST`/`PUT`/`DELETE` on -`/departments` does. +`cdb_bereich` (Bereiche) and `cc_securitylevel` (Sicherheitslevel) are the two +dimensions `ct` **reads but does not manage**. Neither has a REST write verb — +live-probed against the instance's own OpenAPI spec (eqrm prod, CT 3.135.2, +2026-08-13): `GET /departments` and `GET /securitylevels` exist; no +`POST`/`PUT`/`DELETE` on either does. -So a department reference resolves by name on every host — which is what makes -`churchdb:view alldata` ("Personen eines Bereiches sehen") declarable at all — -but there is no `ct.department` resource, no `ct adopt department`, and a name -that matches nothing is a **hard error** rather than a create: +That is a fact about the REST API and about what this tool drives — **not** a +claim that ChurchTools cannot create them. Both are rows in CT's editable +master-data registry, which the admin UI writes through a legacy endpoint that +appears in no OpenAPI spec. Making them declarable is tracked in +[#108](https://github.com/eqrm/ct-cli/issues/108) / +[#109](https://github.com/eqrm/ct-cli/issues/109). + +So a reference of either kind resolves by name on every host — which is what +makes `churchdb:view alldata` ("Personen eines Bereiches sehen") and +`churchdb:+see persons` declarable at all — but there is no `ct.department` / +`ct.securityLevel` resource, no `ct adopt` for them, and a name that matches +nothing is a **hard error** rather than a create: ``` Cannot resolve department:nope referenced at … : no live department at -/departments matches key "nope". departments are a read-only catalog in -ChurchTools — they cannot be declared or adopted, so fix the key/name -(list them with `ct get departments`) or use a numeric id. +/departments matches key "nope". ct reads departments but does not manage +them, so it cannot create this one — fix the key/name (list them with +`ct get departments`), create it in the ChurchTools admin UI, or use a +numeric id. ``` -Discover the names with `ct get departments`. Because the id always comes from -the live catalog, a department scope is never "pending" and is never re-resolved -at apply time — it is host-correct the moment it resolves. A managed resource -that happens to share the key does **not** shadow it (that would be exactly the -misgrant this feature exists to prevent). +Discover the names with `ct get departments` / `ct get security-levels`. Because +the id always comes from the live catalog, such a scope is never "pending" and is +never re-resolved at apply time — it is host-correct the moment it resolves. A +managed resource that happens to share the key does **not** shadow it (that would +be exactly the misgrant this feature exists to prevent). + +**Why security levels got a reference form (#110).** They were previously treated +as _numeric-universal_: `scope: [1, 2, 3]` was considered portable because the ids +"mean the same thing on every instance". They do — by convention. `cc_securitylevel` +is an admin-editable master-data table with an auto-increment id and a `sortkey`, +so someone adding or reordering a level on one host silently changes what a +hard-coded `[1, 2, 3]` grants there, with a green plan. Numerics still work +everywhere; the trade-off is that level names are localised German strings +(`"Stufe 3 (Hoch)"` → `stufe_3_hoch`), so a **rename** breaks a reference where a +number would have survived. Pick per config which risk you would rather carry. ### Numeric scope escape hatch (#49) @@ -392,9 +428,13 @@ e.g.: "cc_securitylevel"` For these, a `dataId` like `1`, `2`, `3` names a security level or a -comment-viewer bucket — **not** a group — so `GET /groups/{1,2,3}` 404s and -there is no logical/managed key to reference it by. A `scope` array entry may -therefore be a plain number instead of a string: +comment-viewer bucket — **not** a group — so `GET /groups/{1,2,3}` 404s. A +`scope` array entry may therefore be a plain number instead of a string: + +(Security levels gained a name-based form in #110 — see +[Departments and security levels](#departments-and-security-levels-referenceable-not-declarable). +Numerics keep working there; `cdb_comment_viewer` and friends still have nothing +but numerics.) ```ts { right: "churchdb:security level view own data", scope: [1, 2, 3, 5] }, @@ -487,7 +527,12 @@ to be accepted by `ct plan` (the round trip is locked by tests): clearly-marked placeholder comment telling you to `ct adopt group ` first — scope keys must be state keys (see [Scope resolution](#scope-resolution)). - - **Any other scope dimension** (`cc_securitylevel`, `cdb_comment_viewer`, + - **A catalog dimension `ct` reads but does not manage** (`cdb_bereich`, + `cc_securitylevel`): the emitter is pure — no client, no fetch — so it cannot + turn the id into a name. It emits the number plus a `NOTE` giving the portable + form (`{ department: "" }` / `{ securityLevel: "" }`) to write by + hand. + - **Any other scope dimension** (`cdb_comment_viewer`, `oauth_client`, …): there is no group to adopt, so the `dataId`(s) are emitted directly as a numeric `scope: [1, 2, 3]` — always an active line, with a comment naming the right's actual scope dimension. `ct adopt group ` is never @@ -643,9 +688,8 @@ routinely has two declarable roles and one blocked one, and group granularity would hide exactly that. A role instance is declarable when every authored grant is either unscoped, uses the `-1` ALL sentinel, scopes by a dimension with a [logical reference form](#scope-resolution) (`cdb_gruppe`, `cdb_station`, -`cdb_gruppentyp`, `cdb_bereich`), or scopes by a numeric-but-host-independent -dimension (`cc_securitylevel`). Anything else is module data with no resource -behind it, and is named as the blocker. +`cdb_gruppentyp`, `cdb_bereich`, `cc_securitylevel`). Anything else scopes by a +dimension `ct` cannot yet name portably, and is named as the blocker. ## Example diff --git a/docs/runbook-manual-surface.md b/docs/runbook-manual-surface.md index b7ca3c0..c782730 100644 --- a/docs/runbook-manual-surface.md +++ b/docs/runbook-manual-surface.md @@ -5,24 +5,33 @@ apply "selective adoption" deliberately instead of guessing. If reproducing an instance from code means "rebuild the scaffold with `ct apply`, then do _these_ specific clicks by hand," this is the list of those clicks. -Every item below falls into exactly one of three buckets: - -- **API gap** — ChurchTools does not expose a write endpoint (or any - endpoint) for this. Nothing in `ct` can close this until CT ships one. -- **Not yet implemented** — the ChurchTools API supports it, but `ct` doesn't - drive it yet. Tracked by an open issue; closing the issue removes the item - from this runbook. -- **Out of tool scope** — deliberately, permanently unmanaged. Not a gap to - close; a boundary the tool is designed to respect. - -## API gap — CT does not expose a write endpoint - -| Item | What it is | Why manual | Where in the CT admin UI | How to verify | -| ---------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| Group member-statuses | The set of "member status" values a group can assign to its members (e.g. active/candidate) — NOT the same dimension as a group's own `groupStatusId`, which IS fully managed (`ct.group({ groupStatusId })`, [`docs/group-field-decisions.md`](group-field-decisions.md)); do not conflate the two (#67) | Only `GET /group/memberstatus` exists; no create/update/delete endpoint ([`docs/api-coverage.md`](api-coverage.md) #8) | Group settings → member status admin (org-wide master data, not per-group) | `ct get raw /group/memberstatus` (no dedicated `ct get member-statuses` subcommand yet — the generic `raw` path covers it) and diff by eye against the expected list below | -| Meeting points (Treffpunkte) | A group's meeting-location master data | No endpoint at all — zero matches for `treffpunkt`/`meetingpoint` anywhere in the OpenAPI spec ([`docs/api-coverage.md`](api-coverage.md) #11) | Group admin → meeting point field on a group | No API verification possible; visually confirm in the UI. (Do not confuse with _meeting templates_ `/group/meetingtemplates` or _group meetings_ `/groups/{id}/meetings`, both full CRUD but different concepts — confirm with product if "meeting point" was meant to be one of those instead) | -| Bereiche (departments) — creating/renaming one | The Bereich master data (`/departments`) — the `cdb_bereich` permission scope dimension | `GET /departments` exists but no write verb does — no `POST`/`PUT`/`DELETE`, and no `/departments/{id}` path at all (live-probed against the instance OpenAPI spec, eqrm prod CT 3.135.2, 2026-08-13; [`docs/api-coverage.md`](api-coverage.md) #14) | Master data admin → Bereiche | `ct get departments` lists them. **Grants scoped to one ARE declarable** — `scope: [{ department: "" }]` resolves by name per host (#98) — so only CREATING or RENAMING a Bereich is manual. Renaming one in CT breaks every config reference to its old name, by design: the reference then hard-errors instead of silently regranting elsewhere | -| Permission name↔authId catalog | The mapping from human-readable `module:right` names to the numeric `authId` the API actually writes | Not exposed by the REST API at all; only servable via the legacy `POST /index.php?q=churchauth/ajax&func=getMasterData` call ([`src/permissions/README.md`](../src/permissions/README.md)) | Permission editor (any role's right-picker enumerates the live set) | Regeneration procedure below (**Permission catalog lifecycle**) | +Every item below falls into exactly one of three buckets. Keeping them apart +matters, because they say very different things and only one of them is about +ChurchTools' capabilities at all (#111 — the three were routinely conflated, +and "no REST endpoint" kept getting written down as "impossible"): + +- **No write API `ct` can drive** — a _capability fact about the interfaces this + tool speaks_, dated and sourced. It says: neither the REST/OpenAPI surface nor + the legacy master-data registry offers a write path we can use today. It does + **not** say ChurchTools cannot do it — CT's admin UI writes plenty that no API + we audit exposes. +- **Not yet implemented** — the API supports it, but `ct` doesn't drive it yet. + Tracked by an open issue; closing the issue removes the item from this runbook. +- **Out of tool scope** — a _product decision_, not an API limit. Deliberately + unmanaged; not a gap to close, a boundary the tool is designed to respect. + +Absolutes ("never", "permanently", "no API at all") belong only on items where +**both** probes in [the re-audit procedure](#re-audit-procedure-for-new-ct-releases) +were actually run and recorded, with dates — or on out-of-scope items, where the +absolute is about our own intent rather than about CT. + +## No write API `ct` can drive + +| Item | What it is | Why manual | Where in the CT admin UI | How to verify | +| ------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Group member-statuses | The set of "member status" values a group can assign to its members (e.g. active/candidate) — NOT the same dimension as a group's own `groupStatusId`, which IS fully managed (`ct.group({ groupStatusId })`, [`docs/group-field-decisions.md`](group-field-decisions.md)); do not conflate the two (#67) | Only `GET /group/memberstatus` exists; no create/update/delete endpoint ([`docs/api-coverage.md`](api-coverage.md) #8) | Group settings → member status admin (org-wide master data, not per-group) | `ct get raw /group/memberstatus` (no dedicated `ct get member-statuses` subcommand yet — the generic `raw` path covers it) and diff by eye against the expected list below | +| Meeting points (Treffpunkte) | A group's meeting-location master data | No endpoint at all — zero matches for `treffpunkt`/`meetingpoint` anywhere in the OpenAPI spec ([`docs/api-coverage.md`](api-coverage.md) #11) | Group admin → meeting point field on a group | No API verification possible; visually confirm in the UI. (Do not confuse with _meeting templates_ `/group/meetingtemplates` or _group meetings_ `/groups/{id}/meetings`, both full CRUD but different concepts — confirm with product if "meeting point" was meant to be one of those instead) | +| Permission name↔authId catalog | The mapping from human-readable `module:right` names to the numeric `authId` the API actually writes | Not exposed by the REST API at all; only servable via the legacy `POST /index.php?q=churchauth/ajax&func=getMasterData` call ([`src/permissions/README.md`](../src/permissions/README.md)) | Permission editor (any role's right-picker enumerates the live set) | Regeneration procedure below (**Permission catalog lifecycle**) | **Expected values for a given instance:** left blank here deliberately — this runbook is generic (part of `ct-cli`, the tool repo). The per-instance @@ -31,16 +40,18 @@ in that instance's own config repo, in a runbook following this doc's structure. ## Not yet implemented — API supports it, `ct` doesn't drive it yet -| Item | What it is | Tracking issue | Manual workaround today | -| --------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| Group/group-type field decision table | Fields deliberately left unmanaged (decided out of scope): visibility, note, `autoAccept`/open-for-members, chat status, sort key. The triage **shipped** as a committed decision table ([`docs/group-field-decisions.md`](group-field-decisions.md)) | [#21](https://github.com/eqrm/ct-cli/issues/21) (decided) | Set by hand; these fields are intentionally not diffed — `ct` will neither preserve nor revert them. Promote one later only with its own registry entry + tests | -| Portable/logical references | **Shipped (#20, #25).** Configs reference master data by name/key — `campus`/`groupType` on a group, `ref.campus(...)` in ruleset `var` values, `groupType: ""` for a `group_type_role` domain, and now `group: "", role: ""` for a `group_role` domain (#25) — and the per-host resolver maps each to that instance's id at plan time (managed resources ∪ live catalogs). A same-run campus resolves at apply time. Numeric ids still work as an escape hatch. **`status` (group status) is NOT part of this** (#67) — group statuses have no REST catalog, so `status:` fails fast at eval time; declare the numeric `groupStatusId` directly | [#20](https://github.com/eqrm/ct-cli/issues/20) (done), [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | None needed for the shipped surface. Write logical names; run `ct plan`. The `group_role` pairing-id resolution is verified live (row below) | -| Environments (dev → prod promotion) | Named `(host, token, state file)` profiles and a `--env` flag; today one config + one state file = one host | [#22](https://github.com/eqrm/ct-cli/issues/22) | Point `CT_HOST`/state file manually at each target and re-run; keep dev and prod state files apart yourself, and be careful — nothing stops you from applying a dev-shaped config against prod today | -| Permission `group_role` domain by reference **(shipped, verified live)** | `ct.groupRole({ group, role })` now resolves the (group, role) pair to its pairing domainId at plan time (#25). **Confirmed live 2026-08-13 (CT 3.135.2):** it reads the group's role list (`GET /groups/{groupId}/roles`) and takes the matched role row's `id` as the pairing domainId. Two anchors on different group types: each row's `id` is a live `group_role` domainId carrying that role's grants, while its type-level `groupTypeRoleId` appears nowhere in the domainId set | [#25](https://github.com/eqrm/ct-cli/issues/25) (done, verified) | None needed. Works by reference for managed, already-created groups; numeric `id:` remains a supported escape hatch ([`docs/handbuch/permissions.md`](handbuch/permissions.md) "domainId semantics") | -| ~~Grant adoption~~ **(shipped)** | ~~existing rights structures must be hand-transcribed~~ — **`ct adopt grants ` ships this** (#25): it reads the live rows, applies the planner's normalization, and prints a paste-ready `ct.groupRole` / `ct.groupTypeRole` block (baseline/inherited excluded, denies noted-and-preserved, scope dataIds mapped back to managed-group keys). See [`docs/handbuch/permissions.md`](handbuch/permissions.md) "Adopting existing grants" | [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | No workaround needed — run `ct adopt grants group_role ` (or `group_type_role`), review the `WARNING`/`NOTE` comments, paste into config | -| ~~Permission catalog lifecycle~~ **(shipped)** | ~~`catalog.json` is a one-off HAR-trace snapshot with no staleness detection~~ — **shipped (#25):** `npm run regenerate:permission-catalog` rewrites it from a live instance (records the CT version in `$meta`), and `ct plan` now warns on a version mismatch or an unknown-authId live grant (which it leaves untouched, never revoking a right it cannot name). See [`docs/handbuch/permissions.md`](handbuch/permissions.md) "Catalog lifecycle & staleness" | [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | No workaround needed — run the command; heed the `ct plan` warnings | -| Field definitions & security levels (person + group custom fields) **(read-only, shipped #47/#48)** | The person master-data model, the security-level enumeration, and the data-field DEFINITIONS ("Datenfelder") for persons and groups — structural schema, not per-record values | [#47](https://github.com/eqrm/ct-cli/issues/47), [#48](https://github.com/eqrm/ct-cli/issues/48) (read shipped; write is an API gap — see note) | Read with `ct get person-masterdata` (model + security levels) and `ct get data-fields` (all field definitions, person + group, discriminated by `fieldCategory`). **Mutation stays manual:** field definitions have no REST write endpoint — only the legacy churchdb admin AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) — so create/edit/delete them by hand in the master-data admin UI. Decision + evidence: [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md) | -| API re-audit for new CT releases | CT's OpenAPI spec is self-trimming (only shows endpoints your version has), so a new write endpoint (e.g. a member-status write, or — separately — a first-ever group-status list/write endpoint, #67) appears silently between CT upgrades | tracked by this issue ([#26](https://github.com/eqrm/ct-cli/issues/26)) | Procedure below (**Re-audit procedure for new CT releases**) | +| Item | What it is | Tracking issue | Manual workaround today | +| --------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Group/group-type field decision table | Fields deliberately left unmanaged (decided out of scope): visibility, note, `autoAccept`/open-for-members, chat status, sort key. The triage **shipped** as a committed decision table ([`docs/group-field-decisions.md`](group-field-decisions.md)) | [#21](https://github.com/eqrm/ct-cli/issues/21) (decided) | Set by hand; these fields are intentionally not diffed — `ct` will neither preserve nor revert them. Promote one later only with its own registry entry + tests | +| Portable/logical references | **Shipped (#20, #25).** Configs reference master data by name/key — `campus`/`groupType` on a group, `ref.campus(...)` in ruleset `var` values, `groupType: ""` for a `group_type_role` domain, and now `group: "", role: ""` for a `group_role` domain (#25) — and the per-host resolver maps each to that instance's id at plan time (managed resources ∪ live catalogs). A same-run campus resolves at apply time. Numeric ids still work as an escape hatch. **`status` (group status) is NOT part of this** (#67) — group statuses have no REST catalog, so `status:` fails fast at eval time; declare the numeric `groupStatusId` directly | [#20](https://github.com/eqrm/ct-cli/issues/20) (done), [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | None needed for the shipped surface. Write logical names; run `ct plan`. The `group_role` pairing-id resolution is verified live (row below) | +| Environments (dev → prod promotion) | Named `(host, token, state file)` profiles and a `--env` flag; today one config + one state file = one host | [#22](https://github.com/eqrm/ct-cli/issues/22) | Point `CT_HOST`/state file manually at each target and re-run; keep dev and prod state files apart yourself, and be careful — nothing stops you from applying a dev-shaped config against prod today | +| Permission `group_role` domain by reference **(shipped, verified live)** | `ct.groupRole({ group, role })` now resolves the (group, role) pair to its pairing domainId at plan time (#25). **Confirmed live 2026-08-13 (CT 3.135.2):** it reads the group's role list (`GET /groups/{groupId}/roles`) and takes the matched role row's `id` as the pairing domainId. Two anchors on different group types: each row's `id` is a live `group_role` domainId carrying that role's grants, while its type-level `groupTypeRoleId` appears nowhere in the domainId set | [#25](https://github.com/eqrm/ct-cli/issues/25) (done, verified) | None needed. Works by reference for managed, already-created groups; numeric `id:` remains a supported escape hatch ([`docs/handbuch/permissions.md`](handbuch/permissions.md) "domainId semantics") | +| ~~Grant adoption~~ **(shipped)** | ~~existing rights structures must be hand-transcribed~~ — **`ct adopt grants ` ships this** (#25): it reads the live rows, applies the planner's normalization, and prints a paste-ready `ct.groupRole` / `ct.groupTypeRole` block (baseline/inherited excluded, denies noted-and-preserved, scope dataIds mapped back to managed-group keys). See [`docs/handbuch/permissions.md`](handbuch/permissions.md) "Adopting existing grants" | [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | No workaround needed — run `ct adopt grants group_role ` (or `group_type_role`), review the `WARNING`/`NOTE` comments, paste into config | +| ~~Permission catalog lifecycle~~ **(shipped)** | ~~`catalog.json` is a one-off HAR-trace snapshot with no staleness detection~~ — **shipped (#25):** `npm run regenerate:permission-catalog` rewrites it from a live instance (records the CT version in `$meta`), and `ct plan` now warns on a version mismatch or an unknown-authId live grant (which it leaves untouched, never revoking a right it cannot name). See [`docs/handbuch/permissions.md`](handbuch/permissions.md) "Catalog lifecycle & staleness" | [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | No workaround needed — run the command; heed the `ct plan` warnings | +| Field definitions & security levels (person + group custom fields) **(read-only, shipped #47/#48)** | The person master-data model, the security-level enumeration, and the data-field DEFINITIONS ("Datenfelder") for persons and groups — structural schema, not per-record values | [#47](https://github.com/eqrm/ct-cli/issues/47), [#48](https://github.com/eqrm/ct-cli/issues/48) (read shipped; write is an API gap — see note) | Read with `ct get person-masterdata` (model + security levels) and `ct get data-fields` (all field definitions, person + group, discriminated by `fieldCategory`). **Mutation stays manual:** field definitions have no REST write endpoint — only the legacy churchdb admin AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) — so create/edit/delete them by hand in the master-data admin UI. Decision + evidence: [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md) | +| Bereiche (departments) — creating/renaming one | The Bereich master data — the `cdb_bereich` permission scope dimension | [#108](https://github.com/eqrm/ct-cli/issues/108), [#109](https://github.com/eqrm/ct-cli/issues/109) | **Moved here from "API gap" by #111.** `/departments` has no REST write verb (live-probed, eqrm prod CT 3.135.2, 2026-08-13) — but the admin UI creates Bereiche through `POST /index.php?q=churchdb/ajax` `func=saveMasterData` (captured 2026-08-13), so the old "can never be created" verdict was an artefact of auditing OpenAPI only. Until `ct` drives that endpoint: create/rename in **Master data admin → Bereiche**; `ct get departments` lists them. Grants scoped to one ARE declarable — `scope: [{ department: "" }]` resolves by name per host (#98). Renaming one in CT breaks every config reference to its old name, by design: the reference hard-errors instead of silently regranting elsewhere | +| Security levels — creating/reordering one | The `cc_securitylevel` master data (`GET /securitylevels`) — the scope dimension of `churchdb:+see persons` and friends | [#109](https://github.com/eqrm/ct-cli/issues/109), [#110](https://github.com/eqrm/ct-cli/issues/110) (ref form done) | No REST write verb (live-probed, eqrm prod CT 3.135.2, 2026-08-13); `cc_securitylevel` IS in the legacy master-data registry, which `ct` does not drive yet. Create/reorder in **Master data admin → Sicherheitslevel**; `ct get security-levels` lists them. Grants scoped to one are declarable by name — `scope: [{ securityLevel: "" }]` (#110) — and numeric `scope: [1, 2, 3]` still works, but check the ids per host before relying on them: they are auto-increment rows, not protocol constants | +| API re-audit for new CT releases | CT's OpenAPI spec is self-trimming (only shows endpoints your version has), so a new write endpoint (e.g. a member-status write, or — separately — a first-ever group-status list/write endpoint, #67) appears silently between CT upgrades | tracked by this issue ([#26](https://github.com/eqrm/ct-cli/issues/26)) | Procedure below (**Re-audit procedure for new CT releases**) | ## Out of tool scope — deliberate, not a gap @@ -129,21 +140,49 @@ CT's OpenAPI spec (`GET $CT_HOST/system/runtime/swagger/openapi.json`, pulled by `npm run generate:client`) is **self-trimming**: `info.description` states it "will always show only those endpoints you can use with your ChurchTools installation." That means a version bump can silently add a write endpoint -this runbook still lists as an API gap (most plausibly: a `group/memberstatus` -write endpoint, or a meeting-point endpoint). - -Until a scripted diff exists, re-audit manually after any CT upgrade: - -1. Re-fetch the spec: `npm run generate:client` (writes - `src/api/schema.d.ts`) or fetch `openapi.json` directly and open it. -2. For each item in the **API gap** table above, re-check whether its path - now has additional methods (grep the spec for `/group/memberstatus`, - `treffpunkt`/`meetingpoint`, etc.). -3. If a write method appeared: promote the item — add it to - `src/resources/registry.ts` (or the relevant synthetic field), extend - the DSL, add tests, and delete its row from this runbook's API-gap table. -4. Re-run `GET /info` to confirm the CT `version`/`build` this audit was - performed against, and note it in the commit that updates this file. +this runbook still lists as unavailable. + +**The spec is not the whole surface.** This is the defect #111 was filed about: +this procedure used to say "grep the OpenAPI spec", which can only ever discover +REST endpoints — so it was structurally incapable of finding the legacy +`POST /index.php?q=…/ajax` interface the admin UI writes a large amount of master +data through, and yet it concluded "permanent" / "never" on that basis. Bereiche +(#108), person statuses (#96) and security levels (#110) were all recorded as +impossible on evidence that only ruled out REST. `ct` has depended on that same +legacy surface the whole time — the permission catalog is generated from +`POST /index.php?q=churchauth/ajax` `func=getMasterData` +([`src/permissions/README.md`](../src/permissions/README.md)). + +So a "no write API" verdict now requires **both** probes, and the third where +neither settles it: + +1. **REST probe.** Re-fetch the spec: `npm run generate:client` (writes + `src/api/schema.d.ts`) or fetch `openapi.json` directly. For each item in the + no-write-API table, re-check whether its path now has additional methods (grep + for `/group/memberstatus`, `treffpunkt`/`meetingpoint`, etc.). +2. **Master-data registry probe.** `POST /index.php?q=churchdb/ajax` with body + `func=getMasterData` returns `data.masterDataTables`: every editable table with + its label, shortname, physical table name and a full `DESCRIBE`-style column + list. If the thing you are auditing has a table there (`cdb_bereich`, + `cdb_status`, `cc_securitylevel`, `cdb_comment_viewer`, …), it **is** writable — + `func=saveMasterData&table=&id=&col0=…&value0=…` (empty `id` creates) — and + the item belongs under _Not yet implemented_, not here. +3. **UI capture**, where neither probe settles it: perform the action in the CT + admin UI with devtools recording, and read what it actually posted. Save the + request line/body into the item's row. +4. **Record the probe.** Note the date, the CT `version`/`build` from `GET /info`, + and _which_ probes were run, in the row and in the commit — the way the + permission-catalog note already does. A reader must be able to tell a verified + negative from an inherited assumption. +5. If a write path appeared: promote the item — add it to + `src/resources/registry.ts` (or the relevant synthetic field), extend the DSL, + add tests, and move its row out of this table. + +**Wording rule.** Write what was probed, not what you assume. "No REST write +endpoint (probed 2026-08-13, CT 3.135.2)" is a fact. "Cannot be created", +"permanently", "not a resource at all" are claims that need step 2 (and usually +step 3) behind them — and if the item is unmanaged because we _chose_ that, it +belongs in **Out of tool scope**, phrased as a decision. This mirrors (and should eventually replace by scripting) the Phase 0 spike that produced `docs/api-coverage.md` — see that doc for the full method. @@ -199,4 +238,8 @@ in where they'd otherwise be silently skipped: - "Meeting point" (Treffpunkt) has no confirmed CT concept mapping — `docs/api-coverage.md` flags that it might actually mean _meeting templates_ or _group meetings_ (both full CRUD, i.e. not actually manual at all). Confirm with product - before treating it as a permanent API gap. + before treating it as unavailable. +- The two remaining rows in the no-write-API table (**group member-statuses**, + **meeting points**) carry a REST probe but **no master-data-registry probe** + yet — step 2 of the procedure above postdates them. Until someone runs it, + read them as "no REST write endpoint", not as "impossible". diff --git a/src/commands/get.ts b/src/commands/get.ts index 2a5f508..5bc058f 100644 --- a/src/commands/get.ts +++ b/src/commands/get.ts @@ -46,6 +46,11 @@ const RESOURCE_PATHS: Record = { // (GET only; no write verb exists), so this is the way to discover the names a // `scope: [{ department: "…" }]` reference resolves against. Never adoptable. departments: { path: "/departments" }, + // Security levels — the `cc_securitylevel` permission scope dimension (#110). Flat + // `[{id, name, sortKey}]`. This is how you discover the names a `scope: [{ securityLevel: "…" }]` + // reference resolves against, and how you check whether an instance's ids really are the usual + // 1..4 — they are editable master data, not a protocol constant. + "security-levels": { path: "/securitylevels" }, // PERSON statuses — master data (the enumeration), never person records. The domain a `ct.status` // permission declaration hangs off, and an adoptable resource since #96. statuses: { path: "/statuses" }, diff --git a/src/coverage/report.ts b/src/coverage/report.ts index 6d80335..66a12ac 100644 --- a/src/coverage/report.ts +++ b/src/coverage/report.ts @@ -24,12 +24,12 @@ import { ALL_SCOPE_SENTINEL, SCOPE_REF_KIND } from "../permissions/scope.js"; import { fromInformation } from "../resources/registry.js"; import type { State } from "../state/state.js"; -/** - * Scope dimensions whose numeric dataIds mean the same thing on EVERY ChurchTools instance, so a - * numeric literal is portable and the grant is declarable without a logical reference form. - * `cc_securitylevel` is the enumeration 1/2/3 — not a resource, but not host-specific either. - */ -const NUMERIC_UNIVERSAL_SCOPE_FIELDS: ReadonlySet = new Set(["cc_securitylevel"]); +// `cc_securitylevel` used to be listed here as "numeric-universal" — a dimension whose ids mean the +// same thing on every instance, so a numeric literal counted as declarable. #110 retired that idea: +// security levels are admin-editable master-data rows with an auto-increment id, so the alignment held +// by convention, not by construction. They now have a real reference form (`{ securityLevel: "…" }`, +// resolved through `GET /securitylevels`), so `SCOPE_REF_KIND` covers them like any other dimension +// and no universality exemption is needed. /** A live (group, role) permission domain — the granularity declarability is decided at. */ export interface RoleInstance { @@ -107,8 +107,8 @@ export interface CoverageInput { * A grant is declarable when its right is nameable AND its scope can be written portably: * - unscoped rights always are; * - the `-1` ALL sentinel always is (CT reads it back verbatim on every dimension); - * - a dimension with a logical reference form (#98: group, campus, group type, department) is; - * - a numeric-but-universal dimension (`cc_securitylevel`) is. + * - a dimension with a logical reference form is (#98: group, campus, group type, department; + * #110 added security levels). * Everything else — calendar categories, HTML templates, wiki categories, OAuth clients — names * module data this tool has no resource for, so it blocks the whole instance under the strict * ownership default. (With `preserveUnknown` (#102) those same instances become declarable while @@ -130,7 +130,6 @@ export function declarability(rows: RawPermission[]): DeclarabilityVerdict { if (scopeField === null) continue; if (t.dataId.every((id) => id === ALL_SCOPE_SENTINEL)) continue; if (SCOPE_REF_KIND[scopeField] !== undefined) continue; - if (NUMERIC_UNIVERSAL_SCOPE_FIELDS.has(scopeField)) continue; blockedBy.add(scopeField); } return { diff --git a/src/permissions/adopt.ts b/src/permissions/adopt.ts index b0f4ec1..d5bf42e 100644 --- a/src/permissions/adopt.ts +++ b/src/permissions/adopt.ts @@ -32,6 +32,7 @@ const SCOPE_SUGAR_FIELD: Readonly> = { campus: "campus", "group-type": "groupType", department: "department", + "security-level": "securityLevel", }; interface ReverseEntry { @@ -258,7 +259,7 @@ function grantLines(g: CollapsedGrant, rev: Map, state: St if (dimension && !dimension.managed) { const field = SCOPE_SUGAR_FIELD[dimension.type] ?? dimension.type; out.push( - ` // NOTE: "${entry.name}" scopes by "${entry.scopeField}" (${dimension.type}), a read-only catalog —`, + ` // NOTE: "${entry.name}" scopes by "${entry.scopeField}" (${dimension.type}), a catalog ct reads but does not manage —`, ); out.push( ` // the id below is host-specific. Portable form: { ${field}: "" } (\`ct get ${dimension.type}s\`).`, diff --git a/src/permissions/apply.ts b/src/permissions/apply.ts index 837fc3d..7eae15c 100644 --- a/src/permissions/apply.ts +++ b/src/permissions/apply.ts @@ -12,7 +12,11 @@ import type { PermissionPlanItem } from "./plan.js"; import type { GrantTuple } from "./grants.js"; import { reresolveTuple } from "./scope.js"; import { pendingRef, refLabel } from "../resolve/refs.js"; -import { reresolvePendingValue } from "../resolve/resolver.js"; +import { + reresolvePendingValue, + resolvePendingGroupRoleDomain, + type GroupRoleRowCache, +} from "../resolve/resolver.js"; import { formatError } from "../ui.js"; /** How many permission tuples to write at once. Tuples are independent rows, so a modest fan-out is safe. */ @@ -65,17 +69,18 @@ export interface PermissionApplyResult { */ export async function applyPermissionPlan( items: PermissionPlanItem[], - client: Pick, + client: Pick & Partial>, state?: State, ): Promise { const ops: WriteOp[] = []; + /** Shared across items so several pending group_role domains on one group cost a single fetch. */ + const roleLists: GroupRoleRowCache = new Map(); // Concurrent writes are race-free only because evaluateConfig rejects two declarations targeting // the same (domainType, domainId) — so all ops for a path come from ONE item's disjoint diff. // Programmatic callers bypassing evaluateConfig must uphold that invariant themselves. for (const item of items) { - // A pending domain (#69) is a group type created THIS run: its numeric id is only known after - // executePlan, so re-resolve it against the POST-execute state now — reusing the SAME machinery - // that re-resolves resource pending refs (reresolvePendingValue). Requires `state`: a pending + // A pending domain (#69) names a resource created THIS run: its numeric id is only known after + // executePlan, so complete it against the POST-execute state now. Requires `state`: a pending // domain can never be applied statelessly. let domainId = item.domainId; if (item.pendingDomain) { @@ -85,7 +90,30 @@ export async function applyPermissionPlan( `without post-execute state — it names a resource created in the same run.`, ); } - domainId = reresolvePendingValue(pendingRef(item.pendingDomain), state) as number; + const site = `${item.domainType} "${item.key}".domainId`; + if (item.pendingDomain.kind === "group-role") { + // The one pending domain a state lookup cannot finish (#106): a (group, role) PAIRING id is + // not the group's id — it lives on the freshly created group's own role list, so completing + // it takes a live fetch. Needs a reading client; the real CtClient always has one, and the + // failure mode if a caller passes a write-only double is a clear error, not a wrong domain. + if (!client.get) { + throw new Error( + `Pending permission domain ${refLabel(item.pendingDomain)} ("${item.key}") needs to read ` + + `the created group's role list, but this client cannot GET. Pass a full CtClient.`, + ); + } + domainId = await resolvePendingGroupRoleDomain( + item.pendingDomain, + state, + { get: client.get, getAll: client.getAll }, + site, + roleLists, + ); + } else { + // Every other pending domain (group type, person status) IS its resource's own id, so the + // SAME machinery that re-resolves resource pending refs finishes it from state alone. + domainId = reresolvePendingValue(pendingRef(item.pendingDomain), state) as number; + } } const path = `/permissions/${item.domainType}/${domainId}`; assertNotPeople(path); diff --git a/src/permissions/plan.ts b/src/permissions/plan.ts index d7b60f8..ff687b6 100644 --- a/src/permissions/plan.ts +++ b/src/permissions/plan.ts @@ -142,11 +142,17 @@ type ResolvedPermission = /** * Resolve every permission's domainId (#20). A numeric domainId passes straight through; a Ref * (e.g. `groupType: "…"`) resolves against managed state ∪ the live catalog. A domainId that - * resolves to a same-run-created group type (PendingRef) is NOT rejected (#69): it is carried as a + * resolves to a same-run-created resource (PendingRef) is NOT rejected (#69): it is carried as a * `pendingDomain` and re-resolved against post-execute state at apply time — this is what lets a - * fresh-instance plan render the create-set + pending grants instead of aborting. A group_role ref - * resolves to its concrete (group, role) pairing id in the resolver and never goes pending — the - * pairing id needs a live `/groups/{id}/roles` fetch, so a same-run group is a hard error there (#25). + * fresh-instance plan render the create-set + pending grants instead of aborting. + * + * That now includes a `group_role` domain on a group declared in this same config (#106). Its pairing + * id needs a live `/groups/{id}/roles` fetch rather than a bare state lookup, so it is completed by + * `applyPermissionPlan` (via `resolvePendingGroupRoleDomain`) instead of `reresolvePendingValue` — but + * from the planner's side it is the same pending domain as any other. Before #106 it was a hard error, + * which made any config declaring a group AND its own group-role plan on prod (where the group exists) + * and fail on dev (where it does not) — non-portable by construction. + * * The hard error remains ONLY for genuinely unresolvable references (key not in config/state at all). * * After resolution, the authoritative duplicate-target guard runs on the resolved identities (concrete @@ -164,7 +170,10 @@ async function resolveDomainIds( continue; } const site = `${p.domainType} "${p.key}".domainId`; - const res = await resolver.resolve(p.domainId, site); + // `pendingGroupRole` is opt-in per position (#106): this is the ONLY call site that can finish a + // pending group_role, because `applyPermissionPlan` runs after `executePlan` and holds a client to + // fetch the freshly created group's role list with. + const res = await resolver.resolve(p.domainId, site, { pendingGroupRole: true }); if (isPendingRef(res)) { resolved.push({ key: p.key, @@ -257,11 +266,11 @@ export async function buildPermissionPlan( } for (const p of resolved) { if (p.pendingDomain !== undefined) { - // The domain (a group type) is created THIS run (#69), so it has no live grants yet: the - // actual set is empty and every desired grant lands in toPut as a pending grant block. Its - // numeric domainId is unknown until the resource tier applies — the pending marker is - // re-resolved against post-execute state at apply time (applyPermissionPlan). Rendered with a - // `` marker consistent with resource pending refs. + // The domain (a group type, a person status, or the group behind a group_role — #106) is created + // THIS run (#69), so it has no live grants yet: the actual set is empty and every desired grant + // lands in toPut as a pending grant block. Its numeric domainId is unknown until the resource + // tier applies — the pending marker is completed at apply time (applyPermissionPlan). Rendered + // with a `` marker consistent with resource pending refs. items.push({ key: p.key, domainType: p.domainType, diff --git a/src/permissions/scope.ts b/src/permissions/scope.ts index bb80078..af879e4 100644 --- a/src/permissions/scope.ts +++ b/src/permissions/scope.ts @@ -56,23 +56,26 @@ export const GROUP_SCOPE_FIELD = "cdb_gruppe"; * - `cdb_gruppentyp` → group types (`/group/grouptypes`) * - `cdb_bereich` → departments (`/departments`) — Bereiche, e.g. `churchdb:view alldata` * ("Personen eines Bereiches sehen") + * - `cc_securitylevel` → security levels (`/securitylevels`, #110) — e.g. `churchdb:+see persons` * - * `managed` is what separates the last one from the rest. Groups, campuses and group types are + * `managed` is what separates the last two from the rest. Groups, campuses and group types are * MANAGED resource kinds: a scope target can be declared in the same config (resolving pending, then - * re-resolved at apply time) and a state-backed id is worth re-resolving. Departments are a READ-ONLY - * ref catalog — `GET /departments` exists, but no POST/PUT/DELETE does (live-probed against the - * instance's own OpenAPI spec, eqrm prod CT 3.135.2, 2026-08-13) — so a department reference always - * resolves by NAME against the live catalog and hard-errors when the name is absent, which is - * strictly better than a silent numeric misgrant but is never a create. + * re-resolved at apply time) and a state-backed id is worth re-resolving. Departments and security + * levels are catalogs `ct` READS but does not manage, so a reference always resolves by NAME against + * the live catalog and hard-errors when the name is absent — strictly better than a silent numeric + * misgrant, but never a create. (Neither has a REST write verb; both ARE writable through the legacy + * master-data endpoint the admin UI uses, which `ct` does not drive yet — #108/#109.) * - * Every other scoped dimension (`cc_securitylevel`, `ccm_data_category`, `oauth_client`, …) stays - * numeric-only by nature rather than by omission: its values are not resources at all. + * The remaining scoped dimensions (`ccm_data_category`, `cdb_comment_viewer`, `oauth_client`, …) stay + * numeric-only because this tool has no way to address their values by a host-independent name today — + * an omission to revisit (#109), not a proof that they are "not resources". */ export const SCOPE_REF_KIND: Readonly> = { [GROUP_SCOPE_FIELD]: { kind: "group", type: "group", managed: true }, cdb_station: { kind: "campus", type: "campus", managed: true }, cdb_gruppentyp: { kind: "group-type", type: "group-type", managed: true }, cdb_bereich: { kind: "department", type: "department", managed: false }, + cc_securitylevel: { kind: "security-level", type: "security-level", managed: false }, }; /** The DSL's object sugar for a typed scope ref: exactly one dimension field, e.g. `{ campus: "koblenz" }`. */ @@ -81,6 +84,7 @@ const SCOPE_SUGAR: Readonly Ref>> = { campus: ref.campus, groupType: ref.groupType, department: ref.department, + securityLevel: ref.securityLevel, }; /** @@ -254,8 +258,8 @@ function expectedDimension( * pre-resolved for this host by {@link resolveScopeRefs} and read out of `refs` here. This is what * makes a campus-scoped grant portable: campus ids differ per host (dev Mainz = 6, prod Mainz = 0). * - a **raw numeric dataId** (`number`, #49) — an escape hatch that passes straight through with no - * lookup at all. Still the only form for dimensions whose values are not resources this tool can - * name (`cc_securitylevel`, `cdb_bereich`, `oauth_client`, …). + * lookup at all. Still the only form for dimensions whose values this tool cannot yet address by a + * host-independent name (`ccm_data_category`, `cdb_comment_viewer`, `oauth_client`, …). * * A logical key that names a managed resource in state resolves to its id. A logical key that names a * resource DECLARED in this config but not yet in state (`declaredGroupKeys` for the string form; the diff --git a/src/permissions/types.ts b/src/permissions/types.ts index 59b7515..f289d72 100644 --- a/src/permissions/types.ts +++ b/src/permissions/types.ts @@ -14,17 +14,23 @@ import type { Ref } from "../resolve/refs.js"; * (see `normalizeScopeEntry` in src/permissions/scope.ts), so `ref.campus("koblenz")` is identical. */ export type ScopeSugar = - { group: string } | { campus: string } | { groupType: string } | { department: string }; + | { group: string } + | { campus: string } + | { groupType: string } + | { department: string } + | { securityLevel: string }; /** * One entry of a scoped grant's `scope`: * * - a **logical group key** (`string`) — a group managed by this tool. Group dimension only. * - a **typed logical ref** ({@link ScopeSugar} / {@link Ref}, #98) — the portable form for the other - * dimensions this tool can name by key: campuses (`cdb_station`), group types (`cdb_gruppentyp`) - * and departments (`cdb_bereich`, a read-only catalog — referenceable but never declarable). - * - a **raw numeric dataId** (`number`, #49 escape hatch) — the only form for dimensions whose values - * are not resources at all (`cc_securitylevel`, `ccm_data_category`, `oauth_client`, …). + * dimensions this tool can name by key: campuses (`cdb_station`), group types (`cdb_gruppentyp`), + * departments (`cdb_bereich`) and security levels (`cc_securitylevel`, #110). The last two are + * catalogs `ct` reads but does not manage: referenceable by name on any host, never declarable. + * - a **raw numeric dataId** (`number`, #49 escape hatch) — still supported everywhere, and the only + * form for dimensions this tool cannot yet address by a host-independent name + * (`ccm_data_category`, `cdb_comment_viewer`, `oauth_client`, …). */ export type ScopeEntry = string | number | Ref | ScopeSugar; diff --git a/src/resolve/refs.ts b/src/resolve/refs.ts index df6392a..5928f06 100644 --- a/src/resolve/refs.ts +++ b/src/resolve/refs.ts @@ -21,6 +21,7 @@ export type RefKind = | "campus" | "department" + | "security-level" | "group-type" | "group-status" | "person-status" @@ -44,10 +45,18 @@ export const GROUP_STATUS_NO_CATALOG = `group statuses have no REST catalog (GET /group/memberstatus is a different dimension: member ` + `statuses, string ids — verified 2026-07-10). Declare a numeric "groupStatusId" instead (e.g. "groupStatusId: 1").`; -/** Simple key-addressed reference: campus / department / group type / group status / person status / role definition / group. */ +/** Simple key-addressed reference: campus / department / security level / group type / group status / person status / role definition / group. */ export interface SimpleRef { __ctRef: true; - kind: "campus" | "department" | "group-type" | "group-status" | "person-status" | "role-def" | "group"; + kind: + | "campus" + | "department" + | "security-level" + | "group-type" + | "group-status" + | "person-status" + | "role-def" + | "group"; key: string; } @@ -101,17 +110,41 @@ export const ref = { campus: (key: string): SimpleRef => ({ __ctRef: true, kind: "campus", key: requireKey("campus", key) }), /** * A Bereich/DEPARTMENT (`/departments`), the scope dimension of `cdb_bereich` rights such as - * `churchdb:view alldata` ("Personen eines Bereiches sehen") — #98. Unlike campuses and group - * types, departments are a READ-ONLY ref catalog: `GET /departments` exists but there is no POST / - * PUT / DELETE (live-probed against the instance OpenAPI spec, eqrm prod CT 3.135.2, 2026-08-13). - * So a department can be REFERENCED by name on any host, but never declared, adopted or created — - * an unresolvable name is a hard error, not a create. + * `churchdb:view alldata` ("Personen eines Bereiches sehen") — #98. + * + * Catalog-only FOR NOW: `ct` reads `GET /departments` and resolves a name against it, but declares + * and creates nothing, so an unresolvable name is a hard error rather than a create. That is a + * statement about what this tool drives today, NOT about ChurchTools: `/departments` has no REST + * write verb (live-probed against the instance OpenAPI spec, eqrm prod CT 3.135.2, 2026-08-13), but + * the admin UI does create Bereiche, through the legacy `POST /index.php?q=churchdb/ajax` + * `func=saveMasterData` interface that appears in no OpenAPI spec (captured 2026-08-13, #108/#109). + * Making departments declarable is tracked there. */ department: (key: string): SimpleRef => ({ __ctRef: true, kind: "department", key: requireKey("department", key), }), + /** + * A SECURITY LEVEL (`/securitylevels`), the scope dimension of `cc_securitylevel` rights such as + * `churchdb:+see persons` — #110. + * + * Exists because "security-level ids are universal, so a numeric literal is portable" was an + * assumption, not a guarantee: `cc_securitylevel` is an admin-editable master-data table (`id`, + * `bezeichnung`, `sortkey`) with an auto-increment id, so 1/2/3 line up across instances by + * convention — because nobody has added or reordered a level — not by construction. Referencing a + * level by name makes a config say what it means on any host, and hard-error instead of silently + * granting the wrong level when the name is absent. + * + * Numeric scope entries stay fully supported (#49): the names are localised German strings with + * parentheses ("Stufe 3 (Hoch)" slugs to `stufe_3_hoch`), so a rename breaks a ref where a number + * would have survived. Pick per config which risk you would rather carry. + */ + securityLevel: (key: string): SimpleRef => ({ + __ctRef: true, + kind: "security-level", + key: requireKey("security-level", key), + }), groupType: (key: string): SimpleRef => ({ __ctRef: true, kind: "group-type", @@ -142,8 +175,13 @@ export const ref = { /** * A `group_role` permission domain, by its (group, role) pair (#25). The resolver maps it to the * numeric pairing domainId at plan time by matching the role name against the group's role list - * (see the ASSUMPTION block in src/resolve/resolver.ts — the exact source is unverified; the - * numeric `id:` escape hatch remains the fallback). The Ref itself is an inert sentinel until then. + * (see the VERIFIED LIVE block in src/resolve/resolver.ts; the numeric `id:` escape hatch remains + * the fallback). The Ref itself is an inert sentinel until then. + * + * When the GROUP is declared in the same config but does not exist on the target host yet, the + * domain resolves to a {@link PendingRef} and is completed during apply (#106) — after the group is + * created, from its own `/groups/{id}/roles`. That is what keeps one config plannable on a host + * where the group already exists AND on a fresh one where it does not. */ groupRole: (group: string, role: string): GroupRoleRef => ({ __ctRef: true, @@ -251,16 +289,20 @@ export function collectRefs(value: unknown): Ref[] { /** * Collect the managed logical keys named by every {@link PendingRef} in a value. Pending markers * always point at a same-run declared resource, so these keys are exactly the apply-order - * dependencies the referencing resource needs (group-role refs resolve to a concrete id at plan - * time and never go pending). + * dependencies the referencing resource needs. + * + * The compound kinds contribute nothing here. `group-type-role` never goes pending at all. A + * `group-role` CAN (#106), but only in the permission-DOMAIN position — permissions are applied after + * every resource tier, so they need no per-resource ordering edge — and never inside a resource's + * field bag, which is the only thing this function walks. */ export function collectPendingRefKeys(value: unknown): string[] { const out: string[] = []; const walk = (v: unknown): void => { if (isPendingRef(v)) { const r = v.__pendingRef; - // Compound refs (group-role, group-type-role) resolve to a concrete id at plan time and never go - // pending, so only the simple, single-`key` kinds contribute an apply-order dependency here. + // Only the simple, single-`key` kinds contribute an apply-order dependency here — see the note + // above for why the compound ones cannot reach this walk. if (r.kind !== "group-role" && r.kind !== "group-type-role") out.push(r.key); return; } diff --git a/src/resolve/resolver.ts b/src/resolve/resolver.ts index 5ed8ca1..ea7c42a 100644 --- a/src/resolve/resolver.ts +++ b/src/resolve/resolver.ts @@ -81,6 +81,12 @@ const CATALOG_PATH: Partial> = { // eqrm prod, CT 3.135.2, 2026-08-13), so a department is resolvable by name on every host yet can // never be declared, adopted or created. Rows carry {id, name, nameTranslated, sortKey, shorty}. department: "/departments", + // Security levels — the `cc_securitylevel` scope dimension (#110). `GET /securitylevels` returns a + // flat `[{id, name, sortKey}]` array ("Stufe 1 (Niedrig)" … "Stufe 4 (Sehr hoch)"), live-verified on + // eqrm prod, CT 3.135.2, 2026-08-13. Catalog-only, like departments: `ct` resolves a level by name + // but never creates one. It exists because the ids are NOT a protocol constant — `cc_securitylevel` + // is an editable master-data table, so a hard-coded `scope: [1, 2, 3]` is portable only by convention. + "security-level": "/securitylevels", "group-type": "/group/grouptypes", // PERSON statuses — the domain of a `status` permission declaration (#90). Unlike GROUP statuses // (see the note above), these DO have a flat REST catalog: `GET /statuses` returns @@ -139,6 +145,23 @@ export interface ResolverDeps { const GROUP_ROLE_ENDPOINT = (groupId: number): string => `/groups/${groupId}/roles`; const GROUP_ROLE_PAIRING_FIELD = "id"; +/** Options for one {@link Resolver.resolve} call — what the CALLING POSITION can cope with. */ +export interface ResolveOptions { + /** + * Allow a `group-role` ref whose group is declared-but-not-yet-created to resolve to a + * {@link PendingRef} instead of hard-erroring (#106). + * + * Off by default, and deliberately opt-in per call site rather than global: a pending group-role + * cannot be finished by {@link reresolvePendingValue} the way every other pending ref can — the + * pairing id lives on `GET /groups/{id}/roles`, so completing it needs a LIVE FETCH after the group + * exists. Only the permission-domain position can do that (`applyPermissionPlan` runs after + * `executePlan` and holds a client), so only `resolveDomainIds` passes this. Every other position + * (resource id fields, query `var` values) keeps the old fail-fast error, which is still the right + * answer there. + */ + pendingGroupRole?: boolean; +} + export class Resolver { private readonly client: Pick & Partial>; private readonly state: State; @@ -164,8 +187,8 @@ export class Resolver { } /** Resolve one Ref to a numeric id, or a {@link PendingRef} for a same-run-created managed target. */ - async resolve(r: Ref, site: string): Promise { - if (r.kind === "group-role") return this.resolveGroupRole(r, site); + async resolve(r: Ref, site: string, opts: ResolveOptions = {}): Promise { + if (r.kind === "group-role") return this.resolveGroupRole(r, site, opts); if (r.kind === "group-type-role") return this.resolveGroupTypeRole(r, site); // (1) managed desired ∪ state by logical key const type = REF_KIND_TYPE[r.kind]; @@ -236,43 +259,24 @@ export class Resolver { /** * Resolve a `group_role` domain by its (group, role) pair to the numeric pairing domainId (#25). * See the VERIFIED LIVE block above the {@link GROUP_ROLE_ENDPOINT} constant for the model this - * implements and the evidence for it. Returns a number — never a {@link PendingRef}: the pairing id only exists - * once the group does, so a same-run-declared (not-yet-created) group is a hard error here (its id - * cannot be known at plan time), telling the author to apply the group first or pass a numeric id. + * implements and the evidence for it. + * + * Returns a number, EXCEPT when the group is declared in this config but not yet created AND the + * call site opted into {@link ResolveOptions.pendingGroupRole} (#106): then it returns a + * {@link PendingRef}, because the pairing id only exists once the group does. The permission-domain + * position takes that path — `applyPermissionPlan` finishes it with a live fetch after `executePlan` + * (see {@link resolvePendingGroupRoleDomain}). Everywhere else a same-run group stays a hard error, + * telling the author to apply the group first or pass a numeric id. */ - private async resolveGroupRole(r: GroupRoleRef, site: string): Promise { - const groupId = this.groupIdForRole(r, site); + private async resolveGroupRole( + r: GroupRoleRef, + site: string, + opts: ResolveOptions = {}, + ): Promise { + const groupId = this.groupIdForRole(r, site, opts); + if (typeof groupId !== "number") return groupId; const rows = await this.groupRoleList(groupId); - // slug-primary, exact-name secondary — identical matching to resolveFromCatalog, so a role named - // e.g. "Leiter" resolves whether the author writes "leiter" (slug) or "Leiter" (exact). - const bySlug = rows.filter((row) => typeof row.name === "string" && slug(row.name) === slug(r.role)); - const matches = bySlug.length >= 1 ? bySlug : rows.filter((row) => row.name === r.role); - if (matches.length === 0) { - const available = rows - .map((row) => (typeof row.name === "string" ? JSON.stringify(row.name) : `#${row.id}`)) - .join(", "); - throw new Error( - `Cannot resolve ${refLabel(r)} referenced at ${site} on ${this.host}: group #${groupId} has ` + - `no role named "${r.role}"${available ? ` (available: ${available})` : ""}. Fix the role name, ` + - `or pass a numeric id.`, - ); - } - if (matches.length > 1) { - const list = matches.map((c) => `${JSON.stringify(c.name)} (#${c.id})`).join(", "); - throw new Error( - `Ambiguous ${refLabel(r)} referenced at ${site} on ${this.host}: ${matches.length} roles on ` + - `group #${groupId} match — ${list}. Rename to disambiguate, or pass a numeric id.`, - ); - } - const domainId = matches[0]![GROUP_ROLE_PAIRING_FIELD]; - if (typeof domainId !== "number") { - throw new Error( - `Cannot resolve ${refLabel(r)} referenced at ${site} on ${this.host}: the matched role row ` + - `carries no numeric "${GROUP_ROLE_PAIRING_FIELD}" (the assumed pairing domainId — see #25). ` + - `Pass a numeric id.`, - ); - } - return domainId; + return pickGroupRolePairingId(rows, r, groupId, site, this.host); } /** @@ -332,11 +336,16 @@ export class Resolver { return resolved; } - /** Resolve the group half of a group_role ref to a managed group id (state ∪ declared). */ - private groupIdForRole(r: GroupRoleRef, site: string): number { + /** + * Resolve the group half of a group_role ref to a managed group id (state ∪ declared). A group + * declared in this run resolves to a {@link PendingRef} when the call site can finish it later + * (#106) and stays a hard error otherwise. + */ + private groupIdForRole(r: GroupRoleRef, site: string, opts: ResolveOptions): number | PendingRef { const managed = this.state.resources[r.group]; if (managed && managed.type === "group") return managed.id; if (this.declaredByType.get("group")?.has(r.group)) { + if (opts.pendingGroupRole) return pendingRef(r); throw new Error( `Cannot resolve ${refLabel(r)} referenced at ${site} on ${this.host}: group "${r.group}" is ` + `declared in this config but not yet created — its (group, role) pairing id only exists once ` + @@ -352,13 +361,7 @@ export class Resolver { private groupRoleList(groupId: number): Promise { let p = this.groupRoleLists.get(groupId); if (!p) { - // Paginated for the same reason as `catalog()` above: a group with more than CT's default page - // of roles would otherwise hide its later ones behind "group #N has no role named …". - const path = GROUP_ROLE_ENDPOINT(groupId); - const rows = this.client.getAll - ? this.client.getAll(path).then((page) => page.data) - : this.client.get(path); - p = rows.then((r) => (Array.isArray(r) ? r : [])); + p = fetchGroupRoleRows(this.client, groupId); this.groupRoleLists.set(groupId, p); } return p; @@ -378,15 +381,16 @@ export class Resolver { } const catalog = CATALOG_PATH[r.kind]; // A catalog-only kind has no managed resource type, so "Declare/adopt it" is advice the tool - // cannot honour (#96's exact complaint about the old person-status message). Departments are - // read-only in ChurchTools — GET /departments exists, no write verb does — so the only real - // fixes are correcting the name or falling back to a numeric id. + // cannot honour (#96's exact complaint about the old person-status message). The message says + // what `ct` does — reads this catalog, never writes it — rather than claiming ChurchTools makes + // it impossible: departments and security levels ARE writable through the legacy master-data + // endpoint the admin UI uses, just not through anything `ct` drives today (#108/#109/#111). if (catalog && REF_KIND_TYPE[r.kind] === undefined) { return new Error( `Cannot resolve ${refLabel(r)} referenced at ${site} on ${this.host}: no live ${r.kind} at ` + - `${catalog} matches key "${r.key}". ${r.kind}s are a read-only catalog in ChurchTools — they ` + - `cannot be declared or adopted, so fix the key/name (list them with \`ct get ${r.kind}s\`) or ` + - `use a numeric id.`, + `${catalog} matches key "${r.key}". ct reads ${r.kind}s but does not manage them, so it cannot ` + + `create this one — fix the key/name (list them with \`ct get ${r.kind}s\`), create it in the ` + + `ChurchTools admin UI, or use a numeric id.`, ); } const where = catalog @@ -407,6 +411,104 @@ export class Resolver { } } +/** The client shape both the plan-time resolver and the apply-time group-role completion need. */ +type RoleListReader = Pick & Partial>; + +/** Per-group role-list cache, so several pending group_role domains on one group share one fetch. */ +export type GroupRoleRowCache = Map>; + +/** + * Read one group's role list. Paginated for the same reason as `Resolver.catalog()`: a group with + * more than CT's default page of roles would otherwise hide its later ones behind "group #N has no + * role named …". `getAll` is optional only so `{ get }` test doubles stay usable. + */ +function fetchGroupRoleRows(client: RoleListReader, groupId: number): Promise { + const path = GROUP_ROLE_ENDPOINT(groupId); + const rows = client.getAll + ? client.getAll(path).then((page) => page.data) + : client.get(path); + return rows.then((r) => (Array.isArray(r) ? r : [])); +} + +/** + * Pick the (group, role) pairing domainId out of a group's role list. Shared by the plan-time path + * (group already exists) and the apply-time path (#106: group created in this run), so the matching + * rules and — importantly — the error messages are identical whichever side a config lands on. + * slug-primary, exact-name secondary, matching every other catalog lookup in this file. + */ +function pickGroupRolePairingId( + rows: CatalogRecord[], + r: GroupRoleRef, + groupId: number, + site: string, + host: string, +): number { + const bySlug = rows.filter((row) => typeof row.name === "string" && slug(row.name) === slug(r.role)); + const matches = bySlug.length >= 1 ? bySlug : rows.filter((row) => row.name === r.role); + if (matches.length === 0) { + const available = rows + .map((row) => (typeof row.name === "string" ? JSON.stringify(row.name) : `#${row.id}`)) + .join(", "); + throw new Error( + `Cannot resolve ${refLabel(r)} referenced at ${site} on ${host}: group #${groupId} has ` + + `no role named "${r.role}"${available ? ` (available: ${available})` : ""}. Fix the role name, ` + + `or pass a numeric id.`, + ); + } + if (matches.length > 1) { + const list = matches.map((c) => `${JSON.stringify(c.name)} (#${c.id})`).join(", "); + throw new Error( + `Ambiguous ${refLabel(r)} referenced at ${site} on ${host}: ${matches.length} roles on ` + + `group #${groupId} match — ${list}. Rename to disambiguate, or pass a numeric id.`, + ); + } + const domainId = matches[0]![GROUP_ROLE_PAIRING_FIELD]; + if (typeof domainId !== "number") { + throw new Error( + `Cannot resolve ${refLabel(r)} referenced at ${site} on ${host}: the matched role row ` + + `carries no numeric "${GROUP_ROLE_PAIRING_FIELD}" (the assumed pairing domainId — see #25). ` + + `Pass a numeric id.`, + ); + } + return domainId; +} + +/** + * Finish a PENDING `group_role` domain at apply time (#106). + * + * This is the one pending ref {@link reresolvePendingValue} cannot complete from state alone: the + * (group, role) pairing id is not the group's id, it lives on `GET /groups/{id}/roles` and only + * exists once ChurchTools has created the group. So completion is a two-step — look the freshly + * created group up in POST-execute state, then fetch ITS role list and match the declared role name — + * and it is necessarily async, which is why it sits here rather than in `reresolvePendingValue`. + * + * A role name the created group does not have is a hard error listing the available roles, exactly as + * at plan time (shared {@link pickGroupRolePairingId}). `cache` dedupes the fetch when several pending + * domains name the same group. + */ +export async function resolvePendingGroupRoleDomain( + r: GroupRoleRef, + state: State, + client: RoleListReader, + site: string, + cache?: GroupRoleRowCache, +): Promise { + const managed = state.resources[r.group]; + if (!managed || managed.type !== "group") { + throw new Error( + `Pending ${refLabel(r)} did not resolve after apply — "${r.group}" is not a managed group in ` + + `state. The resource tier creates groups before permissions are applied, so this usually means ` + + `the group's create failed earlier in this run.`, + ); + } + let rows = cache?.get(managed.id); + if (!rows) { + rows = fetchGroupRoleRows(client, managed.id); + cache?.set(managed.id, rows); + } + return pickGroupRolePairingId(await rows, r, managed.id, site, state.host); +} + /** * Re-resolve every {@link PendingRef} in a value against the POST-execute state, at apply time. * The referenced resource's tier applies before the referencing resource's (campus tier 0 < group @@ -427,9 +529,15 @@ export function reresolvePendingValue(value: unknown, state: State): unknown { function pendingIdFromState(r: Ref, state: State): number { if (r.kind === "group-role") { - // A group_role ref resolves to a concrete pairing id at plan time (never a PendingRef — a - // same-run group is rejected up front), so a pending one should never reach apply. - throw new Error(`Pending ${refLabel(r)} reached apply — group_role refs never go pending (#25).`); + // A pending group_role IS legal now (#106), but it cannot be completed from state alone: the + // pairing id lives on GET /groups/{id}/roles, so it needs a live fetch. `applyPermissionPlan` + // routes it through `resolvePendingGroupRoleDomain` instead. Reaching HERE means a pending + // group-role turned up in a value-shaped position (a resource id field, a query var), which the + // resolver only ever produces for the permission-domain call site — so it is a wiring bug. + throw new Error( + `Pending ${refLabel(r)} reached the state-only re-resolver — a group_role pairing id needs a ` + + `live /groups/{id}/roles fetch, so it can only be completed by applyPermissionPlan (#106).`, + ); } if (r.kind === "group-type-role") { // A group-type-role ref resolves to a concrete /group/roles id at plan time (the catalog id exists diff --git a/tests/permission-adopt.test.ts b/tests/permission-adopt.test.ts index d404458..4ac0559 100644 --- a/tests/permission-adopt.test.ts +++ b/tests/permission-adopt.test.ts @@ -345,8 +345,10 @@ describe("emitAdoptedGrants", () => { ]; const state = stateWithKids(); const block = emitAdoptedGrants({ domainType: "group_type_role", domainId: 9, rows, state }); - // the admin-authored member rights are emitted as ACTIVE grants, never NOTE comments - expect(block).not.toContain("NOTE:"); + // the admin-authored member rights are emitted as ACTIVE grants, never as omitted WARNINGs. The + // security-level-scoped ones carry a portable-form NOTE (#110) — advice, not an omission — so the + // assertion is about what is DECLARED, which `parseEmittedGrants` below reads back in full. + expect(block).not.toContain("WARNING:"); expect(block).toContain("churchdb:+add person"); const grants = parseEmittedGrants(block); @@ -443,11 +445,22 @@ describe("emitAdoptedGrants", () => { expect(block).toContain('{ right: "churchdb:view alldata", scope: [4] }'); }); - it("a dimension with no logical form still gets the numeric-escape-hatch note", () => { + it("a security-level scope gets the portable-form note, not the numeric-escape-hatch one (#110)", () => { const rows: RawPermission[] = [ { authId: 125, dataId: 2, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, // cc_securitylevel ]; const block = emitAdoptedGrants({ domainType: "group_role", domainId: 42, rows, state: emptyState() }); + expect(block).toContain('Portable form: { securityLevel: "" }'); + expect(block).not.toContain("not a group"); + expect(block).toContain('{ right: "churchdb:security level person", scope: [2] }'); + }); + + it("a dimension with no logical form still gets the numeric-escape-hatch note", () => { + const rows: RawPermission[] = [ + // cdb_comment_viewer — master data with names, but no ref kind here yet (#109 tracks it). + { authId: 113, dataId: 2, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, + ]; + const block = emitAdoptedGrants({ domainType: "group_role", domainId: 42, rows, state: emptyState() }); expect(block).toContain("not a group"); expect(block).not.toContain("Portable form"); }); diff --git a/tests/permission-pending-domain.test.ts b/tests/permission-pending-domain.test.ts index 238fc83..1cb6213 100644 --- a/tests/permission-pending-domain.test.ts +++ b/tests/permission-pending-domain.test.ts @@ -14,6 +14,7 @@ import { renderPermissionPlan } from "../src/permissions/render.js"; import { executePlan } from "../src/engine/execute.js"; import { emptyState, type State } from "../src/state/state.js"; import { ref } from "../src/resolve/refs.js"; +import { Resolver } from "../src/resolve/resolver.js"; import type { Plan, DesiredResource } from "../src/engine/types.js"; import type { DesiredPermission } from "../src/permissions/types.js"; import type { CtClient } from "../src/api/ctClient.js"; @@ -165,27 +166,137 @@ describe("pending domain: prod-like scenario (type already in state) is unchange }); }); -describe("group_role symmetry: a same-run group is NOT made pending — it stays a hard error (#69/#25)", () => { - it("keeps the specific 'apply the group first' message for a group_role domain on a declared-not-created group", async () => { - // A group_role domainId is the (group, role) PAIRING id, exposed only on GET /groups/{id}/roles — - // so re-resolution needs a LIVE fetch after the group exists, not just a state lookup. That is a - // materially harder case than group_type_role (whose domainId is the group type's OWN id, present - // in post-execute state). So group_role deliberately does NOT go pending: it fails fast at - // resolve time with its own actionable message, unchanged by this fix. - const declaredGroup: DesiredResource[] = [ - { type: "group", key: "kids_area", fields: { name: "Kids" }, dependsOn: [] }, - ]; - const grPerm: DesiredPermission = { - key: "kids_lead", - domainType: "group_role", - domainId: ref.groupRole("kids_area", "Leiter"), - grants: [], - }; - const { client } = mockClient(); - await expect(buildPermissionPlan(client, emptyState(HOST), [grPerm], declaredGroup)).rejects.toThrow( - /group "kids_area" is declared in this config but not yet created/, +describe("group_role symmetry: a same-run group DOES go pending and completes in one apply (#106)", () => { + // The domain half of #29's deadlock. A group_role domainId is the (group, role) PAIRING id, exposed + // only on GET /groups/{id}/roles — so it cannot be completed from post-execute state alone the way a + // group_type_role domain can. It is completed with a live fetch inside applyPermissionPlan instead. + // Before #106 this was a hard error, which made the very same config plan clean on prod (group + // exists) and exit 1 on dev (group does not) — non-portable by construction. + const GROUP_ID = 4711; + const PAIRING_ID = 44675; + + const declaredGroup: DesiredResource[] = [ + { type: "group", key: "kids_area", fields: { name: "Kids" }, dependsOn: [] }, + ]; + const grPerm: DesiredPermission = { + key: "kids_lead", + domainType: "group_role", + domainId: ref.groupRole("kids_area", "Leiter"), + grants: ["churchgroup:administer groups"], + }; + const createGroupPlan: Plan = { + items: [ + { + type: "group", + key: "kids_area", + id: null, + action: "create", + changes: [{ field: "name", from: undefined, to: "Kids" }], + }, + ], + }; + + /** Like `mockClient`, but POST /groups mints GROUP_ID and the group's role list is readable. */ + function groupClient(roles: unknown[] = [{ id: PAIRING_ID, name: "Leiter", groupTypeRoleId: 12 }]) { + const calls: { method: string; path: string; body?: unknown }[] = []; + const request = vi.fn(async (method: string, path: string, body?: unknown) => { + calls.push({ method, path, body }); + if (method === "POST" && path === "/groups") return { id: GROUP_ID }; + return {}; + }); + const get = vi.fn(async (path: string) => (path === `/groups/${GROUP_ID}/roles` ? roles : [])); + return { client: { request, get } as unknown as CtClient, calls, get }; + } + + it("plans from EMPTY state as a pending domain instead of the old hard error", async () => { + const { client, get } = groupClient(); + const { items, fetchErrors } = await buildPermissionPlan( + client, + emptyState(HOST), + [grPerm], + declaredGroup, + ); + + expect(fetchErrors).toEqual([]); + expect(items[0]?.domainId).toBeNull(); + expect(items[0]?.pendingDomain).toEqual(ref.groupRole("kids_area", "Leiter")); + expect(items[0]?.diff.toPut).toEqual([{ authId: 1113, dataId: [], type: "grant" }]); + // Nothing is fetched at plan time — the group does not exist yet, so neither does its role list. + expect(get).not.toHaveBeenCalled(); + expect(renderPermissionPlan(items)).toContain( + "", + ); + }); + + it("applies in ONE run — create the group, read its roles, grant on the pairing id", async () => { + const { client, calls, get } = groupClient(); + const state = emptyState(HOST); + const { items } = await buildPermissionPlan(client, state, [grPerm], declaredGroup); + + await executePlan(createGroupPlan, { client, state, statePath: "unused", save: async () => {} }); + expect(state.resources.kids_area?.id).toBe(GROUP_ID); + + const res = await applyPermissionPlan(items, client, state); + expect(res.granted).toBe(1); + expect(res.failed).toEqual([]); + // The role list is read from the FRESHLY created group, and the pairing id — not the group id — + // is what lands in the write path. + expect(get).toHaveBeenCalledWith(`/groups/${GROUP_ID}/roles`); + const put = calls.find((c) => c.method === "PUT"); + expect(put?.path).toBe(`/permissions/group_role/${PAIRING_ID}`); + expect(put?.body).toEqual({ authId: 1113, type: "grant" }); + }); + + it("still hard-errors on a role the created group does not have, listing what it does have", async () => { + const { client } = groupClient([ + { id: PAIRING_ID, name: "Mitglied" }, + { id: PAIRING_ID + 1, name: "Organisator" }, + ]); + const state = emptyState(HOST); + const { items } = await buildPermissionPlan(client, state, [grPerm], declaredGroup); + await executePlan(createGroupPlan, { client, state, statePath: "unused", save: async () => {} }); + + await expect(applyPermissionPlan(items, client, state)).rejects.toThrow( + /group #4711 has no role named "Leiter" \(available: "Mitglied", "Organisator"\)/, ); }); + + it("is unchanged on a host where the group already exists — concrete domain, no pending path", async () => { + const { client, get } = groupClient(); + const state: State = { + version: 1, + host: HOST, + resources: { + kids_area: { + type: "group", + id: GROUP_ID, + key: "kids_area", + fields: { name: "Kids" }, + adoptedAt: "t", + updatedAt: "t", + }, + }, + }; + const { items, fetchErrors } = await buildPermissionPlan(client, state, [grPerm], declaredGroup); + expect(fetchErrors).toEqual([]); + expect(items[0]?.pendingDomain).toBeUndefined(); + expect(items[0]?.domainId).toBe(PAIRING_ID); + // Resolved at PLAN time here, from the already-existing group's role list. + expect(get).toHaveBeenCalledWith(`/groups/${GROUP_ID}/roles`); + }); + + it("keeps the fail-fast message where a pending group_role can NOT be completed (query var)", async () => { + // Only the permission-domain position opts into pending group-roles, because only it gets a live + // fetch after the group exists. A group-role ref anywhere else keeps the old actionable error. + const resolver = new Resolver({ + client: { get: async () => [] } as unknown as CtClient, + state: emptyState(HOST), + desired: declaredGroup, + }); + await expect( + resolver.resolve(ref.groupRole("kids_area", "Leiter"), 'dynamic group "x" var'), + ).rejects.toThrow(/group "kids_area" is declared in this config but not yet created/); + }); }); describe("pending domain: a TRUE typo (key absent from config AND state AND catalog) still hard-errors (#69)", () => { diff --git a/tests/permission-scope-refs.test.ts b/tests/permission-scope-refs.test.ts index d75e642..1b47c03 100644 --- a/tests/permission-scope-refs.test.ts +++ b/tests/permission-scope-refs.test.ts @@ -175,11 +175,23 @@ describe("scope-dimension validation (#98)", () => { key: "p", domainType: "group_role", domainId: 1, - // churchdb:security level person scopes by cc_securitylevel — security levels are not resources. + // churchdb:view comments scopes by cdb_comment_viewer, which has no ref kind here yet (#109). + grants: [{ right: "churchdb:view comments", scope: [ref.campus("koblenz")] }], + }; + await expect(resolveScopeRefs([perm], resolverFor(emptyState(HOST)), emptyState(HOST))).rejects.toThrow( + /campus:koblenz.*cdb_comment_viewer.*no logical reference form/s, + ); + }); + + it("rejects a ref whose kind does not match the security-level dimension (#110)", async () => { + const perm: DesiredPermission = { + key: "p", + domainType: "group_role", + domainId: 1, grants: [{ right: "churchdb:security level person", scope: [ref.campus("koblenz")] }], }; await expect(resolveScopeRefs([perm], resolverFor(emptyState(HOST)), emptyState(HOST))).rejects.toThrow( - /campus:koblenz.*cc_securitylevel.*no logical reference form/s, + /campus:koblenz.*"cc_securitylevel".*takes a security-level reference/s, ); }); @@ -263,9 +275,11 @@ describe("scope-dimension validation (#98)", () => { }); }); -describe("department scopes are a READ-ONLY ref catalog (cdb_bereich, #98)", () => { +describe("department scopes are a read-but-not-managed ref catalog (cdb_bereich, #98)", () => { // `/departments` is GET-only — live-probed 2026-08-13 against the instance's own OpenAPI spec - // (eqrm prod, CT 3.135.2). So a department resolves by NAME on every host but is never declarable. + // (eqrm prod, CT 3.135.2) — and `ct` drives no other write path for it, so a department resolves by + // NAME on every host but is never declarable HERE. (ChurchTools itself can create one, through the + // legacy master-data endpoint the admin UI uses — #108/#109.) const departments = [ { id: 7, name: "Equippers Koblenz" }, { id: 1, name: "Equippers Rhein-Main" }, @@ -291,7 +305,7 @@ describe("department scopes are a READ-ONLY ref catalog (cdb_bereich, #98)", () it("hard-errors on an unknown department, and does NOT advise declaring it", async () => { await expect(tuplesFor(perm("nope"), emptyState(HOST), [], client)).rejects.toThrow( - /no live department at \/departments matches key "nope".*read-only catalog.*cannot be declared or adopted/s, + /no live department at \/departments matches key "nope".*ct reads departments but does not manage them/s, ); }); @@ -317,6 +331,48 @@ describe("department scopes are a READ-ONLY ref catalog (cdb_bereich, #98)", () }); }); +describe("security-level scopes resolve by name (cc_securitylevel, #110)", () => { + // "Security-level ids are universal, so a numeric literal is portable" was an assumption, not a + // guarantee: cc_securitylevel is admin-editable master data with an auto-increment id. These pin the + // ref form that says what it means on any host — while keeping the numeric hatch working. + const levels = [ + { id: 1, name: "Stufe 1 (Niedrig)", sortKey: 1 }, + { id: 2, name: "Stufe 2 (Mittel)", sortKey: 2 }, + { id: 3, name: "Stufe 3 (Hoch)", sortKey: 3 }, + ]; + const client = { + get: vi.fn(async (path: string) => (path === "/securitylevels" ? levels : [])), + } as unknown as CtClient; + + const perm = (scope: unknown[]): DesiredPermission => ({ + key: "p", + domainType: "group_role", + domainId: 1, + grants: [{ right: "churchdb:security level person", scope: scope as never }], + }); + + it("resolves a level by its slugged name against the live catalog", async () => { + // No scopeKey: catalog-resolved ids are host-correct already and have no managed identity. + expect(await tuplesFor(perm([{ securityLevel: "stufe_3_hoch" }]), emptyState(HOST), [], client)).toEqual([ + { authId: 125, dataId: [3], type: "grant" }, + ]); + }); + + it("still accepts the numeric escape hatch, byte-identically (#49)", async () => { + expect(await tuplesFor(perm([1, 2, 3]), emptyState(HOST), [], client)).toEqual([ + { authId: 125, dataId: [1], type: "grant" }, + { authId: 125, dataId: [2], type: "grant" }, + { authId: 125, dataId: [3], type: "grant" }, + ]); + }); + + it("hard-errors on a level name this host does not have, instead of granting the wrong one", async () => { + await expect( + tuplesFor(perm([{ securityLevel: "stufe_9" }]), emptyState(HOST), [], client), + ).rejects.toThrow(/no live security-level at \/securitylevels matches key "stufe_9"/); + }); +}); + describe("scope object sugar", () => { it('compiles { campus: "koblenz" } to the same Ref as ref.campus(...)', () => { const { ct, permissions } = createContext(); diff --git a/tests/portable-refs.test.ts b/tests/portable-refs.test.ts index 1040b01..a38fb85 100644 --- a/tests/portable-refs.test.ts +++ b/tests/portable-refs.test.ts @@ -225,7 +225,10 @@ describe("permission domainId resolution", () => { expect(items[0]?.diff.toPut).toEqual([{ authId: 1113, dataId: [], type: "grant" }]); }); - it("still rejects a group_role reference to a not-yet-created group (pass a numeric id) (#25)", async () => { + it("carries a group_role reference to a not-yet-created group as a PENDING domain (#106)", async () => { + // Was a hard error until #106. The pairing id only exists once the group does, so the domain plans + // pending and is completed with a live /groups/{id}/roles fetch inside applyPermissionPlan — + // which is what makes this config plannable on a fresh host as well as on one where it exists. const { permissions } = await evaluateConfig((ct) => { ct.group({ key: "kids", name: "Kids", groupTypeId: 2 }); ct.groupRole({ key: "p", group: "kids", role: "Leiter", grants: ["churchgroup:administer groups"] }); @@ -233,9 +236,10 @@ describe("permission domainId resolution", () => { const client = { get: async (): Promise => [] as T }; // `desired` includes the same-run group, so the resolver knows it is declared-but-pending. const desired = [{ type: "group", key: "kids", fields: {}, dependsOn: [] }]; - await expect(buildPermissionPlan(client, emptyState("h"), permissions, desired)).rejects.toThrow( - /declared in this config but not yet created.*pass a numeric id/is, - ); + const { items } = await buildPermissionPlan(client, emptyState("h"), permissions, desired); + expect(items[0]?.domainId).toBeNull(); + expect(items[0]?.pendingDomain).toEqual(ref.groupRole("kids", "Leiter")); + expect(items[0]?.diff.toPut).toEqual([{ authId: 1113, dataId: [], type: "grant" }]); }); }); From 515e59da2e2f6b5cfdc0beeee920906eb8b3ff5b Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Fri, 14 Aug 2026 09:10:48 +0200 Subject: [PATCH 2/7] =?UTF-8?q?fix:=20correct=20the=20security-level=20wri?= =?UTF-8?q?te=20claims=20=E2=80=94=20/securitylevels/{id}=20has=20POST/PAT?= =?UTF-8?q?CH/DELETE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-probed against eqrm-dev (CT 3.135.2, 2026-08-14) with the two-probe procedure this PR added, and it immediately caught an error in this PR's own text: `/securitylevels` is GET-only, but `/securitylevels/{id}` exposes POST (create, body `{name}`), PATCH (update, body `{name, newid, forcereorder}` — reordering is a first-class supported operation) and DELETE. #110 asserted "no REST write verb", inherited it from the issue, and I repeated it. What actually changes: security levels stay a catalog `ct` reads and does not manage, but for a different and much weaker reason — the id-bearing create path does not fit the resource registry's "POST the collection path" contract, so promotion is a design call, not an API limit. The reference form (#110's actual ask) is unaffected and stays exactly as it was. Also strengthens the case for the ref form rather than weakening it: a supported `newid`/`forcereorder` reorder means "the ids are stable in practice" was never going to hold. Corrected in scope.ts, refs.ts, resolver.ts, api-coverage.md, the runbook, and both handbook pages. Departments re-probed on dev and unchanged: GET only. Claude-Session: https://claude.ai/code/session_016TacHkv3oVczwXGHBwckq2 --- docs/api-coverage.md | 10 ++++----- docs/handbuch/field-definitions.md | 17 +++++++++------ docs/handbuch/permissions.md | 34 ++++++++++++++++++------------ docs/runbook-manual-surface.md | 2 +- src/permissions/scope.ts | 8 +++++-- src/resolve/refs.ts | 15 +++++++++---- src/resolve/resolver.ts | 9 +++++--- 7 files changed, 60 insertions(+), 35 deletions(-) diff --git a/docs/api-coverage.md b/docs/api-coverage.md index 3fcfede..5c61766 100644 --- a/docs/api-coverage.md +++ b/docs/api-coverage.md @@ -85,11 +85,11 @@ API-client libraries (5pm-HDH `churchtools-api` @ CT 3.104, bensteUEM `src/api/schema.d.ts` is git-ignored and was not available offline. Re-verify per the runbook's re-audit procedure once the schema is regenerated. -| Resource | Matched path(s) | GET | POST/PUT/PATCH/DELETE | Verdict | -| ------------------------------------ | ----------------------------- | ------------------ | --------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Person master-data model | `/person/masterdata` | ✅ (single object) | ❌ | **Read-only.** Versionable master-data model incl. the `securityLevels` enumeration. No write endpoint (edited in the CT master-data admin UI). | -| Data-field definitions (Datenfelder) | `/dbfields`, `/dbfields/{id}` | list ✅ / by-id ✅ | ❌ | **Read-only.** Unified person + group field definitions, discriminated by `fieldCategory`. Mutation only via legacy churchdb AJAX (`db_insert/update/deletefields`), not REST. | -| Security levels | `/securitylevels` | list ✅ | ❌ (no REST write) | **Read here, writable elsewhere.** Flat `[{id, name, sortKey}]` — live-verified 2026-08-13 (eqrm prod, CT 3.135.2). The `cc_securitylevel` permission scope dimension, resolvable by name since #110. The ids are NOT protocol constants: `cc_securitylevel` is an editable master-data table, writable through the legacy `getMasterData`/`saveMasterData` endpoint (#109/#110). | +| Resource | Matched path(s) | GET | POST/PUT/PATCH/DELETE | Verdict | +| ------------------------------------ | ----------------------------------------- | ------------------ | ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Person master-data model | `/person/masterdata` | ✅ (single object) | ❌ | **Read-only.** Versionable master-data model incl. the `securityLevels` enumeration. No write endpoint (edited in the CT master-data admin UI). | +| Data-field definitions (Datenfelder) | `/dbfields`, `/dbfields/{id}` | list ✅ / by-id ✅ | ❌ | **Read-only.** Unified person + group field definitions, discriminated by `fieldCategory`. Mutation only via legacy churchdb AJAX (`db_insert/update/deletefields`), not REST. | +| Security levels | `/securitylevels`, `/securitylevels/{id}` | list ✅ / by-id ✅ | ✅ POST, PATCH, DELETE | **Full CRUD — but not on the usual paths.** Live-probed 2026-08-14 (eqrm-dev, CT 3.135.2): `/securitylevels` is GET-only, and **create is `POST /securitylevels/{id}`** (body `{name}`), update is `PATCH /securitylevels/{id}` (body `{name, newid, forcereorder}` — reordering is first-class), delete is `DELETE /securitylevels/{id}`. Not a managed resource yet: the registry models create as "POST the collection path", which this id-bearing create does not fit (#110). Resolvable by name as a scope ref since #110. The ids are NOT protocol constants — `cc_securitylevel` is an editable master-data table with an auto-increment id. | See [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md) for the full writability decision, evidence, and the schema/values boundary. diff --git a/docs/handbuch/field-definitions.md b/docs/handbuch/field-definitions.md index e45aeec..ce902b6 100644 --- a/docs/handbuch/field-definitions.md +++ b/docs/handbuch/field-definitions.md @@ -54,7 +54,10 @@ ct get data-fields # all data-field definitions (person + group), one ro earlier version of this page said no standalone security-levels resource existed; it does, live-verified 2026-08-13 on CT 3.135.2.) Reading it per host is also how you check whether the usual ids `1..4` really hold there — they are - editable master-data rows, not protocol constants. + editable master-data rows, not protocol constants. **Security levels are the + one item on this page that is _not_ read-only in ChurchTools:** + `/securitylevels/{id}` takes `POST`/`PATCH`/`DELETE` (probed 2026-08-14). + `ct` only reads them today — see #110. - `ct get data-fields` → `GET /dbfields` (auto-paginated). The **unified** data-field definition catalog. Person master-data fields **and** group custom fields live in the same list, discriminated per-row by `fieldCategory` @@ -113,12 +116,12 @@ re-deciding rather than merely re-reading. ## Endpoint reference -| Purpose | Path | Methods (this CT) | `ct` surface | -| ------------------------------------------ | ------------------------------------------------------------------------------ | ------------------- | -------------------------- | -| Person master-data model + security levels | `/person/masterdata` | GET (read-only) | `ct get person-masterdata` | -| Security levels, standalone | `/securitylevels` | GET (no REST write) | `ct get security-levels` | -| Data-field definitions (person + group) | `/dbfields`, `/dbfields/{id}` | GET (read-only) | `ct get data-fields` | -| Field-definition **mutation** | legacy `churchdb` AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) | non-REST | **not managed — manual** | +| Purpose | Path | Methods (this CT) | `ct` surface | +| ------------------------------------------ | ------------------------------------------------------------------------------ | ---------------------------------------------- | -------------------------------------------- | +| Person master-data model + security levels | `/person/masterdata` | GET (read-only) | `ct get person-masterdata` | +| Security levels, standalone | `/securitylevels`, `/securitylevels/{id}` | GET; POST/PATCH/DELETE on the item path (#110) | `ct get security-levels` (read only for now) | +| Data-field definitions (person + group) | `/dbfields`, `/dbfields/{id}` | GET (read-only) | `ct get data-fields` | +| Field-definition **mutation** | legacy `churchdb` AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) | non-REST | **not managed — manual** | All paths verified against public CT client libraries + CT Academy docs, **not** against this repo's (git-ignored, ungenerated) `src/api/schema.d.ts`. diff --git a/docs/handbuch/permissions.md b/docs/handbuch/permissions.md index 6de9ae0..748a9d4 100644 --- a/docs/handbuch/permissions.md +++ b/docs/handbuch/permissions.md @@ -5,7 +5,7 @@ sources: - src/resolve/resolver.ts - src/resolve/refs.ts - src/config/context.ts -sources_hash: e3b937247ebebc2a +sources_hash: 0775204b798e0aae reviewed: 2026-08-13 --- @@ -373,17 +373,23 @@ Three things are hard errors at **plan** time, never a guessed `dataId`: ### Departments and security levels: referenceable, not declarable `cdb_bereich` (Bereiche) and `cc_securitylevel` (Sicherheitslevel) are the two -dimensions `ct` **reads but does not manage**. Neither has a REST write verb — -live-probed against the instance's own OpenAPI spec (eqrm prod, CT 3.135.2, -2026-08-13): `GET /departments` and `GET /securitylevels` exist; no -`POST`/`PUT`/`DELETE` on either does. - -That is a fact about the REST API and about what this tool drives — **not** a -claim that ChurchTools cannot create them. Both are rows in CT's editable -master-data registry, which the admin UI writes through a legacy endpoint that -appears in no OpenAPI spec. Making them declarable is tracked in -[#108](https://github.com/eqrm/ct-cli/issues/108) / -[#109](https://github.com/eqrm/ct-cli/issues/109). +dimensions `ct` **reads but does not manage**. They are unmanaged for two +different reasons, and neither reason is "ChurchTools cannot do it": + +- **Bereiche** genuinely have no REST write path — live-probed against the + instance's own OpenAPI spec (eqrm prod CT 3.135.2, 2026-08-13; re-probed on + eqrm-dev 2026-08-14): `GET /departments` exists, nothing else on that path + does. The admin UI creates them through the legacy master-data endpoint, which + appears in no OpenAPI spec and which `ct` does not drive + ([#108](https://github.com/eqrm/ct-cli/issues/108) / + [#109](https://github.com/eqrm/ct-cli/issues/109)). +- **Security levels do have REST writes.** `/securitylevels/{id}` exposes + `POST` (create), `PATCH` (update, including reorder via `newid` + + `forcereorder`) and `DELETE` — live-probed 2026-08-14, CT 3.135.2. They are + unmanaged here only because that create path carries an id, which the resource + registry's "POST to the collection path" contract does not model. Promotion is + a deliberate open question on + [#110](https://github.com/eqrm/ct-cli/issues/110), not an API limit. So a reference of either kind resolves by name on every host — which is what makes `churchdb:view alldata` ("Personen eines Bereiches sehen") and @@ -410,7 +416,9 @@ as _numeric-universal_: `scope: [1, 2, 3]` was considered portable because the i "mean the same thing on every instance". They do — by convention. `cc_securitylevel` is an admin-editable master-data table with an auto-increment id and a `sortkey`, so someone adding or reordering a level on one host silently changes what a -hard-coded `[1, 2, 3]` grants there, with a green plan. Numerics still work +hard-coded `[1, 2, 3]` grants there, with a green plan. Reordering is not even an +edge case: `PATCH /securitylevels/{id}` takes a `newid` and a `forcereorder` flag, +i.e. CT ships it as a supported operation. Numerics still work everywhere; the trade-off is that level names are localised German strings (`"Stufe 3 (Hoch)"` → `stufe_3_hoch`), so a **rename** breaks a reference where a number would have survived. Pick per config which risk you would rather carry. diff --git a/docs/runbook-manual-surface.md b/docs/runbook-manual-surface.md index c782730..a3dc29d 100644 --- a/docs/runbook-manual-surface.md +++ b/docs/runbook-manual-surface.md @@ -50,7 +50,7 @@ in that instance's own config repo, in a runbook following this doc's structure. | ~~Permission catalog lifecycle~~ **(shipped)** | ~~`catalog.json` is a one-off HAR-trace snapshot with no staleness detection~~ — **shipped (#25):** `npm run regenerate:permission-catalog` rewrites it from a live instance (records the CT version in `$meta`), and `ct plan` now warns on a version mismatch or an unknown-authId live grant (which it leaves untouched, never revoking a right it cannot name). See [`docs/handbuch/permissions.md`](handbuch/permissions.md) "Catalog lifecycle & staleness" | [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | No workaround needed — run the command; heed the `ct plan` warnings | | Field definitions & security levels (person + group custom fields) **(read-only, shipped #47/#48)** | The person master-data model, the security-level enumeration, and the data-field DEFINITIONS ("Datenfelder") for persons and groups — structural schema, not per-record values | [#47](https://github.com/eqrm/ct-cli/issues/47), [#48](https://github.com/eqrm/ct-cli/issues/48) (read shipped; write is an API gap — see note) | Read with `ct get person-masterdata` (model + security levels) and `ct get data-fields` (all field definitions, person + group, discriminated by `fieldCategory`). **Mutation stays manual:** field definitions have no REST write endpoint — only the legacy churchdb admin AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) — so create/edit/delete them by hand in the master-data admin UI. Decision + evidence: [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md) | | Bereiche (departments) — creating/renaming one | The Bereich master data — the `cdb_bereich` permission scope dimension | [#108](https://github.com/eqrm/ct-cli/issues/108), [#109](https://github.com/eqrm/ct-cli/issues/109) | **Moved here from "API gap" by #111.** `/departments` has no REST write verb (live-probed, eqrm prod CT 3.135.2, 2026-08-13) — but the admin UI creates Bereiche through `POST /index.php?q=churchdb/ajax` `func=saveMasterData` (captured 2026-08-13), so the old "can never be created" verdict was an artefact of auditing OpenAPI only. Until `ct` drives that endpoint: create/rename in **Master data admin → Bereiche**; `ct get departments` lists them. Grants scoped to one ARE declarable — `scope: [{ department: "" }]` resolves by name per host (#98). Renaming one in CT breaks every config reference to its old name, by design: the reference hard-errors instead of silently regranting elsewhere | -| Security levels — creating/reordering one | The `cc_securitylevel` master data (`GET /securitylevels`) — the scope dimension of `churchdb:+see persons` and friends | [#109](https://github.com/eqrm/ct-cli/issues/109), [#110](https://github.com/eqrm/ct-cli/issues/110) (ref form done) | No REST write verb (live-probed, eqrm prod CT 3.135.2, 2026-08-13); `cc_securitylevel` IS in the legacy master-data registry, which `ct` does not drive yet. Create/reorder in **Master data admin → Sicherheitslevel**; `ct get security-levels` lists them. Grants scoped to one are declarable by name — `scope: [{ securityLevel: "" }]` (#110) — and numeric `scope: [1, 2, 3]` still works, but check the ids per host before relying on them: they are auto-increment rows, not protocol constants | +| Security levels — creating/reordering one | The `cc_securitylevel` master data (`GET /securitylevels`) — the scope dimension of `churchdb:+see persons` and friends | [#110](https://github.com/eqrm/ct-cli/issues/110) (ref form done; promotion open) | **REST CRUD exists** — live-probed 2026-08-14 (eqrm-dev, CT 3.135.2): create `POST /securitylevels/{id}`, update/reorder `PATCH /securitylevels/{id}` (`newid`, `forcereorder`), delete `DELETE /securitylevels/{id}`. So this is `ct`'s gap, not CT's: the id-bearing create does not fit the registry's "POST the collection path" contract. Until it is promoted, create/reorder in **Master data admin → Sicherheitslevel**; `ct get security-levels` lists them. Grants scoped to one are declarable by name — `scope: [{ securityLevel: "" }]` (#110) — and numeric `scope: [1, 2, 3]` still works, but check the ids per host before relying on them: they are auto-increment rows, not protocol constants | | API re-audit for new CT releases | CT's OpenAPI spec is self-trimming (only shows endpoints your version has), so a new write endpoint (e.g. a member-status write, or — separately — a first-ever group-status list/write endpoint, #67) appears silently between CT upgrades | tracked by this issue ([#26](https://github.com/eqrm/ct-cli/issues/26)) | Procedure below (**Re-audit procedure for new CT releases**) | ## Out of tool scope — deliberate, not a gap diff --git a/src/permissions/scope.ts b/src/permissions/scope.ts index af879e4..133a253 100644 --- a/src/permissions/scope.ts +++ b/src/permissions/scope.ts @@ -63,8 +63,12 @@ export const GROUP_SCOPE_FIELD = "cdb_gruppe"; * re-resolved at apply time) and a state-backed id is worth re-resolving. Departments and security * levels are catalogs `ct` READS but does not manage, so a reference always resolves by NAME against * the live catalog and hard-errors when the name is absent — strictly better than a silent numeric - * misgrant, but never a create. (Neither has a REST write verb; both ARE writable through the legacy - * master-data endpoint the admin UI uses, which `ct` does not drive yet — #108/#109.) + * misgrant, but never a create. They are unmanaged for DIFFERENT reasons, and neither is "CT cannot": + * - `cdb_bereich` has no REST write at all; the admin UI writes it through the legacy master-data + * endpoint `ct` does not drive (#108/#109). + * - `cc_securitylevel` DOES have REST writes (`/securitylevels/{id}`: POST/PATCH/DELETE, probed + * 2026-08-14) — it is unmanaged only because its id-bearing create does not fit the resource + * registry's `collectionPath` contract (#110). * * The remaining scoped dimensions (`ccm_data_category`, `cdb_comment_viewer`, `oauth_client`, …) stay * numeric-only because this tool has no way to address their values by a host-independent name today — diff --git a/src/resolve/refs.ts b/src/resolve/refs.ts index 5928f06..ad52305 100644 --- a/src/resolve/refs.ts +++ b/src/resolve/refs.ts @@ -131,10 +131,17 @@ export const ref = { * * Exists because "security-level ids are universal, so a numeric literal is portable" was an * assumption, not a guarantee: `cc_securitylevel` is an admin-editable master-data table (`id`, - * `bezeichnung`, `sortkey`) with an auto-increment id, so 1/2/3 line up across instances by - * convention — because nobody has added or reordered a level — not by construction. Referencing a - * level by name makes a config say what it means on any host, and hard-error instead of silently - * granting the wrong level when the name is absent. + * `bezeichnung`, `sortkey`) with an auto-increment id — confirmed in the instance's own master-data + * registry, eqrm-dev CT 3.135.2, 2026-08-14 — so 1/2/3 line up across instances by convention, not + * by construction. Reordering is not even exotic: `PATCH /securitylevels/{id}` takes `newid` + + * `forcereorder`. Referencing a level by name makes a config say what it means on any host, and + * hard-error instead of silently granting the wrong level when the name is absent. + * + * Catalog-only TODAY, and unlike {@link ref.department} that is not forced by CT: + * `/securitylevels/{id}` exposes POST, PATCH and DELETE (live-probed 2026-08-14), so security + * levels could become a fully managed resource. They are not one yet because CREATE is + * `POST /securitylevels/{id}` — an id-bearing create path the registry's `collectionPath` contract + * does not model. Tracked on #110. * * Numeric scope entries stay fully supported (#49): the names are localised German strings with * parentheses ("Stufe 3 (Hoch)" slugs to `stufe_3_hoch`), so a rename breaks a ref where a number diff --git a/src/resolve/resolver.ts b/src/resolve/resolver.ts index ea7c42a..c2592a4 100644 --- a/src/resolve/resolver.ts +++ b/src/resolve/resolver.ts @@ -83,9 +83,12 @@ const CATALOG_PATH: Partial> = { department: "/departments", // Security levels — the `cc_securitylevel` scope dimension (#110). `GET /securitylevels` returns a // flat `[{id, name, sortKey}]` array ("Stufe 1 (Niedrig)" … "Stufe 4 (Sehr hoch)"), live-verified on - // eqrm prod, CT 3.135.2, 2026-08-13. Catalog-only, like departments: `ct` resolves a level by name - // but never creates one. It exists because the ids are NOT a protocol constant — `cc_securitylevel` - // is an editable master-data table, so a hard-coded `scope: [1, 2, 3]` is portable only by convention. + // eqrm prod (CT 3.135.2, 2026-08-13) and eqrm-dev (2026-08-14). Read here because the ids are NOT a + // protocol constant — `cc_securitylevel` is an editable master-data table with an auto-increment id + // and a supported reorder (`PATCH …/{id}` with `newid`), so a hard-coded `scope: [1, 2, 3]` is + // portable only by convention. Unlike `department`, catalog-only is OUR limit, not CT's: + // `/securitylevels/{id}` has POST/PATCH/DELETE, but its create is id-bearing and does not fit the + // registry's `collectionPath` contract, so promotion to a managed resource is a separate call (#110). "security-level": "/securitylevels", "group-type": "/group/grouptypes", // PERSON statuses — the domain of a `status` permission declaration (#90). Unlike GROUP statuses From d52921ed1567a65e8017a21914409466896ef310 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Fri, 14 Aug 2026 09:17:26 +0200 Subject: [PATCH 3/7] =?UTF-8?q?feat(permissions):=20commentViewer=20scope?= =?UTF-8?q?=20refs=20=E2=80=94=20the=20cdb=5Fcomment=5Fviewer=20half=20of?= =?UTF-8?q?=20#102,=20via=20REST?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/person/commentviewers` is conventional REST CRUD (`[{id, name, sortKey}]` plus POST/PUT/DELETE on the item path), live-probed on eqrm-dev CT 3.135.2 2026-08-14. So `{ commentViewer: "" }` is one more CATALOG_PATH entry and RefKind, exactly like the security-level ref — no legacy master-data endpoint needed, which is what #109 assumed this would take. Unblocks the three role instances on Bereich Pastoral Care that each carried exactly one comment_viewer-scoped grant, and so fell out of `ct coverage` as undeclarable in their entirety. `id: 0` is a real row on a live instance ("Alle"), so there is a test pinning that a falsy id resolves rather than reading as "not found". Two tests that used cc_securitylevel and then cdb_comment_viewer as their stand-in for "a dimension with no logical form" now use cc_calcategory, which is a more honest example: the dimensions still without a ref kind are overwhelmingly those of modules outside this tool's mandate (calendars, resources, services, wiki, finance) — a scope decision, not an inability to name them. Comments and docs say that now instead of "not resources at all". Claude-Session: https://claude.ai/code/session_016TacHkv3oVczwXGHBwckq2 --- docs/handbuch/field-definitions.md | 2 +- docs/handbuch/permissions.md | 64 +++++++++++++++++------------ src/commands/get.ts | 3 ++ src/permissions/adopt.ts | 1 + src/permissions/scope.ts | 22 ++++++---- src/permissions/types.ts | 3 +- src/resolve/refs.ts | 22 ++++++++++ src/resolve/resolver.ts | 4 ++ tests/permission-adopt.test.ts | 4 +- tests/permission-scope-refs.test.ts | 48 ++++++++++++++++++++-- 10 files changed, 132 insertions(+), 41 deletions(-) diff --git a/docs/handbuch/field-definitions.md b/docs/handbuch/field-definitions.md index ce902b6..9970df4 100644 --- a/docs/handbuch/field-definitions.md +++ b/docs/handbuch/field-definitions.md @@ -3,7 +3,7 @@ title: Field definitions & security levels sources: - src/commands/get.ts - src/api/ctClient.ts -sources_hash: 93ba678d9f747e3c +sources_hash: 4ee434f900165c79 reviewed: 2026-08-13 --- diff --git a/docs/handbuch/permissions.md b/docs/handbuch/permissions.md index 748a9d4..31d783c 100644 --- a/docs/handbuch/permissions.md +++ b/docs/handbuch/permissions.md @@ -5,7 +5,7 @@ sources: - src/resolve/resolver.ts - src/resolve/refs.ts - src/config/context.ts -sources_hash: 0775204b798e0aae +sources_hash: 6b8acc0778bf1fd5 reviewed: 2026-08-13 --- @@ -335,13 +335,14 @@ ct.groupRole({ `{ campus: "koblenz" }` is sugar for `ref.campus("koblenz")` — the same `Ref` the rest of the DSL uses — so both spellings are interchangeable. -| `scopeField` | Reference form | Resolved against | -| ------------------ | ----------------------------------------- | ------------------------------------------------- | -| `cdb_gruppe` | `{ group: "" }` (or the bare string) | managed groups | -| `cdb_station` | `{ campus: "" }` | managed campuses, then `GET /campuses` | -| `cdb_gruppentyp` | `{ groupType: "" }` | managed group types, then `GET /group/grouptypes` | -| `cdb_bereich` | `{ department: "" }` | `GET /departments` **only** — see below | -| `cc_securitylevel` | `{ securityLevel: "" }` | `GET /securitylevels` **only** — see below (#110) | +| `scopeField` | Reference form | Resolved against | +| -------------------- | ----------------------------------------- | ------------------------------------------------- | +| `cdb_gruppe` | `{ group: "" }` (or the bare string) | managed groups | +| `cdb_station` | `{ campus: "" }` | managed campuses, then `GET /campuses` | +| `cdb_gruppentyp` | `{ groupType: "" }` | managed group types, then `GET /group/grouptypes` | +| `cdb_bereich` | `{ department: "" }` | `GET /departments` **only** — see below | +| `cc_securitylevel` | `{ securityLevel: "" }` | `GET /securitylevels` **only** — see below (#110) | +| `cdb_comment_viewer` | `{ commentViewer: "" }` | `GET /person/commentviewers` **only** (#102) | **Why this matters:** campus ids are host-specific — Mainz is `0` on eqrm prod and `6` on eqrm dev. A campus-scoped grant written as a numeric literal is @@ -363,18 +364,19 @@ Three things are hard errors at **plan** time, never a guessed `dataId`: - a reference whose dimension does not match the right's `scopeField` — e.g. `{ groupType: … }` on `churchdb:view station` (a `cdb_station` right); - a reference on a dimension that has **no** logical form yet - (`cdb_comment_viewer`, `ccm_data_category`, `oauth_client`, … — values `ct` - has no host-independent way to name today) — the message points at the numeric + (`ccm_data_category`, `oauth_client`, `cc_calcategory`, … — mostly dimensions + of modules outside this tool's mandate) — the message points at the numeric escape hatch; - a **bare string** on a non-group dimension. A string always means "managed group", so on e.g. a `cdb_station` right it would either fail confusingly or — worse — match an unrelated group that happens to carry that key. -### Departments and security levels: referenceable, not declarable +### Catalog dimensions: referenceable, not declarable -`cdb_bereich` (Bereiche) and `cc_securitylevel` (Sicherheitslevel) are the two -dimensions `ct` **reads but does not manage**. They are unmanaged for two -different reasons, and neither reason is "ChurchTools cannot do it": +`cdb_bereich` (Bereiche), `cc_securitylevel` (Sicherheitslevel) and +`cdb_comment_viewer` (Kommentare-Viewer) are the dimensions `ct` **reads but does +not manage**. Each is unmanaged for a different reason, and none of the reasons +is "ChurchTools cannot do it": - **Bereiche** genuinely have no REST write path — live-probed against the instance's own OpenAPI spec (eqrm prod CT 3.135.2, 2026-08-13; re-probed on @@ -390,12 +392,17 @@ different reasons, and neither reason is "ChurchTools cannot do it": registry's "POST to the collection path" contract does not model. Promotion is a deliberate open question on [#110](https://github.com/eqrm/ct-cli/issues/110), not an API limit. - -So a reference of either kind resolves by name on every host — which is what -makes `churchdb:view alldata` ("Personen eines Bereiches sehen") and -`churchdb:+see persons` declarable at all — but there is no `ct.department` / -`ct.securityLevel` resource, no `ct adopt` for them, and a name that matches -nothing is a **hard error** rather than a create: +- **Comment viewers have plain REST CRUD** — `/person/commentviewers` plus + `POST`/`PUT`/`DELETE` on the item path, which would fit the resource registry + unchanged. They are catalog-only purely because nothing has needed to declare + one yet ([#102](https://github.com/eqrm/ct-cli/issues/102)). + +So a reference of any of these kinds resolves by name on every host — which is +what makes `churchdb:view alldata` ("Personen eines Bereiches sehen"), +`churchdb:+see persons` and `churchdb:view comments` declarable at all — but +there is no `ct.department` / `ct.securityLevel` / `ct.commentViewer` resource, +no `ct adopt` for them, and a name that matches nothing is a **hard error** +rather than a create: ``` Cannot resolve department:nope referenced at … : no live department at @@ -405,7 +412,8 @@ them, so it cannot create this one — fix the key/name (list them with numeric id. ``` -Discover the names with `ct get departments` / `ct get security-levels`. Because +Discover the names with `ct get departments`, `ct get security-levels` or +`ct get comment-viewers`. Because the id always comes from the live catalog, such a scope is never "pending" and is never re-resolved at apply time — it is host-correct the moment it resolves. A managed resource that happens to share the key does **not** shadow it (that would @@ -440,9 +448,10 @@ comment-viewer bucket — **not** a group — so `GET /groups/{1,2,3}` 404s. A `scope` array entry may therefore be a plain number instead of a string: (Security levels gained a name-based form in #110 — see -[Departments and security levels](#departments-and-security-levels-referenceable-not-declarable). -Numerics keep working there; `cdb_comment_viewer` and friends still have nothing -but numerics.) +[Catalog dimensions](#catalog-dimensions-referenceable-not-declarable). +Comment viewers gained one too, in #102. Numerics keep working for both; +calendar categories, OAuth clients and the other module dimensions still have +nothing but numerics.) ```ts { right: "churchdb:security level view own data", scope: [1, 2, 3, 5] }, @@ -540,7 +549,7 @@ to be accepted by `ct plan` (the round trip is locked by tests): turn the id into a name. It emits the number plus a `NOTE` giving the portable form (`{ department: "" }` / `{ securityLevel: "" }`) to write by hand. - - **Any other scope dimension** (`cdb_comment_viewer`, `oauth_client`, + - **Any other scope dimension** (`cc_calcategory`, `oauth_client`, …): there is no group to adopt, so the `dataId`(s) are emitted directly as a numeric `scope: [1, 2, 3]` — always an active line, with a comment naming the right's actual scope dimension. `ct adopt group ` is never @@ -696,8 +705,9 @@ routinely has two declarable roles and one blocked one, and group granularity would hide exactly that. A role instance is declarable when every authored grant is either unscoped, uses the `-1` ALL sentinel, scopes by a dimension with a [logical reference form](#scope-resolution) (`cdb_gruppe`, `cdb_station`, -`cdb_gruppentyp`, `cdb_bereich`, `cc_securitylevel`). Anything else scopes by a -dimension `ct` cannot yet name portably, and is named as the blocker. +`cdb_gruppentyp`, `cdb_bereich`, `cc_securitylevel`, `cdb_comment_viewer`). +Anything else scopes by a dimension `ct` cannot yet name portably — in practice +the modules outside its mandate — and is named as the blocker. ## Example diff --git a/src/commands/get.ts b/src/commands/get.ts index 5bc058f..6a2b5f7 100644 --- a/src/commands/get.ts +++ b/src/commands/get.ts @@ -51,6 +51,9 @@ const RESOURCE_PATHS: Record = { // reference resolves against, and how you check whether an instance's ids really are the usual // 1..4 — they are editable master data, not a protocol constant. "security-levels": { path: "/securitylevels" }, + // Comment viewers — the `cdb_comment_viewer` permission scope dimension (#102). The names a + // `scope: [{ commentViewer: "…" }]` reference resolves against. NB `id: 0` ("Alle") is a real row. + "comment-viewers": { path: "/person/commentviewers" }, // PERSON statuses — master data (the enumeration), never person records. The domain a `ct.status` // permission declaration hangs off, and an adoptable resource since #96. statuses: { path: "/statuses" }, diff --git a/src/permissions/adopt.ts b/src/permissions/adopt.ts index d5bf42e..66f46c5 100644 --- a/src/permissions/adopt.ts +++ b/src/permissions/adopt.ts @@ -33,6 +33,7 @@ const SCOPE_SUGAR_FIELD: Readonly> = { "group-type": "groupType", department: "department", "security-level": "securityLevel", + "comment-viewer": "commentViewer", }; interface ReverseEntry { diff --git a/src/permissions/scope.ts b/src/permissions/scope.ts index 133a253..27c4530 100644 --- a/src/permissions/scope.ts +++ b/src/permissions/scope.ts @@ -57,22 +57,28 @@ export const GROUP_SCOPE_FIELD = "cdb_gruppe"; * - `cdb_bereich` → departments (`/departments`) — Bereiche, e.g. `churchdb:view alldata` * ("Personen eines Bereiches sehen") * - `cc_securitylevel` → security levels (`/securitylevels`, #110) — e.g. `churchdb:+see persons` + * - `cdb_comment_viewer` → comment viewers (`/person/commentviewers`, #102) — `churchdb:view comments` * - * `managed` is what separates the last two from the rest. Groups, campuses and group types are + * `managed` is what separates the last three from the rest. Groups, campuses and group types are * MANAGED resource kinds: a scope target can be declared in the same config (resolving pending, then - * re-resolved at apply time) and a state-backed id is worth re-resolving. Departments and security - * levels are catalogs `ct` READS but does not manage, so a reference always resolves by NAME against - * the live catalog and hard-errors when the name is absent — strictly better than a silent numeric - * misgrant, but never a create. They are unmanaged for DIFFERENT reasons, and neither is "CT cannot": + * re-resolved at apply time) and a state-backed id is worth re-resolving. Departments, security levels + * and comment viewers are catalogs `ct` READS but does not manage, so a reference always resolves by + * NAME against the live catalog and hard-errors when the name is absent — strictly better than a + * silent numeric misgrant, but never a create. Each is unmanaged for a DIFFERENT reason, and none of + * them is "ChurchTools cannot": * - `cdb_bereich` has no REST write at all; the admin UI writes it through the legacy master-data * endpoint `ct` does not drive (#108/#109). * - `cc_securitylevel` DOES have REST writes (`/securitylevels/{id}`: POST/PATCH/DELETE, probed * 2026-08-14) — it is unmanaged only because its id-bearing create does not fit the resource * registry's `collectionPath` contract (#110). + * - `cdb_comment_viewer` has conventional REST CRUD that WOULD fit the registry unchanged (#102) — + * it is catalog-only here purely because nothing has needed to declare one yet. * - * The remaining scoped dimensions (`ccm_data_category`, `cdb_comment_viewer`, `oauth_client`, …) stay + * The remaining scoped dimensions (`ccm_data_category`, `oauth_client`, `cc_calcategory`, …) stay * numeric-only because this tool has no way to address their values by a host-independent name today — - * an omission to revisit (#109), not a proof that they are "not resources". + * an omission to revisit, not a proof that they are "not resources". Most of them belong to modules + * outside this tool's mandate (calendars, resources, services, wiki, finance), which is the real + * reason none has a ref kind, and a better thing to say than that they are "not resources at all". */ export const SCOPE_REF_KIND: Readonly> = { [GROUP_SCOPE_FIELD]: { kind: "group", type: "group", managed: true }, @@ -80,6 +86,7 @@ export const SCOPE_REF_KIND: Readonly Ref>> = { groupType: ref.groupType, department: ref.department, securityLevel: ref.securityLevel, + commentViewer: ref.commentViewer, }; /** diff --git a/src/permissions/types.ts b/src/permissions/types.ts index f289d72..f700c81 100644 --- a/src/permissions/types.ts +++ b/src/permissions/types.ts @@ -18,7 +18,8 @@ export type ScopeSugar = | { campus: string } | { groupType: string } | { department: string } - | { securityLevel: string }; + | { securityLevel: string } + | { commentViewer: string }; /** * One entry of a scoped grant's `scope`: diff --git a/src/resolve/refs.ts b/src/resolve/refs.ts index ad52305..e94f781 100644 --- a/src/resolve/refs.ts +++ b/src/resolve/refs.ts @@ -22,6 +22,7 @@ export type RefKind = | "campus" | "department" | "security-level" + | "comment-viewer" | "group-type" | "group-status" | "person-status" @@ -52,6 +53,7 @@ export interface SimpleRef { | "campus" | "department" | "security-level" + | "comment-viewer" | "group-type" | "group-status" | "person-status" @@ -152,6 +154,26 @@ export const ref = { kind: "security-level", key: requireKey("security-level", key), }), + /** + * A COMMENT VIEWER (`/person/commentviewers`), the scope dimension of `cdb_comment_viewer` rights + * such as `churchdb:view comments` — #102. + * + * The dimension existed in the catalog all along but had no logical form, which is what made three + * role instances on `Bereich Pastoral Care` undeclarable: each was blocked by exactly ONE + * comment_viewer-scoped grant, so the whole role instance fell out of `ct coverage` as unmanageable. + * #109 assumed this would need the legacy master-data endpoint; it does not — `/person/commentviewers` + * is conventional REST (`[{id, name, sortKey}]`, plus POST/PUT/DELETE), live-probed on eqrm-dev + * CT 3.135.2, 2026-08-14. + * + * Catalog-only here, like {@link ref.securityLevel}: resolvable by name on any host, not declarable. + * Its REST surface would fit the resource registry unchanged, so promoting it later is a scope + * decision rather than a technical one. + */ + commentViewer: (key: string): SimpleRef => ({ + __ctRef: true, + kind: "comment-viewer", + key: requireKey("comment-viewer", key), + }), groupType: (key: string): SimpleRef => ({ __ctRef: true, kind: "group-type", diff --git a/src/resolve/resolver.ts b/src/resolve/resolver.ts index c2592a4..4c60682 100644 --- a/src/resolve/resolver.ts +++ b/src/resolve/resolver.ts @@ -90,6 +90,10 @@ const CATALOG_PATH: Partial> = { // `/securitylevels/{id}` has POST/PATCH/DELETE, but its create is id-bearing and does not fit the // registry's `collectionPath` contract, so promotion to a managed resource is a separate call (#110). "security-level": "/securitylevels", + // Comment viewers — the `cdb_comment_viewer` scope dimension (#102). `[{id, name, sortKey}]`, + // live-verified on eqrm-dev CT 3.135.2, 2026-08-14. NB `id: 0` is a real row here ("Alle"), which + // is why nothing in the resolve path may treat a falsy id as "missing". + "comment-viewer": "/person/commentviewers", "group-type": "/group/grouptypes", // PERSON statuses — the domain of a `status` permission declaration (#90). Unlike GROUP statuses // (see the note above), these DO have a flat REST catalog: `GET /statuses` returns diff --git a/tests/permission-adopt.test.ts b/tests/permission-adopt.test.ts index 4ac0559..0282d5f 100644 --- a/tests/permission-adopt.test.ts +++ b/tests/permission-adopt.test.ts @@ -457,8 +457,8 @@ describe("emitAdoptedGrants", () => { it("a dimension with no logical form still gets the numeric-escape-hatch note", () => { const rows: RawPermission[] = [ - // cdb_comment_viewer — master data with names, but no ref kind here yet (#109 tracks it). - { authId: 113, dataId: 2, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, + // cc_calcategory — a calendar dimension, outside this tool's mandate, so no ref kind exists. + { authId: 403, dataId: 2, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, ]; const block = emitAdoptedGrants({ domainType: "group_role", domainId: 42, rows, state: emptyState() }); expect(block).toContain("not a group"); diff --git a/tests/permission-scope-refs.test.ts b/tests/permission-scope-refs.test.ts index 1b47c03..2bfc6d9 100644 --- a/tests/permission-scope-refs.test.ts +++ b/tests/permission-scope-refs.test.ts @@ -175,11 +175,12 @@ describe("scope-dimension validation (#98)", () => { key: "p", domainType: "group_role", domainId: 1, - // churchdb:view comments scopes by cdb_comment_viewer, which has no ref kind here yet (#109). - grants: [{ right: "churchdb:view comments", scope: [ref.campus("koblenz")] }], + // churchcal:view category scopes by cc_calcategory — a calendar dimension, outside this tool's + // mandate, so it has no ref kind and the numeric hatch is the only honest answer. + grants: [{ right: "churchcal:view category", scope: [ref.campus("koblenz")] }], }; await expect(resolveScopeRefs([perm], resolverFor(emptyState(HOST)), emptyState(HOST))).rejects.toThrow( - /campus:koblenz.*cdb_comment_viewer.*no logical reference form/s, + /campus:koblenz.*cc_calcategory.*no logical reference form/s, ); }); @@ -373,6 +374,47 @@ describe("security-level scopes resolve by name (cc_securitylevel, #110)", () => }); }); +describe("comment-viewer scopes resolve by name (cdb_comment_viewer, #102)", () => { + // The dimension that made three Pastoral Care role instances undeclarable: each was blocked by + // exactly ONE comment_viewer-scoped grant. #109 assumed this needed the legacy master-data + // endpoint; `/person/commentviewers` is conventional REST, so it is just another catalog. + const viewers = [ + { id: 0, name: "Alle", sortKey: 0 }, + { id: 1, name: "Gemeindeleitung", sortKey: 1 }, + { id: 2, name: "Admins", sortKey: 2 }, + ]; + const client = { + get: vi.fn(async (path: string) => (path === "/person/commentviewers" ? viewers : [])), + } as unknown as CtClient; + + const perm = (scope: unknown[]): DesiredPermission => ({ + key: "p", + domainType: "group_role", + domainId: 1, + grants: [{ right: "churchdb:view comments", scope: scope as never }], + }); + + it("resolves a viewer by name", async () => { + expect( + await tuplesFor(perm([{ commentViewer: "gemeindeleitung" }]), emptyState(HOST), [], client), + ).toEqual([{ authId: 113, dataId: [1], type: "grant" }]); + }); + + it("resolves the id-0 row — a falsy id must not read as 'not found'", async () => { + // "Alle" is id 0 on a real instance. Anything treating 0 as missing would silently drop the scope + // (or worse, fall through to a different row), so this is pinned deliberately. + expect(await tuplesFor(perm([{ commentViewer: "alle" }]), emptyState(HOST), [], client)).toEqual([ + { authId: 113, dataId: [0], type: "grant" }, + ]); + }); + + it("hard-errors on a viewer name this host does not have", async () => { + await expect(tuplesFor(perm([{ commentViewer: "nope" }]), emptyState(HOST), [], client)).rejects.toThrow( + /no live comment-viewer at \/person\/commentviewers matches key "nope"/, + ); + }); +}); + describe("scope object sugar", () => { it('compiles { campus: "koblenz" } to the same Ref as ref.campus(...)', () => { const { ct, permissions } = createContext(); From 08874b94db6dbee0c1e07feb381f711ce1968dc9 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Fri, 14 Aug 2026 09:29:56 +0200 Subject: [PATCH 4/7] feat(resources): security levels are a managed resource with a declared id (#110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-probed on eqrm-dev (CT 3.135.2, 2026-08-14; created id 99 next to levels 1–4, renamed it, deleted it, instance restored): POST /securitylevels/99 {name} → 200 {"id":99,"name":…,"sortkey":99} PATCH /securitylevels/99 {name} → 200 DELETE /securitylevels/99 → 200 Levels 1–4 were untouched throughout and sortkey mirrored the id, so an insert does not implicitly renumber — reordering is opt-in via newid + forcereorder. The interesting part is that the id is CALLER-ASSIGNED: you POST to the id you want and CT 409s if it is taken. Every other managed type lets CT mint the id. That is deliberate on CT's side — a security level id is referenced instance-wide (person fields' securityLevelId, cc_securitylevel grant scopes), so it has to be reproducible rather than whatever a fresh instance auto-increments to. It also resolves #110's original worry from the other end: a config that declares its levels makes the numeric `scope: [1, 2, 3]` form genuinely portable. Engine: two additions to the resource registry, both opt-in and both currently used by exactly one type (pinned by a test). - `createPath(body)` — the POST target for a create, defaulting to collectionPath. Security levels return `/securitylevels/{declared id}`. - `callerAssignedId` — declares that the id comes from the config. The planner refuses a CHANGED id for such a type: that is a renumber, which rewrites what every numeric scope on the dimension grants, and the executor would otherwise emit a plain `PATCH {id}` that CT either ignores or 409s — a plan that never converges. The error names the real operation instead. `id` is a managed field so drift is visible; `ct adopt security-level ` emits it, which is what lets an adopted config recreate the level at the same id on a fresh host. destroyWarning matches person-status: a delete reaches every person field and grant referencing the level. Also makes `security-level` a managed ref kind, so a level declared in the config resolves from state before the catalog and an adopted grant scoped to a managed level now emits `{ securityLevel: "stufe_2_mittel" }` instead of a bare number. Claude-Session: https://claude.ai/code/session_016TacHkv3oVczwXGHBwckq2 --- docs/api-coverage.md | 10 +- docs/handbuch/blueprints.md | 2 +- docs/handbuch/field-definitions.md | 17 +-- docs/handbuch/permissions.md | 101 +++++++++------ docs/runbook-manual-surface.md | 12 +- src/config/context.ts | 20 +++ src/engine/execute.ts | 7 +- src/engine/plan.ts | 29 +++++ src/permissions/scope.ts | 22 ++-- src/resolve/resolver.ts | 17 ++- src/resources/registry.ts | 59 ++++++++- tests/permission-adopt.test.ts | 27 +++- tests/security-level-resource.test.ts | 176 ++++++++++++++++++++++++++ 13 files changed, 417 insertions(+), 82 deletions(-) create mode 100644 tests/security-level-resource.test.ts diff --git a/docs/api-coverage.md b/docs/api-coverage.md index 5c61766..6e2d3dd 100644 --- a/docs/api-coverage.md +++ b/docs/api-coverage.md @@ -85,11 +85,11 @@ API-client libraries (5pm-HDH `churchtools-api` @ CT 3.104, bensteUEM `src/api/schema.d.ts` is git-ignored and was not available offline. Re-verify per the runbook's re-audit procedure once the schema is regenerated. -| Resource | Matched path(s) | GET | POST/PUT/PATCH/DELETE | Verdict | -| ------------------------------------ | ----------------------------------------- | ------------------ | ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Person master-data model | `/person/masterdata` | ✅ (single object) | ❌ | **Read-only.** Versionable master-data model incl. the `securityLevels` enumeration. No write endpoint (edited in the CT master-data admin UI). | -| Data-field definitions (Datenfelder) | `/dbfields`, `/dbfields/{id}` | list ✅ / by-id ✅ | ❌ | **Read-only.** Unified person + group field definitions, discriminated by `fieldCategory`. Mutation only via legacy churchdb AJAX (`db_insert/update/deletefields`), not REST. | -| Security levels | `/securitylevels`, `/securitylevels/{id}` | list ✅ / by-id ✅ | ✅ POST, PATCH, DELETE | **Full CRUD — but not on the usual paths.** Live-probed 2026-08-14 (eqrm-dev, CT 3.135.2): `/securitylevels` is GET-only, and **create is `POST /securitylevels/{id}`** (body `{name}`), update is `PATCH /securitylevels/{id}` (body `{name, newid, forcereorder}` — reordering is first-class), delete is `DELETE /securitylevels/{id}`. Not a managed resource yet: the registry models create as "POST the collection path", which this id-bearing create does not fit (#110). Resolvable by name as a scope ref since #110. The ids are NOT protocol constants — `cc_securitylevel` is an editable master-data table with an auto-increment id. | +| Resource | Matched path(s) | GET | POST/PUT/PATCH/DELETE | Verdict | +| ------------------------------------ | ----------------------------------------- | ------------------ | ---------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Person master-data model | `/person/masterdata` | ✅ (single object) | ❌ | **Read-only.** Versionable master-data model incl. the `securityLevels` enumeration. No write endpoint (edited in the CT master-data admin UI). | +| Data-field definitions (Datenfelder) | `/dbfields`, `/dbfields/{id}` | list ✅ / by-id ✅ | ❌ | **Read-only.** Unified person + group field definitions, discriminated by `fieldCategory`. Mutation only via legacy churchdb AJAX (`db_insert/update/deletefields`), not REST. | +| Security levels | `/securitylevels`, `/securitylevels/{id}` | list ✅ / by-id ✅ | ✅ POST, PATCH, DELETE | **Full CRUD — but not on the usual paths.** Live-probed 2026-08-14 (eqrm-dev, CT 3.135.2): `/securitylevels` is GET-only, and **create is `POST /securitylevels/{id}`** (body `{name}`), update is `PATCH /securitylevels/{id}` (body `{name, newid, forcereorder}` — reordering is first-class), delete is `DELETE /securitylevels/{id}`. **Managed since #110** — the registry gained a `createPath` hook and a `callerAssignedId` flag for it; `ct.securityLevel({ key, id, name })` declares one, `ct adopt security-level ` adopts one. Changing a declared id is refused at plan time (that is a renumber, which rewrites what every numeric `cc_securitylevel` scope means). Resolvable by name as a scope ref since #110. The ids are NOT protocol constants — `cc_securitylevel` is an editable master-data table with an auto-increment id. | See [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md) for the full writability decision, evidence, and the schema/values boundary. diff --git a/docs/handbuch/blueprints.md b/docs/handbuch/blueprints.md index 8e6227e..aa5a439 100644 --- a/docs/handbuch/blueprints.md +++ b/docs/handbuch/blueprints.md @@ -4,7 +4,7 @@ sources: - src/config/context.ts - src/engine/graph.ts - src/engine/hierarchy.ts -sources_hash: 0124644b70452450 +sources_hash: cd5587ca3b619995 reviewed: 2026-08-13 --- diff --git a/docs/handbuch/field-definitions.md b/docs/handbuch/field-definitions.md index 9970df4..aa6920f 100644 --- a/docs/handbuch/field-definitions.md +++ b/docs/handbuch/field-definitions.md @@ -56,8 +56,9 @@ ct get data-fields # all data-field definitions (person + group), one ro is also how you check whether the usual ids `1..4` really hold there — they are editable master-data rows, not protocol constants. **Security levels are the one item on this page that is _not_ read-only in ChurchTools:** - `/securitylevels/{id}` takes `POST`/`PATCH`/`DELETE` (probed 2026-08-14). - `ct` only reads them today — see #110. + `/securitylevels/{id}` takes `POST`/`PATCH`/`DELETE` (probed 2026-08-14), and + since #110 they are a **managed resource** — `ct.securityLevel({ key, id, name })`, + `ct adopt security-level `. See [`permissions.md`](permissions.md). - `ct get data-fields` → `GET /dbfields` (auto-paginated). The **unified** data-field definition catalog. Person master-data fields **and** group custom fields live in the same list, discriminated per-row by `fieldCategory` @@ -116,12 +117,12 @@ re-deciding rather than merely re-reading. ## Endpoint reference -| Purpose | Path | Methods (this CT) | `ct` surface | -| ------------------------------------------ | ------------------------------------------------------------------------------ | ---------------------------------------------- | -------------------------------------------- | -| Person master-data model + security levels | `/person/masterdata` | GET (read-only) | `ct get person-masterdata` | -| Security levels, standalone | `/securitylevels`, `/securitylevels/{id}` | GET; POST/PATCH/DELETE on the item path (#110) | `ct get security-levels` (read only for now) | -| Data-field definitions (person + group) | `/dbfields`, `/dbfields/{id}` | GET (read-only) | `ct get data-fields` | -| Field-definition **mutation** | legacy `churchdb` AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) | non-REST | **not managed — manual** | +| Purpose | Path | Methods (this CT) | `ct` surface | +| ------------------------------------------ | ------------------------------------------------------------------------------ | --------------------------------------- | --------------------------------------------------- | +| Person master-data model + security levels | `/person/masterdata` | GET (read-only) | `ct get person-masterdata` | +| Security levels, standalone | `/securitylevels`, `/securitylevels/{id}` | GET; POST/PATCH/DELETE on the item path | `ct get security-levels`, `ct.securityLevel` (#110) | +| Data-field definitions (person + group) | `/dbfields`, `/dbfields/{id}` | GET (read-only) | `ct get data-fields` | +| Field-definition **mutation** | legacy `churchdb` AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) | non-REST | **not managed — manual** | All paths verified against public CT client libraries + CT Academy docs, **not** against this repo's (git-ignored, ungenerated) `src/api/schema.d.ts`. diff --git a/docs/handbuch/permissions.md b/docs/handbuch/permissions.md index 31d783c..b31ea11 100644 --- a/docs/handbuch/permissions.md +++ b/docs/handbuch/permissions.md @@ -5,7 +5,7 @@ sources: - src/resolve/resolver.ts - src/resolve/refs.ts - src/config/context.ts -sources_hash: 6b8acc0778bf1fd5 +sources_hash: afcc2dc8de2d4ee1 reviewed: 2026-08-13 --- @@ -335,14 +335,14 @@ ct.groupRole({ `{ campus: "koblenz" }` is sugar for `ref.campus("koblenz")` — the same `Ref` the rest of the DSL uses — so both spellings are interchangeable. -| `scopeField` | Reference form | Resolved against | -| -------------------- | ----------------------------------------- | ------------------------------------------------- | -| `cdb_gruppe` | `{ group: "" }` (or the bare string) | managed groups | -| `cdb_station` | `{ campus: "" }` | managed campuses, then `GET /campuses` | -| `cdb_gruppentyp` | `{ groupType: "" }` | managed group types, then `GET /group/grouptypes` | -| `cdb_bereich` | `{ department: "" }` | `GET /departments` **only** — see below | -| `cc_securitylevel` | `{ securityLevel: "" }` | `GET /securitylevels` **only** — see below (#110) | -| `cdb_comment_viewer` | `{ commentViewer: "" }` | `GET /person/commentviewers` **only** (#102) | +| `scopeField` | Reference form | Resolved against | +| -------------------- | ----------------------------------------- | ---------------------------------------------------------- | +| `cdb_gruppe` | `{ group: "" }` (or the bare string) | managed groups | +| `cdb_station` | `{ campus: "" }` | managed campuses, then `GET /campuses` | +| `cdb_gruppentyp` | `{ groupType: "" }` | managed group types, then `GET /group/grouptypes` | +| `cdb_bereich` | `{ department: "" }` | `GET /departments` **only** — see below | +| `cc_securitylevel` | `{ securityLevel: "" }` | managed security levels, then `GET /securitylevels` (#110) | +| `cdb_comment_viewer` | `{ commentViewer: "" }` | `GET /person/commentviewers` **only** (#102) | **Why this matters:** campus ids are host-specific — Mainz is `0` on eqrm prod and `6` on eqrm dev. A campus-scoped grant written as a numeric literal is @@ -373,10 +373,9 @@ Three things are hard errors at **plan** time, never a guessed `dataId`: ### Catalog dimensions: referenceable, not declarable -`cdb_bereich` (Bereiche), `cc_securitylevel` (Sicherheitslevel) and -`cdb_comment_viewer` (Kommentare-Viewer) are the dimensions `ct` **reads but does -not manage**. Each is unmanaged for a different reason, and none of the reasons -is "ChurchTools cannot do it": +`cdb_bereich` (Bereiche) and `cdb_comment_viewer` (Kommentare-Viewer) are the +dimensions `ct` **reads but does not manage**. Neither is unmanaged because +"ChurchTools cannot do it": - **Bereiche** genuinely have no REST write path — live-probed against the instance's own OpenAPI spec (eqrm prod CT 3.135.2, 2026-08-13; re-probed on @@ -385,24 +384,19 @@ is "ChurchTools cannot do it": appears in no OpenAPI spec and which `ct` does not drive ([#108](https://github.com/eqrm/ct-cli/issues/108) / [#109](https://github.com/eqrm/ct-cli/issues/109)). -- **Security levels do have REST writes.** `/securitylevels/{id}` exposes - `POST` (create), `PATCH` (update, including reorder via `newid` + - `forcereorder`) and `DELETE` — live-probed 2026-08-14, CT 3.135.2. They are - unmanaged here only because that create path carries an id, which the resource - registry's "POST to the collection path" contract does not model. Promotion is - a deliberate open question on - [#110](https://github.com/eqrm/ct-cli/issues/110), not an API limit. - **Comment viewers have plain REST CRUD** — `/person/commentviewers` plus `POST`/`PUT`/`DELETE` on the item path, which would fit the resource registry unchanged. They are catalog-only purely because nothing has needed to declare one yet ([#102](https://github.com/eqrm/ct-cli/issues/102)). -So a reference of any of these kinds resolves by name on every host — which is -what makes `churchdb:view alldata` ("Personen eines Bereiches sehen"), -`churchdb:+see persons` and `churchdb:view comments` declarable at all — but -there is no `ct.department` / `ct.securityLevel` / `ct.commentViewer` resource, -no `ct adopt` for them, and a name that matches nothing is a **hard error** -rather than a create: +So a reference of either kind resolves by name on every host — which is what +makes `churchdb:view alldata` ("Personen eines Bereiches sehen") and +`churchdb:view comments` declarable at all — but there is no `ct.department` / +`ct.commentViewer` resource, no `ct adopt` for them, and a name that matches +nothing is a **hard error** rather than a create: + +(Security levels were in this list until #110. They are a managed resource now — +see the next section.) ``` Cannot resolve department:nope referenced at … : no live department at @@ -412,24 +406,53 @@ them, so it cannot create this one — fix the key/name (list them with numeric id. ``` -Discover the names with `ct get departments`, `ct get security-levels` or -`ct get comment-viewers`. Because +Discover the names with `ct get departments` or `ct get comment-viewers`. Because the id always comes from the live catalog, such a scope is never "pending" and is never re-resolved at apply time — it is host-correct the moment it resolves. A managed resource that happens to share the key does **not** shadow it (that would be exactly the misgrant this feature exists to prevent). -**Why security levels got a reference form (#110).** They were previously treated -as _numeric-universal_: `scope: [1, 2, 3]` was considered portable because the ids -"mean the same thing on every instance". They do — by convention. `cc_securitylevel` -is an admin-editable master-data table with an auto-increment id and a `sortkey`, -so someone adding or reordering a level on one host silently changes what a -hard-coded `[1, 2, 3]` grants there, with a green plan. Reordering is not even an -edge case: `PATCH /securitylevels/{id}` takes a `newid` and a `forcereorder` flag, -i.e. CT ships it as a supported operation. Numerics still work -everywhere; the trade-off is that level names are localised German strings -(`"Stufe 3 (Hoch)"` → `stufe_3_hoch`), so a **rename** breaks a reference where a -number would have survived. Pick per config which risk you would rather carry. +### Security levels: a managed resource with a declared id (#110) + +Security levels were treated as _numeric-universal_ for a long time: +`scope: [1, 2, 3]` was considered portable because the ids "mean the same thing +on every instance". They do — **by convention**. `cc_securitylevel` is an +admin-editable table with an auto-increment id and a `sortkey`, and reordering is +not even an edge case: `PATCH /securitylevels/{id}` takes `newid` + +`forcereorder`, i.e. CT ships renumbering as a supported operation. So someone +adding or reordering a level on one host silently changes what a hard-coded +`[1, 2, 3]` grants there, with a green plan. + +Two things address that, and you can use either: + +**1. Reference a level by name.** `{ securityLevel: "stufe_3_hoch" }` resolves +against managed levels first, then `GET /securitylevels`. The trade-off: names +are localised German strings (`"Stufe 3 (Hoch)"` → `stufe_3_hoch`), so a +**rename** breaks a reference where a number would have survived. + +**2. Declare the levels themselves**, which makes the numeric form portable too, +because the config now owns the ids: + +```ts +ct.securityLevel({ key: "stufe_3_hoch", id: 3, name: "Stufe 3 (Hoch)" }); +``` + +`id` is **required** here and it is the only declaration where you choose the id. +ChurchTools creates a level with `POST /securitylevels/{id}` — the client picks +the id, CT 409s if it is taken — precisely so a level is reproducible rather than +whatever a fresh instance auto-increments to. Live-probed on eqrm-dev 2026-08-14: +creating id 99 next to levels 1–4 left 1–4 untouched and set `sortkey` to the id, +so an insert does **not** implicitly renumber. + +Two guard rails: + +- **Changing a declared `id` is refused at plan time.** That is a renumber, not a + field update, and it rewrites what every numeric `cc_securitylevel` scope on the + instance grants. `ct` does not drive `newid`/`forcereorder`; do it in the admin + UI deliberately and update the config to match. +- **`ct destroy` warns.** Deleting a level reaches every person field + (`securityLevelId`) and every grant scoped to it. Same treatment as person + statuses. ### Numeric scope escape hatch (#49) diff --git a/docs/runbook-manual-surface.md b/docs/runbook-manual-surface.md index a3dc29d..034539b 100644 --- a/docs/runbook-manual-surface.md +++ b/docs/runbook-manual-surface.md @@ -50,7 +50,6 @@ in that instance's own config repo, in a runbook following this doc's structure. | ~~Permission catalog lifecycle~~ **(shipped)** | ~~`catalog.json` is a one-off HAR-trace snapshot with no staleness detection~~ — **shipped (#25):** `npm run regenerate:permission-catalog` rewrites it from a live instance (records the CT version in `$meta`), and `ct plan` now warns on a version mismatch or an unknown-authId live grant (which it leaves untouched, never revoking a right it cannot name). See [`docs/handbuch/permissions.md`](handbuch/permissions.md) "Catalog lifecycle & staleness" | [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | No workaround needed — run the command; heed the `ct plan` warnings | | Field definitions & security levels (person + group custom fields) **(read-only, shipped #47/#48)** | The person master-data model, the security-level enumeration, and the data-field DEFINITIONS ("Datenfelder") for persons and groups — structural schema, not per-record values | [#47](https://github.com/eqrm/ct-cli/issues/47), [#48](https://github.com/eqrm/ct-cli/issues/48) (read shipped; write is an API gap — see note) | Read with `ct get person-masterdata` (model + security levels) and `ct get data-fields` (all field definitions, person + group, discriminated by `fieldCategory`). **Mutation stays manual:** field definitions have no REST write endpoint — only the legacy churchdb admin AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) — so create/edit/delete them by hand in the master-data admin UI. Decision + evidence: [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md) | | Bereiche (departments) — creating/renaming one | The Bereich master data — the `cdb_bereich` permission scope dimension | [#108](https://github.com/eqrm/ct-cli/issues/108), [#109](https://github.com/eqrm/ct-cli/issues/109) | **Moved here from "API gap" by #111.** `/departments` has no REST write verb (live-probed, eqrm prod CT 3.135.2, 2026-08-13) — but the admin UI creates Bereiche through `POST /index.php?q=churchdb/ajax` `func=saveMasterData` (captured 2026-08-13), so the old "can never be created" verdict was an artefact of auditing OpenAPI only. Until `ct` drives that endpoint: create/rename in **Master data admin → Bereiche**; `ct get departments` lists them. Grants scoped to one ARE declarable — `scope: [{ department: "" }]` resolves by name per host (#98). Renaming one in CT breaks every config reference to its old name, by design: the reference hard-errors instead of silently regranting elsewhere | -| Security levels — creating/reordering one | The `cc_securitylevel` master data (`GET /securitylevels`) — the scope dimension of `churchdb:+see persons` and friends | [#110](https://github.com/eqrm/ct-cli/issues/110) (ref form done; promotion open) | **REST CRUD exists** — live-probed 2026-08-14 (eqrm-dev, CT 3.135.2): create `POST /securitylevels/{id}`, update/reorder `PATCH /securitylevels/{id}` (`newid`, `forcereorder`), delete `DELETE /securitylevels/{id}`. So this is `ct`'s gap, not CT's: the id-bearing create does not fit the registry's "POST the collection path" contract. Until it is promoted, create/reorder in **Master data admin → Sicherheitslevel**; `ct get security-levels` lists them. Grants scoped to one are declarable by name — `scope: [{ securityLevel: "" }]` (#110) — and numeric `scope: [1, 2, 3]` still works, but check the ids per host before relying on them: they are auto-increment rows, not protocol constants | | API re-audit for new CT releases | CT's OpenAPI spec is self-trimming (only shows endpoints your version has), so a new write endpoint (e.g. a member-status write, or — separately — a first-ever group-status list/write endpoint, #67) appears silently between CT upgrades | tracked by this issue ([#26](https://github.com/eqrm/ct-cli/issues/26)) | Procedure below (**Re-audit procedure for new CT releases**) | ## Out of tool scope — deliberate, not a gap @@ -214,11 +213,14 @@ in where they'd otherwise be silently skipped: declaration, adopt them now with `ct adopt grants ` (paste the emitted block into config) so they don't silently diverge from what `ct plan` believes is desired. -7. **Field definitions & security levels** — read the current schema with +7. **Field definitions** — read the current schema with `ct get person-masterdata` and `ct get data-fields` to confirm the expected - fields, field groups, and security levels exist; create/edit any missing - _definitions_ by hand in the master-data admin UI (no REST write endpoint — - see the field-definitions row above and [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md)). + fields and field groups exist; create/edit any missing _definitions_ by hand in + the master-data admin UI (no REST write endpoint — see the field-definitions row + above and [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md)). + **Security levels are no longer manual** (#110): declare them + (`ct.securityLevel({ key, id, name })`) and step 1's `ct apply` creates them at + the declared ids. 8. **Anything from the "out of tool scope" table** — persons, memberships, per-record field values, calendars, services, resource booking, forms, check-in, wiki, finance, sync, module-level settings, i18n — configure per diff --git a/src/config/context.ts b/src/config/context.ts index e4daf81..90887fc 100644 --- a/src/config/context.ts +++ b/src/config/context.ts @@ -266,6 +266,25 @@ export interface ConfigContext { * Master data, not people — a status is the enumeration, never a person or a membership. */ personStatus(input: ResourceInput): void; + /** + * A SECURITY LEVEL (`/securitylevels` — "Stufe 1 (Niedrig)" … ), #110. The **only** declaration + * whose `id` you choose yourself, and it is REQUIRED: + * + * ```ts + * ct.securityLevel({ key: "stufe_3_hoch", id: 3, name: "Stufe 3 (Hoch)" }); + * ``` + * + * ChurchTools creates a level with `POST /securitylevels/{id}` — the client picks the id and CT + * 409s if it is taken. That is deliberate on CT's side: the id IS the level (person fields carry a + * `securityLevelId`, grants scope by `cc_securitylevel`), so it has to be reproducible rather than + * whatever a fresh instance happens to auto-increment to. Declaring it is what makes a numeric + * `scope: [1, 2, 3]` genuinely portable for a config that owns its levels. + * + * Changing a declared `id` later is refused at plan time: that is a renumber, which rewrites what + * every existing grant on that dimension means. Master data, never people — but note the + * `destroyWarning`: a delete reaches every person field and grant referencing the level. + */ + securityLevel(input: ResourceInput): void; /** Master-data group role (`/group/roles`). Named `roleDefinition` to avoid colliding * with the `groupRole` permission function below. */ roleDefinition(input: ResourceInput): void; @@ -544,6 +563,7 @@ export function createContext(): { targetGroup: define("target-group"), relationshipType: define("relationship-type"), personStatus: define("person-status"), + securityLevel: define("security-level"), roleDefinition: define("group-role"), groupRole: definePermission("group_role"), groupTypeRole: definePermission("group_type_role"), diff --git a/src/engine/execute.ts b/src/engine/execute.ts index 22b2242..2c83dbc 100644 --- a/src/engine/execute.ts +++ b/src/engine/execute.ts @@ -161,8 +161,11 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise("POST", spec.collectionPath, createBody); + // A caller-assigned-id type (#110: security levels) POSTs to the DECLARED id, not the + // collection — `createPath` reads it out of the body. Everything else posts to the collection. + const createTarget = spec.createPath ? spec.createPath(createBody) : spec.collectionPath; + assertNotPeople(createTarget); + const res = await client.request<{ id: number }>("POST", createTarget, createBody); if (typeof res.id !== "number") { throw new Error(`create returned no numeric id (got ${JSON.stringify(res.id)})`); } diff --git a/src/engine/plan.ts b/src/engine/plan.ts index 268f06f..faeed29 100644 --- a/src/engine/plan.ts +++ b/src/engine/plan.ts @@ -13,6 +13,34 @@ import type { State } from "../state/state.js"; import type { DesiredResource, FieldChange, Plan, PlanItem } from "./types.js"; import { orderKeys, isKnownType } from "./graph.js"; +import { RESOURCES } from "../resources/registry.js"; + +/** + * Refuse to plan a changed `id` on a CALLER-ASSIGNED-ID type (#110). + * + * For those types the id is not an opaque handle the tool owns — it is the resource's meaning, and + * it is referenced from all over the instance (a security level id appears on person fields and in + * every `cc_securitylevel` grant scope). ChurchTools models changing one as a RENUMBER — `PATCH` + * with `newid` + `forcereorder` — not as a field update, and doing it silently rewrites what every + * numeric grant on that dimension grants. + * + * The executor would otherwise emit a plain `PATCH {id: }`, which is the wrong shape: CT would + * either ignore it (a plan that never converges) or 409. So this is a hard error naming the real + * operation, and the author performs it deliberately in ChurchTools. + */ +function assertNoRenumber(type: string, key: string, changes: FieldChange[], currentId: number): void { + if (!RESOURCES[type]?.callerAssignedId) return; + const idChange = changes.find((c) => c.field === "id"); + if (!idChange) return; + throw new Error( + `${type} "${key}": cannot change id ${currentId} → ${JSON.stringify(idChange.to)}. For ${type}s ` + + `the id is chosen by the config and referenced across the instance, so changing it is a ` + + `RENUMBER (ChurchTools: PATCH with "newid" + "forcereorder"), not a field update — and it ` + + `silently changes what every numeric scope naming the old id grants. \`ct\` does not drive ` + + `that. Renumber in the ChurchTools admin UI and update the config to match, or declare a ` + + `separate ${type} under a new key.`, + ); +} /** * Structural deep-equal. Order-independent for objects, so a mere key-order @@ -205,6 +233,7 @@ export function computePlan( continue; } const changes = diffFields(d.fields, a); + assertNoRenumber(d.type, d.key, changes, managed.id); const drift = driftFields(managed.fields, a); const driftedFields = new Set(drift.map((c) => c.field)); updates.push({ diff --git a/src/permissions/scope.ts b/src/permissions/scope.ts index 27c4530..d54e2de 100644 --- a/src/permissions/scope.ts +++ b/src/permissions/scope.ts @@ -59,20 +59,16 @@ export const GROUP_SCOPE_FIELD = "cdb_gruppe"; * - `cc_securitylevel` → security levels (`/securitylevels`, #110) — e.g. `churchdb:+see persons` * - `cdb_comment_viewer` → comment viewers (`/person/commentviewers`, #102) — `churchdb:view comments` * - * `managed` is what separates the last three from the rest. Groups, campuses and group types are - * MANAGED resource kinds: a scope target can be declared in the same config (resolving pending, then - * re-resolved at apply time) and a state-backed id is worth re-resolving. Departments, security levels - * and comment viewers are catalogs `ct` READS but does not manage, so a reference always resolves by - * NAME against the live catalog and hard-errors when the name is absent — strictly better than a - * silent numeric misgrant, but never a create. Each is unmanaged for a DIFFERENT reason, and none of - * them is "ChurchTools cannot": + * `managed` separates the MANAGED resource kinds — groups, campuses, group types and (since #110) + * security levels — from the read-only catalogs. For a managed kind a scope target can be declared in + * the same config (resolving pending, then re-resolved at apply time) and a state-backed id is worth + * re-resolving. For a catalog kind the reference always resolves by NAME against the live catalog and + * hard-errors when the name is absent — strictly better than a silent numeric misgrant, but never a + * create. Neither remaining catalog kind is unmanaged because "ChurchTools cannot": * - `cdb_bereich` has no REST write at all; the admin UI writes it through the legacy master-data - * endpoint `ct` does not drive (#108/#109). - * - `cc_securitylevel` DOES have REST writes (`/securitylevels/{id}`: POST/PATCH/DELETE, probed - * 2026-08-14) — it is unmanaged only because its id-bearing create does not fit the resource - * registry's `collectionPath` contract (#110). + * endpoint (#108/#109). * - `cdb_comment_viewer` has conventional REST CRUD that WOULD fit the registry unchanged (#102) — - * it is catalog-only here purely because nothing has needed to declare one yet. + * it is catalog-only purely because nothing has needed to declare one yet. * * The remaining scoped dimensions (`ccm_data_category`, `oauth_client`, `cc_calcategory`, …) stay * numeric-only because this tool has no way to address their values by a host-independent name today — @@ -85,7 +81,7 @@ export const SCOPE_REF_KIND: Readonly> = { // status declared in the same config usable as a permission domain (it resolves to a PendingRef, // which buildPermissionPlan carries as a pending domain, #69). "person-status": "person-status", + // Security levels became a managed resource in #110, so — exactly like person statuses above — a + // `{ securityLevel: "…" }` ref resolves from managed state / this run's declarations FIRST and only + // falls through to the `/securitylevels` catalog for a level this config does not own. That + // ordering is what lets a config declare a level and scope a grant to it in the same run (the ref + // resolves to a PendingRef, carried as a pending scope). + "security-level": "security-level", "role-def": "group-role", group: "group", }; @@ -83,12 +89,11 @@ const CATALOG_PATH: Partial> = { department: "/departments", // Security levels — the `cc_securitylevel` scope dimension (#110). `GET /securitylevels` returns a // flat `[{id, name, sortKey}]` array ("Stufe 1 (Niedrig)" … "Stufe 4 (Sehr hoch)"), live-verified on - // eqrm prod (CT 3.135.2, 2026-08-13) and eqrm-dev (2026-08-14). Read here because the ids are NOT a - // protocol constant — `cc_securitylevel` is an editable master-data table with an auto-increment id - // and a supported reorder (`PATCH …/{id}` with `newid`), so a hard-coded `scope: [1, 2, 3]` is - // portable only by convention. Unlike `department`, catalog-only is OUR limit, not CT's: - // `/securitylevels/{id}` has POST/PATCH/DELETE, but its create is id-bearing and does not fit the - // registry's `collectionPath` contract, so promotion to a managed resource is a separate call (#110). + // eqrm prod (CT 3.135.2, 2026-08-13) and eqrm-dev (2026-08-14). Unlike `department` this kind ALSO + // has a REF_KIND_TYPE entry: levels are a managed resource, so this catalog is the fallback for a + // level the config does not own, not the only source. Reading it by name matters because the ids + // are not a protocol constant — an editable table with an auto-increment id and a supported + // reorder, so a hard-coded `scope: [1, 2, 3]` is portable only by convention. "security-level": "/securitylevels", // Comment viewers — the `cdb_comment_viewer` scope dimension (#102). `[{id, name, sortKey}]`, // live-verified on eqrm-dev CT 3.135.2, 2026-08-14. NB `id: 0` is a real row here ("Alle"), which diff --git a/src/resources/registry.ts b/src/resources/registry.ts index b91a8cf..7b0c7cb 100644 --- a/src/resources/registry.ts +++ b/src/resources/registry.ts @@ -8,8 +8,30 @@ */ export interface AdoptableResource { - /** Collection path: `POST` here creates. */ + /** Collection path: `POST` here creates, unless {@link createPath} overrides the target. */ collectionPath: string; + /** + * POST target for a CREATE, when it is not {@link collectionPath} (#110). + * + * Exists for ChurchTools' one CALLER-ASSIGNED-ID resource: a security level is created with + * `POST /securitylevels/{id}` — the client picks the id, and CT 409s if it is taken. That is the + * opposite of every other type here, where CT mints the id and the tool records the mapping. The + * hook receives the create body (which for such a type carries the declared `id`) and returns the + * path to POST to. Types whose create is a plain collection POST omit it. + */ + createPath?: (body: Record) => string; + /** + * True when the resource's id is chosen by the CONFIG, not minted by ChurchTools (#110). + * + * For these, the id is not an opaque handle but part of the resource's meaning — security level 3 + * IS "Stufe 3", referenced by `securityLevelId` on person fields and by `cc_securitylevel` grant + * scopes across the whole instance. Two consequences the engine has to respect: + * - CREATE must send the declared id (see {@link createPath}), and + * - CHANGING it is a RENUMBER, not a field update. CT models that as `PATCH` with `newid` + + * `forcereorder`, which rewrites what every existing numeric grant on that dimension means. + * The planner refuses it rather than emitting a wrong-shaped PATCH (see `computePlan`). + */ + callerAssignedId?: boolean; /** GET/PUT/PATCH/DELETE path for a single resource by id. */ itemPath: (id: number) => string; /** Update verb: `group` is PATCH; every other type is PUT. */ @@ -243,6 +265,41 @@ export const RESOURCES: Record = { securityLevelId: r.securityLevelId, }), }), + /** + * SECURITY LEVELS (#110) — `cc_securitylevel`, the scope dimension of `churchdb:+see persons` and + * the `securityLevelId` on every person field. The only CALLER-ASSIGNED-ID type here. + * + * Live-probed on eqrm-dev, CT 3.135.2, 2026-08-14 (create → rename → delete, instance restored): + * + * POST /securitylevels/{id} body {name} → 200 {id, name, sortKey}; 409 if taken + * PATCH /securitylevels/{id} body {name[, newid]} ?forcereorder → 200 {id, name} + * DELETE /securitylevels/{id} → 200 + * + * Creating id 99 alongside 1–4 left 1–4 untouched and `sortkey` mirrored the id, so an insert does + * NOT implicitly renumber — reordering is opt-in through `newid` + `forcereorder`, which this tool + * deliberately does not drive (it would silently change what every numeric `scope: [1,2,3]` grant + * means instance-wide). `id` is therefore a MANAGED field: it is part of the declaration, it is sent + * at create, and a changed one is refused by the planner rather than PATCHed into a wrong shape. + * + * Managing `name` only (besides `id`) is safe here where it is not for person-status: PATCH is a + * partial update, not a full replace, so unmanaged siblings are left alone. + */ + "security-level": define({ + collectionPath: "/securitylevels", + // Caller-assigned: POST goes to the DECLARED id, not the collection. CT 409s if it is taken. + createPath: (body) => `/securitylevels/${String(body.id)}`, + callerAssignedId: true, + updateMethod: "PATCH", + // Tier 0: person statuses carry a `securityLevelId`, and grants scope by one, so levels must + // exist before anything that references them. + tier: 0, + destroyWarning: + "deleting a security level reaches every person field and grant scoped to it — the level id " + + "is referenced instance-wide (person-field `securityLevelId`, `cc_securitylevel` grant " + + "scopes). Verify it is unused first (`ct get security-levels`, `ct get data-fields`).", + deriveKey: (r) => slug(str(r, "name")), + managedFields: (r) => ({ id: r.id, name: r.name }), + }), "group-role": define({ collectionPath: "/group/roles", updateMethod: "PUT", diff --git a/tests/permission-adopt.test.ts b/tests/permission-adopt.test.ts index 0282d5f..e1da3e1 100644 --- a/tests/permission-adopt.test.ts +++ b/tests/permission-adopt.test.ts @@ -445,16 +445,39 @@ describe("emitAdoptedGrants", () => { expect(block).toContain('{ right: "churchdb:view alldata", scope: [4] }'); }); - it("a security-level scope gets the portable-form note, not the numeric-escape-hatch one (#110)", () => { + it("points an UNMANAGED security-level scope at `ct adopt security-level` (#110)", () => { + // Security levels are a managed resource now, so the honest advice is the same as for any other + // managed dimension: adopt the level, then re-adopt the grants to get the portable ref form. const rows: RawPermission[] = [ { authId: 125, dataId: 2, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, // cc_securitylevel ]; const block = emitAdoptedGrants({ domainType: "group_role", domainId: 42, rows, state: emptyState() }); - expect(block).toContain('Portable form: { securityLevel: "" }'); + expect(block).toContain("ct adopt security-level 2"); expect(block).not.toContain("not a group"); expect(block).toContain('{ right: "churchdb:security level person", scope: [2] }'); }); + it("emits a MANAGED security-level scope as the portable ref (#110)", () => { + const state: State = { + ...emptyState(), + resources: { + stufe_2_mittel: { + type: "security-level", + id: 2, + key: "stufe_2_mittel", + fields: { id: 2, name: "Stufe 2 (Mittel)" }, + adoptedAt: "t", + updatedAt: "t", + }, + }, + }; + const rows: RawPermission[] = [ + { authId: 125, dataId: 2, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, + ]; + const block = emitAdoptedGrants({ domainType: "group_role", domainId: 42, rows, state }); + expect(block).toContain('scope: [{ securityLevel: "stufe_2_mittel" }]'); + }); + it("a dimension with no logical form still gets the numeric-escape-hatch note", () => { const rows: RawPermission[] = [ // cc_calcategory — a calendar dimension, outside this tool's mandate, so no ref kind exists. diff --git a/tests/security-level-resource.test.ts b/tests/security-level-resource.test.ts new file mode 100644 index 0000000..d1ff14c --- /dev/null +++ b/tests/security-level-resource.test.ts @@ -0,0 +1,176 @@ +/** + * Security levels as a managed resource (#110) — ChurchTools' only CALLER-ASSIGNED-ID type. + * + * Every other managed type lets CT mint the id and records the mapping in state. A security level + * is created with `POST /securitylevels/{id}`: the config picks the id, and CT 409s if it is taken. + * That is not an API quirk to paper over — the id IS the level (person fields carry a + * `securityLevelId`, grants scope by `cc_securitylevel`), so it has to be reproducible across hosts + * rather than whatever a fresh instance auto-increments to. + * + * Live-probed on eqrm-dev, CT 3.135.2, 2026-08-14 (create id 99 → rename → delete; levels 1–4 + * untouched throughout, instance restored): + * POST /securitylevels/99 {name} → 200 {"id":99,"name":"…","sortkey":99,"sortKey":99} + * PATCH /securitylevels/99 {name} → 200 {"id":99,"name":"…"} + * DELETE /securitylevels/99 → 200 + */ +import { describe, it, expect } from "vitest"; +import { executePlan } from "../src/engine/execute.js"; +import { computePlan } from "../src/engine/plan.js"; +import { evaluateConfig } from "../src/config/context.js"; +import { RESOURCES, configSnippet, knownFields } from "../src/resources/registry.js"; +import { emptyState, type State } from "../src/state/state.js"; +import type { Plan, DesiredResource } from "../src/engine/types.js"; + +const noSave: (path: string, state: State) => Promise = async () => {}; +const fixedNow = () => "2026-08-14T00:00:00.000Z"; + +function recorder(responses: Record = {}) { + const calls: { method: string; path: string; body?: unknown }[] = []; + const client = { + request: async (method: string, path: string, body?: unknown): Promise => { + calls.push({ method, path, body }); + return (responses[`${method} ${path}`] ?? {}) as T; + }, + }; + return { client, calls }; +} + +/** State holding security level 3 under key `stufe_3_hoch`, as a prior apply would have left it. */ +function stateWithLevel3(): State { + return { + version: 1, + host: "h", + resources: { + stufe_3_hoch: { + type: "security-level", + id: 3, + key: "stufe_3_hoch", + fields: { id: 3, name: "Stufe 3 (Hoch)" }, + adoptedAt: "t", + updatedAt: "t", + }, + }, + }; +} + +describe("create posts to the DECLARED id, not the collection (#110)", () => { + it("POSTs /securitylevels/3 and records id 3 in state", async () => { + const state = emptyState("h"); + const { client, calls } = recorder({ "POST /securitylevels/3": { id: 3, name: "Stufe 3 (Hoch)" } }); + const plan: Plan = { + items: [ + { + type: "security-level", + key: "stufe_3_hoch", + id: null, + action: "create", + changes: [ + { field: "id", from: undefined, to: 3 }, + { field: "name", from: undefined, to: "Stufe 3 (Hoch)" }, + ], + }, + ], + }; + + const res = await executePlan(plan, { client, state, statePath: "unused", save: noSave, now: fixedNow }); + expect(res.failed).toBeUndefined(); + expect(res.created).toEqual(["stufe_3_hoch"]); + // The whole point: the id is in the PATH. A POST to /securitylevels would 404/405 on a live host. + expect(calls).toEqual([ + { method: "POST", path: "/securitylevels/3", body: { id: 3, name: "Stufe 3 (Hoch)" } }, + ]); + expect(state.resources.stufe_3_hoch).toMatchObject({ type: "security-level", id: 3 }); + }); + + it("leaves every other type posting to its collection path", async () => { + // The createPath hook must not leak into the normal contract — pinned so a future refactor that + // makes it unconditional is caught here rather than on a live instance. + const withCreatePath = Object.entries(RESOURCES) + .filter(([, spec]) => spec.createPath !== undefined) + .map(([type]) => type); + expect(withCreatePath).toEqual(["security-level"]); + const callerAssigned = Object.entries(RESOURCES) + .filter(([, spec]) => spec.callerAssignedId) + .map(([type]) => type); + expect(callerAssigned).toEqual(["security-level"]); + }); +}); + +describe("changing a declared id is refused at plan time (#110)", () => { + const desired = (id: number): DesiredResource[] => [ + { type: "security-level", key: "stufe_3_hoch", fields: { id, name: "Stufe 3 (Hoch)" }, dependsOn: [] }, + ]; + const actual = new Map([["stufe_3_hoch", { id: 3, name: "Stufe 3 (Hoch)" }]]); + + it("throws, naming the renumber semantics rather than emitting a wrong-shaped PATCH", () => { + // CT models this as PATCH with `newid` + `forcereorder`, which rewrites what every numeric + // cc_securitylevel scope on the instance grants. A plain PATCH {id: 4} would either 409 or be + // ignored — a plan that never converges. + expect(() => computePlan(desired(4), stateWithLevel3(), actual)).toThrow( + /cannot change id 3 → 4.*RENUMBER.*forcereorder/s, + ); + }); + + it("is a clean no-op when the declared id matches", () => { + const plan = computePlan(desired(3), stateWithLevel3(), actual); + expect(plan.items.map((i) => i.action)).toEqual(["no-op"]); + }); + + it("does not fire for a normal minted-id type", () => { + // `id` is not a managed field anywhere else, so this guard can never touch campuses/groups — + // but pin it, because the guard reads a field name that exists on every state entry. + const campusState: State = { + version: 1, + host: "h", + resources: { + mainz: { + type: "campus", + id: 0, + key: "mainz", + fields: { name: "Mainz" }, + adoptedAt: "t", + updatedAt: "t", + }, + }, + }; + const plan = computePlan( + [{ type: "campus", key: "mainz", fields: { name: "Mainz 2" }, dependsOn: [] }], + campusState, + new Map([["mainz", { name: "Mainz" }]]), + ); + expect(plan.items.map((i) => i.action)).toEqual(["update"]); + }); +}); + +describe("adopt → config → plan round-trips to a no-op (#110)", () => { + it("emits the id in the snippet, and the re-read config diffs clean", async () => { + // `ct adopt security-level 3` reads the live row through managedFields and emits this snippet. + const live = { id: 3, name: "Stufe 3 (Hoch)", sortkey: 3, sortKey: 3 }; + const fields = RESOURCES["security-level"]!.managedFields(live); + expect(fields).toEqual({ id: 3, name: "Stufe 3 (Hoch)" }); + const snippet = configSnippet("security-level", "stufe_3_hoch", fields); + expect(snippet).toContain("securityLevel("); + // The id MUST survive adoption — without it the re-applied config could not recreate the level + // on a fresh host at the same id, which is the whole reason the type is managed. + expect(snippet).toContain("id: 3"); + + const { resources } = await evaluateConfig((ct) => { + ct.securityLevel({ key: "stufe_3_hoch", id: 3, name: "Stufe 3 (Hoch)" }); + }); + expect(resources[0]?.fields).toEqual({ id: 3, name: "Stufe 3 (Hoch)" }); + const plan = computePlan(resources, stateWithLevel3(), new Map([["stufe_3_hoch", live]])); + expect(plan.items.map((i) => i.action)).toEqual(["no-op"]); + }); + + it("accepts `id` and `name` as known fields — no unknown-field warning", () => { + expect(knownFields("security-level")).toEqual(new Set(["id", "name"])); + }); +}); + +describe("destroy carries the instance-wide warning (#110)", () => { + it("names what a delete reaches, the way person-status does", () => { + const warning = RESOURCES["security-level"]!.destroyWarning; + expect(warning).toMatch(/every person field and grant scoped to it/); + expect(warning).toMatch(/ct get security-levels/); + }); +}); From 0ef18cc2769dfad9abe27bb4cd1fc1cd02402e30 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Fri, 14 Aug 2026 09:41:13 +0200 Subject: [PATCH 5/7] feat(resources): Bereiche are a managed resource, written through the legacy master-data endpoint (#108) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reads stay REST (`GET /departments`); only writes take the legacy `POST /index.php?q=churchdb/ajax` route the admin UI uses. Verified end to end against eqrm-dev (CT 3.135.2, 2026-08-14) with the real CLI — plan, apply, re-plan, apply an update, delete — and the instance was left as found: func=saveMasterData table=cdb_bereich id= → creates (server assigns the id) func=saveMasterData table=cdb_bereich id=N → updates that row func=deleteMasterData table=cdb_bereich id=N → deletes it Before this, a Bereich-scoped grant only planned on a host where somebody had already created the Bereich by hand, so `ct` could not stand up a fresh instance from config — the assumption the whole dev→prod promotion model rests on. Scope: ONE table. Classifying all 24 tables in the registry against dev's own OpenAPI spec found 15 with a REST write path — REST stays authoritative for every one of them, per the standing rule — and of the 9 without, 8 are person master data or field option lists outside this tool's mandate. Bereiche were the only table both in-mandate and REST-less, which is why #109's generic driver is not built: it would have had exactly one in-scope user. Engine, all opt-in and each used by exactly one type (pinned by tests): - `CtClient.ajax()` — the legacy channel, reusing the session cookie and CSRF token the client already holds. A failure there is a 200 with `{"status":"error"}`, so the envelope is checked rather than the status code. - `writer` — create/update/(remove) for a type whose writes are not REST. `create` returns the id itself because saveMasterData does not return one; it re-reads `/departments` and matches the name, refusing to guess when 0 or 2+ rows match rather than binding state to the wrong Bereich permanently. - `fetchOne` — read one row when `GET {itemPath}` does not exist. That last one is a bug the live run caught and no unit test could have: there is no `/departments/{id}` path at all, so the default read 404d, every caller read that as "vanished in ChurchTools", and the second plan proposed creating the same Bereich again — forever. A mock answers whatever path it is asked for. Fixed and pinned. Columns are validated against the instance's own DESCRIBE before posting, because the legacy endpoint does not 400 on an unknown colN — it ignores it, so an unvalidated write looks like a success while silently dropping the field. Bereiche also become a managed ref kind: a `{ department: "…" }` scope now resolves from state first, and can name a Bereich declared in the same run. Claude-Session: https://claude.ai/code/session_016TacHkv3oVczwXGHBwckq2 --- docs/api-coverage.md | 32 +-- docs/handbuch/blueprints.md | 2 +- docs/handbuch/field-definitions.md | 2 +- docs/handbuch/permissions.md | 74 ++++--- docs/runbook-manual-surface.md | 21 +- src/api/ctClient.ts | 67 +++++++ src/api/masterdata.ts | 182 ++++++++++++++++++ src/commands/destroy.ts | 19 +- src/config/context.ts | 15 ++ src/engine/build.ts | 11 +- src/engine/execute.ts | 32 ++- src/permissions/scope.ts | 2 +- src/resolve/resolver.ts | 4 + src/resources/registry.ts | 162 ++++++++++++++++ tests/department-resource.test.ts | 289 ++++++++++++++++++++++++++++ tests/permission-adopt.test.ts | 15 +- tests/permission-scope-refs.test.ts | 6 +- 17 files changed, 863 insertions(+), 72 deletions(-) create mode 100644 src/api/masterdata.ts create mode 100644 tests/department-resource.test.ts diff --git a/docs/api-coverage.md b/docs/api-coverage.md index 6e2d3dd..385fc0a 100644 --- a/docs/api-coverage.md +++ b/docs/api-coverage.md @@ -15,22 +15,22 @@ Analysis of the ChurchTools OpenAPI spec (`openapi.json`, OpenAPI 3.1.0, 487 pat Methods marked only if they actually exist on the matched path. "Update" = PUT or PATCH (noted). Collection paths (list/create) vs item paths (`/{id}`) are separated. -| # | Resource | Matched path(s) | GET (list / by-id) | POST (create) | PUT/PATCH (update) | DELETE | Verdict | -| --- | ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------- | ------------------------------------------- | ----------------------- | ------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 1 | `ct_campus` | `/campuses`, `/campuses/{id}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** | -| 2 | `ct_group_type` | `/group/grouptypes`, `/group/grouptypes/{groupTypeId}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** | -| 3 | `ct_group` | `/groups`, `/groups/{groupId}` | list ✅ / by-id ✅ | ✅ | ✅ PATCH | ✅ | **Full CRUD** | -| 4 | `ct_group_hierarchy` | `/groups/hierarchies` (GET), `/groups/{groupId}/children` (GET), `/groups/{groupId}/parents` (GET), `/groups/{groupId}/parents/{parentGroupId}` (PUT/DELETE) | list ✅ (hierarchies/children/parents) | — (no collection POST) | ✅ PUT links a parent | ✅ unlinks a parent | **Writable** — parent/child edges created & removed via PUT/DELETE on the item path (no POST needed) | -| 5 | `ct_group_role` | `/group/roles`, `/group/roles/{roleId}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** (master-data roles). Per-group assignment lives separately at `/groups/{groupId}/roles` GET + `/groups/{groupId}/roles/{roleId}` PATCH. | -| 6 | `ct_dynamic_group` | `/dynamicgroups` (GET), `/dynamicgroups/{groupId}/ruleset` (GET/PUT/DELETE), `/dynamicgroups/{groupId}/status` (GET/PUT), `/dynamicgroups/refresh` & `/dynamicgroups/{groupId}/refresh` (POST) | list ✅ / ruleset & status by-id ✅ | ⚠️ POST only triggers _refresh_, not create | ✅ PUT ruleset & status | ✅ ruleset DELETE | **Partial** — ruleset is fully updatable/deletable; the group entity itself is created via `/groups` (POST) then given a ruleset. No dedicated create/delete of the dynamic-group record. | -| 7 | `ct_permission` | `/permissions/global` (GET), `/permissions/{domainType}` (GET), `/permissions/{domainType}/{domainId}` (GET/PUT/DELETE); `/permissions/internal/...` (GET) | list ✅ / by-id ✅ | — (no collection POST) | ✅ PUT sets permission | ✅ | **Writable** — assign/revoke via PUT/DELETE on `/{domainType}/{domainId}` | -| 8 | `ct_group_status` | none — see note | ❌ | ❌ | ❌ | ❌ | **No REST endpoint at all → manual, permanently (#67).** `/group/memberstatus` looks like a match but is a DIFFERENT dimension — **member** statuses (`{id: "active", name: "Active"}`, STRING ids, assignable per-membership), not group statuses (`groupStatusId`, numeric, e.g. 1 = active / 4 = archived). Live-verified 2026-07-10 on eqrm prod: `/groups/statuses` parses as `/groups/{groupId}` (400), `/group/statuses` and `/groupstatuses` both 404 — no read OR write endpoint for group statuses exists. (`/statuses` + `/statuses/{id}` DO offer full CRUD, but that is the person/community **Status** master data, tag `Status`, a third, unrelated dimension — do not conflate any of the three.) `groupStatusId` stays a plain numeric field on `ct_group`, authored directly — never resolved by name. | -| 9 | `ct_age_group` | `/group/agegroups`, `/group/agegroups/{ageGroupId}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** | -| 10 | `ct_target_group` | `/group/targetgroups`, `/group/targetgroups/{targetGroupId}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** | -| 11 | `ct_meeting_point` | — none — | ❌ | ❌ | ❌ | ❌ | **Not in API → fully manual.** Zero matches for `treffpunkt`/`meetingpoint`/`meeting point` anywhere in the spec. Closest neighbours are _meeting templates_ (`/group/meetingtemplates`, full CRUD) and _group meetings_ (`/groups/{groupId}/meetings`, CRUD) — different concepts; confirm with product whether "meeting point" was meant to be one of those. | -| 12 | `ct_relationship_type` | `/person/relationshiptypes`, `/person/relationshiptypes/{id}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** | -| 13 | `ct_person_status` | `/statuses`, `/statuses/{id}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD, live-verified 2026-08-13** (eqrm prod, CT 3.135.2, read from the instance OpenAPI spec): `/statuses` → GET, POST; `/statuses/{id}` → GET, PUT, DELETE. `POST` requires `name`, `shorty`, `isMember`; `PUT` requires ALL of `name`, `shorty`, `isMember`, `isSearchable`, `sortKey`, `securityLevelId` — uniquely strict among managed types (every other managed PUT declares no required fields), and since PUT is a full replace the registry manages all six rather than a subset. The person/community **Status** master data (tag `Status`): "0 - First", "3 - Group Active", …, the domain a `ct.status` permission grant hangs off. Adoptable since #96, which is what makes a config using that domain self-sufficient across hosts. Do NOT conflate with `ct_group_status` (row 8, no endpoint at all) or `/group/memberstatus` (member statuses, string ids). Master data — never a person record; the people guard is unaffected. | -| 14 | `ct_department` | `/departments` | list ✅ / by-id ❌ | ❌ | ❌ | ❌ | **No REST write path — a ref catalog here, not a managed resource.** Live-probed 2026-08-13 (eqrm prod, CT 3.135.2): `GET /departments` returns `[{id, name, nameTranslated, sortKey, shorty}]`; no `POST`/`PUT`/`DELETE` on `/departments` exists in the spec, and there is no `/departments/{id}` path at all. Bereiche are the `cdb_bereich` permission scope dimension (`churchdb:view alldata`), so `ct` resolves them BY NAME for a `scope: [{ department: "…" }]` reference (#98) and surfaces them via `ct get departments` — but `ct` cannot declare, adopt or create one today. **Not a ChurchTools limitation:** the admin UI creates Bereiche through the legacy `POST /index.php?q=churchdb/ajax` `func=saveMasterData` interface (captured 2026-08-13), which appears in no OpenAPI spec — see #108/#109, and #111 for why an OpenAPI-only audit could never have found it. | +| # | Resource | Matched path(s) | GET (list / by-id) | POST (create) | PUT/PATCH (update) | DELETE | Verdict | +| --- | ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------- | ------------------------------------------- | ----------------------- | ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `ct_campus` | `/campuses`, `/campuses/{id}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** | +| 2 | `ct_group_type` | `/group/grouptypes`, `/group/grouptypes/{groupTypeId}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** | +| 3 | `ct_group` | `/groups`, `/groups/{groupId}` | list ✅ / by-id ✅ | ✅ | ✅ PATCH | ✅ | **Full CRUD** | +| 4 | `ct_group_hierarchy` | `/groups/hierarchies` (GET), `/groups/{groupId}/children` (GET), `/groups/{groupId}/parents` (GET), `/groups/{groupId}/parents/{parentGroupId}` (PUT/DELETE) | list ✅ (hierarchies/children/parents) | — (no collection POST) | ✅ PUT links a parent | ✅ unlinks a parent | **Writable** — parent/child edges created & removed via PUT/DELETE on the item path (no POST needed) | +| 5 | `ct_group_role` | `/group/roles`, `/group/roles/{roleId}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** (master-data roles). Per-group assignment lives separately at `/groups/{groupId}/roles` GET + `/groups/{groupId}/roles/{roleId}` PATCH. | +| 6 | `ct_dynamic_group` | `/dynamicgroups` (GET), `/dynamicgroups/{groupId}/ruleset` (GET/PUT/DELETE), `/dynamicgroups/{groupId}/status` (GET/PUT), `/dynamicgroups/refresh` & `/dynamicgroups/{groupId}/refresh` (POST) | list ✅ / ruleset & status by-id ✅ | ⚠️ POST only triggers _refresh_, not create | ✅ PUT ruleset & status | ✅ ruleset DELETE | **Partial** — ruleset is fully updatable/deletable; the group entity itself is created via `/groups` (POST) then given a ruleset. No dedicated create/delete of the dynamic-group record. | +| 7 | `ct_permission` | `/permissions/global` (GET), `/permissions/{domainType}` (GET), `/permissions/{domainType}/{domainId}` (GET/PUT/DELETE); `/permissions/internal/...` (GET) | list ✅ / by-id ✅ | — (no collection POST) | ✅ PUT sets permission | ✅ | **Writable** — assign/revoke via PUT/DELETE on `/{domainType}/{domainId}` | +| 8 | `ct_group_status` | none — see note | ❌ | ❌ | ❌ | ❌ | **No REST endpoint at all → manual, permanently (#67).** `/group/memberstatus` looks like a match but is a DIFFERENT dimension — **member** statuses (`{id: "active", name: "Active"}`, STRING ids, assignable per-membership), not group statuses (`groupStatusId`, numeric, e.g. 1 = active / 4 = archived). Live-verified 2026-07-10 on eqrm prod: `/groups/statuses` parses as `/groups/{groupId}` (400), `/group/statuses` and `/groupstatuses` both 404 — no read OR write endpoint for group statuses exists. (`/statuses` + `/statuses/{id}` DO offer full CRUD, but that is the person/community **Status** master data, tag `Status`, a third, unrelated dimension — do not conflate any of the three.) `groupStatusId` stays a plain numeric field on `ct_group`, authored directly — never resolved by name. | +| 9 | `ct_age_group` | `/group/agegroups`, `/group/agegroups/{ageGroupId}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** | +| 10 | `ct_target_group` | `/group/targetgroups`, `/group/targetgroups/{targetGroupId}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** | +| 11 | `ct_meeting_point` | — none — | ❌ | ❌ | ❌ | ❌ | **Not in API → fully manual.** Zero matches for `treffpunkt`/`meetingpoint`/`meeting point` anywhere in the spec. Closest neighbours are _meeting templates_ (`/group/meetingtemplates`, full CRUD) and _group meetings_ (`/groups/{groupId}/meetings`, CRUD) — different concepts; confirm with product whether "meeting point" was meant to be one of those. | +| 12 | `ct_relationship_type` | `/person/relationshiptypes`, `/person/relationshiptypes/{id}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD** | +| 13 | `ct_person_status` | `/statuses`, `/statuses/{id}` | list ✅ / by-id ✅ | ✅ | ✅ PUT | ✅ | **Full CRUD, live-verified 2026-08-13** (eqrm prod, CT 3.135.2, read from the instance OpenAPI spec): `/statuses` → GET, POST; `/statuses/{id}` → GET, PUT, DELETE. `POST` requires `name`, `shorty`, `isMember`; `PUT` requires ALL of `name`, `shorty`, `isMember`, `isSearchable`, `sortKey`, `securityLevelId` — uniquely strict among managed types (every other managed PUT declares no required fields), and since PUT is a full replace the registry manages all six rather than a subset. The person/community **Status** master data (tag `Status`): "0 - First", "3 - Group Active", …, the domain a `ct.status` permission grant hangs off. Adoptable since #96, which is what makes a config using that domain self-sufficient across hosts. Do NOT conflate with `ct_group_status` (row 8, no endpoint at all) or `/group/memberstatus` (member statuses, string ids). Master data — never a person record; the people guard is unaffected. | +| 14 | `ct_department` | `/departments` | list ✅ / by-id ❌ | ❌ | ❌ | ❌ | **No REST write path — a ref catalog here, not a managed resource.** Live-probed 2026-08-13 (eqrm prod, CT 3.135.2): `GET /departments` returns `[{id, name, nameTranslated, sortKey, shorty}]`; no `POST`/`PUT`/`DELETE` on `/departments` exists in the spec, and there is no `/departments/{id}` path at all. Bereiche are the `cdb_bereich` permission scope dimension (`churchdb:view alldata`), so `ct` resolves them BY NAME for a `scope: [{ department: "…" }]` reference (#98) and surfaces them via `ct get departments` — but `ct.department` declares one and `ct adopt department ` adopts one. **Managed since #108** — `ct` writes Bereiche through the legacy `POST /index.php?q=churchdb/ajax` `func=saveMasterData` interface the admin UI uses (create/update/delete all verified live on eqrm-dev 2026-08-14). It appears in no OpenAPI spec, which is why an OpenAPI-only audit could never have found it (#111). Note there is no `/departments/{id}` at all, so a single Bereich is read by filtering the collection. | > **Create-time required fields (#73).** The "POST ✅" marks above were spec-derived, not live-exercised for > create. CT's POST validators require fields the tool does not manage for diffing: `group-type` needs diff --git a/docs/handbuch/blueprints.md b/docs/handbuch/blueprints.md index aa5a439..1dbbe48 100644 --- a/docs/handbuch/blueprints.md +++ b/docs/handbuch/blueprints.md @@ -4,7 +4,7 @@ sources: - src/config/context.ts - src/engine/graph.ts - src/engine/hierarchy.ts -sources_hash: cd5587ca3b619995 +sources_hash: 284e69c26599d21a reviewed: 2026-08-13 --- diff --git a/docs/handbuch/field-definitions.md b/docs/handbuch/field-definitions.md index aa6920f..c52c6bd 100644 --- a/docs/handbuch/field-definitions.md +++ b/docs/handbuch/field-definitions.md @@ -3,7 +3,7 @@ title: Field definitions & security levels sources: - src/commands/get.ts - src/api/ctClient.ts -sources_hash: 4ee434f900165c79 +sources_hash: 047a27a2bd82b120 reviewed: 2026-08-13 --- diff --git a/docs/handbuch/permissions.md b/docs/handbuch/permissions.md index b31ea11..d26d293 100644 --- a/docs/handbuch/permissions.md +++ b/docs/handbuch/permissions.md @@ -5,7 +5,7 @@ sources: - src/resolve/resolver.ts - src/resolve/refs.ts - src/config/context.ts -sources_hash: afcc2dc8de2d4ee1 +sources_hash: e6d4211dd673a741 reviewed: 2026-08-13 --- @@ -340,7 +340,7 @@ the rest of the DSL uses — so both spellings are interchangeable. | `cdb_gruppe` | `{ group: "" }` (or the bare string) | managed groups | | `cdb_station` | `{ campus: "" }` | managed campuses, then `GET /campuses` | | `cdb_gruppentyp` | `{ groupType: "" }` | managed group types, then `GET /group/grouptypes` | -| `cdb_bereich` | `{ department: "" }` | `GET /departments` **only** — see below | +| `cdb_bereich` | `{ department: "" }` | managed Bereiche, then `GET /departments` (#108) | | `cc_securitylevel` | `{ securityLevel: "" }` | managed security levels, then `GET /securitylevels` (#110) | | `cdb_comment_viewer` | `{ commentViewer: "" }` | `GET /person/commentviewers` **only** (#102) | @@ -373,45 +373,67 @@ Three things are hard errors at **plan** time, never a guessed `dataId`: ### Catalog dimensions: referenceable, not declarable -`cdb_bereich` (Bereiche) and `cdb_comment_viewer` (Kommentare-Viewer) are the -dimensions `ct` **reads but does not manage**. Neither is unmanaged because -"ChurchTools cannot do it": - -- **Bereiche** genuinely have no REST write path — live-probed against the - instance's own OpenAPI spec (eqrm prod CT 3.135.2, 2026-08-13; re-probed on - eqrm-dev 2026-08-14): `GET /departments` exists, nothing else on that path - does. The admin UI creates them through the legacy master-data endpoint, which - appears in no OpenAPI spec and which `ct` does not drive - ([#108](https://github.com/eqrm/ct-cli/issues/108) / - [#109](https://github.com/eqrm/ct-cli/issues/109)). +`cdb_comment_viewer` (Kommentare-Viewer) is the one remaining dimension `ct` +**reads but does not manage**, and not because ChurchTools cannot do it: + - **Comment viewers have plain REST CRUD** — `/person/commentviewers` plus `POST`/`PUT`/`DELETE` on the item path, which would fit the resource registry unchanged. They are catalog-only purely because nothing has needed to declare one yet ([#102](https://github.com/eqrm/ct-cli/issues/102)). -So a reference of either kind resolves by name on every host — which is what -makes `churchdb:view alldata` ("Personen eines Bereiches sehen") and -`churchdb:view comments` declarable at all — but there is no `ct.department` / -`ct.commentViewer` resource, no `ct adopt` for them, and a name that matches -nothing is a **hard error** rather than a create: +So the reference resolves by name on every host — which is what makes +`churchdb:view comments` declarable at all — but there is no `ct.commentViewer` +resource, no `ct adopt` for it, and a name that matches nothing is a **hard +error** rather than a create: -(Security levels were in this list until #110. They are a managed resource now — -see the next section.) +(Security levels left this list in #110 and Bereiche in #108. Both are managed +resources now — see below.) ``` -Cannot resolve department:nope referenced at … : no live department at -/departments matches key "nope". ct reads departments but does not manage -them, so it cannot create this one — fix the key/name (list them with -`ct get departments`), create it in the ChurchTools admin UI, or use a -numeric id. +Cannot resolve comment-viewer:nope referenced at … : no managed resource and +no live comment-viewer at /person/commentviewers matches key "nope". +Declare/adopt it, fix the key/name, or use a numeric id. ``` -Discover the names with `ct get departments` or `ct get comment-viewers`. Because +Discover the names with `ct get comment-viewers`. Because the id always comes from the live catalog, such a scope is never "pending" and is never re-resolved at apply time — it is host-correct the moment it resolves. A managed resource that happens to share the key does **not** shadow it (that would be exactly the misgrant this feature exists to prevent). +### Bereiche: managed, but written through a non-REST endpoint (#108) + +Bereiche have **no REST write verb** — live-probed against the instance's own +OpenAPI spec (eqrm prod CT 3.135.2, 2026-08-13; re-probed on eqrm-dev +2026-08-14): `GET /departments` exists, nothing else on that path does. The CT +admin UI creates them through the legacy master-data endpoint +(`POST /index.php?q=churchdb/ajax`, `func=saveMasterData`), which appears in no +OpenAPI spec — which is exactly why an OpenAPI-only audit concluded for months +that a Bereich "can never be created" (#111). + +`ct` drives that endpoint now, for this one table: + +```ts +ct.department({ key: "equippers_koblenz", name: "Equippers Koblenz", shorty: "EQKO" }); +``` + +- **Reads stay REST.** `GET /departments` remains the only read path — cleaner, + paginated, already wired. Only writes take the legacy route. +- **Columns are validated against the instance's own schema.** The registry + describes each table's columns, and a declared field the instance does not + report is a hard error. This matters because the legacy endpoint does not 400 + on an unknown column — it ignores it, so an unvalidated write would look like a + success and silently drop the field. +- **`ct apply` never deletes**, as everywhere else. `ct destroy` can, and warns: + a Bereich delete reaches every person assigned to it and every grant scoped to + it. + +This is the only table `ct` writes through the legacy endpoint. A live +classification of all 24 tables in the registry (eqrm-dev, 2026-08-14) found 15 +with a REST write path — REST stays authoritative for every one of them — and of +the 9 without, 8 are person master data or field option lists outside this tool's +mandate. Bereiche were the only table both in-mandate and REST-less. + ### Security levels: a managed resource with a declared id (#110) Security levels were treated as _numeric-universal_ for a long time: diff --git a/docs/runbook-manual-surface.md b/docs/runbook-manual-surface.md index 034539b..f1e28cf 100644 --- a/docs/runbook-manual-surface.md +++ b/docs/runbook-manual-surface.md @@ -40,17 +40,16 @@ in that instance's own config repo, in a runbook following this doc's structure. ## Not yet implemented — API supports it, `ct` doesn't drive it yet -| Item | What it is | Tracking issue | Manual workaround today | -| --------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Group/group-type field decision table | Fields deliberately left unmanaged (decided out of scope): visibility, note, `autoAccept`/open-for-members, chat status, sort key. The triage **shipped** as a committed decision table ([`docs/group-field-decisions.md`](group-field-decisions.md)) | [#21](https://github.com/eqrm/ct-cli/issues/21) (decided) | Set by hand; these fields are intentionally not diffed — `ct` will neither preserve nor revert them. Promote one later only with its own registry entry + tests | -| Portable/logical references | **Shipped (#20, #25).** Configs reference master data by name/key — `campus`/`groupType` on a group, `ref.campus(...)` in ruleset `var` values, `groupType: ""` for a `group_type_role` domain, and now `group: "", role: ""` for a `group_role` domain (#25) — and the per-host resolver maps each to that instance's id at plan time (managed resources ∪ live catalogs). A same-run campus resolves at apply time. Numeric ids still work as an escape hatch. **`status` (group status) is NOT part of this** (#67) — group statuses have no REST catalog, so `status:` fails fast at eval time; declare the numeric `groupStatusId` directly | [#20](https://github.com/eqrm/ct-cli/issues/20) (done), [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | None needed for the shipped surface. Write logical names; run `ct plan`. The `group_role` pairing-id resolution is verified live (row below) | -| Environments (dev → prod promotion) | Named `(host, token, state file)` profiles and a `--env` flag; today one config + one state file = one host | [#22](https://github.com/eqrm/ct-cli/issues/22) | Point `CT_HOST`/state file manually at each target and re-run; keep dev and prod state files apart yourself, and be careful — nothing stops you from applying a dev-shaped config against prod today | -| Permission `group_role` domain by reference **(shipped, verified live)** | `ct.groupRole({ group, role })` now resolves the (group, role) pair to its pairing domainId at plan time (#25). **Confirmed live 2026-08-13 (CT 3.135.2):** it reads the group's role list (`GET /groups/{groupId}/roles`) and takes the matched role row's `id` as the pairing domainId. Two anchors on different group types: each row's `id` is a live `group_role` domainId carrying that role's grants, while its type-level `groupTypeRoleId` appears nowhere in the domainId set | [#25](https://github.com/eqrm/ct-cli/issues/25) (done, verified) | None needed. Works by reference for managed, already-created groups; numeric `id:` remains a supported escape hatch ([`docs/handbuch/permissions.md`](handbuch/permissions.md) "domainId semantics") | -| ~~Grant adoption~~ **(shipped)** | ~~existing rights structures must be hand-transcribed~~ — **`ct adopt grants ` ships this** (#25): it reads the live rows, applies the planner's normalization, and prints a paste-ready `ct.groupRole` / `ct.groupTypeRole` block (baseline/inherited excluded, denies noted-and-preserved, scope dataIds mapped back to managed-group keys). See [`docs/handbuch/permissions.md`](handbuch/permissions.md) "Adopting existing grants" | [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | No workaround needed — run `ct adopt grants group_role ` (or `group_type_role`), review the `WARNING`/`NOTE` comments, paste into config | -| ~~Permission catalog lifecycle~~ **(shipped)** | ~~`catalog.json` is a one-off HAR-trace snapshot with no staleness detection~~ — **shipped (#25):** `npm run regenerate:permission-catalog` rewrites it from a live instance (records the CT version in `$meta`), and `ct plan` now warns on a version mismatch or an unknown-authId live grant (which it leaves untouched, never revoking a right it cannot name). See [`docs/handbuch/permissions.md`](handbuch/permissions.md) "Catalog lifecycle & staleness" | [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | No workaround needed — run the command; heed the `ct plan` warnings | -| Field definitions & security levels (person + group custom fields) **(read-only, shipped #47/#48)** | The person master-data model, the security-level enumeration, and the data-field DEFINITIONS ("Datenfelder") for persons and groups — structural schema, not per-record values | [#47](https://github.com/eqrm/ct-cli/issues/47), [#48](https://github.com/eqrm/ct-cli/issues/48) (read shipped; write is an API gap — see note) | Read with `ct get person-masterdata` (model + security levels) and `ct get data-fields` (all field definitions, person + group, discriminated by `fieldCategory`). **Mutation stays manual:** field definitions have no REST write endpoint — only the legacy churchdb admin AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) — so create/edit/delete them by hand in the master-data admin UI. Decision + evidence: [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md) | -| Bereiche (departments) — creating/renaming one | The Bereich master data — the `cdb_bereich` permission scope dimension | [#108](https://github.com/eqrm/ct-cli/issues/108), [#109](https://github.com/eqrm/ct-cli/issues/109) | **Moved here from "API gap" by #111.** `/departments` has no REST write verb (live-probed, eqrm prod CT 3.135.2, 2026-08-13) — but the admin UI creates Bereiche through `POST /index.php?q=churchdb/ajax` `func=saveMasterData` (captured 2026-08-13), so the old "can never be created" verdict was an artefact of auditing OpenAPI only. Until `ct` drives that endpoint: create/rename in **Master data admin → Bereiche**; `ct get departments` lists them. Grants scoped to one ARE declarable — `scope: [{ department: "" }]` resolves by name per host (#98). Renaming one in CT breaks every config reference to its old name, by design: the reference hard-errors instead of silently regranting elsewhere | -| API re-audit for new CT releases | CT's OpenAPI spec is self-trimming (only shows endpoints your version has), so a new write endpoint (e.g. a member-status write, or — separately — a first-ever group-status list/write endpoint, #67) appears silently between CT upgrades | tracked by this issue ([#26](https://github.com/eqrm/ct-cli/issues/26)) | Procedure below (**Re-audit procedure for new CT releases**) | +| Item | What it is | Tracking issue | Manual workaround today | +| --------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Group/group-type field decision table | Fields deliberately left unmanaged (decided out of scope): visibility, note, `autoAccept`/open-for-members, chat status, sort key. The triage **shipped** as a committed decision table ([`docs/group-field-decisions.md`](group-field-decisions.md)) | [#21](https://github.com/eqrm/ct-cli/issues/21) (decided) | Set by hand; these fields are intentionally not diffed — `ct` will neither preserve nor revert them. Promote one later only with its own registry entry + tests | +| Portable/logical references | **Shipped (#20, #25).** Configs reference master data by name/key — `campus`/`groupType` on a group, `ref.campus(...)` in ruleset `var` values, `groupType: ""` for a `group_type_role` domain, and now `group: "", role: ""` for a `group_role` domain (#25) — and the per-host resolver maps each to that instance's id at plan time (managed resources ∪ live catalogs). A same-run campus resolves at apply time. Numeric ids still work as an escape hatch. **`status` (group status) is NOT part of this** (#67) — group statuses have no REST catalog, so `status:` fails fast at eval time; declare the numeric `groupStatusId` directly | [#20](https://github.com/eqrm/ct-cli/issues/20) (done), [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | None needed for the shipped surface. Write logical names; run `ct plan`. The `group_role` pairing-id resolution is verified live (row below) | +| Environments (dev → prod promotion) | Named `(host, token, state file)` profiles and a `--env` flag; today one config + one state file = one host | [#22](https://github.com/eqrm/ct-cli/issues/22) | Point `CT_HOST`/state file manually at each target and re-run; keep dev and prod state files apart yourself, and be careful — nothing stops you from applying a dev-shaped config against prod today | +| Permission `group_role` domain by reference **(shipped, verified live)** | `ct.groupRole({ group, role })` now resolves the (group, role) pair to its pairing domainId at plan time (#25). **Confirmed live 2026-08-13 (CT 3.135.2):** it reads the group's role list (`GET /groups/{groupId}/roles`) and takes the matched role row's `id` as the pairing domainId. Two anchors on different group types: each row's `id` is a live `group_role` domainId carrying that role's grants, while its type-level `groupTypeRoleId` appears nowhere in the domainId set | [#25](https://github.com/eqrm/ct-cli/issues/25) (done, verified) | None needed. Works by reference for managed, already-created groups; numeric `id:` remains a supported escape hatch ([`docs/handbuch/permissions.md`](handbuch/permissions.md) "domainId semantics") | +| ~~Grant adoption~~ **(shipped)** | ~~existing rights structures must be hand-transcribed~~ — **`ct adopt grants ` ships this** (#25): it reads the live rows, applies the planner's normalization, and prints a paste-ready `ct.groupRole` / `ct.groupTypeRole` block (baseline/inherited excluded, denies noted-and-preserved, scope dataIds mapped back to managed-group keys). See [`docs/handbuch/permissions.md`](handbuch/permissions.md) "Adopting existing grants" | [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | No workaround needed — run `ct adopt grants group_role ` (or `group_type_role`), review the `WARNING`/`NOTE` comments, paste into config | +| ~~Permission catalog lifecycle~~ **(shipped)** | ~~`catalog.json` is a one-off HAR-trace snapshot with no staleness detection~~ — **shipped (#25):** `npm run regenerate:permission-catalog` rewrites it from a live instance (records the CT version in `$meta`), and `ct plan` now warns on a version mismatch or an unknown-authId live grant (which it leaves untouched, never revoking a right it cannot name). See [`docs/handbuch/permissions.md`](handbuch/permissions.md) "Catalog lifecycle & staleness" | [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | No workaround needed — run the command; heed the `ct plan` warnings | +| Field definitions & security levels (person + group custom fields) **(read-only, shipped #47/#48)** | The person master-data model, the security-level enumeration, and the data-field DEFINITIONS ("Datenfelder") for persons and groups — structural schema, not per-record values | [#47](https://github.com/eqrm/ct-cli/issues/47), [#48](https://github.com/eqrm/ct-cli/issues/48) (read shipped; write is an API gap — see note) | Read with `ct get person-masterdata` (model + security levels) and `ct get data-fields` (all field definitions, person + group, discriminated by `fieldCategory`). **Mutation stays manual:** field definitions have no REST write endpoint — only the legacy churchdb admin AJAX (`db_insertfields`/`db_updatefields`/`db_deletefields`) — so create/edit/delete them by hand in the master-data admin UI. Decision + evidence: [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md) | +| API re-audit for new CT releases | CT's OpenAPI spec is self-trimming (only shows endpoints your version has), so a new write endpoint (e.g. a member-status write, or — separately — a first-ever group-status list/write endpoint, #67) appears silently between CT upgrades | tracked by this issue ([#26](https://github.com/eqrm/ct-cli/issues/26)) | Procedure below (**Re-audit procedure for new CT releases**) | ## Out of tool scope — deliberate, not a gap diff --git a/src/api/ctClient.ts b/src/api/ctClient.ts index 608bc9f..3b1c227 100644 --- a/src/api/ctClient.ts +++ b/src/api/ctClient.ts @@ -157,6 +157,73 @@ export class CtClient { return (envelope.data ?? envelope) as T; } + /** + * Call ChurchTools' LEGACY AJAX interface — `POST /index.php?q=/ajax`, form-encoded, + * `{status, data}` envelope. Not REST, not in any OpenAPI spec (#111), and reached only where CT + * exposes no REST equivalent. + * + * Today that is the master-data registry (`churchdb/ajax`), which is the only write path for + * Bereiche (#108). The permission catalog reads the sibling `churchauth/ajax` through its own + * script rather than this method, because that runs outside an authenticated session. + * + * Reuses the session this client already holds: the login-token handshake set the cookie, and the + * legacy endpoint enforces the SAME `CSRF-Token` header as the REST writes (verified live — without + * it, `churchdb/ajax` answers 401 "CSRF-Token is invalid" rather than redirecting to a login page). + * + * Unlike REST, a failure here is a 200 with `{"status":"error"}`, so the status code alone proves + * nothing — the envelope is checked and a non-success is raised as a {@link CtApiError} carrying + * CT's own message. (An unknown `func` returns exactly that, which is how the delete verb was + * confirmed to exist rather than being silently ignored: `deleteMasterData` succeeded where a + * made-up `delMasterData` came back "was not defined as Function!".) + */ + async ajax(module: string, params: Record): Promise { + if (!this.cookie) { + throw new CtApiError("Not authenticated — run `ct auth login` first", 401, null); + } + if (!this.csrfToken) { + await this.refreshCsrfToken(); + } + const url = `${this.config.host}/index.php?q=${module}/ajax`; + const label = `POST ${module}/ajax ${params.func ?? ""}`.trim(); + // `redirect: "manual"` on purpose: an expired session makes this endpoint bounce through the + // login page, and following that lands on a redirect loop whose eventual error names neither + // the endpoint nor the real cause ("redirect count exceeded"). + const res = await fetchWithRetry( + url, + { + method: "POST", + headers: { + Accept: "application/json", + Cookie: this.cookie, + "CSRF-Token": this.csrfToken ?? "", + "Content-Type": "application/x-www-form-urlencoded", + }, + body: new URLSearchParams(params).toString(), + redirect: "manual", + }, + { isIdempotent: false }, + ); + this.captureCookie(res); + if (!res.ok) { + throw new CtApiError(`${label} failed`, res.status, await safeBody(res)); + } + const text = await res.text(); + let envelope: { status?: string; message?: string; data?: T }; + try { + envelope = JSON.parse(text) as typeof envelope; + } catch { + throw new CtApiError(`${label} returned a non-JSON body`, res.status, text.slice(0, 500)); + } + if (envelope.status !== "success") { + throw new CtApiError( + `${label} returned status "${envelope.status ?? "?"}"${envelope.message ? `: ${envelope.message}` : ""}`, + res.status, + envelope, + ); + } + return envelope.data as T; + } + /** * Fetch every page of a ChurchTools list endpoint and concatenate them, so * callers see the whole collection instead of just CT's default first page diff --git a/src/api/masterdata.ts b/src/api/masterdata.ts new file mode 100644 index 0000000..bc4bbd5 --- /dev/null +++ b/src/api/masterdata.ts @@ -0,0 +1,182 @@ +/** + * ChurchTools' LEGACY master-data registry (#108/#109) — the write path the admin UI uses for tables + * that have no REST write endpoint. + * + * `POST /index.php?q=churchdb/ajax` with `func=getMasterData` returns, among the data itself, + * `data.masterDataTables`: a SELF-DESCRIBING registry of every editable table, keyed by a numeric id, + * each carrying its label, shortname, physical table name and a full `DESCRIBE`-style column list. + * Writes go to the same endpoint: + * + * func=saveMasterData&table=cdb_bereich&id=&col0=bezeichnung&value0=…&col1=kuerzel&value1=… + * → {"status":"success","data":null} (empty `id` creates; a non-empty `id` updates that row) + * func=deleteMasterData&table=cdb_bereich&id=3 + * → {"status":"success","data":null} + * + * All three verbs verified live on eqrm-dev, CT 3.135.2, 2026-08-14: created a Bereich, updated it by + * id, deleted it, and confirmed `GET /departments` reflected each step (instance left as found). + * `deleteMasterData` is real rather than a silently-ignored unknown verb — a made-up `delMasterData` + * against the same endpoint answered `{"status":"error","message":"Function delMasterData was not + * defined as Function!"}`, so the endpoint validates function names. + * + * SCOPE. `ct` drives this registry for exactly ONE table: `cdb_bereich`. A live classification of all + * 24 tables on eqrm-dev (2026-08-14) against the instance's own OpenAPI spec found 15 with a REST + * write path — campuses, group types, person statuses, comment viewers, group categories and more — + * and REST stays authoritative for every one of them. Of the 9 without, 8 are person master data or + * field option lists outside this tool's structural mandate (marital status, nationality, custom + * choice/selection lists, privacy-policy types, meeting days). Bereiche are the only table that is + * both in-mandate and REST-less, which is why #109's generic driver is not built: it would have had + * exactly one in-scope user. + * + * RE-PROBE on a CT major bump, per the procedure in docs/runbook-manual-surface.md — this endpoint is + * undocumented and self-trimming in neither direction. + */ +import type { CtClient } from "./ctClient.js"; + +/** One column of a master-data table, straight out of the instance's `DESCRIBE`. */ +export interface MasterDataColumn { + field: string; + type: string; + null: string; + key: string; + default: string | null; + extra: string; +} + +/** One editable table as the instance describes itself. */ +export interface MasterDataTable { + id: number; + /** Human label, e.g. "Bereich". Localised, and empty for a few internal tables. */ + bezeichnung: string; + /** The registry's own short name, e.g. "dep". */ + shortname: string; + /** The physical table, e.g. "cdb_bereich" — what `saveMasterData` takes as `table`. */ + tablename: string; + /** Column name → its description. */ + desc: Record; +} + +/** The physical table behind Bereiche/departments — the one table this tool writes here (#108). */ +export const DEPARTMENT_TABLE = "cdb_bereich"; + +/** The module whose legacy AJAX endpoint serves the master-data registry. */ +const MASTER_DATA_MODULE = "churchdb"; + +interface RawMasterData { + masterDataTables?: Record; +} + +/** + * Read the instance's own table registry, once per client. + * + * Cached on the client because `getMasterData` returns the whole master-data payload — ~3 MB on a + * real instance — and a plan that writes several Bereiche would otherwise re-fetch it per write. + */ +const registryCache = new WeakMap>>(); + +export function masterDataTables(client: Pick): Promise> { + let p = registryCache.get(client); + if (!p) { + p = client.ajax(MASTER_DATA_MODULE, { func: "getMasterData" }).then((d) => { + const tables = d?.masterDataTables; + if (!tables || typeof tables !== "object") { + throw new Error( + `ChurchTools' master-data registry (getMasterData) returned no "masterDataTables". The ` + + `legacy endpoint is undocumented and may have changed — re-probe it per the re-audit ` + + `procedure in docs/runbook-manual-surface.md before trusting any master-data write.`, + ); + } + return tables; + }); + registryCache.set(client, p); + } + return p; +} + +/** Find one table by its physical name, or throw naming what the instance does report. */ +export async function masterDataTable( + client: Pick, + tablename: string, +): Promise { + const tables = await masterDataTables(client); + const hit = Object.values(tables).find((t) => t.tablename === tablename); + if (!hit) { + const available = Object.values(tables) + .map((t) => t.tablename) + .sort() + .join(", "); + throw new Error( + `ChurchTools' master-data registry does not report a table "${tablename}" on this instance. ` + + `It reports: ${available}.`, + ); + } + return hit; +} + +/** + * Turn a field bag into `colN`/`valueN` pairs, VALIDATED against the instance's own column list. + * + * The validation is the point (#109's acceptance): posting an unknown `colN` to a legacy endpoint is + * not a 400 — it is a silent partial write. So a declared field the instance does not report is a + * hard error naming the columns it does report, and `id` is skipped because it travels as its own + * parameter rather than as a column. + */ +export function masterDataColumns( + table: MasterDataTable, + fields: Record, +): Record { + const params: Record = {}; + let n = 0; + for (const [name, value] of Object.entries(fields)) { + if (name === "id") continue; // the row key, not a column — sent separately + if (!table.desc[name]) { + const known = Object.keys(table.desc).join(", "); + throw new Error( + `Master-data table "${table.tablename}" on this instance has no column "${name}" — it has: ` + + `${known}. Refusing to post an unknown column (the legacy endpoint would accept it silently).`, + ); + } + if (value === undefined || value === null) continue; + params[`col${n}`] = name; + params[`value${n}`] = String(value); + n += 1; + } + if (n === 0) { + throw new Error(`Nothing to write to "${table.tablename}": no non-empty managed columns.`); + } + return params; +} + +/** + * Create or update one master-data row. An empty `id` creates; a numeric one updates that row. + * + * Returns nothing useful on create — CT answers `{"status":"success","data":null}` with no id — so + * callers that need the new id must re-read the resource's REST catalog afterward. That is why the + * department writer below matches on the name it just wrote. + */ +export async function saveMasterData( + client: Pick, + tablename: string, + fields: Record, + id?: number, +): Promise { + const table = await masterDataTable(client, tablename); + await client.ajax(MASTER_DATA_MODULE, { + func: "saveMasterData", + table: tablename, + id: id === undefined ? "" : String(id), + ...masterDataColumns(table, fields), + }); +} + +/** + * Delete one master-data row. Verified to exist (see the header), but `ct apply` never deletes — + * this is reached only from `ct destroy`, behind its own typed confirmation. + */ +export async function deleteMasterData( + client: Pick, + tablename: string, + id: number, +): Promise { + await masterDataTable(client, tablename); // validate the table exists before asking CT to drop a row + await client.ajax(MASTER_DATA_MODULE, { func: "deleteMasterData", table: tablename, id: String(id) }); +} diff --git a/src/commands/destroy.ts b/src/commands/destroy.ts index dc354c8..6f68659 100644 --- a/src/commands/destroy.ts +++ b/src/commands/destroy.ts @@ -4,7 +4,7 @@ import { CtApiError, type CtClient } from "../api/ctClient.js"; import { resolveConfig } from "../config.js"; import { prepareEnv } from "../env/context.js"; import { loadState, saveState, type State } from "../state/state.js"; -import { RESOURCES } from "../resources/registry.js"; +import { RESOURCES, type CtWriteClient } from "../resources/registry.js"; import { assertNotPeople } from "../engine/guard.js"; import { orderKeys } from "../engine/graph.js"; import { fetchActual } from "../engine/build.js"; @@ -240,7 +240,22 @@ export async function runDeleteLoop(ctx: DeleteLoopCtx): Promise { const path = spec.itemPath(managed.id); assertNotPeople(path); try { - await client.request("DELETE", path); + if (spec.writer) { + // A type whose writes are not REST (#108: Bereiche). Without a `remove` it has no delete path + // at all — say so instead of issuing a DELETE the endpoint does not implement, which would + // 404/405 and read as "already deleted". + if (!spec.writer.remove) { + error( + `${managed.type}.${key} (#${managed.id}) cannot be deleted by \`ct\` — ChurchTools exposes ` + + `no delete for this type. Remove it in the ChurchTools admin UI, then re-run to drop it ` + + `from state.`, + ); + continue; + } + await spec.writer.remove({ client: client as CtWriteClient, id: managed.id }); + } else { + await client.request("DELETE", path); + } } catch (err) { if (err instanceof CtApiError && err.status === 404) { delete state.resources[key]; diff --git a/src/config/context.ts b/src/config/context.ts index 90887fc..e0b3c45 100644 --- a/src/config/context.ts +++ b/src/config/context.ts @@ -285,6 +285,20 @@ export interface ConfigContext { * `destroyWarning`: a delete reaches every person field and grant referencing the level. */ securityLevel(input: ResourceInput): void; + /** + * A BEREICH / department (`/departments`), #108 — the `cdb_bereich` scope dimension of + * `churchdb:view alldata` ("Personen eines Bereiches sehen"). + * + * ```ts + * ct.department({ key: "equippers_koblenz", name: "Equippers Koblenz", shorty: "EQKO" }); + * ``` + * + * The one managed type ChurchTools has no REST write for: reads come from `GET /departments`, + * writes go through the legacy master-data endpoint the admin UI uses. Declaring Bereiche is what + * lets a config stand up a fresh host — before this, a Bereich-scoped grant only planned where + * someone had already created the Bereich by hand. + */ + department(input: ResourceInput): void; /** Master-data group role (`/group/roles`). Named `roleDefinition` to avoid colliding * with the `groupRole` permission function below. */ roleDefinition(input: ResourceInput): void; @@ -564,6 +578,7 @@ export function createContext(): { relationshipType: define("relationship-type"), personStatus: define("person-status"), securityLevel: define("security-level"), + department: define("department"), roleDefinition: define("group-role"), groupRole: definePermission("group_role"), groupTypeRole: definePermission("group_type_role"), diff --git a/src/engine/build.ts b/src/engine/build.ts index aa49ff7..381cc11 100644 --- a/src/engine/build.ts +++ b/src/engine/build.ts @@ -9,7 +9,7 @@ import type { CtClient } from "../api/ctClient.js"; import { CtApiError } from "../api/ctClient.js"; import type { ManagedResource, State } from "../state/state.js"; import type { DesiredResource, Plan } from "./types.js"; -import { RESOURCES } from "../resources/registry.js"; +import { RESOURCES, type CtWriteClient } from "../resources/registry.js"; import { computePlan } from "./plan.js"; import { foldSynthetic } from "./synthetic.js"; import { Resolver } from "../resolve/resolver.js"; @@ -65,7 +65,14 @@ export async function fetchActual( return; } try { - const raw = await client.get>(spec.itemPath(managed.id)); + // A type with no `GET {itemPath}` (#108: Bereiche have only the collection) reads through its + // own `fetchOne`, which returns null for "genuinely absent" — the same meaning a 404 carries + // below. Without this the default read 404s on every plan and the resource looks vanished, so + // apply proposes creating it again forever. + const raw = spec.fetchOne + ? await spec.fetchOne(client as CtWriteClient, managed.id) + : await client.get>(spec.itemPath(managed.id)); + if (raw === null) return; // absent in CT — same handling as a 404 actual.set(managed.key, spec.managedFields(raw)); } catch (err) { if (err instanceof CtApiError && err.status === 404) { diff --git a/src/engine/execute.ts b/src/engine/execute.ts index 2c83dbc..b0054c9 100644 --- a/src/engine/execute.ts +++ b/src/engine/execute.ts @@ -13,7 +13,7 @@ import { CtApiError } from "../api/ctClient.js"; import type { ManagedResource, State } from "../state/state.js"; import { upsert, saveState } from "../state/state.js"; import type { FieldChange, Plan, PlanItem } from "./types.js"; -import { RESOURCES } from "../resources/registry.js"; +import { RESOURCES, type CtWriteClient } from "../resources/registry.js"; import { assertNotPeople } from "./guard.js"; import { isSyntheticField, syntheticField } from "./synthetic.js"; import { reresolvePendingValue } from "../resolve/resolver.js"; @@ -165,10 +165,19 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise("POST", createTarget, createBody); - if (typeof res.id !== "number") { - throw new Error(`create returned no numeric id (got ${JSON.stringify(res.id)})`); + // A type whose writes are not REST (#108: Bereiche, via the legacy master-data endpoint) + // supplies its own writer and returns the new id itself. Everything else POSTs. + let newId: number; + if (spec.writer) { + newId = await spec.writer.create({ client: client as CtWriteClient, body: createBody }); + } else { + const res = await client.request<{ id: number }>("POST", createTarget, createBody); + newId = res.id; } + if (typeof newId !== "number") { + throw new Error(`create returned no numeric id (got ${JSON.stringify(newId)})`); + } + const res = { id: newId }; // A "recreate" create item (resource vanished from CT but still in state) leaves the // stale entry — with the *old* id — under this key. upsert would read the new id as a // key collision and throw, so the just-created resource never lands in state and every @@ -192,12 +201,19 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise !isSyntheticField(c.field)); if (hasFieldChange) { - // PATCH resources take only the changed fields (unchanged/drifted siblings are left alone); - // PUT resources replace the whole object, so send actual ∪ changes to preserve those siblings. - const body = spec.updateMethod === "PATCH" ? snapshotFromChanges({}, changes) : snapshot; const path = spec.itemPath(id); assertNotPeople(path); - await client.request(spec.updateMethod, path, body); + if (spec.writer) { + // Non-REST write path (#108). Always handed the FULL snapshot, never just the changed + // fields: the legacy master-data endpoint writes the columns it is given and is closer to + // a PUT than a PATCH, so sending a subset would blank the untouched ones. + await spec.writer.update({ client: client as CtWriteClient, body: snapshot, id }); + } else { + // PATCH resources take only the changed fields (unchanged/drifted siblings are left alone); + // PUT resources replace the whole object, so send actual ∪ changes to preserve those siblings. + const body = spec.updateMethod === "PATCH" ? snapshotFromChanges({}, changes) : snapshot; + await client.request(spec.updateMethod, path, body); + } } upsert(state, { type: item.type, id, key: item.key, fields: snapshot }, now()); mirrorPreventDestroy(state.resources[item.key]!, item.preventDestroy); diff --git a/src/permissions/scope.ts b/src/permissions/scope.ts index d54e2de..e5142ef 100644 --- a/src/permissions/scope.ts +++ b/src/permissions/scope.ts @@ -80,7 +80,7 @@ export const SCOPE_REF_KIND: Readonly> = { // status declared in the same config usable as a permission domain (it resolves to a PendingRef, // which buildPermissionPlan carries as a pending domain, #69). "person-status": "person-status", + // Bereiche became a managed resource in #108 (writes go through the legacy master-data endpoint, + // reads stay `GET /departments`), so a `{ department: "…" }` ref resolves from managed state first + // and only falls back to the catalog for a Bereich this config does not own. + department: "department", // Security levels became a managed resource in #110, so — exactly like person statuses above — a // `{ securityLevel: "…" }` ref resolves from managed state / this run's declarations FIRST and only // falls through to the `/securitylevels` catalog for a level this config does not own. That diff --git a/src/resources/registry.ts b/src/resources/registry.ts index 7b0c7cb..5d26e52 100644 --- a/src/resources/registry.ts +++ b/src/resources/registry.ts @@ -7,6 +7,26 @@ * matrix (docs/api-coverage.md). Adding a type = adding an entry here. */ +/** What a custom {@link AdoptableResource.writer} is handed to perform one write. */ +export interface ResourceWriteContext { + /** The full authenticated client — REST plus the legacy `ajax` channel. */ + client: CtWriteClient; + /** The managed field bag to write (create: the declared fields; update: actual ∪ changes). */ + body: Record; +} + +/** + * The client surface a custom writer may use. Deliberately a structural subset rather than the whole + * {@link CtClient}, so the engine's test doubles stay small and a writer cannot reach for anything + * the executor does not already guarantee. + */ +export interface CtWriteClient { + request(method: string, path: string, body?: unknown): Promise; + ajax?(module: string, params: Record): Promise; + getAll?(path: string, options?: { limit?: number }): Promise<{ data: T[] }>; + get?(path: string): Promise; +} + export interface AdoptableResource { /** Collection path: `POST` here creates, unless {@link createPath} overrides the target. */ collectionPath: string; @@ -20,6 +40,18 @@ export interface AdoptableResource { * path to POST to. Types whose create is a plain collection POST omit it. */ createPath?: (body: Record) => string; + /** + * Read ONE resource by id when `GET {itemPath}` does not exist (#108). + * + * Bereiche have no `/departments/{id}` path at all — only the collection — so the default read + * 404s, which every caller correctly interprets as "vanished in ChurchTools". The visible symptom + * is a plan that proposes creating the same Bereich on every run: apply creates it, the next plan + * cannot read it back, and proposes creating it again. Caught by a live dev apply, not by any unit + * test, because a mock happily answers whatever path it is asked for. + * + * Returning `null` means genuinely absent (the caller treats it as a 404). + */ + fetchOne?: (client: CtWriteClient, id: number) => Promise | null>; /** * True when the resource's id is chosen by the CONFIG, not minted by ChurchTools (#110). * @@ -32,6 +64,29 @@ export interface AdoptableResource { * The planner refuses it rather than emitting a wrong-shaped PATCH (see `computePlan`). */ callerAssignedId?: boolean; + /** + * Override the WRITE path for a type whose reads are REST but whose writes are not (#108). + * + * Exactly one type needs this: Bereiche/departments. `GET /departments` is a normal REST catalog, + * but no REST write verb exists for it on any probed version — the admin UI writes Bereiche through + * the legacy `saveMasterData` endpoint instead. Rather than teach the executor about that endpoint, + * the type supplies its own create/update, and everything else in the engine (planning, diffing, + * state, tiers, `preventDestroy`) is unchanged. + * + * `create` returns the new id, because the legacy endpoint does not: it answers + * `{"status":"success","data":null}`, so the implementation re-reads the REST catalog to find the + * row it just wrote. Reads therefore stay REST-only, which is what #108 asked for. + */ + writer?: { + create(ctx: ResourceWriteContext): Promise; + update(ctx: ResourceWriteContext & { id: number }): Promise; + /** + * `ct destroy`'s delete. Optional and separate from the two above because `ct apply` NEVER + * deletes — a type can be fully applyable without one, and omitting it makes `ct destroy` say so + * rather than issue a REST DELETE the endpoint does not have. + */ + remove?(ctx: { client: CtWriteClient; id: number }): Promise; + }; /** GET/PUT/PATCH/DELETE path for a single resource by id. */ itemPath: (id: number) => string; /** Update verb: `group` is PATCH; every other type is PUT. */ @@ -132,6 +187,78 @@ export function fromInformation(resource: Record, key: string): return information[key] ?? resource[key]; } +/** + * Bereich writes, through the legacy master-data endpoint (#108). Reads stay REST — including the + * read this create performs to learn the new id, because `saveMasterData` does not return one. + * + * Column mapping: the config and `ct get departments` speak the REST names (`name`, `shorty`, + * `sortKey`); the legacy table speaks `bezeichnung`, `kuerzel`, `sortkey`. Mapping here keeps that + * detail out of the config surface, and `masterDataColumns` validates the result against the + * instance's OWN column list, so a CT rename surfaces as a named error rather than a silent + * partial write. + */ +const DEPARTMENT_COLUMNS: Record = { + name: "bezeichnung", + shorty: "kuerzel", + sortKey: "sortkey", +}; + +function departmentRow(body: Record): Record { + const row: Record = {}; + for (const [restName, column] of Object.entries(DEPARTMENT_COLUMNS)) { + if (body[restName] !== undefined) row[column] = body[restName]; + } + return row; +} + +/** Read the whole `/departments` collection — the only read path CT offers for Bereiche. */ +async function departmentRows(client: CtWriteClient): Promise<{ id: number; name?: string }[]> { + if (client.getAll) return (await client.getAll<{ id: number; name?: string }>("/departments")).data; + return (await client.get?.<{ id: number; name?: string }[]>("/departments")) ?? []; +} + +function requireAjax(client: CtWriteClient): Required> { + if (!client.ajax) { + throw new Error( + "Departments are written through ChurchTools' legacy master-data endpoint, which needs the " + + "full authenticated client (`ajax`). This client cannot reach it.", + ); + } + return { ajax: client.ajax.bind(client) }; +} + +const departmentWriter: AdoptableResource["writer"] = { + async create({ client, body }): Promise { + const { saveMasterData, DEPARTMENT_TABLE } = await import("../api/masterdata.js"); + await saveMasterData(requireAjax(client), DEPARTMENT_TABLE, departmentRow(body)); + // `saveMasterData` answers {"status":"success","data":null} — no id. Re-read the REST catalog and + // match the name just written. A duplicate name would be ambiguous, so that is refused rather + // than guessed: picking either row would bind state to the wrong Bereich for good. + const name = body.name; + const rows = await departmentRows(client); + const matches = rows.filter((r) => r.name === name); + if (matches.length !== 1) { + throw new Error( + `Created the Bereich "${String(name)}", but ${matches.length === 0 ? "it did not appear in" : `${matches.length} rows match it in`} ` + + `GET /departments — cannot record its id. The legacy master-data endpoint returns no id, so ` + + `the new row is identified by name. Resolve the duplicate/absence in ChurchTools and re-run.`, + ); + } + return matches[0]!.id; + }, + async update({ client, body, id }): Promise { + const { saveMasterData, DEPARTMENT_TABLE } = await import("../api/masterdata.js"); + // A non-empty `id` makes the same call an UPDATE of that row (verified live, eqrm-dev 2026-08-14). + await saveMasterData(requireAjax(client), DEPARTMENT_TABLE, departmentRow(body), id); + }, + async remove({ client, id }): Promise { + const { deleteMasterData, DEPARTMENT_TABLE } = await import("../api/masterdata.js"); + // `deleteMasterData` is a real verb, not a silently-ignored unknown one: a made-up `delMasterData` + // against the same endpoint answers "was not defined as Function!" (verified, eqrm-dev 2026-08-14). + await deleteMasterData(requireAjax(client), DEPARTMENT_TABLE, id); + }, +}; + export const RESOURCES: Record = { campus: define({ collectionPath: "/campuses", @@ -265,6 +392,41 @@ export const RESOURCES: Record = { securityLevelId: r.securityLevelId, }), }), + /** + * BEREICHE / DEPARTMENTS (#108) — `cdb_bereich`, the scope dimension of `churchdb:view alldata` + * ("Personen eines Bereiches sehen"). The one managed type whose READS are REST and whose WRITES + * are not. + * + * `GET /departments` is a normal catalog; no REST write verb exists for it (live-probed on eqrm + * prod CT 3.135.2 2026-08-13 and re-probed on eqrm-dev 2026-08-14 — no POST/PUT/DELETE, and no + * `/departments/{id}` path at all). The admin UI creates Bereiche through the legacy master-data + * endpoint, which is what {@link writer} drives. That asymmetry is the whole of #108: without it a + * Bereich-scoped grant only planned on hosts where somebody had already made the Bereich by hand, + * so a config could not stand up a fresh instance — the promotion model's core assumption. + * + * Managed columns are the three the instance's own registry reports as writable for `cdb_bereich` + * (`bezeichnung`, `kuerzel`, `sortkey`), surfaced under their REST names so config and `ct get + * departments` agree: `name`, `shorty`, `sortKey`. The writer maps them back. + */ + department: define({ + collectionPath: "/departments", + // Never used for writes — `writer` handles those — but the executor's update path still reads it + // for `assertNotPeople`, and `ct adopt department ` reads the row through `itemPath`. + updateMethod: "PUT", + // Tier 0: grants scope by Bereich, and permissions apply after every resource tier. + tier: 0, + destroyWarning: + "deleting a Bereich reaches every person assigned to it and every grant scoped to it " + + "(`churchdb:view alldata`). ChurchTools offers no REST delete for Bereiche at all — this goes " + + "through the legacy master-data endpoint. Verify it is unused first (`ct get departments`).", + deriveKey: (r) => slug(str(r, "name")), + managedFields: (r) => ({ name: r.name, shorty: r.shorty, sortKey: r.sortKey ?? 0 }), + // There is no `GET /departments/{id}` — filter the collection instead. Without this every plan + // after a create would read a 404 and propose creating the same Bereich again. + fetchOne: async (client, id) => + ((await departmentRows(client)) as Record[]).find((r) => r.id === id) ?? null, + writer: departmentWriter, + }), /** * SECURITY LEVELS (#110) — `cc_securitylevel`, the scope dimension of `churchdb:+see persons` and * the `securityLevelId` on every person field. The only CALLER-ASSIGNED-ID type here. diff --git a/tests/department-resource.test.ts b/tests/department-resource.test.ts new file mode 100644 index 0000000..f8031a4 --- /dev/null +++ b/tests/department-resource.test.ts @@ -0,0 +1,289 @@ +/** + * Bereiche/departments as a managed resource (#108) — the one type whose READS are REST and whose + * WRITES are not. + * + * `GET /departments` is a normal catalog; no REST write verb exists for it. The admin UI creates a + * Bereich through the legacy master-data endpoint, and so does `ct` now. Verified live on eqrm-dev, + * CT 3.135.2, 2026-08-14 (create → update-by-id → delete, `GET /departments` reflecting each step, + * instance left as found): + * + * func=saveMasterData&table=cdb_bereich&id=&col0=bezeichnung&value0=… → {"status":"success","data":null} + * func=saveMasterData&table=cdb_bereich&id=3&col0=bezeichnung&value0=… → {"status":"success","data":null} + * func=deleteMasterData&table=cdb_bereich&id=3 → {"status":"success","data":null} + * + * Why it matters: before this, a Bereich-scoped grant only planned on a host where somebody had + * already created the Bereich by hand, so `ct` could not stand up a fresh instance from config — the + * assumption the whole dev→prod promotion model rests on. + */ +import { describe, it, expect } from "vitest"; +import { executePlan } from "../src/engine/execute.js"; +import { fetchActual } from "../src/engine/build.js"; +import { RESOURCES } from "../src/resources/registry.js"; +import { masterDataColumns, masterDataTable, DEPARTMENT_TABLE } from "../src/api/masterdata.js"; +import { emptyState, type State } from "../src/state/state.js"; +import type { Plan } from "../src/engine/types.js"; + +const noSave: (path: string, state: State) => Promise = async () => {}; +const fixedNow = () => "2026-08-14T00:00:00.000Z"; + +/** The `cdb_bereich` descriptor exactly as eqrm-dev reports it (CT 3.135.2, 2026-08-14). */ +const BEREICH_TABLE = { + id: 3, + bezeichnung: "Bereich", + shortname: "dep", + tablename: "cdb_bereich", + desc: { + id: { field: "id", type: "int(11)", null: "NO", key: "PRI", default: null, extra: "auto_increment" }, + bezeichnung: { field: "bezeichnung", type: "varchar(50)", null: "NO", key: "", default: null, extra: "" }, + kuerzel: { field: "kuerzel", type: "varchar(10)", null: "NO", key: "", default: null, extra: "" }, + sortkey: { field: "sortkey", type: "int(11)", null: "NO", key: "", default: "0", extra: "" }, + }, +}; + +/** A client exposing REST `request`/`getAll` plus the legacy `ajax` channel, recording both. */ +function recorder(departments: { id: number; name: string; shorty?: string; sortKey?: number }[] = []) { + const ajaxCalls: { module: string; params: Record }[] = []; + const restCalls: { method: string; path: string; body?: unknown }[] = []; + const client = { + request: async (method: string, path: string, body?: unknown): Promise => { + restCalls.push({ method, path, body }); + return {} as T; + }, + getAll: async (path: string): Promise<{ data: T[] }> => { + restCalls.push({ method: "GET", path }); + return { data: departments as T[] }; + }, + ajax: async (module: string, params: Record): Promise => { + ajaxCalls.push({ module, params }); + if (params.func === "getMasterData") return { masterDataTables: { "3": BEREICH_TABLE } } as T; + return null as T; + }, + }; + return { client, ajaxCalls, restCalls }; +} + +describe("create goes through saveMasterData, then re-reads REST for the id (#108)", () => { + it("posts the mapped columns with an EMPTY id, then records the id from GET /departments", async () => { + const state = emptyState("h"); + // The Bereich exists in the catalog by the time the create re-reads it. + const { client, ajaxCalls, restCalls } = recorder([{ id: 7, name: "Equippers Koblenz" }]); + const plan: Plan = { + items: [ + { + type: "department", + key: "equippers_koblenz", + id: null, + action: "create", + changes: [ + { field: "name", from: undefined, to: "Equippers Koblenz" }, + { field: "shorty", from: undefined, to: "EQKO" }, + { field: "sortKey", from: undefined, to: 0 }, + ], + }, + ], + }; + + const res = await executePlan(plan, { client, state, statePath: "unused", save: noSave, now: fixedNow }); + expect(res.failed).toBeUndefined(); + expect(res.created).toEqual(["equippers_koblenz"]); + + // No REST write — that is the point. The only REST call is the id lookup afterwards. + expect(restCalls.filter((c) => c.method !== "GET")).toEqual([]); + expect(restCalls).toEqual([{ method: "GET", path: "/departments" }]); + + // The registry is read first (to validate columns), then the write. + expect(ajaxCalls[0]).toEqual({ module: "churchdb", params: { func: "getMasterData" } }); + expect(ajaxCalls[1]).toEqual({ + module: "churchdb", + params: { + func: "saveMasterData", + table: "cdb_bereich", + id: "", // empty = create + col0: "bezeichnung", + value0: "Equippers Koblenz", + col1: "kuerzel", + value1: "EQKO", + col2: "sortkey", + value2: "0", + }, + }); + expect(state.resources.equippers_koblenz).toMatchObject({ type: "department", id: 7 }); + }); + + it("fails loudly when the created Bereich cannot be identified by name afterwards", async () => { + // saveMasterData returns no id, so the new row is found by name. Two rows sharing that name — or + // none — must stop the apply rather than bind state to a guessed id, which would be permanent. + const state = emptyState("h"); + const { client } = recorder([ + { id: 7, name: "Doppelt" }, + { id: 8, name: "Doppelt" }, + ]); + const plan: Plan = { + items: [ + { + type: "department", + key: "doppelt", + id: null, + action: "create", + changes: [{ field: "name", from: undefined, to: "Doppelt" }], + }, + ], + }; + const res = await executePlan(plan, { client, state, statePath: "unused", save: noSave, now: fixedNow }); + expect(res.failed?.key).toBe("doppelt"); + expect(res.failed?.message).toMatch(/2 rows match it in GET \/departments/); + expect(state.resources.doppelt).toBeUndefined(); + }); +}); + +describe("update reuses the same call with a non-empty id (#108)", () => { + it("sends the FULL snapshot, not just the changed column", async () => { + // The legacy endpoint writes the columns it is handed and is closer to a PUT than a PATCH, so a + // subset would blank the untouched ones. + const state: State = { + version: 1, + host: "h", + resources: { + equippers_koblenz: { + type: "department", + id: 7, + key: "equippers_koblenz", + fields: { name: "Equippers Koblenz", shorty: "EQKO", sortKey: 0 }, + adoptedAt: "t", + updatedAt: "t", + }, + }, + }; + const { client, ajaxCalls, restCalls } = recorder(); + const plan: Plan = { + items: [ + { + type: "department", + key: "equippers_koblenz", + id: 7, + action: "update", + actual: { name: "Equippers Koblenz", shorty: "EQKO", sortKey: 0 }, + changes: [{ field: "name", from: "Equippers Koblenz", to: "Equippers KO" }], + }, + ], + }; + + const res = await executePlan(plan, { client, state, statePath: "unused", save: noSave, now: fixedNow }); + expect(res.failed).toBeUndefined(); + expect(res.updated).toEqual(["equippers_koblenz"]); + expect(restCalls).toEqual([]); // no REST write at all + expect(ajaxCalls[1]?.params).toEqual({ + func: "saveMasterData", + table: "cdb_bereich", + id: "7", // non-empty = update that row + col0: "bezeichnung", + value0: "Equippers KO", + col1: "kuerzel", + value1: "EQKO", // carried through, not blanked + col2: "sortkey", + value2: "0", + }); + }); +}); + +describe("columns are validated against the instance's own DESCRIBE (#109 acceptance)", () => { + it("refuses a column this instance does not report, instead of posting it silently", async () => { + // A legacy endpoint does not 400 on an unknown colN — it just ignores it. So an unvalidated write + // looks like a success and silently drops the field. Hard-error naming the real columns instead. + expect(() => masterDataColumns(BEREICH_TABLE, { farbe: "blau" })).toThrow( + /has no column "farbe" — it has: id, bezeichnung, kuerzel, sortkey/, + ); + }); + + it("skips `id` (it travels as its own parameter, not as a column)", () => { + expect(masterDataColumns(BEREICH_TABLE, { id: 7, bezeichnung: "X" })).toEqual({ + col0: "bezeichnung", + value0: "X", + }); + }); + + it("names what the instance DOES report when the table is missing entirely", async () => { + const { client } = recorder(); + await expect(masterDataTable(client, "cdb_nope")).rejects.toThrow( + /does not report a table "cdb_nope".*It reports: cdb_bereich/s, + ); + }); +}); + +describe("reads go through fetchOne, because GET /departments/{id} does not exist (#108)", () => { + // Regression for a bug a live dev apply caught and no mock could: `/departments` has NO item path. + // The default read 404s, every caller reads that as "vanished in ChurchTools", and the plan proposes + // creating the same Bereich again — forever. A mock answers whatever path it is asked for, so this + // only showed up against a real instance. Pinned here now. + it("filters the collection instead of fetching an item path", async () => { + const { client, restCalls } = recorder([{ id: 7, name: "Equippers Koblenz" }]); + const raw = await RESOURCES.department!.fetchOne!(client, 7); + expect(raw).toMatchObject({ id: 7, name: "Equippers Koblenz" }); + expect(restCalls).toEqual([{ method: "GET", path: "/departments" }]); + expect(restCalls.some((c) => c.path.startsWith("/departments/"))).toBe(false); + }); + + it("returns null for an id the collection does not contain (treated as a 404 by callers)", async () => { + const { client } = recorder([{ id: 7, name: "Equippers Koblenz" }]); + expect(await RESOURCES.department!.fetchOne!(client, 99)).toBeNull(); + }); + + it("makes a managed Bereich diff clean instead of re-proposing a create", async () => { + const state: State = { + version: 1, + host: "h", + resources: { + equippers_koblenz: { + type: "department", + id: 7, + key: "equippers_koblenz", + fields: { name: "Equippers Koblenz", shorty: "EQKO", sortKey: 0 }, + adoptedAt: "t", + updatedAt: "t", + }, + }, + }; + const { client } = recorder([{ id: 7, name: "Equippers Koblenz", shorty: "EQKO", sortKey: 0 }]); + const { actual } = await fetchActual(client as never, Object.values(state.resources)); + expect(actual.get("equippers_koblenz")).toEqual({ + name: "Equippers Koblenz", + shorty: "EQKO", + sortKey: 0, + }); + }); +}); + +describe("destroy deletes through the legacy verb (#108)", () => { + it("calls deleteMasterData rather than a REST DELETE", async () => { + const { client, ajaxCalls, restCalls } = recorder(); + await RESOURCES.department!.writer!.remove!({ client, id: 7 }); + expect(restCalls).toEqual([]); + expect(ajaxCalls.at(-1)).toEqual({ + module: "churchdb", + params: { func: "deleteMasterData", table: "cdb_bereich", id: "7" }, + }); + }); +}); + +describe("the department type stays REST for reads (#108 acceptance)", () => { + it("reads through /departments and writes through the writer hook", () => { + const spec = RESOURCES.department!; + expect(spec.collectionPath).toBe("/departments"); + expect(spec.itemPath(7)).toBe("/departments/7"); + expect(spec.writer).toBeDefined(); + expect(DEPARTMENT_TABLE).toBe("cdb_bereich"); + }); + + it("is the only type with a custom writer", () => { + // Pinned: the legacy surface is deliberately reached for exactly one table (#109 — the other 23 + // registry tables either have a REST write or are outside this tool's mandate). + const withWriter = Object.entries(RESOURCES) + .filter(([, spec]) => spec.writer !== undefined) + .map(([type]) => type); + expect(withWriter).toEqual(["department"]); + }); + + it("warns about the blast radius on destroy", () => { + expect(RESOURCES.department!.destroyWarning).toMatch(/every person assigned to it/); + }); +}); diff --git a/tests/permission-adopt.test.ts b/tests/permission-adopt.test.ts index e1da3e1..3400e73 100644 --- a/tests/permission-adopt.test.ts +++ b/tests/permission-adopt.test.ts @@ -434,14 +434,25 @@ describe("emitAdoptedGrants", () => { it("a catalog-only dimension gets ONE note — the portable form, not the 'not a group' line", () => { const rows: RawPermission[] = [ - { authId: 102, dataId: 4, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, // cdb_bereich + // cdb_comment_viewer — a catalog ct reads but does not manage (#102). Bereiche used to be the + // example here; they became a managed resource in #108, so they now take the adopt-hint path. + { authId: 113, dataId: 4, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, ]; const block = emitAdoptedGrants({ domainType: "group_role", domainId: 42, rows, state: emptyState() }); - expect(block).toContain('Portable form: { department: "" }'); + expect(block).toContain('Portable form: { commentViewer: "" }'); // The numeric-escape-hatch line is for dimensions with NO logical form; emitting it here too // would contradict the portable-form note directly above it. expect(block).not.toContain("not a group"); + expect(block).toContain('{ right: "churchdb:view comments", scope: [4] }'); + }); + + it("points an unmanaged Bereich scope at `ct adopt department` now that Bereiche are managed (#108)", () => { + const rows: RawPermission[] = [ + { authId: 102, dataId: 4, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, // cdb_bereich + ]; + const block = emitAdoptedGrants({ domainType: "group_role", domainId: 42, rows, state: emptyState() }); + expect(block).toContain("ct adopt department 4"); expect(block).toContain('{ right: "churchdb:view alldata", scope: [4] }'); }); diff --git a/tests/permission-scope-refs.test.ts b/tests/permission-scope-refs.test.ts index 2bfc6d9..d2b97c1 100644 --- a/tests/permission-scope-refs.test.ts +++ b/tests/permission-scope-refs.test.ts @@ -304,9 +304,11 @@ describe("department scopes are a read-but-not-managed ref catalog (cdb_bereich, ]); }); - it("hard-errors on an unknown department, and does NOT advise declaring it", async () => { + it("hard-errors on an unknown department, and NOW advises declaring it (#108)", async () => { + // Before #108 this said departments could not be declared or adopted. They can: `ct.department` + // creates one through the legacy master-data endpoint, so the generic advice is correct again. await expect(tuplesFor(perm("nope"), emptyState(HOST), [], client)).rejects.toThrow( - /no live department at \/departments matches key "nope".*ct reads departments but does not manage them/s, + /no managed resource and no live department at \/departments matches key "nope".*Declare\/adopt it/s, ); }); From a4e7fcfddfa1298a9c51f429ee9b825bf129df73 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Fri, 14 Aug 2026 10:34:03 +0200 Subject: [PATCH 6/7] =?UTF-8?q?fix:=20address=20the=20#112=20review=20?= =?UTF-8?q?=E2=80=94=20detached=20client=20reads,=20write-then-check=20cre?= =?UTF-8?q?ates,=20and=20a=20missing=20item=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight findings from the review of this PR, fixed with a regression test each. CRITICAL. `applyPermissionPlan` handed the role-list fetcher `{ get: client.get }`, which DETACHES the prototype method from its instance — the real `CtClient.get` calls `this.requestEnvelope`, so every pending group_role domain (#106's whole scenario) died with "this.requestEnvelope is not a function" at apply time. Bind instead. The existing tests could not see it: their doubles are plain objects that close over nothing, so the new test double is a CLASS, state on `this`, method on the prototype — verified to fail without the fix. HIGH. `departmentWriter.create` wrote first and checked for ambiguity second, so a Bereich created by hand on the target host made the create succeed, the ambiguity check throw, nothing land in state — and every re-run add another orphan. Snapshot the catalog ids BEFORE the write, identify the new row by set difference, and refuse a pre-existing name up front (pointing at `ct adopt`). `ct adopt department ` GET `/departments/{id}`, a path that does not exist — every invocation 404'd, while the docs advertised the command. Adopt now prefers the spec's own `fetchOne`. The master-data registry cache never hit: it keys a WeakMap on the client, but `requireAjax` returned a fresh literal per call, so each write refetched the ~3 MB `getMasterData` payload. Memoize the wrapper per client. Likewise `department.fetchOne` re-read the whole `/departments` collection once per managed Bereich under the `fetchActual` fan-out — share one read, dropped by every write so it can never go stale. A `securityLevel` declaration missing `id` POSTed to a literal `/securitylevels/undefined` (and a string `"3"` diffed forever against the numeric actual). Nothing validated it — `id` is a plain managed field, not one of `ID_FIELDS`. Require a non-negative integer at eval time for any caller-assigned-id type. `ct destroy` exited 0 on the "no delete path" branch, reporting success while the resource stayed in ChurchTools AND in state; same for a missing write spec. Both now exit 1. Finally, `assertNotPeople` guards REST paths only and cannot see the legacy `ajax` channel, while the master-data registry includes person tables. The one-table scope is now enforced by an allowlist in masterdata.ts rather than only documented. Claude-Session: https://claude.ai/code/session_016TacHkv3oVczwXGHBwckq2 --- src/api/masterdata.ts | 20 ++++ src/commands/adopt.ts | 9 +- src/commands/destroy.ts | 6 ++ src/config/context.ts | 16 +++- src/permissions/apply.ts | 9 +- src/resources/registry.ts | 74 +++++++++++--- tests/adopt.test.ts | 38 ++++++++ tests/department-resource.test.ts | 122 +++++++++++++++++++++--- tests/destroy.test.ts | 33 +++++++ tests/permission-pending-domain.test.ts | 35 +++++++ tests/registry.test.ts | 5 +- tests/security-level-resource.test.ts | 29 ++++++ 12 files changed, 364 insertions(+), 32 deletions(-) diff --git a/src/api/masterdata.ts b/src/api/masterdata.ts index bc4bbd5..adce35e 100644 --- a/src/api/masterdata.ts +++ b/src/api/masterdata.ts @@ -61,6 +61,24 @@ export const DEPARTMENT_TABLE = "cdb_bereich"; /** The module whose legacy AJAX endpoint serves the master-data registry. */ const MASTER_DATA_MODULE = "churchdb"; +/** + * The ONLY tables this layer may write. `assertNotPeople` guards REST *paths* and cannot see the + * legacy channel at all, and the registry that `getMasterData` reports includes person master data + * (marital status, nationality, …) — so the "we only ever pass cdb_bereich" of the SCOPE note above + * is enforced here rather than merely documented. Widening this list is a deliberate act. + */ +const WRITABLE_TABLES: ReadonlySet = new Set(["cdb_bereich"]); + +function assertWritableTable(tablename: string): void { + if (!WRITABLE_TABLES.has(tablename)) { + throw new Error( + `Refusing to write master-data table "${tablename}" through the legacy endpoint. \`ct\` drives ` + + `exactly one table here: ${[...WRITABLE_TABLES].join(", ")}. Everything else is either REST-managed ` + + `or outside this tool's structural mandate (person master data).`, + ); + } +} + interface RawMasterData { masterDataTables?: Record; } @@ -159,6 +177,7 @@ export async function saveMasterData( fields: Record, id?: number, ): Promise { + assertWritableTable(tablename); const table = await masterDataTable(client, tablename); await client.ajax(MASTER_DATA_MODULE, { func: "saveMasterData", @@ -177,6 +196,7 @@ export async function deleteMasterData( tablename: string, id: number, ): Promise { + assertWritableTable(tablename); await masterDataTable(client, tablename); // validate the table exists before asking CT to drop a row await client.ajax(MASTER_DATA_MODULE, { func: "deleteMasterData", table: tablename, id: String(id) }); } diff --git a/src/commands/adopt.ts b/src/commands/adopt.ts index 9cceaf5..c861136 100644 --- a/src/commands/adopt.ts +++ b/src/commands/adopt.ts @@ -41,7 +41,14 @@ export function adoptCommand(): Command { const state = await loadState(statePath, config.host); const { client } = await authedSession(); - const resource = await client.get>(spec.itemPath(id)); + // A type whose reads have no item path (#108: Bereiche — CT offers `/departments` only) reads + // through the spec's own `fetchOne`. Going via `itemPath` would 404 on every invocation. + const resource = spec.fetchOne + ? await spec.fetchOne(client, id) + : await client.get>(spec.itemPath(id)); + if (!resource) { + throw new Error(`No ${type} with id ${id} exists in ChurchTools.`); + } const key = opts.key?.trim() || spec.deriveKey(resource); if (!key) { diff --git a/src/commands/destroy.ts b/src/commands/destroy.ts index 6f68659..0bdc763 100644 --- a/src/commands/destroy.ts +++ b/src/commands/destroy.ts @@ -234,7 +234,11 @@ export async function runDeleteLoop(ctx: DeleteLoopCtx): Promise { const managed = state.resources[key]!; const spec = RESOURCES[managed.type]; if (!spec) { + // Skipped, not destroyed: the resource is still in ChurchTools AND still in state, so exit + // non-zero like every other failure below — a `ct destroy` that reports success while its + // targets survive is the one outcome a caller must never see. error(`No write spec for type "${managed.type}" — skipping ${key}.`); + process.exitCode = 1; continue; } const path = spec.itemPath(managed.id); @@ -250,6 +254,8 @@ export async function runDeleteLoop(ctx: DeleteLoopCtx): Promise { `no delete for this type. Remove it in the ChurchTools admin UI, then re-run to drop it ` + `from state.`, ); + // Still in ChurchTools and still in state — a skip, not a success. Exit non-zero. + process.exitCode = 1; continue; } await spec.writer.remove({ client: client as CtWriteClient, id: managed.id }); diff --git a/src/config/context.ts b/src/config/context.ts index e0b3c45..cfa19ff 100644 --- a/src/config/context.ts +++ b/src/config/context.ts @@ -19,7 +19,7 @@ import type { DesiredPermission, Grant, PreserveUnknown } from "../permissions/t import { KNOWN_SCOPE_FIELDS } from "../permissions/catalog.js"; import { GROUP_STATUS_NO_CATALOG, isRef, ref, refKey, type Ref } from "../resolve/refs.js"; import { normalizeScopeEntry } from "../permissions/scope.js"; -import { conventionalRulesetRef, knownFields } from "../resources/registry.js"; +import { conventionalRulesetRef, isCallerAssignedId, knownFields } from "../resources/registry.js"; import { warn } from "../ui.js"; // Re-exported so a config file can pull the query DSL from the same module as // `ConfigContext`: `import { q, churchQuery } from "../../src/config/context.js"`. @@ -425,6 +425,20 @@ function toDesired(type: string, input: ResourceInput, location?: string): Desir `reference${hint}.`, ); } + // A caller-assigned-id type (#110: security levels) is CREATED at its declared id — `createPath` + // interpolates `body.id` straight into the POST path. An omitted id would POST to a literal + // ".../undefined", and a string one ("3") would both post oddly and diff forever against CT's + // numeric actual. Neither is caught anywhere else: `id` is a plain managed field here, not one of + // the ID_FIELDS checked above. So require a real, non-negative integer at eval time. + if (isCallerAssignedId(type)) { + const declaredId = fields.id; + if (typeof declaredId !== "number" || !Number.isInteger(declaredId) || declaredId < 0) { + throw new Error( + `${type} "${key}": "id" is required and must be a non-negative integer — this resource is ` + + `created at the id you declare (got ${JSON.stringify(declaredId)}).`, + ); + } + } // Warn (never throw) on a declared field the registry does not manage for this type — e.g. a // seeded config using campus's vestigial `shortName` instead of the real `shorty` (#51). The // field still passes through into `fields` unchanged (unrecognised fields have always been sent diff --git a/src/permissions/apply.ts b/src/permissions/apply.ts index 7eae15c..0017735 100644 --- a/src/permissions/apply.ts +++ b/src/permissions/apply.ts @@ -105,7 +105,14 @@ export async function applyPermissionPlan( domainId = await resolvePendingGroupRoleDomain( item.pendingDomain, state, - { get: client.get, getAll: client.getAll }, + // BIND, don't copy: `CtClient`'s reads are prototype methods that call `this.request` / + // `this.requestEnvelope` internally, so handing on a bare `{ get: client.get }` detaches + // them and every pending group_role domain dies with "this.requestEnvelope is not a + // function" the moment it fetches the role list. + { + get: client.get.bind(client), + ...(client.getAll ? { getAll: client.getAll.bind(client) } : {}), + }, site, roleLists, ); diff --git a/src/resources/registry.ts b/src/resources/registry.ts index 5d26e52..108b1d6 100644 --- a/src/resources/registry.ts +++ b/src/resources/registry.ts @@ -217,6 +217,30 @@ async function departmentRows(client: CtWriteClient): Promise<{ id: number; name return (await client.get?.<{ id: number; name?: string }[]>("/departments")) ?? []; } +/** + * `fetchOne` has no `/departments/{id}` to hit, so it filters the whole collection — and + * `fetchActual` runs it once per managed Bereich, concurrently. Share one read across that fan-out, + * the way the resolver caches identical catalog reads. Every write below drops the entry, so the + * cache can never outlive the state it describes. + */ +const departmentReads = new WeakMap>(); + +function cachedDepartmentRows(client: CtWriteClient): Promise<{ id: number; name?: string }[]> { + let p = departmentReads.get(client); + if (!p) { + p = departmentRows(client); + departmentReads.set(client, p); + } + return p; +} + +/** + * Wrap a client's `ajax` once PER CLIENT. The wrapper is what `masterdata.ts` keys its ~3 MB + * registry cache on (a WeakMap on the object it is handed), so returning a fresh literal per call + * would miss that cache on every single write and re-fetch the whole payload each time. + */ +const ajaxWrappers = new WeakMap>>(); + function requireAjax(client: CtWriteClient): Required> { if (!client.ajax) { throw new Error( @@ -224,38 +248,57 @@ function requireAjax(client: CtWriteClient): Required { const { saveMasterData, DEPARTMENT_TABLE } = await import("../api/masterdata.js"); - await saveMasterData(requireAjax(client), DEPARTMENT_TABLE, departmentRow(body)); - // `saveMasterData` answers {"status":"success","data":null} — no id. Re-read the REST catalog and - // match the name just written. A duplicate name would be ambiguous, so that is refused rather - // than guessed: picking either row would bind state to the wrong Bereich for good. const name = body.name; - const rows = await departmentRows(client); - const matches = rows.filter((r) => r.name === name); - if (matches.length !== 1) { + // `saveMasterData` answers {"status":"success","data":null} — no id — so the new row has to be + // identified by re-reading the REST catalog. Snapshot the ids BEFORE writing and diff after: + // matching on the name alone would be wrong the moment a Bereich of that name already exists + // (created by hand on the target host), because the create would still have succeeded and the + // post-write ambiguity error would leave an orphan row behind that every retry duplicates. + const before = await departmentRows(client); + const collision = before.filter((r) => r.name === name); + if (collision.length > 0) { + throw new Error( + `A Bereich named "${String(name)}" already exists in ChurchTools (#${collision.map((r) => r.id).join(", #")}). ` + + `Refusing to create a second one: the legacy master-data endpoint returns no id, so the new ` + + `row could not be told apart from the existing one. Adopt it instead (\`ct adopt department ` + + `${collision[0]!.id}\`) or rename one of the two.`, + ); + } + const known = new Set(before.map((r) => r.id)); + await saveMasterData(requireAjax(client), DEPARTMENT_TABLE, departmentRow(body)); + departmentReads.delete(client); + const fresh = (await departmentRows(client)).filter((r) => !known.has(r.id)); + if (fresh.length !== 1) { throw new Error( - `Created the Bereich "${String(name)}", but ${matches.length === 0 ? "it did not appear in" : `${matches.length} rows match it in`} ` + - `GET /departments — cannot record its id. The legacy master-data endpoint returns no id, so ` + - `the new row is identified by name. Resolve the duplicate/absence in ChurchTools and re-run.`, + `Created the Bereich "${String(name)}", but ${fresh.length === 0 ? "no new row appeared in" : `${fresh.length} new rows appeared in`} ` + + `GET /departments — cannot record its id. Check ChurchTools for a stray Bereich before re-running.`, ); } - return matches[0]!.id; + return fresh[0]!.id; }, async update({ client, body, id }): Promise { const { saveMasterData, DEPARTMENT_TABLE } = await import("../api/masterdata.js"); // A non-empty `id` makes the same call an UPDATE of that row (verified live, eqrm-dev 2026-08-14). await saveMasterData(requireAjax(client), DEPARTMENT_TABLE, departmentRow(body), id); + departmentReads.delete(client); }, async remove({ client, id }): Promise { const { deleteMasterData, DEPARTMENT_TABLE } = await import("../api/masterdata.js"); // `deleteMasterData` is a real verb, not a silently-ignored unknown one: a made-up `delMasterData` // against the same endpoint answers "was not defined as Function!" (verified, eqrm-dev 2026-08-14). await deleteMasterData(requireAjax(client), DEPARTMENT_TABLE, id); + departmentReads.delete(client); }, }; @@ -424,7 +467,7 @@ export const RESOURCES: Record = { // There is no `GET /departments/{id}` — filter the collection instead. Without this every plan // after a create would read a 404 and propose creating the same Bereich again. fetchOne: async (client, id) => - ((await departmentRows(client)) as Record[]).find((r) => r.id === id) ?? null, + ((await cachedDepartmentRows(client)) as Record[]).find((r) => r.id === id) ?? null, writer: departmentWriter, }), /** @@ -500,6 +543,11 @@ export function knownFields(type: string): Set { return new Set(Object.keys(resourceType(type).managedFields({}))); } +/** True when `type`'s id is chosen by the config rather than minted by CT (#110: security levels). */ +export function isCallerAssignedId(type: string): boolean { + return RESOURCES[type]?.callerAssignedId === true; +} + /** Camel-case a hyphenated type name: `group-type` → `groupType`. */ function camelCase(type: string): string { return type.replace(/-([a-z])/g, (_, c: string) => c.toUpperCase()); diff --git a/tests/adopt.test.ts b/tests/adopt.test.ts index 0236d5b..daec5e8 100644 --- a/tests/adopt.test.ts +++ b/tests/adopt.test.ts @@ -90,6 +90,44 @@ describe("ct adopt", () => { await expect(readFile(statePath, "utf8")).rejects.toMatchObject({ code: "ENOENT" }); }); + // Bereiche have NO `/departments/{id}` path (#108) — reading through the default item path 404s on + // every single invocation, so adopt has to go through the spec's own `fetchOne`. + it("reads a type with no item path through fetchOne, not through //", async () => { + const rows = [{ id: 7, name: "Equippers Koblenz", shorty: "EQKO", sortKey: 0 }]; + const paths: string[] = []; + const original = getMock.getMockImplementation()!; + getMock.mockImplementation(async (path?: string) => { + paths.push(path!); + if (path === "/departments") return rows as unknown as Record; + return original(path); + }); + try { + await runAdopt(["department", "7", "--state", statePath]); + } finally { + getMock.mockImplementation(original); + } + + const state = await loadState(statePath, HOST); + expect(state.resources.equippers_koblenz).toMatchObject({ type: "department", id: 7 }); + expect(paths).toContain("/departments"); + expect(paths.some((p) => p.startsWith("/departments/"))).toBe(false); + }); + + it("errors clearly when a fetchOne type has no row with that id", async () => { + const original = getMock.getMockImplementation()!; + getMock.mockImplementation(async (path?: string) => { + if (path === "/departments") return [] as unknown as Record; + return original(path); + }); + try { + await expect(runAdopt(["department", "99", "--state", statePath])).rejects.toThrow( + /No department with id 99 exists in ChurchTools/, + ); + } finally { + getMock.mockImplementation(original); + } + }); + it("rejects an unknown resource type before any API call", async () => { await expect(runAdopt(["widget", "1", "--state", statePath])).rejects.toThrow(/Adoptable types/); expect(getMock).not.toHaveBeenCalled(); diff --git a/tests/department-resource.test.ts b/tests/department-resource.test.ts index f8031a4..a035db9 100644 --- a/tests/department-resource.test.ts +++ b/tests/department-resource.test.ts @@ -40,10 +40,22 @@ const BEREICH_TABLE = { }, }; -/** A client exposing REST `request`/`getAll` plus the legacy `ajax` channel, recording both. */ -function recorder(departments: { id: number; name: string; shorty?: string; sortKey?: number }[] = []) { +/** + * A client exposing REST `request`/`getAll` plus the legacy `ajax` channel, recording both. + * + * The catalog is LIVE, not a frozen list: a `saveMasterData` create appends a row the way CT does, + * so `GET /departments` afterwards differs from before. That asymmetry is the whole mechanism the + * writer relies on to learn the new id (the endpoint returns none), so a double that answered the + * same rows before and after would test nothing. `options.createdId` forces the id CT mints; + * `options.mintsNothing` makes the write a silent no-op (the "row never appeared" failure path). + */ +function recorder( + departments: { id: number; name: string; shorty?: string; sortKey?: number }[] = [], + options: { createdId?: number; mintsNothing?: boolean } = {}, +) { const ajaxCalls: { module: string; params: Record }[] = []; const restCalls: { method: string; path: string; body?: unknown }[] = []; + const rows = [...departments]; const client = { request: async (method: string, path: string, body?: unknown): Promise => { restCalls.push({ method, path, body }); @@ -51,11 +63,15 @@ function recorder(departments: { id: number; name: string; shorty?: string; sort }, getAll: async (path: string): Promise<{ data: T[] }> => { restCalls.push({ method: "GET", path }); - return { data: departments as T[] }; + return { data: [...rows] as T[] }; }, ajax: async (module: string, params: Record): Promise => { ajaxCalls.push({ module, params }); if (params.func === "getMasterData") return { masterDataTables: { "3": BEREICH_TABLE } } as T; + if (params.func === "saveMasterData" && params.id === "" && !options.mintsNothing) { + // `col0=bezeichnung&value0=` — mirror CT and append the row under its REST name. + rows.push({ id: options.createdId ?? 7, name: params.value0! }); + } return null as T; }, }; @@ -65,8 +81,8 @@ function recorder(departments: { id: number; name: string; shorty?: string; sort describe("create goes through saveMasterData, then re-reads REST for the id (#108)", () => { it("posts the mapped columns with an EMPTY id, then records the id from GET /departments", async () => { const state = emptyState("h"); - // The Bereich exists in the catalog by the time the create re-reads it. - const { client, ajaxCalls, restCalls } = recorder([{ id: 7, name: "Equippers Koblenz" }]); + // Empty instance; the write itself puts the Bereich in the catalog (see `recorder`). + const { client, ajaxCalls, restCalls } = recorder([], { createdId: 7 }); const plan: Plan = { items: [ { @@ -87,9 +103,13 @@ describe("create goes through saveMasterData, then re-reads REST for the id (#10 expect(res.failed).toBeUndefined(); expect(res.created).toEqual(["equippers_koblenz"]); - // No REST write — that is the point. The only REST call is the id lookup afterwards. + // No REST write — that is the point. The only REST calls are the id lookups either side of it: + // the new row is identified by DIFFING the catalog, so both reads are load-bearing. expect(restCalls.filter((c) => c.method !== "GET")).toEqual([]); - expect(restCalls).toEqual([{ method: "GET", path: "/departments" }]); + expect(restCalls).toEqual([ + { method: "GET", path: "/departments" }, + { method: "GET", path: "/departments" }, + ]); // The registry is read first (to validate columns), then the write. expect(ajaxCalls[0]).toEqual({ module: "churchdb", params: { func: "getMasterData" } }); @@ -110,14 +130,12 @@ describe("create goes through saveMasterData, then re-reads REST for the id (#10 expect(state.resources.equippers_koblenz).toMatchObject({ type: "department", id: 7 }); }); - it("fails loudly when the created Bereich cannot be identified by name afterwards", async () => { - // saveMasterData returns no id, so the new row is found by name. Two rows sharing that name — or - // none — must stop the apply rather than bind state to a guessed id, which would be permanent. + it("refuses BEFORE writing when a Bereich of that name already exists", async () => { + // The ordinary collision: somebody made the Bereich by hand on the target host. Checking only + // AFTER the write would be worse than useless — the create would have succeeded, the ambiguity + // error would abort before state recorded anything, and every re-run would add another orphan. const state = emptyState("h"); - const { client } = recorder([ - { id: 7, name: "Doppelt" }, - { id: 8, name: "Doppelt" }, - ]); + const { client, ajaxCalls } = recorder([{ id: 7, name: "Doppelt" }]); const plan: Plan = { items: [ { @@ -131,9 +149,31 @@ describe("create goes through saveMasterData, then re-reads REST for the id (#10 }; const res = await executePlan(plan, { client, state, statePath: "unused", save: noSave, now: fixedNow }); expect(res.failed?.key).toBe("doppelt"); - expect(res.failed?.message).toMatch(/2 rows match it in GET \/departments/); + expect(res.failed?.message).toMatch(/already exists in ChurchTools \(#7\)/); + // Nothing was written: no saveMasterData reached the legacy endpoint at all. + expect(ajaxCalls.filter((c) => c.params.func === "saveMasterData")).toEqual([]); expect(state.resources.doppelt).toBeUndefined(); }); + + it("fails loudly when the write leaves no new row in the catalog", async () => { + const state = emptyState("h"); + const { client } = recorder([{ id: 7, name: "Andere" }], { mintsNothing: true }); + const plan: Plan = { + items: [ + { + type: "department", + key: "neu", + id: null, + action: "create", + changes: [{ field: "name", from: undefined, to: "Neu" }], + }, + ], + }; + const res = await executePlan(plan, { client, state, statePath: "unused", save: noSave, now: fixedNow }); + expect(res.failed?.key).toBe("neu"); + expect(res.failed?.message).toMatch(/no new row appeared in GET \/departments/); + expect(state.resources.neu).toBeUndefined(); + }); }); describe("update reuses the same call with a non-empty id (#108)", () => { @@ -253,6 +293,58 @@ describe("reads go through fetchOne, because GET /departments/{id} does not exis }); }); +describe("repeated reads and writes share their fetches", () => { + it("reads /departments ONCE across a whole fetchActual fan-out", async () => { + // `fetchActual` runs fetchOne per managed Bereich, concurrently — without sharing, a config with + // N departments issues N full collection GETs on every plan AND every apply. + const { client, restCalls } = recorder([ + { id: 7, name: "A" }, + { id: 8, name: "B" }, + { id: 9, name: "C" }, + ]); + const spec = RESOURCES.department!; + const ids = [7, 8, 9]; + const rows = await Promise.all(ids.map((id) => spec.fetchOne!(client, id))); + expect(rows.map((r) => r?.id)).toEqual(ids); + expect(restCalls).toEqual([{ method: "GET", path: "/departments" }]); + }); + + it("re-reads /departments after a write, so the shared read can never go stale", async () => { + const { client, restCalls } = recorder([{ id: 7, name: "A" }]); + const spec = RESOURCES.department!; + await spec.fetchOne!(client, 7); + await spec.writer!.update!({ client, body: { name: "A2" }, id: 7 }); + await spec.fetchOne!(client, 7); + expect(restCalls.filter((c) => c.path === "/departments")).toHaveLength(2); + }); + + it("fetches the ~3 MB master-data registry ONCE across several writes", async () => { + // The registry is cached per client inside masterdata.ts. The writer must therefore hand on a + // STABLE wrapper object — a fresh literal per call misses that cache and refetches every time. + const { client, ajaxCalls } = recorder([{ id: 7, name: "A" }]); + const spec = RESOURCES.department!; + await spec.writer!.update!({ client, body: { name: "A2" }, id: 7 }); + await spec.writer!.update!({ client, body: { name: "A3" }, id: 7 }); + await spec.writer!.remove!({ client, id: 7 }); + expect(ajaxCalls.filter((c) => c.params.func === "getMasterData")).toHaveLength(1); + }); +}); + +describe("the legacy write channel is allowlisted to one table", () => { + // `assertNotPeople` guards REST PATHS and cannot see this channel at all, while the registry the + // instance reports includes person master data. Enforce the one-table scope in code, not prose. + it("refuses to save or delete any table other than cdb_bereich", async () => { + const { client } = recorder(); + const { saveMasterData, deleteMasterData } = await import("../src/api/masterdata.js"); + await expect(saveMasterData(client, "cdb_familienstand", { bezeichnung: "X" })).rejects.toThrow( + /Refusing to write master-data table "cdb_familienstand"/, + ); + await expect(deleteMasterData(client, "cdb_familienstand", 1)).rejects.toThrow( + /Refusing to write master-data table "cdb_familienstand"/, + ); + }); +}); + describe("destroy deletes through the legacy verb (#108)", () => { it("calls deleteMasterData rather than a REST DELETE", async () => { const { client, ajaxCalls, restCalls } = recorder(); diff --git a/tests/destroy.test.ts b/tests/destroy.test.ts index 6cb65b7..b003b88 100644 --- a/tests/destroy.test.ts +++ b/tests/destroy.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi } from "vitest"; import { parseTargets, orderDestroy, runDeleteLoop, destroyWarnings } from "../src/commands/destroy.js"; import { emptyState, type State } from "../src/state/state.js"; import { CtApiError, type CtClient } from "../src/api/ctClient.js"; +import { RESOURCES } from "../src/resources/registry.js"; function stateWith(...entries: Array<{ key: string; type: string; id: number }>): State { const state = emptyState("h"); @@ -117,6 +118,38 @@ describe("runDeleteLoop", () => { process.exitCode = prevExit; }); + it("exits NON-ZERO when a type has no delete path — the target survives, so it is not a success", async () => { + // A writer without `remove` (#108's branch for a type ChurchTools exposes no delete for) leaves + // the resource in CT *and* in state. Reporting exit 0 there would tell a script the destroy + // succeeded while nothing was destroyed. + const spec = RESOURCES.department!; + const originalRemove = spec.writer!.remove; + delete spec.writer!.remove; + const state = stateWith({ key: "bereich", type: "department", id: 7 }); + const request = vi.fn(async () => ({})); + const save = vi.fn(async () => {}); + const prevExit = process.exitCode; + const errSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + + try { + await runDeleteLoop({ + client: asClient(request), + state, + statePath: "s.json", + ordered: ["bereich"], + save, + }); + expect(process.exitCode).toBe(1); + expect(state.resources.bereich).toBeDefined(); // still managed — nothing was deleted + expect(request).not.toHaveBeenCalled(); // and no DELETE was issued against a path CT lacks + expect(errSpy.mock.calls.map((c) => String(c[0])).join("\n")).toContain("cannot be deleted"); + } finally { + spec.writer!.remove = originalRemove; + errSpy.mockRestore(); + process.exitCode = prevExit; + } + }); + it("renders a CtApiError's HTTP status + body in the stop message, via the shared formatter (#71)", async () => { const state = stateWith({ key: "boom", type: "group", id: 1 }); const request = vi.fn(async (_m: string, path: string) => { diff --git a/tests/permission-pending-domain.test.ts b/tests/permission-pending-domain.test.ts index 1cb6213..0f6996c 100644 --- a/tests/permission-pending-domain.test.ts +++ b/tests/permission-pending-domain.test.ts @@ -261,6 +261,41 @@ describe("group_role symmetry: a same-run group DOES go pending and completes in ); }); + it("survives a client whose reads are PROTOTYPE methods using `this` (the real CtClient shape)", async () => { + // Regression guard. `applyPermissionPlan` hands the role-list fetcher a narrowed reading client; + // building that as `{ get: client.get }` DETACHES the method from its instance, so the real + // CtClient — whose `get` calls `this.requestEnvelope` internally — dies with "this.… is not a + // function" on every pending group_role domain. The plain-object doubles above cannot catch it + // (they close over nothing), so this double mimics the class: state on `this`, method on the + // prototype. + class ProtoClient { + readonly seen: string[] = []; + async request(method: string, path: string, body?: unknown): Promise { + this.seen.push(`${method} ${path}`); + void body; + return method === "POST" && path === "/groups" ? { id: GROUP_ID } : {}; + } + async get(path: string): Promise { + // The load-bearing part: reaching a sibling through `this`, exactly as CtClient.get does. + this.seen.push(`GET ${path}`); + return path === `/groups/${GROUP_ID}/roles` + ? [{ id: PAIRING_ID, name: "Leiter", groupTypeRoleId: 12 }] + : []; + } + } + const proto = new ProtoClient(); + const client = proto as unknown as CtClient; + const state = emptyState(HOST); + const { items } = await buildPermissionPlan(client, state, [grPerm], declaredGroup); + await executePlan(createGroupPlan, { client, state, statePath: "unused", save: async () => {} }); + + const res = await applyPermissionPlan(items, client, state); + expect(res.failed).toEqual([]); + expect(res.granted).toBe(1); + expect(proto.seen).toContain(`GET /groups/${GROUP_ID}/roles`); + expect(proto.seen).toContain(`PUT /permissions/group_role/${PAIRING_ID}`); + }); + it("is unchanged on a host where the group already exists — concrete domain, no pending path", async () => { const { client, get } = groupClient(); const state: State = { diff --git a/tests/registry.test.ts b/tests/registry.test.ts index 4a21e16..6e9d5cb 100644 --- a/tests/registry.test.ts +++ b/tests/registry.test.ts @@ -142,7 +142,10 @@ describe("configSnippet — idiomatic multi-line output (#52 item A)", () => { describe("configSnippet round-trips through the config loader for every adoptable type", () => { it.each(Object.keys(RESOURCES))("%s", async (type) => { const key = `adopted_${type.replace(/-/g, "_")}`; - const snippet = configSnippet(type, key, { name: "Round Trip" }); + // A caller-assigned-id type (#110: security levels) is created AT its declared id, so `id` is a + // managed field that `adopt` always emits and the config DSL requires. Mirror a real adopt. + const fields = RESOURCES[type]?.callerAssignedId ? { id: 3, name: "Round Trip" } : { name: "Round Trip" }; + const snippet = configSnippet(type, key, fields); const dir = mkdtempSync(join(tmpdir(), "ct-adopt-")); writeFileSync(join(dir, "ct.config.ts"), `export default (ct) => { ct.${snippet} };`); diff --git a/tests/security-level-resource.test.ts b/tests/security-level-resource.test.ts index d1ff14c..c3a4694 100644 --- a/tests/security-level-resource.test.ts +++ b/tests/security-level-resource.test.ts @@ -167,6 +167,35 @@ describe("adopt → config → plan round-trips to a no-op (#110)", () => { }); }); +describe("a declared id is REQUIRED and must be a number (#110)", () => { + // The create POSTs to `/securitylevels/${body.id}`. An omitted id would interpolate the literal + // string "undefined" into the path, and a string "3" would diff forever against CT's numeric + // actual — neither is caught by ID_FIELDS (which covers campusId/groupTypeId/groupStatusId only), + // so eval time is the last place either can be stopped. + it("rejects a declaration with no id", async () => { + await expect( + evaluateConfig((ct) => { + ct.securityLevel({ key: "stufe_5", name: "Stufe 5" }); + }), + ).rejects.toThrow(/"id" is required and must be a non-negative integer/); + }); + + it("rejects a STRING id rather than posting it and diffing against a number forever", async () => { + await expect( + evaluateConfig((ct) => { + ct.securityLevel({ key: "stufe_5", id: "5", name: "Stufe 5" }); + }), + ).rejects.toThrow(/"id" is required and must be a non-negative integer/); + }); + + it("leaves every CT-minted-id type alone — a campus needs no declared id", async () => { + const { resources } = await evaluateConfig((ct) => { + ct.campus({ key: "mainz", name: "Mainz", shorty: "MZ" }); + }); + expect(resources[0]?.key).toBe("mainz"); + }); +}); + describe("destroy carries the instance-wide warning (#110)", () => { it("names what a delete reaches, the way person-status does", () => { const warning = RESOURCES["security-level"]!.destroyWarning; From 3595c4a57d939d34d4ebaa5ae4501c6623162624 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Fri, 14 Aug 2026 10:35:43 +0200 Subject: [PATCH 7/7] docs: note that a security level's `id` is now enforced, and re-sign the two pages whose sources moved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The handbook already said `id` is required on `ct.securityLevel`; it is now actually rejected at config-evaluation time (a missing id used to POST to `/securitylevels/undefined`), so say when the rejection happens. blueprints.md re-read unchanged — its sources moved, its subject matter did not. Claude-Session: https://claude.ai/code/session_016TacHkv3oVczwXGHBwckq2 --- docs/handbuch/blueprints.md | 2 +- docs/handbuch/permissions.md | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/handbuch/blueprints.md b/docs/handbuch/blueprints.md index 1dbbe48..0a61d93 100644 --- a/docs/handbuch/blueprints.md +++ b/docs/handbuch/blueprints.md @@ -4,7 +4,7 @@ sources: - src/config/context.ts - src/engine/graph.ts - src/engine/hierarchy.ts -sources_hash: 284e69c26599d21a +sources_hash: bbadf1a728820f5c reviewed: 2026-08-13 --- diff --git a/docs/handbuch/permissions.md b/docs/handbuch/permissions.md index d26d293..cb6fe3b 100644 --- a/docs/handbuch/permissions.md +++ b/docs/handbuch/permissions.md @@ -5,7 +5,7 @@ sources: - src/resolve/resolver.ts - src/resolve/refs.ts - src/config/context.ts -sources_hash: e6d4211dd673a741 +sources_hash: 5c881a7bede7e01a reviewed: 2026-08-13 --- @@ -459,7 +459,9 @@ because the config now owns the ids: ct.securityLevel({ key: "stufe_3_hoch", id: 3, name: "Stufe 3 (Hoch)" }); ``` -`id` is **required** here and it is the only declaration where you choose the id. +`id` is **required** here — a declaration without one, or with a string `"3"` +instead of the number `3`, is rejected when the config is evaluated, before any +request goes out — and it is the only declaration where you choose the id. ChurchTools creates a level with `POST /securitylevels/{id}` — the client picks the id, CT 409s if it is taken — precisely so a level is reproducible rather than whatever a fresh instance auto-increments to. Live-probed on eqrm-dev 2026-08-14: