feat(kanidm): declare the whole account policy, and record why secrets cannot be #307

Sammanfogat

supernaut sammanfogade 1 incheckning från feat/kanidm-declared-state in i main

2026-08-01 17:52:42 +00:00

supernaut kommenterad

2026-08-01 14:27:39 +00:00

Ägare

Closes #275.

Most of #275 had already shipped through #277–#282 — all four OAuth2 clients are declared and the
scope-map drift gate is enforced. What was left was the account policy and two undocumented decisions.

Account policy is now declared in full

kanidm_account_policy_credential_type_minimum was a single scalar; it is replaced by a
kanidm_account_policy table covering all eight knobs the CLI exposes, each carrying its
server-side fallback and the reason for the declared value. The assertion gate loops the table, so an
upstream default that changes on upgrade now fails a converge instead of silently altering credential
requirements for every person.

Three findings worth reading before merging

1. Client secrets cannot be sourced from vault — this is a limit, not a deferred task. Upstream
Kanidm has no API to set an OAuth2 basic secret; it can only read or reset one. The provisioning
tool's basicSecretFile targets an endpoint that exists only in a patched Kanidm server — its own
source says "Only works when using the patch. Do not specify otherwise!" and "Did you compile kanidm
with the necessary patch?" (src/client.rs:396-426). Forking the identity provider is a different
order of risk from the existing entryManagedBy patch on a helper tool, so it stays undone — but it is
now recorded as a limit, together with its consequence (a rebuild mints fresh random secrets and breaks
Forgejo, the portal and Grafana SSO simultaneously) and the recovery sequence, in the runbook.

2. ⚠ authsession_expiry being unset means never, not 24 hours — so production auth sessions
effectively never expire. Absent resolves to MAXIMUM_AUTH_SESSION_EXPIRY (u32::MAX), not the
test-only DEFAULT_ constant of 86400 s (accountpolicy.rs:27-29). And reset-auth-expiry is an HTTP
DELETE (libs/client/src/group.rs:40-46), so it purges the value rather than restoring a default.
This is recorded as a documented recommendation and not applied — changing session lifetime is its
own reviewed change. Filed separately.

3. The old readability guard had a hole. It skipped silently when credential_type_minimum was
purged — the one case it exists to catch. It now guards on the attribute map being non-empty, which also
distinguishes Kanidm's habit of answering a denied read with HTTP 200 and a null body.

`--no-auto-remove`: recommend keeping it

Declaring the clients does not make dropping the flag safe. Auto-remove is not OAuth2-scoped — it
deletes orphaned groups and persons, and neither the builtin idm_* groups nor a narrowed
forgejo_users is safe under it. It is also not what protects a declared entity (present: false
deletes regardless), and the leak that actually bit us — #278's stale scope map — is invisible to it.
Recorded rather than silently flipped.

Verified

site.yml --syntax-check clean.
ansible-lint roles/kanidm roles/caddy → 14 failures, identical to the HEAD baseline (confirmed
by git stash; the rule mix diff is empty). None on changed lines.
Template renders and validates as JSON: clients ['bitborg-web','bitborg-web-dev','forgejo','grafana'];
basicSecretFile absent everywhere.
The gate was extracted verbatim from the role by script and driven against five mocked payloads:
live-as-is → all 8 pass; purged credential_type_minimum → fail; downgraded to any → fail; an
undeclared authsession_expiry set → only that one fails; a denied read (null body) → all skip.
Client set confirmed read-only against production: per-client OIDC discovery returns 200 for all four
and 404 for a control id.

Applying — read this first

Run the normal apply. The only new runtime behaviour is the account-policy gate, which is read-only.

Watch its output on the first run. That is where the seven null-declared knobs are confirmed
against live for the first time. If one turns out to be set on the server, the play fails with the
value and the reconcile command — that is intended. The fix is to adopt the live value in
kanidm_account_policy with a reason, or unset it on the server.

Two things could not be verified without credentials, and are stated in the runbook rather than glossed:
the live values of those seven knobs (the cached idm_admin session had expired), and whether an
undeclared OAuth2 client exists — OIDC discovery proves the declared ones are real but cannot
enumerate, which needs kanidm system oauth2 list as idm_admin. If the reconciler token cannot read
idm_all_persons' attributes at all, the gate reports "could not verify" and skips by design, and the
whole policy check is inert — so check the first apply's output for that message specifically.

Closes #275. Most of #275 had already shipped through #277–#282 — all four OAuth2 clients are declared and the scope-map drift gate is enforced. What was left was the account policy and two undocumented decisions. ## Account policy is now declared in full `kanidm_account_policy_credential_type_minimum` was a single scalar; it is replaced by a `kanidm_account_policy` table covering **all eight knobs the CLI exposes**, each carrying its server-side fallback and the reason for the declared value. The assertion gate loops the table, so an upstream default that changes on upgrade now fails a converge instead of silently altering credential requirements for every person. ## Three findings worth reading before merging **1. Client secrets cannot be sourced from vault — this is a limit, not a deferred task.** Upstream Kanidm has no API to *set* an OAuth2 basic secret; it can only read or reset one. The provisioning tool's `basicSecretFile` targets an endpoint that exists only in a **patched Kanidm server** — its own source says *"Only works when using the patch. Do not specify otherwise!"* and *"Did you compile kanidm with the necessary patch?"* (`src/client.rs:396-426`). Forking the identity provider is a different order of risk from the existing `entryManagedBy` patch on a helper tool, so it stays undone — but it is now recorded as a limit, together with its consequence (a rebuild mints fresh random secrets and breaks Forgejo, the portal and Grafana SSO simultaneously) and the recovery sequence, in the runbook. **2. ⚠ `authsession_expiry` being unset means *never*, not 24 hours — so production auth sessions effectively never expire.** Absent resolves to `MAXIMUM_AUTH_SESSION_EXPIRY` (`u32::MAX`), not the test-only `DEFAULT_` constant of 86400 s (`accountpolicy.rs:27-29`). And `reset-auth-expiry` is an HTTP `DELETE` (`libs/client/src/group.rs:40-46`), so it purges the value rather than restoring a default. This is recorded as a documented recommendation and **not applied** — changing session lifetime is its own reviewed change. Filed separately. **3. The old readability guard had a hole.** It skipped **silently** when `credential_type_minimum` was purged — the one case it exists to catch. It now guards on the attribute map being non-empty, which also distinguishes Kanidm's habit of answering a denied read with HTTP 200 and a `null` body. ## `--no-auto-remove`: recommend keeping it Declaring the clients does not make dropping the flag safe. Auto-remove is not OAuth2-scoped — it deletes orphaned groups and persons, and neither the builtin `idm_*` groups nor a narrowed `forgejo_users` is safe under it. It is also not what protects a declared entity (`present: false` deletes regardless), and the leak that actually bit us — #278's stale scope map — is invisible to it. Recorded rather than silently flipped. ## Verified - `site.yml --syntax-check` clean. - `ansible-lint roles/kanidm roles/caddy` → 14 failures, **identical to the `HEAD` baseline** (confirmed by `git stash`; the rule mix diff is empty). None on changed lines. - Template renders and validates as JSON: clients `['bitborg-web','bitborg-web-dev','forgejo','grafana']`; `basicSecretFile` absent everywhere. - The gate was extracted verbatim from the role by script and driven against five mocked payloads: live-as-is → all 8 pass; purged `credential_type_minimum` → fail; downgraded to `any` → fail; an undeclared `authsession_expiry` set → only that one fails; a denied read (`null` body) → all skip. - Client set confirmed read-only against production: per-client OIDC discovery returns 200 for all four and 404 for a control id. ## Applying — read this first Run the normal apply. The only new runtime behaviour is the account-policy gate, which is read-only. **Watch its output on the first run.** That is where the seven `null`-declared knobs are confirmed against live for the first time. If one turns out to be set on the server, the play **fails** with the value and the reconcile command — that is intended. The fix is to adopt the live value in `kanidm_account_policy` with a reason, or unset it on the server. Two things could not be verified without credentials, and are stated in the runbook rather than glossed: the live values of those seven knobs (the cached `idm_admin` session had expired), and whether an **undeclared** OAuth2 client exists — OIDC discovery proves the declared ones are real but cannot enumerate, which needs `kanidm system oauth2 list` as `idm_admin`. If the reconciler token cannot read `idm_all_persons`' attributes at all, the gate reports "could not verify" and skips by design, and the whole policy check is inert — so check the first apply's output for that message specifically.

supernaut lade till 1 incheckning

2026-08-01 14:27:39 +00:00

feat(kanidm): declare the whole account policy, and record why secrets cannot be

ci / ci (pull_request) Successful in 1m23s

Detaljer

ac81541c31

Closes #275. Finishes the half of that issue the earlier changes left open: the OAuth2 clients are
declared (#278/#280) and their scope maps gated (#281/#282), but the account policy covered exactly
one attribute and three comments in the role had gone stale enough to mislead.

## Account policy: all eight knobs, not one

`kanidm_account_policy` now lists every knob `kanidm group account-policy` exposes, with the
server-side fallback that applies when each is unset and a reason for the value. The drift gate
loops over the table instead of asserting `credential_type_minimum` alone.

The assertion is **symmetric**, which is the part that makes this more than documentation: a knob
declared `null` asserts the attribute is *absent* from `idm_all_persons`. So "we deliberately
inherit the server fallback" becomes a checked claim, and setting a knob by hand at a terminal is
drift. Still asserted rather than applied — kanidm-provision cannot express account policy, and a
playbook that force-set these would silently apply a security posture nobody reviewed.

Two findings from reading the Kanidm 1.10.4 source rather than the CLI help:

- **`authsession_expiry` unset does not mean 24 h. It means never.** An absent attribute resolves to
  `MAXIMUM_AUTH_SESSION_EXPIRY` (`u32::MAX`), not the 86400 s `DEFAULT_AUTH_SESSION_EXPIRY`, which is
  test-only (`server/lib/src/idm/accountpolicy.rs:27-29`). And `reset-auth-expiry`, whose help says
  "reset to its default value", is an HTTP `DELETE` on the attribute — it purges rather than
  restoring anything (`libs/client/src/group.rs:40-46`). So auth sessions effectively never expire
  today. Recorded as-is with the fix as a recommendation, not applied: bounding session lifetime for
  every person is a security change that deserves its own review, and the runbook carries the
  command and the note that the declaration must move with it.
- **`credential_type_minimum: mfa` is the one attribute the builtin sets** (`migration_data/dl14/
  groups.rs`, "MFA By Default"); absent it would fall back to `any`. Kept at `mfa` deliberately —
  a password ranks below it so Kanidm additionally demands TOTP, a passkey ranks above and satisfies
  it alone. Not raised to `passkey`: it would lock out every password-only person including the
  break-glass path, and there is no downgrade.

The readability guard changed too, and this one was a real hole. It gated on
`credential_type_minimum` being non-empty, so the gate skipped **silently** in the single case it
exists for: that attribute purged from the group, dropping every person to `any`. It now gates on
the attribute map being non-empty, which is also what distinguishes a denied read — Kanidm answers
those with HTTP 200 and a `null` body, so a status check proves nothing.

## Client secrets: not a deferred task, an upstream limit

The natural completion of "declare the clients" is to source their secrets from vault. That is not
available. Upstream Kanidm has no API to *set* an OAuth2 basic secret — only read it and reset it to
a random value — so kanidm-provision's `basicSecretFile` targets an endpoint that exists only in a
**patched kanidm server**; its README says "Only works when using the patch. Do not specify
otherwise!" and the failure is "Did you compile kanidm with the necessary patch?"
(`src/client.rs:396-426`). Carrying a fork of the identity provider is a different order of risk
from the small `entryManagedBy` patch we carry for the provisioning tool, so this stays undone and
is now written down as a limit rather than an omission.

Its consequence is recorded where it will be needed: on a rebuild, provisioning *creates* the
clients with fresh random secrets that will not match the vaulted ones, breaking Forgejo login, the
portal and Grafana SSO at once. The runbook has the read-back-and-revault sequence.

## `--no-auto-remove`: recommend keeping it, and say why

Declaring the full client set is what made dropping the flag conceivable. The recommendation is to
keep it, recorded rather than quietly acted on either way. Auto-remove is not scoped to OAuth2 — it
deletes orphaned groups and persons too, and neither the builtin `idm_*` privilege groups nor a
narrowed `forgejo_users` is safe under that. The flag is not what protects a declared entity anyway
(`"present": false` deletes regardless), and the leak that has actually bitten — a stale scope map
surviving a declaration change, #278 — is invisible to auto-remove and covered by the scope-map gate.

## Stale comments corrected

Three claims in the role were false and actively misleading to the next reader: that `forgejo` and
`grafana` are "DELIBERATELY ABSENT" from the declaration (#280 declared both), that
`systems.oauth2` "renders empty" (it renders four clients), and that the `--no-auto-remove` comment's
rationale is the untracked manual `forgejo` client.

## Verification

No production mutation; the gate is read-only by construction (reconciler token) and nothing here is
applied.

- `site.yml --syntax-check` clean; `ansible-lint roles/kanidm roles/caddy` reports the same 14
  pre-existing findings as `HEAD`, identical rule mix, none in changed lines.
- The state template renders and validates as JSON with the real defaults, still four clients, and
  `basicSecretFile` absent from the output.
- The drift gate was extracted from the role verbatim and driven against five mocked payloads:
  live-as-is passes all eight knobs; a purged `credential_type_minimum` fails; a downgrade to `any`
  fails; an undeclared `authsession_expiry` found set fails while the rest pass; and a denied read
  (`null` body) skips rather than false-failing.
- The declared client set was confirmed against production read-only via per-client OIDC discovery
  (200 for all four, 404 for a control id). That cannot enumerate clients, so an extra undeclared
  client remains unverifiable without idm_admin — stated in the runbook rather than glossed.

## Not done

The live values of the seven `null` knobs were not read from production: no vault access in this
environment means no reconciler token, and the `idm_admin` CLI session had expired. They are declared
from the Kanidm 1.10.4 builtin definition plus the `kanidm group get` output quoted in #275, which
shows `credential_type_minimum` as the only policy attribute set. The first apply's gate output is
the confirmation — if any of them is set live, it fails with the value, which is the intended
behaviour rather than a surprise.