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
5 Commits
Author SHA1 Message Date
jakman85 262c6e4a97 Merge branch 'main' into feat/keycloak-passkey-reconcile
gate / gate (pull_request) Successful in 2m56s
gate / conformance (pull_request) Has been skipped
2026-10-30 05:56:00 +00:00
jakman85andClaude Opus 5.5 6bb8af1d70 docs(decisions): handoff for the passkey reconciler review fixes
gate / gate (pull_request) Failing after 2m55s
gate / conformance (pull_request) Has been skipped
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
jakman85andClaude Opus 5.5 7c4eb439b2 feat(monitoring): alert when the keycloak passkey reconciler's Job fails
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]>
2026-10-29 21:41:15 -05:00
jakman85andClaude Opus 5.5 17eed6e8ae fix(keycloak-passkey): bind only on git's say-so; protect real sub-flows; screen the requirement PUT by body id
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]>
2026-10-29 21:41:14 -05:00
jakman85andClaude Opus 5 f5931944fa feat(keycloak): reconcile the passkey realm config via GitOps, the browser flow blue/green
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]>
2026-10-29 21:22:08 -05:00