Files
home-network-k3s-impl/decisions/2026-10-29-keycloak-passkey-reconcile.md
jakman85andClaude Opus 5.5 6bb8af1d70
gate / gate (pull_request) Failing after 2m55s
gate / conformance (pull_request) Has been skipped
docs(decisions): handoff for the passkey reconciler review fixes
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]>
2026-10-29 21:45:40 -05:00

11 KiB

date, branch, title
date branch title
2026-10-29 feat/keycloak-passkey-reconcile Passkey reconciler review fixes: bind only on git's say-so (rollback sticks), protect the real 26.0.6 sub-flows, screen the requirement PUT by body id, alert on failure

What changed

This branch carries the unmerged passkey reconciler (commit 36fcb7b, cherry-picked with its authorship, recorded in decisions/2026-10-18-keycloak-passkey-reconciler.md) plus the fixes for a review of it. Read that entry for the design; this one records what the review found and what changed.

1. HIGH: the bind was automatic, and rollback did not stick. Within one tick of merge the reconciler would build and BIND browser-mfa-<hash>, a login flow no human had tried. Worse, the README's rollback (rebind browser-mfa) was undone every 15 minutes: the next run found the verified candidate again and rebound it, and a hotfix inside the bound flow was "corrected" by building and binding an -r2.

  • New git-declared switch: bind-browser-flow in platform/keycloak/passkey-desired-state.yaml, a second key of the same ConfigMap, so it reaches the pod through the existing mount. Default "false". It is the one hand-edited line in a generated file: the renderer carries it through on --write, and --check (run by the L0 gate) refuses anything but the quoted strings "true" / "false". An unquoted true is a YAML boolean, which a ConfigMap rejects at apply time and which would have failed the whole platform-config Kustomization, realm import included.
  • "false": pass 3 builds and verifies the candidate (or reuses a verified one), logs WOULD BIND <alias>, exits 0 with KEYCLOAK_PASSKEY_RECONCILED_OK_PENDING_BIND, and never PUTs a changed browserFlow. That holds after an earlier bind too, so the operator's rebind sticks. If the bound flow is the candidate alias and has been hand-edited, it is left alone (no -r2 is built beside it).
  • "true": binds as before, and keeps asserting it. That is GitOps working; the README says so, and says not to hotfix the flow while it is "true".
  • The CronJob passes no flag. --no-bind still exists for a hand-run Job and can only narrow git's value. The L0 check asserts the CronJob carries no -- argument.
  • Rollback is now: a PR setting "false" (first), then one rebind write. README documents the order and why it matters, plus how to test the candidate on one client (a per-client account-console browser-flow override) before the go-live PR.

2. HIGH: the protected-subtree guard read the wrong field for sub-flows. Read from source at tag 26.0.6 (services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java, fetched from GitHub this session): recurseExecutions sets displayName = flowRef.getAlias() and flowId on a sub-flow row and never calls setAlias. alias is set only on authenticator rows, where it holds the bound config's alias. So the protected set was {bound} alone, and a write into browser-mfa-forms would not have been refused. The old stub returned alias on sub-flow rows, which is why all 216 assertions passed anyway.

  • New subflow_alias() reads displayName and cites the source. I did not resolve by flowId: getFlows() (same file) filters to isTopLevel(), so a nested sub-flow's id resolves to nothing through the flows list. A sub-flow row with no displayName makes the listing unreadable, which leaves the flow pass fail-closed.
  • The stub now returns rows shaped like 26.0.6's: sub-flow rows carry displayName, flowId and no alias/providerId. Authenticator rows carry a display type as displayName (not the provider id) and the config alias as alias. Nulls are omitted.
  • New assertions: after a real run, browser-mfa-forms and browser-mfa-conditional-otp are in the protected set, and an execution add or requirement write into them is refused. A missing displayName fails closed.

3. MEDIUM: PUT /flows/{alias}/executions was screened on the wrong field. updateExecutions resolves realm.getAuthenticationExecutionById(rep.getId()) without checking membership in {alias}. The same handler also renames the flow named by rep.getFlowId() when the body carries authenticationFlow + displayName. Now:

  • body["id"] is screened against the protected execution ids.
  • It must also be in _BUILD_EXECUTION_IDS: the ids read back from the flow this call just created, open only inside build_flow.
  • The config POST /executions/{id}/config is held to the same build scope.
  • The body may carry only id, requirement and priority.
  • Selftest cases cover each of these.

4. Stale docs on sensitive paths now describe what ships, including the switch: platform-config/kustomization.yaml, the chainsaw test header, the desired-state header and its renderer template, the renderer's own header and --check message, and the README.

5.

  • backoffLimit: 0: every failure is deterministic within a tick, and a retried build burns another of the 5 candidate aliases.
  • New alert KeycloakPasskeyReconcilerFailing in platform/monitoring/alerts.yaml (warning, for: 15m, kube_job_status_failed on job_name=~"keycloak-passkey-reconciler-.*"), the same shape as the sibling's. It has promtool tests in platform/monitoring/tests/keycloak-passkey-reconciler.test.yaml, in both directions plus "the sibling's failure is not this alert" and "one transient failure stays quiet". The alert ratchet in alert-rules-unit-tested would have refused it untested. A new runbook, runbooks/keycloak-passkey-reconciler.md, is projected by the runbooks generator.
  • The stale "Renovate does not manage plain image: tags" comment is corrected: renovate.json5 enables the kubernetes manager over platform/**. The image is pinned to python:3.13.15-alpine3.23@sha256:6438599575cca0d1df94aeee0d2ae088d4d8846eab554b2ee7784a3a6df0d516, the multi-arch index digest read from Docker Hub today. The tag is mutable: the sibling reconciler's pod on prod reports imageID ...@sha256:75f27d68... under the same tag, which is neither today's index nor today's amd64 manifest.

6. Hygiene. The ~160-line header that appeared in both the YAML and the script is now kept once, in the script, where it ships next to the code and its selftest. A short header above the ConfigMap points to it. Nothing else was restructured, apart from the build's second half moving into _finish_build() so the build scope can be opened and closed around it.

Sensitive paths touched: ci/** (renderer), conformance/** (L0 check, mutations, chainsaw), platform-config/kustomization.yaml (comment only). This needs the operator's /approve-override or a non-author review. I did not post one.

What I verified

  • Local toolbox gate, rc 3 (pinned toolbox via dev.sh, DEV_KUBECONFIG=/dev/null, ENGINE shim bind-mounting the main .git, run on the committed branch). Stages: spec-immutability pass (the spec repo resolved as a sibling), decisions-log pass, lint pass, l0 pass (including keycloak-passkey-reconciler, alert-rules-unit-tested and alert-runbooks-resolvable), render-golden pass, l1 pass, approval-policy pass (N/A offline; it is guaranteed to fail in CI). gitea-settings did not run (no SETTINGS_API_TOKEN locally) and is the only stage that did not run. L2/L3 conformance is skipped by design (no cluster). rc 3 is not green.

  • platform/monitoring/tests/run.sh platform/monitoring/tests/keycloak-passkey-reconciler.test.yaml in the toolbox: promtool check rules SUCCESS (54 rules), promtool test rules SUCCESS, promtool 3.14.0.

  • Selftest: 259 assertions pass (was 216), extracted from the shipped ConfigMap by the L0 check.

  • mutations.py: 40 of 40 injected regressions caught (was 30). The ten new ones each undo one fix:

    • sub-flows read by alias again;
    • an unnameable sub-flow tolerated;
    • the requirement PUT screened on its path only;
    • execution writes not held to the build tree;
    • extra body keys allowed;
    • the bind switch ignored in run();
    • the bind switch ignored in reconcile_flow();
    • an absent switch reading as true;
    • an invalid switch guessed at;
    • -r2 built beside a hand-edited bound flow. "Sub-flows by alias" is caught less cleanly (the selftest aborts on an unreadable listing rather than failing an assertion). It is still caught.
  • The renderer accepts "true" and "false" and refuses true (unquoted) and "yes". I checked this by hand-editing the committed file and running --check.

  • The 26.0.6 source for recurseExecutions, updateExecutions and getFlows was read directly (lines ~225-231, 658-724 and 747-806 of the file at that tag).

  • Read-only kubectl --context k3s-prod: Keycloak pods run quay.io/keycloak/keycloak:26.0.6 (2 replicas). The only CronJob in keycloak is keycloak-client-secret-reconciler, so the passkey reconciler is not live.

What I did NOT verify

  • Nothing has run against a real Keycloak. The stub's row shapes now follow the 26.0.6 source, but they are still my reading of that source, not a captured response. In particular, I have never seen a live executions listing to confirm displayName on a sub-flow row. If it differs, the failure is fail-closed (no build), not fail-open.
  • The candidate flow has never been tested in a browser. That is now a precondition of the go-live PR (README, "Testing the candidate flow before binding it"). Nobody has run those steps. I also have not checked that the per-client account-console override behaves as described on 26.0.6.
  • "Merging changes nothing users see" is true of login, but passes 1 and 2 still write the realm. They set the passwordless policy and enable the enrolment action. That action is enabled by default in 26.0.6, but I did not read the live realm, because no admin API calls were allowed. Pass 3 also creates the unbound candidate flow (realm DB writes, invisible to login).
  • Race in pass 1, not fixed: the policy PUT is a read-modify-write that carries browserFlow back from its own GET. An operator rebind landing between that GET and PUT would be reverted. The window is one HTTP round trip and only exists when the policy has drifted.
  • The alert has never seen a real series. The CronJob has never run, so kube_job_status_failed{job_name=~"keycloak-passkey-reconciler-.*"} does not exist yet. Its shape is copied from the sibling's rule, which does have live series.
  • The digest is of today's index. I did not pull it or run the image. Renovate's handling of a tag@digest pin under the kubernetes manager is from its documentation, not from a Renovate run here.
  • CI's own result is in the PR, not here. approval-policy is expected to fail until the operator overrides.

Follow-ups (out of scope here, deliberately not done)

  • A dedicated least-privilege service-account client for the reconciler, instead of the master-realm break-glass admin it shares with the client-secret reconciler.
  • A NetworkPolicy for the keycloak namespace.
  • Whether a passkey login should skip TOTP for platform-admins. This is the PR #72 design decision, currently "no TOTP after a passkey".
  • A KeycloakPasskeyReconcilerStale rule (no successful run in N hours), like the sibling's. Only the failure alert was in scope.