kanidm: OAuth2 clients and account policy are not declared in the provisioning state

supernaut kommenterad

2026-07-31 09:34:53 +00:00

Ägare

Two categories of Kanidm state are absent from the provisioning state, so live values are whatever
was set by hand or shipped as an upstream default. Neither is visible to a converge, and neither is
reviewable in git.

1. OAuth2 clients are not declared

roles/kanidm/templates/kanidm-state.json.j2 declares an empty map:

"systems": {
  "oauth2": {}
}

and the provisioning run passes --no-auto-remove (roles/kanidm/tasks/main.yml:355), with the
role's own comment acknowledging the gap:

Never delete entities (the manually-created forgejo OAuth2 client isn't tracked by this tool, but
empty systems.oauth2 is declared — be explicit).

So every OAuth2 client is created by hand via kanidm system oauth2 create and tracked only in the
runbook. Consequences:

A client created ad hoc persists indefinitely. No converge will remove it, because nothing
declares what the client set should be. An undeclared client is indistinguishable from an
intentional one.
The prod client inventory has no reviewable source. The runbook documents three
(forgejo, bitborg-web, grafana); confirming that is what actually exists requires querying
the live server.
Scope maps and redirect URIs are unreviewed. These decide which users see an application on
their Kanidm landing page and where authorization codes may be sent — exactly the properties that
benefit from review in a PR rather than being set once at a terminal.

Declaring clients means their secrets have to come from vault, so this is not a trivial change —
which is presumably why it was deferred. Worth doing deliberately rather than left implicit.

2. No account policy is declared

The state template declares only persons, groups, and systems. There is no account policy,
so every policy value is Kanidm's upstream default. Confirmed on the live server:

kanidm group get idm_all_persons
  class: account_policy
  credential_type_minimum: mfa

mfa is the upstream default and is a reasonable value — this is not a misconfiguration. The
problem is that it is not a recorded decision:

Nobody chose it, so nobody reviewed it against principle 4.
An upstream default can change on a Kanidm upgrade, silently altering the credential requirements
for every person, with no diff anywhere in this repo.
The same applies to the other policy knobs the CLI exposes — auth-expiry,
privilege-expiry, password-minimum-length, webauthn-attestation-ca-list,
allow-primary-cred-fallback, and the search limits.

Note when changing credential_type_minimum: the CLI ships reset-* subcommands for the other
policy attributes but not for this one, and Kanidm has treated downgrading it as a security
concern. Treat a change as close to one-way and decide the target value deliberately.

Done when

The set of OAuth2 clients that should exist in production is declared and reviewable, with their
scope maps and redirect URIs, and an undeclared client is visible as drift.
Account policy values are declared explicitly rather than inherited, so an upstream default change
shows up as a diff.

Two categories of Kanidm state are absent from the provisioning state, so live values are whatever was set by hand or shipped as an upstream default. Neither is visible to a converge, and neither is reviewable in git. ## 1. OAuth2 clients are not declared `roles/kanidm/templates/kanidm-state.json.j2` declares an empty map: ```json "systems": { "oauth2": {} } ``` and the provisioning run passes `--no-auto-remove` (`roles/kanidm/tasks/main.yml:355`), with the role's own comment acknowledging the gap: > Never delete entities (the manually-created forgejo OAuth2 client isn't tracked by this tool, but > empty `systems.oauth2` is declared — be explicit). So every OAuth2 client is created by hand via `kanidm system oauth2 create` and tracked only in the runbook. Consequences: - **A client created ad hoc persists indefinitely.** No converge will remove it, because nothing declares what the client set *should* be. An undeclared client is indistinguishable from an intentional one. - **The prod client inventory has no reviewable source.** The runbook documents three (`forgejo`, `bitborg-web`, `grafana`); confirming that is what actually exists requires querying the live server. - **Scope maps and redirect URIs are unreviewed.** These decide which users see an application on their Kanidm landing page and where authorization codes may be sent — exactly the properties that benefit from review in a PR rather than being set once at a terminal. Declaring clients means their secrets have to come from vault, so this is not a trivial change — which is presumably why it was deferred. Worth doing deliberately rather than left implicit. ## 2. No account policy is declared The state template declares only `persons`, `groups`, and `systems`. There is **no account policy**, so every policy value is Kanidm's upstream default. Confirmed on the live server: ``` kanidm group get idm_all_persons class: account_policy credential_type_minimum: mfa ``` `mfa` is the upstream default and is a *reasonable* value — this is not a misconfiguration. The problem is that it is not a recorded decision: - Nobody chose it, so nobody reviewed it against principle 4. - An upstream default can change on a Kanidm upgrade, silently altering the credential requirements for every person, with no diff anywhere in this repo. - The same applies to the other policy knobs the CLI exposes — `auth-expiry`, `privilege-expiry`, `password-minimum-length`, `webauthn-attestation-ca-list`, `allow-primary-cred-fallback`, and the search limits. Note when changing `credential_type_minimum`: the CLI ships `reset-*` subcommands for the other policy attributes but **not** for this one, and Kanidm has treated *downgrading* it as a security concern. Treat a change as close to one-way and decide the target value deliberately. ## Done when - The set of OAuth2 clients that should exist in production is declared and reviewable, with their scope maps and redirect URIs, and an undeclared client is visible as drift. - Account policy values are declared explicitly rather than inherited, so an upstream default change shows up as a diff.

supernaut lade till etiketterna

area/identity

area/security

ready-for-implementation

type

task

2026-07-31 09:35:01 +00:00

supernaut lade till detta till projektet Bitborg Infrastructure

2026-07-31 09:35:39 +00:00

supernaut kommenterad

2026-07-31 09:42:23 +00:00

Upphovsperson

Ägare

Checked what kanidm-provision v1.3.0 can actually express before implementing. The two halves of
this issue turn out to be very different problems.

Account policy CANNOT be declared — do not add it to the state file

Upstream src/state.rs:

pub struct State {
    pub groups: HashMap<String, Group>,
    pub persons: HashMap<String, Person>,
    pub systems: Systems,
}

pub struct Group {
    pub present: bool,
    pub members: Vec<String>,
    pub overwrite_members: bool,
    pub enable_unix: bool,
    pub gid_number: Option<u32>,
}

There is no account-policy support anywhere — not on Group, not on Person, not in Systems.
And since State has no #[serde(deny_unknown_fields)], an accountPolicy key would be parsed
and silently thrown away, producing a state file that looks like it declares the policy while
changing nothing.

That is precisely the failure this role already documents for the dead service-accounts block. So
the naive fix here is actively harmful: it would create a second false declaration.

Real options:

Carry a patch adding account-policy support, following the precedent already set for
entryManagedBy (d732602). Most consistent with how this repo handles upstream gaps, but it is
another patch to maintain.
A dedicated Ansible task invoking kanidm group account-policy credential-type-minimum ….
Needs a read-then-set to stay idempotent, since the CLI has setters only — kanidm group get
parses the current value.
Record the decision without automating it (an ADR or a commented var), accepting that drift
is undetected.

Given credential_type_minimum currently equals the upstream default (mfa) and there is no
reset-credential-type-minimum subcommand, option 2 or 3 is proportionate. Option 1 is only worth it
if more policy knobs get managed.

OAuth2 clients CAN be declared — including removal

pub struct Oauth2System {
    pub present: bool,                                  // false => removes the client
    pub public: bool,
    pub display_name: String,
    pub basic_secret_file: Option<String>,
    pub origin_url: StringOrStrings,
    pub origin_landing: String,
    pub enable_localhost_redirects: bool,
    pub allow_insecure_client_disable_pkce: bool,
    pub scope_maps: HashMap<String, Vec<String>>,
    pub supplementary_scope_maps: HashMap<String, Vec<String>>,
    pub claim_maps: HashMap<String, ClaimMap>,
    // …
}

Two consequences worth noting:

present: false removes a client declaratively. So the stray dev client can be retired through
a reviewed diff and a converge, rather than a manual kanidm system oauth2 delete — much better
given --no-auto-remove means nothing else will ever clean it up.
enable_localhost_redirects is a declared boolean, and scope_maps govern which groups see an
application. These are exactly the properties that should be reviewable in a PR rather than set once
at a terminal.

Declaring the three legitimate clients (forgejo, bitborg-web, grafana) additionally needs
basic_secret_file wired from vault, and a mistake there breaks SSO for the affected service. That
deserves its own change with a careful dry-run — it should not ride along with the dev-client removal.

Suggested split

Declare the stray dev client with present: false and let a converge remove it. Small, reviewable,
immediately valuable. Blocked on confirming the client's real id via
kanidm system oauth2 list — the display name is "Bitborg Portal (dev)" but the state key is the
client id.
Bring the three real clients under declaration, secrets from vault, with their scope maps and
redirect URIs explicit.
Decide account policy via option 2 or 3 above.

Checked what `kanidm-provision` v1.3.0 can actually express before implementing. The two halves of this issue turn out to be very different problems. ## Account policy CANNOT be declared — do not add it to the state file Upstream `src/state.rs`: ```rust pub struct State { pub groups: HashMap<String, Group>, pub persons: HashMap<String, Person>, pub systems: Systems, } pub struct Group { pub present: bool, pub members: Vec<String>, pub overwrite_members: bool, pub enable_unix: bool, pub gid_number: Option<u32>, } ``` There is **no** account-policy support anywhere — not on `Group`, not on `Person`, not in `Systems`. And since `State` has no `#[serde(deny_unknown_fields)]`, an `accountPolicy` key would be **parsed and silently thrown away**, producing a state file that *looks* like it declares the policy while changing nothing. That is precisely the failure this role already documents for the dead `service-accounts` block. So the naive fix here is actively harmful: it would create a second false declaration. Real options: 1. **Carry a patch** adding account-policy support, following the precedent already set for `entryManagedBy` (d732602). Most consistent with how this repo handles upstream gaps, but it is another patch to maintain. 2. **A dedicated Ansible task** invoking `kanidm group account-policy credential-type-minimum …`. Needs a read-then-set to stay idempotent, since the CLI has setters only — `kanidm group get` parses the current value. 3. **Record the decision without automating it** (an ADR or a commented var), accepting that drift is undetected. Given `credential_type_minimum` currently equals the upstream default (`mfa`) and there is **no** `reset-credential-type-minimum` subcommand, option 2 or 3 is proportionate. Option 1 is only worth it if more policy knobs get managed. ## OAuth2 clients CAN be declared — including removal ```rust pub struct Oauth2System { pub present: bool, // false => removes the client pub public: bool, pub display_name: String, pub basic_secret_file: Option<String>, pub origin_url: StringOrStrings, pub origin_landing: String, pub enable_localhost_redirects: bool, pub allow_insecure_client_disable_pkce: bool, pub scope_maps: HashMap<String, Vec<String>>, pub supplementary_scope_maps: HashMap<String, Vec<String>>, pub claim_maps: HashMap<String, ClaimMap>, // … } ``` Two consequences worth noting: - **`present: false` removes a client declaratively.** So the stray dev client can be retired through a reviewed diff and a converge, rather than a manual `kanidm system oauth2 delete` — much better given `--no-auto-remove` means nothing else will ever clean it up. - **`enable_localhost_redirects` is a declared boolean**, and `scope_maps` govern which groups see an application. These are exactly the properties that should be reviewable in a PR rather than set once at a terminal. Declaring the three legitimate clients (`forgejo`, `bitborg-web`, `grafana`) additionally needs `basic_secret_file` wired from vault, and a mistake there breaks SSO for the affected service. That deserves its own change with a careful dry-run — it should not ride along with the dev-client removal. ## Suggested split 1. Declare the stray dev client with `present: false` and let a converge remove it. Small, reviewable, immediately valuable. **Blocked on** confirming the client's real id via `kanidm system oauth2 list` — the display name is "Bitborg Portal (dev)" but the state key is the client id. 2. Bring the three real clients under declaration, secrets from vault, with their scope maps and redirect URIs explicit. 3. Decide account policy via option 2 or 3 above.

supernaut refererade till detta ärende från en incheckning

2026-07-31 10:07:49 +00:00

feat(kanidm): declare the account policy and gate on drift

supernaut refererade till detta ärende

2026-07-31 10:07:50 +00:00

feat(kanidm): declare the account policy and gate on drift #277

supernaut refererade till detta ärende från en incheckning

2026-07-31 10:25:53 +00:00

feat(kanidm): declare the account policy and gate on drift (#277)

supernaut refererade till detta ärende från en incheckning

2026-07-31 10:29:18 +00:00

fix(kanidm): declare the portal OAuth2 clients and drop the localhost origin from production