Home-Network/home-network-k3s-impl
2
0 Fork 0
Code Issues 2 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix(keycloak-passkey): passkey reconciler with review fixes — bind only on git's say-so (default false), protect real sub-flows, body-id screening, failure alert #82

Merged
jakman85 merged 5 commits from feat/keycloak-passkey-reconcile into main 2026-10-30 05:59:11 +00:00
Conversation 1 Commits 5 Files Changed 15
+6148 -15
gitea-admin commented 2026-10-30 02:49:08 +00:00
Owner
Copy link
Copy source

What this is

This PR has two parts:

  • The passkey reconciler: a CronJob that converges the homelab realm's WebAuthn passwordless policy and enrolment required action, and builds the declared browser-mfa login flow blue/green. It is commit f593194, cherry-picked with its original authorship; its design is in decisions/2026-10-18-keycloak-passkey-reconciler.md.
  • Fixes for every finding of a review of that commit, in the commits on top. Handoff: decisions/2026-10-29-keycloak-passkey-reconcile.md.

Merging this changes NOTHING about how anyone logs in

bind-browser-flow in platform/keycloak/passkey-desired-state.yaml ships as "false". With that setting, the reconciler never writes the realm's browserFlow:

  • It builds the candidate flow browser-mfa-<hash> and verifies it by readback.
  • It logs WOULD BIND browser-mfa-<hash> and exits 0.
  • The realm stays bound to browser-mfa, exactly as today.

It does still write some things that no login step uses:

  • Pass 1: the passwordless policy.
  • Pass 2: the webauthn-register-passwordless enrolment action. It is enabled by default in 26.0.6. I could not confirm its live value, because no admin API calls were allowed.
  • Pass 3: it creates the unbound candidate flow.

Going live is a later, one-line PR made by the operator, after testing the candidate in a browser: bind-browser-flow: "true". The test procedure is in platform/keycloak/README.md, "Testing the candidate flow before binding it":

  1. Find the candidate in the Job log (WOULD BIND browser-mfa-<hash>).
  2. Admin console → Clients → account-console → Advanced → Authentication flow overrides → Browser Flow = the candidate. This is temporary.
  3. Password fallback. Open a private window at https://keycloak.<base domain>/realms/homelab/account as a user with no passkey. Expect a username screen, then the passkey / password choice. Sign in with the password.
  4. Platform-admin TOTP on the password branch. Repeat as a platform-admins member. A TOTP prompt must appear after the password. A missing prompt or a 403 means stop, do not bind.
  5. Passkey enrolment. In the account console: Signing in → Passkey → Set up. A browser or OS passkey prompt must appear. If it does not, the RP ID is wrong.
  6. Passkey login. Sign out and sign in again: username, then passkey. You should be signed in with no TOTP prompt.
  7. Remove the override.
  8. Open the PR setting bind-browser-flow: "true". The next run binds the same candidate you tested.

Rollback: first a PR setting bind-browser-flow: "false". Then one rebind write to the previous flow (browser-mfa; the Job log of the bind prints it). With the switch at "false", the reconciler never undoes that rebind.

Review findings and how each was fixed

# Finding Fix Proof
1 HIGH: the bind was automatic, and rollback did not stick. An operator rebind was undone every 15 min, and a hotfix in the bound flow spawned an -r2. New git switch bind-browser-flow, default "false". It is a second key of the desired-state ConfigMap and is read through the same mount. The renderer carries the hand-edited line through, and --check refuses anything but quoted "true"/"false". With "false" the reconciler never PUTs a changed browserFlow, including after an earlier bind, and it leaves a hand-edited bound candidate alone. The CronJob passes no flag; --no-bind can only narrow the switch. Selftest §15: bind=false never binds and still converges passes 1–2; repeat runs make zero writes; after a bind=true run plus an operator rollback, three bind=false runs make no write and the rollback sticks; a hotfix is left alone with no -r2; an absent switch means no bind; an invalid switch is fatal before any write. Four new mutations.
2 HIGH: the protected-subtree guard read alias on sub-flow rows. In 26.0.6, recurseExecutions sets displayName = flowRef.getAlias() and flowId, and never alias. The stub hid this. subflow_alias() reads displayName, citing AuthenticationManagementResource.java at tag 26.0.6. flowId cannot be used: getFlows() lists top-level flows only. A sub-flow row without displayName keeps the pass fail-closed. The stub now returns 26.0.6-shaped rows (no alias on sub-flows; the config alias on authenticator rows). Selftest §14: after a real run, writes into browser-mfa-forms and browser-mfa-conditional-otp are refused; a missing displayName fails closed. Mutations: "sub-flows read by alias again" and "unnameable sub-flow tolerated".
3 MEDIUM: PUT /flows/{alias}/executions was screened only by the path alias. updateExecutions resolves body.id realm-wide. body.id must be outside the protected ids and inside _BUILD_EXECUTION_IDS (read back from the flow this call just built; open only inside build_flow). The config POST is held to the same rule. The body may carry only id, requirement and priority, because the same handler renames any flow named by flowId. Selftest §5b′: a bound execution id under a candidate path, an id outside both sets, no id, and extra flowId/displayName keys are all refused; an in-build id is allowed; everything is refused when no build is running. Three mutations.
4 MEDIUM: stale "audit-only" docs on sensitive paths. Rewritten to describe what ships, including the switch: platform-config/kustomization.yaml, the chainsaw test header, the desired-state header and its renderer template, the renderer header and --check text, and the README. grep for the stale phrases finds nothing.
5 LOW-MEDIUM: backoffLimit: 2; no alert; a stale Renovate comment. backoffLimit: 0. New KeycloakPasskeyReconcilerFailing alert (warning, for: 15m, on kube_job_status_failed{job_name=~"keycloak-passkey-reconciler-.*"}) with promtool tests and a new runbook. The comment is corrected and the image pinned by index digest. promtool tests cover four cases: quiet, fires after 15m, quiet on the sibling's failure, quiet on one transient failure. L0 asserts backoffLimit == 0.
6 Hygiene: the ~160-line header was duplicated. Kept once, in the script, with a short pointer in the YAML.

Selftest: 259 assertions (was 216). mutations.py: 40/40 injected regressions caught (was 30).

Gate

Local toolbox gate (dev.sh, DEV_KUBECONFIG=/dev/null, ENGINE shim, committed tree): rc 3. That is not green.

  • Passed: spec-immutability, decisions-log, lint, l0 (including keycloak-passkey-reconciler, alert-rules-unit-tested, alert-runbooks-resolvable), render-golden, l1, approval-policy (N/A offline).
  • gitea-settings is the only stage that did not run (no token locally).
  • L2/L3 is skipped by design (no cluster).

Sensitive paths: needs the operator's override

This PR touches ci/** (the renderer), conformance/** (L0 check, mutations, chainsaw) and platform-config/kustomization.yaml (comment only). approval-policy will therefore fail until the operator reviews it or posts /approve-override <reason> as a PR review. I have not posted one and will not.

Not verified

  • Nothing has run against a real Keycloak. The sub-flow row shape comes from reading the 26.0.6 source, not from a live response. If it is wrong, the pass fails closed rather than open.
  • The candidate flow has never been tested in a browser. That is now the precondition for the go-live PR.
  • The alert has never seen a real series, because the CronJob has never run.

Out of scope, listed as follow-ups in the decisions entry:

  • a least-privilege service-account client
  • a keycloak NetworkPolicy
  • whether passkey users should skip TOTP
  • a "Stale" companion alert

🤖 Generated with Claude Code

## What this is This PR has two parts: - The passkey reconciler: a CronJob that converges the `homelab` realm's WebAuthn passwordless policy and enrolment required action, and builds the declared `browser-mfa` login flow blue/green. It is commit `f593194`, cherry-picked with its original authorship; its design is in `decisions/2026-10-18-keycloak-passkey-reconciler.md`. - Fixes for every finding of a review of that commit, in the commits on top. Handoff: `decisions/2026-10-29-keycloak-passkey-reconcile.md`. ## Merging this changes NOTHING about how anyone logs in `bind-browser-flow` in `platform/keycloak/passkey-desired-state.yaml` ships as **`"false"`**. With that setting, the reconciler never writes the realm's `browserFlow`: - It builds the candidate flow `browser-mfa-<hash>` and verifies it by readback. - It logs `WOULD BIND browser-mfa-<hash>` and exits 0. - The realm stays bound to `browser-mfa`, exactly as today. It does still write some things that no login step uses: - **Pass 1:** the passwordless policy. - **Pass 2:** the `webauthn-register-passwordless` enrolment action. It is enabled by default in 26.0.6. I could not confirm its live value, because no admin API calls were allowed. - **Pass 3:** it creates the unbound candidate flow. **Going live is a later, one-line PR made by the operator, after testing the candidate in a browser:** `bind-browser-flow: "true"`. The test procedure is in `platform/keycloak/README.md`, "Testing the candidate flow before binding it": 1. Find the candidate in the Job log (`WOULD BIND browser-mfa-<hash>`). 2. Admin console → Clients → `account-console` → Advanced → Authentication flow overrides → Browser Flow = the candidate. This is temporary. 3. **Password fallback.** Open a private window at `https://keycloak.<base domain>/realms/homelab/account` as a user with no passkey. Expect a username screen, then the passkey / password choice. Sign in with the password. 4. **Platform-admin TOTP on the password branch.** Repeat as a `platform-admins` member. A TOTP prompt **must** appear after the password. A missing prompt or a 403 means stop, do not bind. 5. **Passkey enrolment.** In the account console: Signing in → Passkey → Set up. A browser or OS passkey prompt must appear. If it does not, the RP ID is wrong. 6. **Passkey login.** Sign out and sign in again: username, then passkey. You should be signed in with no TOTP prompt. 7. Remove the override. 8. Open the PR setting `bind-browser-flow: "true"`. The next run binds the same candidate you tested. **Rollback:** first a PR setting `bind-browser-flow: "false"`. Then one rebind write to the previous flow (`browser-mfa`; the Job log of the bind prints it). With the switch at `"false"`, the reconciler never undoes that rebind. ## Review findings and how each was fixed | # | Finding | Fix | Proof | |---|---|---|---| | 1 | **HIGH:** the bind was automatic, and rollback did not stick. An operator rebind was undone every 15 min, and a hotfix in the bound flow spawned an `-r2`. | New git switch `bind-browser-flow`, default `"false"`. It is a second key of the desired-state ConfigMap and is read through the same mount. The renderer carries the hand-edited line through, and `--check` refuses anything but quoted `"true"`/`"false"`. With `"false"` the reconciler never PUTs a changed `browserFlow`, including after an earlier bind, and it leaves a hand-edited bound candidate alone. The CronJob passes no flag; `--no-bind` can only narrow the switch. | Selftest §15: bind=false never binds and still converges passes 1–2; repeat runs make zero writes; after a bind=true run plus an operator rollback, three bind=false runs make no write and the rollback sticks; a hotfix is left alone with no `-r2`; an absent switch means no bind; an invalid switch is fatal before any write. Four new mutations. | | 2 | **HIGH:** the protected-subtree guard read `alias` on sub-flow rows. In 26.0.6, `recurseExecutions` sets `displayName = flowRef.getAlias()` and `flowId`, and never `alias`. The stub hid this. | `subflow_alias()` reads `displayName`, citing `AuthenticationManagementResource.java` at tag 26.0.6. `flowId` cannot be used: `getFlows()` lists top-level flows only. A sub-flow row without `displayName` keeps the pass fail-closed. The stub now returns 26.0.6-shaped rows (no `alias` on sub-flows; the config alias on authenticator rows). | Selftest §14: after a real run, writes into `browser-mfa-forms` and `browser-mfa-conditional-otp` are refused; a missing `displayName` fails closed. Mutations: "sub-flows read by `alias` again" and "unnameable sub-flow tolerated". | | 3 | **MEDIUM:** `PUT /flows/{alias}/executions` was screened only by the path alias. `updateExecutions` resolves `body.id` realm-wide. | `body.id` must be outside the protected ids **and** inside `_BUILD_EXECUTION_IDS` (read back from the flow this call just built; open only inside `build_flow`). The config POST is held to the same rule. The body may carry only `id`, `requirement` and `priority`, because the same handler renames any flow named by `flowId`. | Selftest §5b′: a bound execution id under a candidate path, an id outside both sets, no id, and extra `flowId`/`displayName` keys are all refused; an in-build id is allowed; everything is refused when no build is running. Three mutations. | | 4 | **MEDIUM:** stale "audit-only" docs on sensitive paths. | Rewritten to describe what ships, including the switch: `platform-config/kustomization.yaml`, the chainsaw test header, the desired-state header and its renderer template, the renderer header and `--check` text, and the README. | grep for the stale phrases finds nothing. | | 5 | **LOW-MEDIUM:** `backoffLimit: 2`; no alert; a stale Renovate comment. | `backoffLimit: 0`. New `KeycloakPasskeyReconcilerFailing` alert (warning, `for: 15m`, on `kube_job_status_failed{job_name=~"keycloak-passkey-reconciler-.*"}`) with promtool tests and a new runbook. The comment is corrected and the image pinned by index digest. | promtool tests cover four cases: quiet, fires after 15m, quiet on the sibling's failure, quiet on one transient failure. L0 asserts `backoffLimit == 0`. | | 6 | Hygiene: the ~160-line header was duplicated. | Kept once, in the script, with a short pointer in the YAML. | | Selftest: **259 assertions** (was 216). `mutations.py`: **40/40** injected regressions caught (was 30). ## Gate Local toolbox gate (`dev.sh`, `DEV_KUBECONFIG=/dev/null`, ENGINE shim, committed tree): **rc 3**. That is **not** green. - Passed: spec-immutability, decisions-log, lint, l0 (including `keycloak-passkey-reconciler`, `alert-rules-unit-tested`, `alert-runbooks-resolvable`), render-golden, l1, approval-policy (N/A offline). - `gitea-settings` is the only stage that did not run (no token locally). - L2/L3 is skipped by design (no cluster). ## Sensitive paths: needs the operator's override This PR touches `ci/**` (the renderer), `conformance/**` (L0 check, mutations, chainsaw) and `platform-config/kustomization.yaml` (comment only). `approval-policy` will therefore fail until the operator reviews it or posts `/approve-override <reason>` as a PR review. I have not posted one and will not. ## Not verified - Nothing has run against a real Keycloak. The sub-flow row shape comes from reading the 26.0.6 source, not from a live response. If it is wrong, the pass fails closed rather than open. - The candidate flow has never been tested in a browser. That is now the precondition for the go-live PR. - The alert has never seen a real series, because the CronJob has never run. Out of scope, listed as follow-ups in the decisions entry: - a least-privilege service-account client - a `keycloak` NetworkPolicy - whether passkey users should skip TOTP - a "Stale" companion alert 🤖 Generated with [Claude Code](https://claude.com/claude-code)
gitea-admin added 4 commits 2026-10-30 02:49:08 +00:00
feat(keycloak): reconcile the passkey realm config via GitOps, the browser flow blue/green f5931944fa 
PR #72 declared the WebAuthn passwordless policy and a restructured browser-mfa
flow in the realm import. That declaration could not take effect: a
KeycloakRealmImport is a ONE-SHOT, and prod's reads Done=True / Started=False, so
every realm-level field and flow declared after the first import is inert -- in
git, applied by Flux, reported Ready, reaching the realm never. Merging it changed
nothing about how anyone logs in, and nothing anywhere said so. Same failure class
as the 2026-09-06 client-secret incident, third field over.

A SIBLING reconciler converges all three declared things every 15 minutes. It is a
sibling, not a second pass of client-secret-reconciler.yaml, so that reconciler's
allow-list -- and its guarantee that prod's real user and the media-users /
platform-admins / viewers groups are structurally unreachable -- stays literally
true. Its selftest still passes unchanged.

  pass 1  webAuthnPolicyPasswordless* -- strict read-modify-write of
          GET/PUT /admin/realms/homelab, readback-verified.
  pass 2  webauthn-register-passwordless -- strict read-modify-write of ONE
          literally-named alias; enabled, defaultAction false so enrolment stays
          opt-in.
  pass 3  the browser flow -- BLUE/GREEN.

PASS 3 NEVER MUTATES THE FLOW USERS ARE LOGGING IN THROUGH. Converging a
five-flow, ten-execution tree needs create-subflow, create-execution,
set-requirement and bind-config as SEPARATE untransacted calls; doing that to the
BOUND flow means a run dying between them leaves a half-built login path for every
SSO app and kubectl, and this repo has had a total SSO outage from a realm change.
So instead:

  1. BUILD under a NEW, unused alias (declared alias + a hash of the desired
     shape, applied to EVERY alias in the tree -- reusing browser-mfa-forms would
     mean writing INTO the bound tree). A failure leaves an orphaned, unbound flow
     and zero user-visible effect.
  2. VERIFY by readback -- levels, ordering, authenticators, requirements, and the
     conditional-OTP config CONTENTS -- before anything is bound, so the
     regression that once 403'd every platform-admin is caught BEFORE it bites.
  3. BIND with ONE atomic PUT setting browserFlow.
  4. ROLLBACK is one write; no DELETE exists anywhere in the allow-list, so the old
     flow survives as the rollback target.

The widened allow-list is made safe structurally. It cannot say "but not the bound
flow" -- its write entries are alias-parameterised -- so a second guard screens
every write against the live browserFlow's WHOLE subtree (sub-flow aliases and
execution ids) before a socket opens, FAIL-CLOSED until that binding has been
read. The create-flow endpoint carries no alias in its path, so it is screened on
its request BODY; the selftest caught that it otherwise escaped the guard.

Idempotence is load-bearing: a rebuild-and-reflip every 15 minutes would churn the
login path on a schedule. The bound flow is compared STRUCTURALLY and a match
produces zero writes and no new flow. Sub-flow and config ALIASES are deliberately
not compared -- a built flow carries a hash suffix, so comparing labels would make
it permanently unrecognisable as converged.

Single source of truth: the desired state is GENERATED from the realm import by
ci/render-keycloak-passkey-policy.py and gated with --check, so the RP ID -- baked
into every credential at enrolment and never re-keyable -- is declared once. The
pod keeps automountServiceAccountToken: false and reads a mounted ConfigMap, so no
rbac.yaml is added anywhere.

Read from Keycloak 26.0.6 source before being relied on: updateRealm
(DefaultExportImportManager 725-894) makes no getUsers/getClients/getGroups/
getRoles/getRequiredActions/getAuthenticationFlows call; it DOES read
getBrowserFlow(), which is what makes the atomic bind possible and why the policy
write pins it; the passwordless policy is applied all-or-nothing only when
RpEntityName is non-empty, with defaults substituted for omitted keys; and
updateExecutions compares rep.getPriority(), so a body omitting `priority` would
silently reshuffle the whole execution order.

Gate coverage: a new whole-repo L0 check extracts the script and runs its
--selftest (216 assertions, including a build killed at four different points each
asserting the old flow is still bound and the bound subtree was never written),
plus mutations.py (30 injected regressions, all caught) proving the selftest can
fail. An earlier iteration of this work only AUDITED the flow and left the
restructure manual; decisions/2026-10-18-keycloak-passkey-reconciler.md records
that design and why it was reconsidered.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
fix(keycloak-passkey): bind only on git's say-so; protect real sub-flows; screen the requirement PUT by body id 17eed6e8ae 
Review findings on the passkey reconciler, fixed:

1. HIGH - the bind was automatic and rollback did not stick. A git-declared switch,
   `bind-browser-flow` in passkey-desired-state.yaml, now decides whether pass 3 may
   PUT browserFlow. Default "false": build + verify the candidate (or reuse a verified
   one), log WOULD BIND <alias>, never touch the binding - including after an earlier
   bind, so an operator's rollback sticks, and a hand-edited bound candidate is left
   alone rather than shadowed by an -r2 build. "true" binds as before. The renderer
   carries the one hand-edited line through and --check refuses anything but the
   quoted strings "true"/"false". --no-bind can only narrow it.

2. HIGH - the protected-subtree guard read `alias` on sub-flow rows. Keycloak 26.0.6
   AuthenticationManagementResource.recurseExecutions sets displayName =
   flowRef.getAlias() and flowId on a sub-flow row and never sets alias (alias is the
   CONFIG alias on authenticator rows), so the protected set was only the top-level
   alias. Sub-flows are now named by displayName (getFlows() lists top-level flows
   only, so flowId cannot be resolved through it); a sub-flow row without one leaves
   the subtree undetermined and the flow pass fail-closed. The selftest stub now
   returns 26.0.6-shaped rows, and a real run is asserted to refuse writes into
   browser-mfa-forms and browser-mfa-conditional-otp.

3. MEDIUM - PUT /flows/{alias}/executions was screened on the path alias only, but
   updateExecutions resolves the execution realm-wide by body id. body.id is now
   screened against the protected ids AND required to be in the tree this run is
   building (recorded from the build's own readback); the config POST is held to the
   same build scope; and the body may carry only id/requirement/priority (the same
   handler renames any flow named by flowId).

4. Stale "audit-only" docs rewritten (platform-config kustomization, chainsaw test,
   desired-state header and its renderer template, README) to describe what ships.

5. backoffLimit 0 (a retried deterministic build failure burns the candidate-alias
   budget); the stale "Renovate does not manage image: tags" comment corrected and the
   image pinned by index digest.

6. The ~160-line header duplicated in the YAML and the script is now kept once, in the
   script, with a short pointer above it.

Selftest: 259 assertions (was 216). mutations.py: 40 injected regressions (was 30),
including one per finding above, all caught.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
feat(monitoring): alert when the keycloak passkey reconciler's Job fails 7c4eb439b2 
KeycloakPasskeyReconcilerFailing (warning, for 15m) on
kube_job_status_failed{namespace="keycloak", job_name=~"keycloak-passkey-reconciler-.*"},
following the sibling client-secret reconciler's rule. promtool unit tests pin it in
both directions: quiet when succeeding, firing after 15m on a failed Job, quiet on the
SIBLING's failure, and quiet on a single transient failure. Runbook added and projected
by the runbooks ConfigMap generator.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
docs(decisions): handoff for the passkey reconciler review fixes
gate / gate (pull_request) Failing after 2m55s
Details
gate / conformance (pull_request) Has been skipped
Details
6bb8af1d70 
Also states in bind_browser_flow()'s docstring that it is reached only when git's
bind-browser-flow is "true".

Co-Authored-By: Claude Opus 5.5 <[email protected]>
jakman85 added 1 commit 2026-10-30 05:56:02 +00:00
Merge branch 'main' into feat/keycloak-passkey-reconcile
gate / gate (pull_request) Successful in 2m56s
Details
gate / conformance (pull_request) Has been skipped
Details
262c6e4a97
jakman85 approved these changes 2026-10-30 05:57:15 +00:00
jakman85 merged commit dfc432c6ea into main 2026-10-30 05:59:11 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
jakman85
Labels
Clear labels
agent
automated-update
blast-radius
breaking
ci-tooling
dependencies
needs-review
own-image
range-widen
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
gitea-admin jakman85 (Keith Jackson) pm-agent renovate-bot
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Home-Network/home-network-k3s-impl#82