From 17eed6e8ae9d9be1ec1ab63f7e972ff68b29a5d1 Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Thu, 29 Oct 2026 21:41:14 -0500 Subject: [PATCH] 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 , 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 --- ci/render-keycloak-passkey-policy.py | 80 +- .../chainsaw-test.yaml | 51 +- .../keycloak-passkey-reconciler/check.sh | 54 +- .../keycloak-passkey-reconciler/mutations.py | 82 +- platform-config/kustomization.yaml | 11 +- platform/keycloak/README.md | 101 ++- platform/keycloak/passkey-desired-state.yaml | 28 +- platform/keycloak/passkey-reconciler.yaml | 796 ++++++++++++------ 8 files changed, 893 insertions(+), 310 deletions(-) diff --git a/ci/render-keycloak-passkey-policy.py b/ci/render-keycloak-passkey-policy.py index b99c50a..6bd1f26 100644 --- a/ci/render-keycloak-passkey-policy.py +++ b/ci/render-keycloak-passkey-policy.py @@ -18,7 +18,18 @@ # keycloak-realm-import.yaml declares are INERT. They are in git, Flux applied # them, and they reach the realm never. platform/keycloak/passkey-reconciler.yaml # is the level-triggered loop that closes that; this renderer supplies the DESIRED -# STATE it converges towards (and, for the flow, audits against). +# STATE it converges towards. +# +# THE ONE VALUE THAT IS NOT RENDERED: `bind-browser-flow`, git's switch for whether the +# reconciler may BIND its verified candidate browser flow (one atomic PUT of the +# realm's browserFlow). It is an operator DECISION -- "I have tested the candidate +# flow in a browser; make it the login path" -- not a fact derivable from the realm +# import, so it cannot be generated. It is instead CARRIED THROUGH from the committed +# file: --write preserves it, --check accepts any value of it that is exactly the +# string "true" or "false" and rejects everything else (an unquoted YAML true is a +# bool, which a ConfigMap rejects at apply time -- failing the whole platform-config +# Kustomization -- so it is refused HERE, at merge time). An absent file or key +# renders as "false". Going live is therefore a one-line PR flipping that line. # # THE CONSTRAINT THAT SHAPES IT: the reconciler pod runs with # automountServiceAccountToken: false, exactly like its client-secret sibling. It @@ -148,6 +159,33 @@ PASSKEY_AUTHENTICATOR = "webauthn-authenticator-passwordless" ENROL_ACTION = "webauthn-register-passwordless" +BIND_KEY = "bind-browser-flow" +BIND_VALUES = ("true", "false") + + +def committed_bind(root: Path) -> str: + """The operator's bind-browser-flow value from the COMMITTED file, or "false". + + This is the only input read from the output file, and it is validated, not trusted: + anything other than the exact strings "true" / "false" is a hard failure.""" + target = root / OUT_FILE + if not target.is_file(): + return "false" + docs = [d for d in yaml.safe_load_all(target.read_text(encoding="utf-8")) if d] + cm = next((d for d in docs if d.get("kind") == "ConfigMap"), None) or {} + data = cm.get("data") or {} + if BIND_KEY not in data: + return "false" + v = data[BIND_KEY] + if not isinstance(v, str) or v not in BIND_VALUES: + raise SystemExit( + "FATAL: %s data.%s is %r. It must be exactly the QUOTED string \"true\" or " + "\"false\": an unquoted true/false is a YAML boolean, which a ConfigMap rejects " + "at apply time and which would fail the whole platform-config Kustomization" + % (OUT_FILE, BIND_KEY, v)) + return v + + def render(root: Path) -> str: """Return the full text of passkey-desired-state.yaml as it should be committed.""" docs = list(yaml.safe_load_all((root / IMPORT_FILE).read_text())) @@ -323,12 +361,13 @@ def render(root: Path) -> str: body = json.dumps(want, indent=2, sort_keys=True, ensure_ascii=False) indented = "\n".join((" " + ln) if ln else "" for ln in body.splitlines()) - return HEADER + indented + "\n" + return (HEADER + BIND_BLOCK % (BIND_KEY, committed_bind(root)) + + " %s: |\n" % DATA_KEY + indented + "\n") HEADER = """--- # ============================================================================= -# GENERATED FILE — DO NOT EDIT BY HAND. +# GENERATED FILE — DO NOT EDIT BY HAND, except the one bind-browser-flow line below. # # source: platform/keycloak/keycloak-realm-import.yaml (spec.realm) # regenerate: python3 ci/render-keycloak-passkey-policy.py --write @@ -338,8 +377,14 @@ HEADER = """--- # WHAT THIS IS: the DESIRED STATE for the `homelab` realm's PASSKEY configuration — # the WebAuthn passwordless policy, the enrolment required action, and the declared # browser authentication flow tree. platform/keycloak/passkey-reconciler.yaml mounts -# it and (a) converges the policy and the required action onto it, (b) AUDITS the -# live browser flow against it and alarms on divergence without rewriting it. +# it and (a) converges the policy and the required action onto it, (b) builds the +# declared flow tree BLUE/GREEN under a new alias and verifies it by readback, never +# writing the flow users are logging in through, and (c) BINDS that verified flow as +# the realm's browserFlow ONLY when `bind-browser-flow` below is "true". +# +# EVERYTHING HERE IS GENERATED EXCEPT ONE LINE: `bind-browser-flow`. That is an +# operator decision, carried through by the renderer from this committed file (see +# ci/render-keycloak-passkey-policy.py). It defaults to "false". # # WHY IT IS A SEPARATE OBJECT: the realm import is a ONE-SHOT that never re-runs # once Done=True, so a realm-level field or a flow edited in git never reaches the @@ -379,8 +424,25 @@ metadata: app.kubernetes.io/name: %s app.kubernetes.io/part-of: keycloak data: - %s: | -""" % (CONFIGMAP_NAME, NAMESPACE, CONFIGMAP_NAME, DATA_KEY) +""" % (CONFIGMAP_NAME, NAMESPACE, CONFIGMAP_NAME) + +BIND_BLOCK = """ # --------------------------------------------------------------------------- + # THE BIND SWITCH -- the ONLY hand-edited line in this file. Exactly "true" or + # "false", QUOTED. + # + # "false" the reconciler builds and verifies the candidate browser flow and + # logs `WOULD BIND `, but NEVER changes the realm's browserFlow. + # Whatever the realm is bound to -- including a rollback -- is left alone. + # "true" it binds the verified candidate in one atomic PUT, and keeps the + # realm bound to it: a binding changed by hand is changed back. + # + # GO LIVE: test the candidate in a browser first (platform/keycloak/README.md, + # "Passkeys"), then a PR setting this to "true". + # ROLL BACK: a PR setting this to "false" FIRST (or the next run rebinds), then + # one write setting browserFlow back to the previous alias from the Job log. + # --------------------------------------------------------------------------- + %s: "%s" +""" def main() -> int: @@ -410,8 +472,8 @@ def main() -> int: print("passkey-desired-state.yaml is in sync with %s" % IMPORT_FILE) return 0 print("CHECK FAIL: %s has DRIFTED from %s — the reconciler would converge the " - "realm on a passkey policy the realm import does not declare, and audit " - "the login flow against a tree nobody authored.\n" + "realm on a passkey policy the realm import does not declare, and build " + "the login flow from a tree nobody authored.\n" "Regenerate with: python3 ci/render-keycloak-passkey-policy.py --write\n" % (OUT_FILE, IMPORT_FILE), file=sys.stderr) for line in difflib.unified_diff(have.splitlines(True), want.splitlines(True), diff --git a/conformance/tests/l0-static/keycloak-passkey-reconciler/chainsaw-test.yaml b/conformance/tests/l0-static/keycloak-passkey-reconciler/chainsaw-test.yaml index 7d87164..cc6073c 100644 --- a/conformance/tests/l0-static/keycloak-passkey-reconciler/chainsaw-test.yaml +++ b/conformance/tests/l0-static/keycloak-passkey-reconciler/chainsaw-test.yaml @@ -1,6 +1,7 @@ # ============================================================================= # L0 STATIC — the passkey configuration is derived from one declaration, continuously -# reconciled, and the browser login flow is auditable but UNWRITABLE +# reconciled, and the browser login flow converges BLUE/GREEN: the BOUND flow is +# unwritable, and binding a new one is git's explicit decision (bind-browser-flow) # Catalog: docs/14 L0 row ("Pre-apply, in CI") — runs WITHOUT a cluster. # # R-IDP-03 (identity config is declarative) and R-IDP-04 (MFA available) are both @@ -25,25 +26,30 @@ # import declares NO requiredActions block (26.0.6 treats it as REPLACE, which would # delete CONFIGURE_TOTP and lock out every platform-admin); the enrolment action is # enabled but NOT a defaultAction, so enrolment stays opt-in; the reconciler carries -# no ServiceAccount token, mounts only what it needs, and contains no dollar sign; -# the endpoint allow-list contains NO write for any authentication flow, execution -# or authenticator config, and NO required-action write for any alias other than the -# one named literally; the SIBLING client-secret reconciler's allow-list is NOT -# widened; and the reconciler's own `--selftest` (extracted from the shipped -# ConfigMap and executed) proves compare-then-write, idempotence, strict -# read-modify-write, the RpEntityName gate, flow-is-unwritable, the conditional-OTP -# regression and fail-closed — with mutations.py proving that selftest can FAIL. +# no ServiceAccount token, mounts only what it needs, passes no mode flag, and +# contains no dollar sign; the endpoint allow-list contains NO delete, NO +# single-execution or config-edit write, and NO required-action write for any alias +# other than the one named literally; the protected-subtree guard exists, is wired +# into the single request choke point and is fail-closed; the SIBLING client-secret +# reconciler's allow-list is NOT widened; and the reconciler's own `--selftest` +# (extracted from the shipped ConfigMap and executed) proves compare-then-write, +# idempotence, strict read-modify-write, the RpEntityName gate, the bound flow and +# its sub-flows unwritable, the git bind switch and a rollback that sticks, the +# conditional-OTP regression and fail-closed — with mutations.py proving that +# selftest can FAIL. # -# WHY THE FLOW IS AUDITED AND NOT CONVERGED, asserted here rather than only argued: -# converging a five-flow, ten-execution restructure needs create-subflow, -# create-execution, set-requirement, set-priority and bind-config as SEPARATE admin API -# calls with no transaction. A run that dies between them leaves a HALF-BUILT FLOW BOUND -# AS THE LOGIN PATH for Grafana, Gitea, Nextcloud, Headlamp, Semaphore, PiHole, Crafty, -# the *arr stack AND kubectl. This repo has already had a total SSO outage from a realm -# change (2026-09-06), and the conditional-OTP sub-flow — which the declared structure -# moves one level deeper than it sits today — once 403'd every platform-admin. A -# reconciler that half-rewrites the bound flow every 15 minutes is a worse failure than -# the one it replaces, because it is a loop rather than a single bad change. +# WHY THE FLOW IS BUILT BLUE/GREEN AND BOUND ONLY ON GIT'S SAY-SO, asserted here rather +# than only argued: converging a five-flow, ten-execution restructure needs +# create-subflow, create-execution, set-requirement and bind-config as SEPARATE admin API +# calls with no transaction. Doing that to the BOUND flow means a run that dies between +# them leaves a HALF-BUILT FLOW AS THE LOGIN PATH for Grafana, Gitea, Nextcloud, Headlamp, +# Semaphore, PiHole, Crafty, the *arr stack AND kubectl — this repo has had a total SSO +# outage from a realm change (2026-09-06), and the conditional-OTP sub-flow once 403'd +# every platform-admin. So every multi-call write lands on a NEW, unused candidate flow, +# which is verified by readback before anything is bound. Binding it is then ONE atomic +# PUT of browserFlow, made only when passkey-desired-state.yaml sets +# bind-browser-flow: "true" (default "false"): an untested login flow never goes live on +# a schedule tick, and with the switch off an operator's rollback is never undone. # # STATUS: EXECUTABLE. `check.sh` is a pure offline assertion script (git + PyYAML + # stdlib); conformance/run.sh --layer l0 and ci/l0.sh run it directly, and this Chainsaw @@ -93,9 +99,10 @@ spec: - name: audit-passkey-declaration-renderer-and-reconciler description: >- Assert the desired state is derived from the realm import and shares its - substitution pass, that the reconciler's write surface excludes every flow - mutation, that the sibling reconciler is unwidened, and run the reconciler's own - selftest plus the mutation suite that proves it can fail. + substitution pass, that the reconciler cannot write the bound flow or delete + anything and binds only on git's bind-browser-flow, that the sibling reconciler + is unwidened, and run the reconciler's own selftest plus the mutation suite that + proves it can fail. try: - script: content: bash ./check.sh diff --git a/conformance/tests/l0-static/keycloak-passkey-reconciler/check.sh b/conformance/tests/l0-static/keycloak-passkey-reconciler/check.sh index bda08e9..2847467 100755 --- a/conformance/tests/l0-static/keycloak-passkey-reconciler/check.sh +++ b/conformance/tests/l0-static/keycloak-passkey-reconciler/check.sh @@ -62,6 +62,11 @@ # been read. That last one is what the whole design rests on: a reconciler that # half-rewrites the BOUND browser flow is a total SSO outage on a loop, and # this repo has had one (2026-09-06). +# (6b) THE BIND IS GIT'S DECISION: the desired-state ConfigMap carries +# bind-browser-flow as exactly "true" or "false", the script reads it from the +# SAME mount, and the CronJob passes no flag that could widen it. With "false" +# (the default) the reconciler builds and verifies but never PUTs browserFlow, +# so an operator's rollback sticks; the selftest proves that end to end. # (7) THE SIBLING RECONCILER IS UNTOUCHED: client-secret-reconciler.yaml's # allow-list is NOT widened by this work. Its --selftest asserts it refuses # /users, /groups, /roles, /authentication and the whole-realm endpoint, and @@ -152,6 +157,7 @@ RENDERER = "ci/render-keycloak-passkey-policy.py" SIBLING_FILE = "platform/keycloak/client-secret-reconciler.yaml" STATE_CM_NAME = "keycloak-passkey-desired-state" STATE_DATA_KEY = "passkey-desired-state.json" +BIND_KEY = "bind-browser-flow" SCRIPT_KEY = "reconcile-keycloak-passkeys.py" ENROL_ACTION = "webauthn-register-passwordless" DOLLAR = chr(36) @@ -225,10 +231,16 @@ if state_cm is not None: f"{STATE_FILE}: the ConfigMap is named " f"{state_cm['metadata']['name']!r}, but the CronJob mounts {STATE_CM_NAME!r}") data = state_cm.get("data") or {} - check(list(data) == [STATE_DATA_KEY], - f"{STATE_FILE}: expected exactly the data key {STATE_DATA_KEY!r}, got " - f"{sorted(data)} — the reconciler's PASSKEY_STATE_FILE default would not " - f"resolve") + check(sorted(data) == sorted([STATE_DATA_KEY, BIND_KEY]), + f"{STATE_FILE}: expected exactly the data keys {STATE_DATA_KEY!r} and " + f"{BIND_KEY!r}, got {sorted(data)} — the reconciler's PASSKEY_STATE_FILE / " + f"PASSKEY_BIND_FILE defaults would not resolve") + # THE BIND SWITCH. An unquoted true/false is a YAML BOOLEAN, which a ConfigMap rejects + # at apply time -- failing the whole platform-config Kustomization, realm import + # included. The renderer refuses it too; asserted here so the reason is on record. + check(data.get(BIND_KEY) in ("true", "false"), + f"{STATE_FILE}: data.{BIND_KEY} is {data.get(BIND_KEY)!r}; it must be the QUOTED " + f"string \"true\" or \"false\"") raw = data.get(STATE_DATA_KEY, "") try: desired = json.loads(raw) @@ -457,6 +469,25 @@ if cj is not None: f"the mount path — they are a pair and must move together") check((mounts.get(state_vol) or {}).get("readOnly") is True, f"{RECON_FILE}: the desired-state mount must be readOnly") + check(f'"/etc/keycloak-passkey/{BIND_KEY}"' in script, + f"{RECON_FILE}: the script's PASSKEY_BIND_FILE default is not " + f"/etc/keycloak-passkey/{BIND_KEY}. An unresolved bind file reads as \"false\" " + f"(safe, but git's \"true\" would then silently never take effect)") + check(not (state_vol and (cm_vols[state_vol]["configMap"] or {}).get("items")), + f"{RECON_FILE}: the desired-state volume selects `items`, so one of its two " + f"keys may not be projected") + + # The bind decision belongs to git. A container arg could only NARROW it today + # (--no-bind), but nothing in the CronJob may carry a mode flag at all: the shipped + # behaviour must be readable from passkey-desired-state.yaml alone. + cmd = [str(x) for x in (ctr.get("command") or []) + (ctr.get("args") or [])] + check(not [x for x in cmd if x.startswith("--")], + f"{RECON_FILE}: the CronJob passes {[x for x in cmd if x.startswith('--')]} to " + f"the reconciler. Its mode is decided by git (bind-browser-flow), not by flags") + # Retries burn the bounded candidate-alias budget on a deterministic failure. + check(cj["spec"]["jobTemplate"]["spec"].get("backoffLimit") == 0, + f"{RECON_FILE}: backoffLimit must be 0 — every failure here is deterministic " + f"within a tick, and each retried build consumes another candidate alias") # The admin credential, referenced BY NAME. Never decrypted, never a literal. for key in ("KEYCLOAK_ADMIN_USER", "KEYCLOAK_ADMIN_PASSWORD"): @@ -591,6 +622,21 @@ check("_FLOW_CREATE_PATH" in script, f"altogether") # And the one function allowed to change the live binding must be distinct from the # policy write, which must refuse to. +# FINDING 2026-10-29: sub-flows are named by displayName in 26.0.6 (recurseExecutions +# never sets `alias` on a sub-flow row). Reading `alias` shrank the protected subtree to +# the top-level flow alone while every selftest passed against a stub that returned it. +check(re.search(r'^def subflow_alias\(ex\):\n(?:.*\n)*? name = ex\.get\("displayName"\)', + script, re.M) is not None, + f"{RECON_FILE}: subflow_alias() no longer reads displayName. Keycloak 26.0.6's " + f"executions listing names a sub-flow ONLY by displayName; reading `alias` yields " + f"None and the protected subtree silently shrinks to the top-level flow") +# FINDING 2026-10-29: the requirement PUT's real target is body["id"]. +check("_EXEC_UPDATE_PATH" in script and "_BUILD_EXECUTION_IDS" in script, + f"{RECON_FILE}: the requirement PUT (PUT /flows//executions) is no longer " + f"screened on its body id. updateExecutions resolves the execution realm-wide by id " + f"and ignores the path alias, so a path-only screen lets it write the bound flow") +check("def declared_bind(" in script and "BIND_FILE" in script, + f"{RECON_FILE}: the reconciler no longer reads git's bind-browser-flow switch") check("def bind_browser_flow(" in script, f"{RECON_FILE}: there is no bind_browser_flow() function. The atomic flip must be " f"one named, auditable operation, not an incidental field in some other write") diff --git a/conformance/tests/l0-static/keycloak-passkey-reconciler/mutations.py b/conformance/tests/l0-static/keycloak-passkey-reconciler/mutations.py index b552bf1..53ba2c8 100644 --- a/conformance/tests/l0-static/keycloak-passkey-reconciler/mutations.py +++ b/conformance/tests/l0-static/keycloak-passkey-reconciler/mutations.py @@ -78,10 +78,10 @@ MUTATIONS: list[tuple[str, str, str, str]] = [ "module-level initial value is deliberately NOT the anchor here: run() resets " "these at the start of every run, so the initialiser is unreachable and mutating " "it would prove nothing.)", - " global _PROTECTED_FLOW_ALIASES, _PROTECTED_EXECUTION_IDS\n" + " global _PROTECTED_FLOW_ALIASES, _PROTECTED_EXECUTION_IDS, _BUILD_EXECUTION_IDS\n" " _PROTECTED_FLOW_ALIASES = None\n" " _PROTECTED_EXECUTION_IDS = None", - " global _PROTECTED_FLOW_ALIASES, _PROTECTED_EXECUTION_IDS\n" + " global _PROTECTED_FLOW_ALIASES, _PROTECTED_EXECUTION_IDS, _BUILD_EXECUTION_IDS\n" " _PROTECTED_FLOW_ALIASES = frozenset()\n" " _PROTECTED_EXECUTION_IDS = frozenset()"), @@ -117,6 +117,84 @@ MUTATIONS: list[tuple[str, str, str, str]] = [ " \"the existing flow %r\" % candidate):", " if cand_shape is not None:"), + # ------------------------------------- 2026-10-29 review findings, each made falsifiable + ("sub-flows identified by `alias` again", + "read `alias` off a sub-flow row, as the first version did -- Keycloak 26.0.6's " + "recurseExecutions sets only displayName (and flowId) on a sub-flow row, so every " + "sub-flow resolves to nothing and the protected subtree is the top-level flow alone: " + "a write into browser-mfa-forms would NOT be refused", + " name = ex.get(\"displayName\")\n" + " return name if isinstance(name, str) and name else None", + " name = ex.get(\"alias\")\n" + " return name if isinstance(name, str) and name else None"), + + ("an unnameable sub-flow is tolerated", + "carry on when a sub-flow row has no displayName -- the protected subtree is then " + "SMALLER than the live tree, which is the fail-open the guard exists to prevent", + " if name is None:\n" + " return None, None", + " if name is None:\n" + " name = \"\""), + + ("the requirement PUT is screened on its path only", + "trust the alias in PUT /flows//executions -- updateExecutions resolves " + "body.id realm-wide and never checks it belongs to , so a candidate alias in " + "the path can carry a BOUND execution's id in the body", + " targets.append((\"execution\", (body or {}).get(\"id\")))", + " pass"), + + ("execution-addressed writes are not held to the build tree", + "'outside the bound subtree is enough' -- it is not: every OTHER flow in the realm is " + "outside it too, so a stray id would edit an unrelated flow", + " if what == \"execution\" and (_BUILD_EXECUTION_IDS is None\n" + " or target not in _BUILD_EXECUTION_IDS):", + " if False:"), + + ("the requirement PUT may carry any key", + "let the body carry flowId / displayName -- the same handler then RENAMES whatever " + "flow flowId names, bound or not", + " if extra:\n" + " raise Refused(", + " if False:\n" + " raise Refused("), + + ("git's bind switch is ignored", + "bind whenever verification passes, as the first version did -- the first live use of " + "an untested login flow becomes a schedule tick, and an operator's rollback is " + "undone within 15 minutes, forever", + " bind = git_bind and not no_bind", + " bind = not no_bind"), + + ("bind=false still binds inside the flow pass", + "keep the flag but forget to honour it where the bind happens", + " if not bind:\n" + " # THE BIND SWITCH IS OFF.", + " if False:\n" + " # THE BIND SWITCH IS OFF."), + + ("an absent bind switch means bind", + "default to true when the key is missing -- a ConfigMap predating the switch, or a " + "deleted line, would silently put an untested flow live", + " except FileNotFoundError:\n" + " return False", + " except FileNotFoundError:\n" + " return True"), + + ("an invalid bind switch is guessed at", + "read anything that is not \"true\" as false -- a typo then passes silently instead of " + "stopping the run, and the operator believes the flow is live", + " if raw == \"false\":\n" + " return False\n" + " raise ValueError(", + " return False\n" + " raise ValueError("), + + ("bind=false rebuilds beside a hand-edited bound flow", + "treat the hand-edited bound candidate as a 'half-built leftover' -- with the switch " + "off this builds an -r2 next to the operator's hotfix on every tick", + " if candidate in have and candidate == bound and not bind:", + " if False:"), + # ------------------------------------------------------------------- idempotence ("sub-flow aliases are compared as names", "compare the sub-flow alias like any other field -- the flows this program builds " diff --git a/platform-config/kustomization.yaml b/platform-config/kustomization.yaml index cffc6bb..fa5a9a5 100644 --- a/platform-config/kustomization.yaml +++ b/platform-config/kustomization.yaml @@ -131,10 +131,13 @@ resources: # browser login flow. Measured on prod 2026-10-18, hours after the passkey declaration # merged: the KeycloakRealmImport read Done=True / Started=False with the whole # webAuthnPolicyPasswordless* block and the restructured browser-mfa flow INERT. This - # CronJob converges the policy and the enrolment required action, and AUDITS the browser - # flow without the ability to rewrite it (no flow-mutating endpoint is on its - # allow-list) — a half-rewritten flow bound as the realm's browserFlow is a total SSO - # outage, and this repo has had one. See the file header for the full rationale. + # CronJob converges the policy and the enrolment required action, and builds the + # declared browser flow BLUE/GREEN: under a new alias, verified by readback, and + # structurally unable to write the flow that is currently bound (a half-rewritten bound + # flow is a total SSO outage, and this repo has had one). It BINDS the verified flow + # (one atomic PUT of browserFlow) ONLY when passkey-desired-state.yaml sets + # bind-browser-flow: "true"; the default "false" leaves the realm's binding untouched. + # See the script header in that file for the full rationale. - ../platform/keycloak/passkey-reconciler.yaml # Its DESIRED STATE, GENERATED from keycloak-realm-import.yaml by # ci/render-keycloak-passkey-policy.py and gated by the L0 check. Same reason as diff --git a/platform/keycloak/README.md b/platform/keycloak/README.md index dee6b58..8b6c6c9 100644 --- a/platform/keycloak/README.md +++ b/platform/keycloak/README.md @@ -31,8 +31,8 @@ files pulled as remote kustomize bases (the kubevirt-stub pattern); Renovate mai | `keycloak-realm-import.yaml` (`KeycloakRealmImport`: realm/groups/clients/MFA) | **platform-config** | config | | `client-secret-reconciler.yaml` (CronJob: client secrets **and** client URLs ← git, every 15m) | **platform-config** | config | | `client-urls.yaml` (**GENERATED** ConfigMap: the client-URL desired state) | **platform-config** | config | -| `passkey-reconciler.yaml` (CronJob: WebAuthn passwordless policy + enrolment action + the **browser flow** ← git every 15m; the flow blue/green, never mutating the bound one) | **platform-config** | config | -| `passkey-desired-state.yaml` (**GENERATED** ConfigMap: the passkey desired state) | **platform-config** | config | +| `passkey-reconciler.yaml` (CronJob: WebAuthn passwordless policy + enrolment action + the **browser flow** ← git every 15m; the flow blue/green, never mutating the bound one, **bound only when git says so**) | **platform-config** | config | +| `passkey-desired-state.yaml` (**GENERATED** ConfigMap: the passkey desired state — plus the one hand-edited line, `bind-browser-flow`) | **platform-config** | config | | `httproute.yaml` (keycloak.home.keithjackson.dev) | **platform-config** | config | | `servicemonitor.yaml` (Prometheus scrape) | **platform-config** | config | @@ -207,7 +207,7 @@ unrecoverable-credential bug, not a cosmetic one. |---|---|---| | `webAuthnPolicyPasswordless*` (realm policy) | **The reconciler. GitOps.** | Realm top-level attributes; strict read-modify-write of `GET`/`PUT /admin/realms/homelab` | | `webauthn-register-passwordless` required action | **The reconciler. GitOps.** | Per-alias `PUT`, one literally-named alias; enabled, `defaultAction: false` | -| The `browser-mfa` flow restructure | **The reconciler. GitOps.** | Blue/green: build under a new alias, verify, flip `browserFlow` in one atomic PUT — see below | +| The `browser-mfa` flow restructure | **The reconciler builds and verifies it. Git decides whether it is bound.** | Blue/green: build under a new alias, verify, and flip `browserFlow` in one atomic PUT **only when `bind-browser-flow: "true"`** — see below | #### The flow converges blue/green — it never mutates the flow you log in through @@ -229,8 +229,46 @@ flow. So the reconciler does not. 2. **Verify** by readback — nesting levels, ordering, authenticators, requirements, and the conditional-OTP config *contents*. Nothing is bound unless this passes, so the regression that once 403'd every platform-admin is caught **before** it can bite. -3. **Bind** with **one atomic PUT** of the realm, setting `browserFlow`. -4. **Rollback is one write**, and nothing is ever deleted, so the old flow stays usable. +3. **Bind** with **one atomic PUT** of the realm, setting `browserFlow` — **only if git says + so** (next section). Otherwise the run stops here and logs `WOULD BIND browser-mfa-`. +4. **Rollback is a PR plus one write** (below), and nothing is ever deleted, so the old flow + stays usable. + +#### The bind switch: `bind-browser-flow` in `passkey-desired-state.yaml` + +That generated file carries exactly one hand-edited line, `bind-browser-flow`, and it is +`"false"` by default. The renderer carries it through on `--write`, and `--check` 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 fail the whole `platform-config` +Kustomization). The CronJob reads it from the same mount as the desired state; no flag can +widen it. + +| `bind-browser-flow` | What the reconciler does to the realm's `browserFlow` | +|---|---| +| `"false"` (default) | **Never writes it.** It builds and verifies the candidate (or reuses an already verified one), logs `WOULD BIND `, and exits 0. Whatever the realm is bound to — including your rollback — is left alone on every run. A hand-edited bound flow is left alone too (no rebuild beside it). | +| `"true"` | Binds the verified candidate in one atomic PUT, **and keeps asserting it**: a binding changed by hand is changed back within 15 minutes, and a hand-edit *inside* the bound flow makes the next run build and bind a fresh copy. That is GitOps doing its job — so never hotfix the flow while this is `"true"`. | + +**Merging with `"false"` changes nothing about how anyone logs in.** Passes 1 and 2 do write +the realm (the passwordless policy and the enrolment action, neither of which any login +step uses until a flow offers passkeys), and pass 3 creates the unbound candidate flow, but +`browserFlow` is untouched. + +**Going live** is a later one-line PR — `bind-browser-flow: "true"` — made **after** the +candidate has been tested in a browser (see "Testing the candidate flow before binding it"). + +**Rolling back**, in this order: + +1. A PR setting `bind-browser-flow: "false"`, merged and applied by Flux. **First** — while it + is `"true"`, the next run rebinds the candidate over your rollback. +2. One write: bind the previous flow again. The Job log of the bind printed it + (`ROLLBACK is one write: set browserFlow back to ...`); it is `browser-mfa` for the first + go-live. Admin console: *Authentication → Flows → → Action → Bind flow → + Browser flow*, or `PUT /admin/realms/homelab` as a read-modify-write of the full realm + representation (a partial body drops the passkey policy). + +If login is broken *right now*, do step 2 immediately and step 1 straight after: the worst +case of that order is that one scheduled run (≤15 min) rebinds the candidate before Flux +applies the PR, and you repeat step 2. **The safety property is structural, not intentional.** The reconciler is *incapable* of writing to the bound flow or anything beneath it: the live `browserFlow` alias and its @@ -257,36 +295,55 @@ flows to clean up rather than creating unbounded sprawl. kubectl --context k3s-prod -n keycloak get jobs -l app.kubernetes.io/name=keycloak-passkey-reconciler kubectl --context k3s-prod -n keycloak logs -l app.kubernetes.io/name=keycloak-passkey-reconciler --tail=200 -# Force a run now rather than wait for the tick. NOTE: this runs the DEFAULT mode, which -# converges (including flipping browserFlow once a new flow verifies). `kubectl create job -# --from=cronjob/...` CANNOT override the container command, so there is no way to get -# --audit or --no-bind out of it — for those, copy the CronJob's pod spec into a one-off -# Job and append the flag. +# Which candidate it would bind (bind-browser-flow "false"): +kubectl --context k3s-prod -n keycloak logs -l app.kubernetes.io/name=keycloak-passkey-reconciler --tail=300 \ + | grep -E 'WOULD BIND|BOUND browserFlow|PROBLEM' + +# Force a run now rather than wait for the tick. It does exactly what the CronJob does, +# including honouring bind-browser-flow. `kubectl create job --from=cronjob/...` CANNOT +# override the container command, so --audit / --no-bind need a one-off Job with the flag +# appended (--no-bind can only NARROW git's switch, never widen it). kubectl --context k3s-prod -n keycloak create job --from=cronjob/keycloak-passkey-reconciler \ passkey-run-once kubectl --context k3s-prod -n keycloak logs job/passkey-run-once -# ROLLBACK — one write. The run log names the previous alias ("ROLLBACK is one write: -# set browserFlow back to ..."). The old flow is never deleted, so this always works. -# browserFlow is a realm setting: Realm settings -> ... or -# PUT /admin/realms/homelab with browserFlow set to the previous alias -# (read-modify-write the realm representation; a partial body drops the passkey policy). -# The admin console path is Authentication -> Flows -> -> Action -> Bind flow -# -> Browser flow. The console does this as one atomic change too. +# ROLLBACK — see "The bind switch" above: PR bind-browser-flow "false" FIRST, then rebind +# the previous flow (Authentication -> Flows -> -> Action -> Bind flow -> +# Browser flow). The old flow is never deleted, so this always works. # Regenerate the desired state after editing the passkey policy or the flow in the realm # import (the merge gate will demand it). python3 ci/render-keycloak-passkey-policy.py --write ``` -**If you would rather the go-live be a human act,** the reconciler takes `--no-bind`: it -builds and verifies the flow and stops, printing the one write needed to bind it. That -automates everything except one atomic operator action. +### Testing the candidate flow before binding it -### Verifying it actually works (nobody has, as of 2026-10-18) +With `bind-browser-flow: "false"` the candidate exists but nothing logs in through it. Test it +on **one client** without touching the realm binding, using a per-client flow override: + +1. Find the candidate: the Job log line `WOULD BIND browser-mfa-`. +2. Admin console → *Clients → `account-console` → Advanced → Authentication flow overrides → + Browser Flow* = `browser-mfa-`. This is a temporary, hand-made client change; remove it + in step 7. +3. **Password fallback.** Private window → `https://keycloak./realms/homelab/account`. + Expect a **username** screen, then a choice: passkey, or the password form. As a user with no + passkey, choose password and sign in. +4. **Platform-admin TOTP on the password branch.** Repeat step 3 as a `platform-admins` member: + after the password, a TOTP prompt **must** appear. If it does not — or you get a 403 — the + conditional-OTP step is wrong: stop, do not bind. +5. **Passkey enrolment.** Signed in to the account console → *Signing in → Passkey → Set up*. A + browser/OS passkey prompt must appear; if it does not, the RP ID is wrong for the origin and + nothing server-side will say so. +6. **Passkey login.** Sign out; private window again; username, then choose passkey. Expect to be + signed in with **no** TOTP prompt, even as a platform-admin. +7. Remove the override from step 2. +8. Only now: the one-line PR `bind-browser-flow: "true"`. The next run binds the candidate you + tested (same alias — the hash is of the declared shape). + +### Verifying it actually works (nobody has, as of 2026-10-29) No passkey has been enrolled and no browser has been pointed at this. The checks that would prove -it, in order: +it on the BOUND flow, after go-live, in order: ```sh # 1. the policy is live (RP ID must read as the base domain, NOT the keycloak host, NOT a URL) diff --git a/platform/keycloak/passkey-desired-state.yaml b/platform/keycloak/passkey-desired-state.yaml index 6d14b5b..e5cb9ac 100644 --- a/platform/keycloak/passkey-desired-state.yaml +++ b/platform/keycloak/passkey-desired-state.yaml @@ -1,6 +1,6 @@ --- # ============================================================================= -# GENERATED FILE — DO NOT EDIT BY HAND. +# GENERATED FILE — DO NOT EDIT BY HAND, except the one bind-browser-flow line below. # # source: platform/keycloak/keycloak-realm-import.yaml (spec.realm) # regenerate: python3 ci/render-keycloak-passkey-policy.py --write @@ -10,8 +10,14 @@ # WHAT THIS IS: the DESIRED STATE for the `homelab` realm's PASSKEY configuration — # the WebAuthn passwordless policy, the enrolment required action, and the declared # browser authentication flow tree. platform/keycloak/passkey-reconciler.yaml mounts -# it and (a) converges the policy and the required action onto it, (b) AUDITS the -# live browser flow against it and alarms on divergence without rewriting it. +# it and (a) converges the policy and the required action onto it, (b) builds the +# declared flow tree BLUE/GREEN under a new alias and verifies it by readback, never +# writing the flow users are logging in through, and (c) BINDS that verified flow as +# the realm's browserFlow ONLY when `bind-browser-flow` below is "true". +# +# EVERYTHING HERE IS GENERATED EXCEPT ONE LINE: `bind-browser-flow`. That is an +# operator decision, carried through by the renderer from this committed file (see +# ci/render-keycloak-passkey-policy.py). It defaults to "false". # # WHY IT IS A SEPARATE OBJECT: the realm import is a ONE-SHOT that never re-runs # once Done=True, so a realm-level field or a flow edited in git never reaches the @@ -51,6 +57,22 @@ metadata: app.kubernetes.io/name: keycloak-passkey-desired-state app.kubernetes.io/part-of: keycloak data: + # --------------------------------------------------------------------------- + # THE BIND SWITCH -- the ONLY hand-edited line in this file. Exactly "true" or + # "false", QUOTED. + # + # "false" the reconciler builds and verifies the candidate browser flow and + # logs `WOULD BIND `, but NEVER changes the realm's browserFlow. + # Whatever the realm is bound to -- including a rollback -- is left alone. + # "true" it binds the verified candidate in one atomic PUT, and keeps the + # realm bound to it: a binding changed by hand is changed back. + # + # GO LIVE: test the candidate in a browser first (platform/keycloak/README.md, + # "Passkeys"), then a PR setting this to "true". + # ROLL BACK: a PR setting this to "false" FIRST (or the next run rebinds), then + # one write setting browserFlow back to the previous alias from the Job log. + # --------------------------------------------------------------------------- + bind-browser-flow: "false" passkey-desired-state.json: | { "authenticatorConfig": { diff --git a/platform/keycloak/passkey-reconciler.yaml b/platform/keycloak/passkey-reconciler.yaml index 53b9d3e..dfa64a5 100644 --- a/platform/keycloak/passkey-reconciler.yaml +++ b/platform/keycloak/passkey-reconciler.yaml @@ -2,165 +2,28 @@ # ============================================================================= # Keycloak PASSKEY RECONCILER (R-IDP-03, R-IDP-04) # -# THE FAILURE THIS EXISTS FOR, MEASURED ON PROD 2026-10-18 -# ------------------------------------------------------- -# platform/keycloak/keycloak-realm-import.yaml declares the WebAuthn PASSWORDLESS -# policy and a restructured `browser-mfa` browser flow (merged as PR #72). That -# declaration CANNOT TAKE EFFECT: +# A CronJob that converges the `homelab` realm's passkey configuration onto +# passkey-desired-state.yaml (GENERATED from keycloak-realm-import.yaml, whose +# KeycloakRealmImport is a ONE-SHOT that never re-runs once Done=True): # -# kubectl -n keycloak get keycloakrealmimport homelab -o jsonpath='{.status}' -# Done=True Started=False HasErrors=False -# -# A KeycloakRealmImport is a ONE-SHOT. Once it rests at Done=True the operator never -# re-runs it, so every realm-level field and every flow the manifest declares after -# the FIRST import is inert — merged into git, applied by Flux, reported Ready, and -# reaching the realm never. Merging the passkey block changed nothing about how -# anyone logs in, and nothing anywhere said so. -# -# This is the SAME failure class platform/keycloak/client-secret-reconciler.yaml -# already closes for client secrets (2026-09-06: all 12 prod clients holding the -# literal `(env:...)` placeholder behind an entirely green cluster) and for -# environment-derived client URLs. This CronJob closes it for the passkey -# configuration, the same way: a one-shot that fails open is replaced by a -# LEVEL-TRIGGERED loop that converges every 15 minutes — the posture Flux gives -# every other resource in this repo (R-GITOPS-03 drift correction), applied to the -# one piece of state Flux itself cannot reach, Keycloak's own database. -# -# ALL THREE DECLARED THINGS CONVERGE. Nothing here is left as a manual step: # pass 1 webAuthnPolicyPasswordless* realm policy strict read-modify-write -# pass 2 the webauthn-register-passwordless action strict read-modify-write, -# ONE literally-named alias -# pass 3 the browser authentication flow BLUE/GREEN +# pass 2 the webauthn-register-passwordless action strict read-modify-write +# pass 3 the browser authentication flow BLUE/GREEN: build a candidate +# under a new alias, verify it by readback, and BIND it (one atomic PUT of +# browserFlow) ONLY when git sets `bind-browser-flow: "true"` in +# passkey-desired-state.yaml. It defaults to "false": build + verify + +# report what WOULD be bound, never touch the realm's binding. # -# WHY A SIBLING AND NOT A SECOND PASS OF THE EXISTING RECONCILER -# ------------------------------------------------------------- -# client-secret-reconciler.yaml's stated guarantee is that prod's real user and the -# media-users / platform-admins / viewers groups survive every run, and its -# --selftest ASSERTS that it refuses /users, /groups, /roles, /authentication and -# the whole-realm endpoint. This work needs the realm endpoint and the -# /authentication tree. Widening that allow-list would break its selftest and, more -# importantly, would retire a guarantee that is currently literally true. So this is -# a SEPARATE program with a SEPARATE, differently-shaped allow-list, and the sibling -# is untouched — its selftest still passes unchanged. -# -# PASS 3: BLUE/GREEN, AND WHY THE OBVIOUS DESIGN IS WRONG -# ------------------------------------------------------ -# The declared restructure is five flows and ten executions, and the realm ALREADY -# holds a `browser-mfa` flow of the OLD shape (one combined -# auth-username-password-form rather than the username/password split a passkey -# branch requires). The admin API offers no transaction: create-flow, add-subflow, -# add-execution, set-requirement and bind-config are all separate calls. -# -# THE FIRST DESIGN OF THIS COMPONENT WAS "AUDIT THE FLOW, NEVER WRITE IT", on the -# reasoning that a run dying part-way through that sequence would leave a HALF-BUILT -# FLOW BOUND AS THE LOGIN PATH for Grafana, Gitea, Nextcloud, Headlamp, Semaphore, -# PiHole, Crafty, the *arr stack AND kubectl — and this repo has had a total SSO -# outage from a realm change (2026-09-06), with the conditional-OTP sub-flow (which -# the declared structure moves ONE LEVEL DEEPER than it sits today) once throwing on -# every evaluation and 403ing every platform-admin. -# -# THAT RISK IS REAL BUT IT IS NOT INHERENT. It follows only from MUTATING THE BOUND -# FLOW. So this does not mutate the bound flow: -# -# 1. BUILD the target structure under a NEW, UNUSED alias — the declared alias -# plus a hash of the desired shape, applied to every flow in the tree. Every -# multi-call mutation happens on a flow NOTHING IS USING. A failure at any -# point leaves an orphaned, unbound flow and ZERO user-visible effect: login -# keeps using the old flow, untouched. -# 2. READ IT BACK AND VERIFY it matches the desired shape — nesting levels, -# ordering, authenticators, requirements, and the conditional-OTP config -# CONTENTS. Nothing is bound until that passes, so the regression the operator -# fears is caught BEFORE it can bite rather than after. -# 3. BIND with ONE ATOMIC WRITE: PUT /admin/realms/homelab setting browserFlow. -# One atomic mutation to live behaviour, not five. -# 4. ROLLBACK IS ONE WRITE. The previous alias is printed in the run log, and -# NOTHING IS EVER DELETED, so the old flow remains a working rollback target. -# -# THE SAFETY PROPERTY, ENFORCED STRUCTURALLY RATHER THAN BY INTENTION -# ------------------------------------------------------------------ -# Blue/green needs flow-create and execution-add endpoints, so the allow-list alone -# is no longer sufficient: its write entries are alias-parameterised and cannot say -# "but not the flow that is bound". A SECOND structural guard sits at the same choke -# point in _http(): -# -# THIS PROGRAM IS INCAPABLE OF WRITING TO THE FLOW THAT IS CURRENTLY BOUND, OR TO -# ANY SUB-FLOW OR EXECUTION BENEATH IT. -# -# Before any flow write the live browserFlow alias is fetched and its ENTIRE SUBTREE -# of sub-flow aliases and execution ids is recorded; every write is screened against -# those sets before a socket is opened. Protecting the whole subtree matters: adding -# an execution to `browser-mfa-forms` changes live login behaviour just as surely as -# touching `browser-mfa` itself. The create-flow endpoint carries no alias in its -# path, so it is screened on its request BODY — otherwise it would escape the guard. -# -# AND IT FAILS CLOSED: the protected sets start each run as "not yet determined", and -# while they are, EVERY flow write is refused. If the bound flow's executions cannot -# be read, they stay undetermined and the flow pass writes nothing at all. So the -# worst a buggy or half-failed run can do is leave an unused flow lying around. -# -# IDEMPOTENCE IS LOAD-BEARING HERE, NOT A NICETY. A rebuild-and-reflip every fifteen -# minutes would be a permanent write loop against the IdP and would churn the realm's -# login path on a schedule. So pass 3 does a REAL STRUCTURAL COMPARISON of the bound -# flow against the desired shape and, when they match, issues ZERO writes and creates -# nothing. Sub-flow aliases and authenticator-config aliases are compared as -# STRUCTURE, not as names — a flow this program built carries a hash suffix on every -# alias, so comparing labels would mean it could never be recognised as converged. -# -# RECOVERY. A crashed earlier run can leave a partial flow under the primary -# candidate alias. A later run reads that flow: if it MATCHES desired it is a -# completed-but-unbound build and is simply BOUND (no rebuild); if it does not, it is -# left alone and the next candidate alias is used. The number of candidates is -# BOUNDED, so a persistently failing build cannot create unbounded flow sprawl — it -# fails loudly instead, naming the flows an operator should clean up. -# -# THE REALM-LEVEL PUT, CHECKED AGAINST 26.0.6 RATHER THAN ASSUMED -# -------------------------------------------------------------- -# Verified against quay.io/keycloak/keycloak:26.0.6, the image the cluster runs: -# * RealmAdminResource.updateRealm is the only handler for PUT /admin/realms/. -# Its javadoc: "Any user, roles or client information in the representation will -# be ignored. This will only update top-level attributes of the realm." -# * It delegates to DefaultExportImportManager.updateRealm, lines 725-894 at tag -# 26.0.6. That body was read in full and brace-matched: NO rep.getUsers(), NO -# getClients(), NO getGroups(), NO getRoles(), NO getRequiredActions(), NO -# getAuthenticationFlows(), NO getAuthenticatorConfig(). Users, groups, clients, -# roles, required actions, flows and authenticator configs are UNREACHABLE from -# this endpoint — not merely ignored by convention. -# * It DOES read rep.getBrowserFlow(). That is what makes the atomic bind in step 3 -# possible. It also means the POLICY pass could rebind the login path by -# accident, so the policy write carries browserFlow back from the GET unchanged -# and refuses to proceed if it would differ. One function may change it, and only -# after verification passes. -# * The passwordless policy is applied ALL-OR-NOTHING and only when RpEntityName is -# non-empty, with a Keycloak DEFAULT substituted for every omitted key. That is -# why the write is a strict read-modify-write of the full live representation and -# never a partial body — a partial PUT would silently reset the keys it left out, -# and one missing RpEntityName would drop the whole policy while returning 204. -# * A third trap, in the flow builder: updateExecutions compares rep.getPriority() -# against the stored priority, and a JSON body omitting `priority` deserialises it -# as the int 0 — which would RESHUFFLE the execution order while only a -# requirement change was intended. Every requirement write carries the -# execution's current priority back. -# -# NOTHING IS EVER DELETED. There is no DELETE method anywhere in the allow-list. -# -# MODES: default reconciles; `--audit` is read-only and builds and binds nothing; -# `--no-bind` builds and VERIFIES but leaves the flip as a one-write human act. +# THE FULL RATIONALE IS IN ONE PLACE ONLY: the header of the script below +# (reconcile-keycloak-passkeys.py). It used to be duplicated here almost verbatim; +# two copies of a safety argument drift apart, and the copy that ships with the code +# and its --selftest is the one kept. Read it before changing anything here. # # CARRIED BY platform-config (dependsOn platform, wait:true), alongside the realm # import whose failure mode it covers and the keycloak-admin-credentials Secret it -# authenticates with. -# -# PROOF: the script's own `--selftest` drives these real code paths against an -# in-process stub Keycloak that MODELS the 26.0.6 behaviours above — including the -# RpEntityName gate, the all-or-nothing policy rebuild, the required-action replace, -# the updateExecutions priority trap, and flow construction where everything is -# created DISABLED. It asserts blue/green build-verify-flip, that a build killed at -# four different points NEVER binds and leaves login untouched, that the bound flow -# is unwritable and fail-closed, idempotence with zero writes, recovery and the -# bounded alias budget, that nothing is ever deleted, and the conditional-OTP -# regression. The L0 static gate EXTRACTS the script from this ConfigMap and runs it, -# and mutations.py proves the selftest can FAIL, so what CI proves is byte-for-byte -# what ships: conformance/tests/l0-static/keycloak-passkey-reconciler/. +# authenticates with. Gated by conformance/tests/l0-static/keycloak-passkey-reconciler/ +# (extracts this script, runs its --selftest, and proves that selftest falsifiable). +# Failures alert via KeycloakPasskeyReconcilerFailing (platform/monitoring/alerts.yaml). # ============================================================================= apiVersion: v1 kind: ConfigMap @@ -200,14 +63,15 @@ data: # client secrets and client URLs (platform/keycloak/client-secret-reconciler.yaml, # 2026-09-06 incident). This program closes it for the passkey configuration. # - # THREE PASSES, ALL OF WHICH CONVERGE - # ----------------------------------- + # THREE PASSES + # ------------ # pass 1 WEBAUTHN PASSWORDLESS POLICY strict read-modify-write of the realm. # pass 2 THE ENROLMENT REQUIRED ACTION strict read-modify-write, ONE hard-coded # alias. # pass 3 THE BROWSER AUTHENTICATION FLOW BLUE/GREEN: build the target structure - # under a NEW alias, verify it, then flip the - # realm's browserFlow in ONE atomic write. + # under a NEW alias and verify it. Flip the + # realm's browserFlow in ONE atomic write ONLY + # if git says bind-browser-flow: "true". # # PASS 3: WHY BLUE/GREEN, AND WHY THE OBVIOUS DESIGN IS WRONG # ----------------------------------------------------------- @@ -236,9 +100,35 @@ data: # contents. Nothing is bound until that passes. The regression the operator fears # becomes detectable BEFORE it can bite, not after. # 3. BIND with ONE ATOMIC WRITE: PUT /admin/realms/ setting browserFlow. One - # atomic mutation to live behaviour, not five. - # 4. ROLLBACK IS ONE WRITE. The previous alias is printed in the run log, and the - # old flow is NEVER DELETED, so it remains a working rollback target. + # atomic mutation to live behaviour, not five -- and ONLY when git asks for it + # (see THE BIND SWITCH below). With the switch off, the run stops after step 2 + # and logs the alias it WOULD bind. + # 4. ROLLBACK IS A PR PLUS ONE WRITE. Set bind-browser-flow back to "false" in git + # (so the reconciler stops asserting the binding), then set browserFlow back to + # the previous alias, which is printed in the run log. The old flow is NEVER + # DELETED, so it remains a working rollback target. + # + # THE BIND SWITCH, AND WHY ITS DEFAULT IS "false" + # ----------------------------------------------- + # passkey-desired-state.yaml carries `bind-browser-flow`, the ONE hand-edited line in + # that otherwise generated file (the renderer carries it through; its --check rejects + # anything but the strings "true" / "false"). It is mounted beside the JSON desired + # state and read every run, so git -- not a CLI default nobody can see -- decides + # whether this program may touch the realm's binding. + # + # "false" (the default) build and verify the candidate (or reuse a verified one), + # log WOULD BIND , and NEVER PUT browserFlow. Whatever + # the realm is bound to -- including an operator's rollback -- + # is left exactly as it is, every run. + # "true" bind the verified candidate in one atomic PUT, and keep + # asserting it: a binding changed by hand is changed back. + # + # Why not just bind: an automatic bind makes the first live use of an UNTESTED login + # flow a schedule tick, and with the bind always on, ROLLBACK DOES NOT STICK -- the + # next run finds the verified candidate again and rebinds it every 15 minutes, and an + # operator hotfix inside the bound flow is "corrected" by building and binding a fresh + # copy. With the switch, going live is a reviewed one-line PR made AFTER a human has + # tested the candidate flow in a browser, and rolling back is that PR reversed. # # THE SAFETY PROPERTY, ENFORCED STRUCTURALLY RATHER THAN BY INTENTION # ------------------------------------------------------------------ @@ -256,6 +146,25 @@ data: # execution to `browser-mfa-forms` would change live login behaviour just as surely as # touching `browser-mfa` itself. # + # HOW A SUB-FLOW IS NAMED IN 26.0.6 -- read from the source, not assumed. The executions + # listing (AuthenticationManagementResource.recurseExecutions, tag 26.0.6) reports a + # sub-flow row with `displayName = flowRef.getAlias()` and `flowId`, and NEVER sets + # `alias` on it; `alias` is set only on an AUTHENTICATOR row, where it is the bound + # authenticator CONFIG's alias. An earlier version of this file read `alias` for + # sub-flow rows, so the protected set held only the top-level alias and a write into + # `browser-mfa-forms` would NOT have been refused. Sub-flows are now named by + # displayName, and a sub-flow row WITHOUT one leaves the subtree undetermined, so the + # flow pass stays fail-closed rather than guessing. + # + # THE ONE WRITE WHOSE TARGET IS IN THE BODY, NOT THE PATH. PUT /flows//executions + # (updateExecutions) resolves realm.getAuthenticationExecutionById(rep.getId()) and + # never checks that the execution belongs to -- so the path alias says nothing + # about what is written. The screen therefore also requires body["id"] to be outside + # the protected execution ids AND inside the flow tree this run is building (recorded + # from the build's own readback), and refuses any body key beyond id / requirement / + # priority: a body carrying flowId + displayName would RENAME an arbitrary flow through + # the same handler. The config-binding POST is held to the same build-tree rule. + # # AND IT FAILS CLOSED: the protected sets start as None, meaning "not yet determined", # and while they are None EVERY flow write is refused. So a code path that tried to # write before discovering what is bound cannot do so. The worst a buggy or @@ -360,21 +269,28 @@ data: # binding is known. # 4. the required-action write can reach exactly ONE alias, named literally. # 5. the realm policy PUT never changes browserFlow; only bind_browser_flow() may, - # and only after verification passes. + # only after verification passes, and only when git sets bind-browser-flow "true". # 6. a flow is BOUND only after a readback proves it matches the desired shape. # 7. no secret is ever printed, in any mode, on any path. # 8. fail closed: an unreadable or malformed desired state writes NOTHING. # 9. nothing is ever deleted. # 10. the three passes are INDEPENDENT -- a failure in one does not suppress the # others. + # 11. with bind-browser-flow "false", NO PUT ever changes browserFlow -- including + # after an earlier bind, so an operator's rollback sticks. + # + # PROOF: --selftest drives these real code paths against an in-process stub Keycloak + # whose executions listing is shaped as 26.0.6's recurseExecutions shapes it. The L0 + # gate EXTRACTS this script from the ConfigMap and runs it, and mutations.py proves the + # selftest can FAIL: conformance/tests/l0-static/keycloak-passkey-reconciler/. # # MODES - # (default) reconcile: compare, and write only what differs. + # (default) reconcile: compare, and write only what differs. Whether pass 3 may + # bind is decided by git (bind-browser-flow), NOT by a flag. # --audit READ-ONLY throughout. Report drift and exit 1 if anything differs. # Builds nothing and binds nothing. - # --no-bind build and VERIFY the new flow but do NOT flip browserFlow. Leaves a - # verified, unbound flow and prints the one-line bind command. The - # deliberate fallback if an operator wants the flip to be a human act. + # --no-bind for a hand-run Job only: never bind, even if git says "true". It can + # only NARROW what git allows; nothing can widen it. # --selftest the offline behavioural proof, against an in-process stub Keycloak. # No cluster; loopback only. This is what the L0 static gate runs, so # the code the gate proves is the code that ships. @@ -408,6 +324,11 @@ data: # access to read the KeycloakRealmImport CR with. STATE_FILE = os.environ.get("PASSKEY_STATE_FILE", "/etc/keycloak-passkey/passkey-desired-state.json") + # THE BIND SWITCH: the second key of the same desired-state ConfigMap, so the same mount. + # Exactly "true" or "false" (whitespace-trimmed); anything else is a FATAL input error. + # The renderer's --check enforces the same, so a typo is caught at merge time. + BIND_FILE = os.environ.get("PASSKEY_BIND_FILE", + "/etc/keycloak-passkey/bind-browser-flow") TIMEOUT = 20 READY_ATTEMPTS = int(os.environ.get("READY_ATTEMPTS", "24")) READY_SLEEP = float(os.environ.get("READY_SLEEP", "5")) @@ -532,6 +453,16 @@ data: # ============================================================================= _PROTECTED_FLOW_ALIASES = None _PROTECTED_EXECUTION_IDS = None + # The execution ids of the flow tree THIS RUN is building, recorded from the build's own + # readback. None means no build is in progress, and then every execution-id-addressed + # write (the requirement PUT, the config POST) is REFUSED. A set membership test on the + # id is the only way to scope those writes: the requirement PUT names a flow alias in its + # path, but Keycloak ignores it when resolving the execution (see _screen_write). + _BUILD_EXECUTION_IDS = None + # The only keys a requirement write may carry. updateExecutions ALSO renames a flow -- + # any flow, by rep.getFlowId() -- when the body carries authenticationFlow, flowId and + # displayName, so a body is held to exactly what the builder needs. + _EXEC_UPDATE_KEYS = frozenset(("id", "requirement", "priority")) # Paths whose write targets must be screened, and how to pull the target out of them. _FLOW_WRITE_PATH = re.compile( @@ -543,6 +474,9 @@ data: # duplicate alias anyway, but "the API happens to reject it" is not a safety property; # this is.) _FLOW_CREATE_PATH = re.compile(r"/admin/realms/[^/]+/authentication/flows") + # PUT /flows//executions -- the requirement write. Its TARGET is body["id"]. + _EXEC_UPDATE_PATH = re.compile( + r"/admin/realms/[^/]+/authentication/flows/[^/]+/executions") def _set_protected(aliases, execution_ids): @@ -554,9 +488,18 @@ data: def _reset_protected(): """Back to NOT DETERMINED, so every flow write is refused again.""" - global _PROTECTED_FLOW_ALIASES, _PROTECTED_EXECUTION_IDS + global _PROTECTED_FLOW_ALIASES, _PROTECTED_EXECUTION_IDS, _BUILD_EXECUTION_IDS _PROTECTED_FLOW_ALIASES = None _PROTECTED_EXECUTION_IDS = None + _BUILD_EXECUTION_IDS = None + + + def _set_build_scope(execution_ids): + """Record the execution ids of the tree being built (from its own readback). Only + these may be targeted by an execution-id-addressed write. None closes the scope.""" + global _BUILD_EXECUTION_IDS + _BUILD_EXECUTION_IDS = (None if execution_ids is None + else frozenset(i for i in execution_ids if i)) class Refused(RuntimeError): @@ -589,26 +532,52 @@ data: "refusing %s %s: flow %r is part of the CURRENTLY BOUND browser flow" % (method, path, target)) return + # Every write below targets (a) a flow by the alias in its path, and/or (b) an + # execution by id -- in the path for the config POST, in the BODY for the + # requirement PUT. Both kinds are screened; one request can carry both. + targets = [] m = _FLOW_WRITE_PATH.fullmatch(path) - if m is None: - m = _EXEC_CONFIG_PATH.fullmatch(path) - if m is None: - return # not a flow write (realm PUT, action PUT, token) - target, protected, what = (m.group(1), _PROTECTED_EXECUTION_IDS, "execution") - else: - target = urllib.parse.unquote(m.group(1)) - protected, what = _PROTECTED_FLOW_ALIASES, "flow" - if protected is None: - raise Refused( - "refusing %s %s: the currently BOUND browser flow has not been determined " - "yet, so this program cannot prove the write would miss it. Flow writes are " - "fail-closed until the binding is known" % (method, path)) - if target in protected: - raise Refused( - "refusing %s %s: %s %r is part of the CURRENTLY BOUND browser flow. This " - "program builds a NEW flow and flips browserFlow atomically; it never " - "mutates the flow users are logging in through, because a half-finished " - "mutation there is a total SSO outage" % (method, path, what, target)) + if m is not None: + targets.append(("flow", urllib.parse.unquote(m.group(1)))) + if method == "PUT" and _EXEC_UPDATE_PATH.fullmatch(path): + # updateExecutions (26.0.6) resolves realm.getAuthenticationExecutionById( + # rep.getId()) and NEVER checks the execution belongs to the path's flow, so the + # path alias alone proves nothing about what is written. Screen the body. + extra = sorted(set(body or {}) - _EXEC_UPDATE_KEYS) + if extra: + raise Refused( + "refusing %s %s: the body carries %s. Only id / requirement / priority " + "may be sent -- updateExecutions also RENAMES any flow named by flowId " + "when a body carries authenticationFlow + flowId + displayName" + % (method, path, ", ".join(extra))) + targets.append(("execution", (body or {}).get("id"))) + m2 = _EXEC_CONFIG_PATH.fullmatch(path) + if m2 is not None: + targets.append(("execution", urllib.parse.unquote(m2.group(1)))) + if not targets: + return # not a flow write (realm PUT, action PUT, token) + for what, target in targets: + protected = (_PROTECTED_FLOW_ALIASES if what == "flow" + else _PROTECTED_EXECUTION_IDS) + if protected is None: + raise Refused( + "refusing %s %s: the currently BOUND browser flow has not been " + "determined yet, so this program cannot prove the write would miss it. " + "Flow writes are fail-closed until the binding is known" + % (method, path)) + if target in protected: + raise Refused( + "refusing %s %s: %s %r is part of the CURRENTLY BOUND browser flow. This " + "program builds a NEW flow and binds it atomically; it never mutates the " + "flow users are logging in through, because a half-finished mutation " + "there is a total SSO outage" % (method, path, what, target)) + if what == "execution" and (_BUILD_EXECUTION_IDS is None + or target not in _BUILD_EXECUTION_IDS): + raise Refused( + "refusing %s %s: execution %r is not part of the flow tree this run is " + "building. An execution-addressed write may only reach the candidate " + "flow's own executions, read back from the build itself" + % (method, path, target)) def _http(method, base, path, query=None, headers=None, form=None, body=None): @@ -1061,13 +1030,43 @@ data: # ============================================================================= # PASS 3 -- the browser authentication flow, BLUE/GREEN # ============================================================================= + def subflow_alias(ex): + """The alias of the sub-flow an executions-listing row refers to, or None. + + KEYCLOAK 26.0.6 NAMES A SUB-FLOW ROW BY displayName, NOT alias. + AuthenticationManagementResource.recurseExecutions (services/src/main/java/org/ + keycloak/services/resources/admin/AuthenticationManagementResource.java, tag 26.0.6, + the authenticatorFlow branch) does: + + AuthenticationFlowModel flowRef = realm.getAuthenticationFlowById( + execution.getFlowId()); + ... + rep.setDisplayName(flowRef.getAlias()); + ... + rep.setFlowId(execution.getFlowId()); + + and never calls rep.setAlias() on that row. `alias` is set only in the AUTHENTICATOR + branch, to the bound authenticator config's alias -- so reading `alias` here would + yield None for every sub-flow (and the protected subtree would silently shrink to + the top-level flow alone). The flows LIST cannot be used instead: getFlows() returns + top-level flows only, so a nested sub-flow's flowId resolves to nothing there. + + A sub-flow row with no non-empty displayName returns None, which the caller treats + as an UNREADABLE subtree -- fail-closed, never a smaller protected set. + """ + name = ex.get("displayName") + return name if isinstance(name, str) and name else None + + def read_executions(base, realm, token, alias): """(shape, raw) for a flow, or (None, None) if it cannot be read. `shape` is the same 6-tuple sequence expected_shape() produces, so the two can be compared element by element. Sub-flow names are blanked for the comparison (see - expected_shape) but the LIVE alias is kept as the 6th member so reports and the - protected-subtree walk can name it. + expected_shape) but the LIVE alias (subflow_alias(): displayName in 26.0.6) is kept + as the 6th member so reports and the protected-subtree walk can name it. A sub-flow + row whose alias cannot be determined makes the WHOLE listing unreadable, so nothing + downstream can act on a partial picture of the tree. """ hdrs = {"Authorization": "Bearer " + token} st, text = _http( @@ -1084,13 +1083,19 @@ data: if not isinstance(ex, dict): continue is_flow = bool(ex.get("authenticationFlow")) + if is_flow: + name = subflow_alias(ex) + if name is None: + return None, None + else: + name = ex.get("providerId") or "" shape.append(( int(ex.get("level") or 0), "flow" if is_flow else "authenticator", "" if is_flow else (ex.get("providerId") or ""), ex.get("requirement") or "", ex.get("authenticationConfig") or "", - (ex.get("alias") if is_flow else ex.get("providerId")) or "", + name, )) return shape, raw @@ -1268,6 +1273,20 @@ data: raise RuntimeError( "built %r has %d execution(s) but git declares %d -- the build did not " "produce the declared structure" % (top, len(raw), len(expected))) + # OPEN THE BUILD SCOPE: from here on, an execution-id-addressed write may reach these + # ids and no others. They come from the readback of the flow THIS CALL just created, + # so an id that belongs anywhere else in the realm stays unwritable. + _set_build_scope(ex.get("id") for ex in raw if isinstance(ex, dict)) + try: + _finish_build(base, realm, top, raw, expected, configs, mapped, hdrs, log) + finally: + _set_build_scope(None) + return top + + + def _finish_build(base, realm, top, raw, expected, configs, mapped, hdrs, log): + """Set requirements and bind configs on the freshly built tree. Every write here is + execution-id-addressed, so it runs only inside build_flow's build scope.""" for idx, ex in enumerate(raw): want_req = expected[idx][3] if ex.get("requirement") == want_req: @@ -1305,7 +1324,6 @@ data: % (want_cfg_alias, idx, top, st, text[:160])) log("flow bound authenticator config %s to step %d" % (mapped(want_cfg_alias), idx)) - return top def bind_browser_flow(base, realm, token, new_alias, log): @@ -1357,8 +1375,14 @@ data: return previous, [] - def reconcile_flow(base, realm, token, state, audit, no_bind, log): - """Return (changed, problems). Blue/green: build new, verify, flip atomically. + # Set by reconcile_flow() when a verified candidate exists but git forbids binding it, + # so the run's final line can say so instead of claiming everything matches git. + _PENDING_BIND = [None] + + + def reconcile_flow(base, realm, token, state, audit, bind, log): + """Return (changed, problems). Blue/green: build new, verify, and -- only if `bind` + (git's bind-browser-flow) -- flip atomically. Order matters and is load-bearing: 1. establish what is BOUND, and protect its whole subtree from writes. @@ -1368,7 +1392,9 @@ data: shape. If a flow already exists under it and MATCHES desired, it is a completed build from an earlier run -- REUSE it rather than rebuild. 4. build, read back, verify. Nothing is bound unless verification passes. - 5. ONE atomic PUT to flip browserFlow. + 5. ONE atomic PUT to flip browserFlow -- ONLY when bind is True. When it is False + the run ends at 4 and reports the alias it WOULD bind; whatever the realm is + bound to is left alone, so an operator's rollback is never undone. """ hdrs = {"Authorization": "Bearer " + token} problems = [] @@ -1441,6 +1467,15 @@ data: for attempt in range(1, MAX_BUILD_ATTEMPTS + 1): candidate_suffix = suffix if attempt == 1 else "%s-r%d" % (suffix, attempt) candidate = "%s-%s" % (state["browserFlow"], candidate_suffix) + if candidate in have and candidate == bound and not bind: + # The candidate alias IS the live binding, and it no longer matches git -- an + # operator edited the flow by hand (a hotfix), or someone else did. With the + # bind switch off, the binding is the operator's: do not build a replacement + # next to it, do not bind anything. Report and stop. + log("flow the BOUND flow %r is this program's own candidate alias but no " + "longer matches git (hand-edited?). bind-browser-flow is not \"true\", so " + "it is LEFT ALONE: nothing is built and nothing is bound" % bound) + return 0, problems if candidate in have: # A flow already exists here. If it is a COMPLETED build from an earlier run # it matches desired and can simply be bound -- which makes a crashed run @@ -1494,10 +1529,17 @@ data: log("flow verified %s against git (%d execution(s), configs included)" % (target, len(expected))) - if no_bind: - log("flow --no-bind: %s is built and VERIFIED but NOT bound. To go live, " - "one write: set the realm's browserFlow to %s" % (target, target)) - return 1, problems + if not bind: + # THE BIND SWITCH IS OFF. The realm's binding is NOT this program's to change: + # not the first time, and not after an operator has rolled it back. Zero writes + # to browserFlow, and not a failure -- this is the declared state. + _PENDING_BIND[0] = target + log("flow WOULD BIND %s -- it is built and VERIFIED against git, but " + "bind-browser-flow is not \"true\" in platform/keycloak/" + "passkey-desired-state.yaml, so browserFlow is LEFT AT %r. To go live: test " + "%s in a browser (README: Passkeys), then set bind-browser-flow: \"true\" in " + "a PR" % (target, bound, target)) + return (1 if built else 0), problems # ---- 5. the atomic flip --------------------------------------------------------- previous, bind_problems = bind_browser_flow(base, realm, token, target, log) @@ -1512,9 +1554,30 @@ data: # ============================================================================= # RUN # ============================================================================= + def declared_bind(path): + """git's bind-browser-flow switch: True, False, or raise ValueError. + + An ABSENT file is False -- the safe reading, and the one a ConfigMap predating the + switch produces. A PRESENT file must hold exactly "true" or "false": a typo must stop + the run, not be guessed at in either direction. + """ + try: + with open(path, "r", encoding="utf-8") as fh: + raw = fh.read().strip() + except FileNotFoundError: + return False + if raw == "true": + return True + if raw == "false": + return False + raise ValueError("%s holds %r; bind-browser-flow must be exactly \"true\" or " + "\"false\"" % (path, raw[:40])) + + def run(argv, log): audit = "--audit" in argv no_bind = "--no-bind" in argv + _PENDING_BIND[0] = None base = os.environ.get("KEYCLOAK_BASE_URL", "").strip() realm = os.environ.get("KEYCLOAK_REALM", "").strip() user = os.environ.get("KEYCLOAK_ADMIN_USER", "") @@ -1529,7 +1592,7 @@ data: log("FATAL: %s is empty" % name) return 1 mode = "audit (read-only)" if audit else ( - "reconcile, build but DO NOT bind" if no_bind else "reconcile") + "reconcile, --no-bind" if no_bind else "reconcile") log("mode=%s realm=%s base=%s" % (mode, realm, base)) # ---- FAIL CLOSED on the desired state -------------------------------------------- @@ -1543,6 +1606,16 @@ data: except (ValueError, TypeError) as e: log("FATAL: %s is unusable: %s. NOTHING WAS WRITTEN." % (STATE_FILE, e)) return 1 + try: + git_bind = declared_bind(BIND_FILE) + except (OSError, ValueError) as e: + log("FATAL: the bind switch is unusable: %s. NOTHING WAS WRITTEN." % e) + return 1 + # --no-bind can only NARROW what git allows; there is no flag that widens it. + bind = git_bind and not no_bind + log("bind-browser-flow (git): %s%s" + % ("true" if git_bind else "false", + " -- overridden to NO BIND by --no-bind" if git_bind and no_bind else "")) wanted_policy = state["webAuthnPolicyPasswordless"] wanted_action = state["requiredAction"] log("declared policy keys (%d): %s" @@ -1580,7 +1653,7 @@ data: wanted_action, audit, log)), ("browser flow (blue/green)", - lambda: reconcile_flow(base, realm, token, state, audit, no_bind, + lambda: reconcile_flow(base, realm, token, state, audit, bind, log))): log("") log("-- pass: %s --" % name) @@ -1619,6 +1692,12 @@ data: log("") log("KEYCLOAK_PASSKEY_AUDIT_OK: the passwordless policy, the enrolment required " "action and the bound browser flow all match git") + elif _PENDING_BIND[0]: + log("") + log("KEYCLOAK_PASSKEY_RECONCILED_OK_PENDING_BIND: the passwordless policy and the " + "enrolment action match git; the browser flow candidate %s is built and " + "verified but NOT bound, because bind-browser-flow is not \"true\" in git. " + "Login is unchanged." % _PENDING_BIND[0]) else: log("") log("KEYCLOAK_PASSKEY_RECONCILED_OK: the passwordless policy, the enrolment " @@ -1634,10 +1713,10 @@ data: # flow-construction endpoints (everything created DISABLED), and the updateExecutions # priority trap. # ============================================================================= - def _refused(method, path): - """True iff _http() refuses (method, path) outright.""" + def _refused(method, path, body=None): + """True iff _http() refuses (method, path, body) outright.""" try: - _http(method, "http://127.0.0.1:1", path, body={}) + _http(method, "http://127.0.0.1:1", path, body={} if body is None else body) except Refused: return True except OSError: @@ -1653,7 +1732,7 @@ data: import tempfile import threading - global STATE_FILE, READY_ATTEMPTS, READY_SLEEP + global STATE_FILE, BIND_FILE, READY_ATTEMPTS, READY_SLEEP ADMIN_PW = "stub-admin-password" RP_ID = "home.keithjackson.dev" @@ -1737,35 +1816,56 @@ data: state = {"realm": {}, "actions": {}, "flows": {}, "configs": {}, "realm_writes": [], "action_writes": [], "touched": [], "ids": [0], "lying_realm": False, "fail_build_after": None, "build_writes": [0], - "sabotage": None, "bind_writes": []} + "sabotage": None, "bind_writes": [], "hide_display_name": False} def new_id(prefix): state["ids"][0] += 1 return "%s-%d" % (prefix, state["ids"][0]) def flat(alias): - """The stub's executions listing: depth-first with levels, priority-ordered.""" + """The stub's executions listing: depth-first with levels, priority-ordered, and + SHAPED AS KEYCLOAK 26.0.6 SHAPES IT (AuthenticationManagementResource. + recurseExecutions). An earlier stub returned `alias` on sub-flow rows, which the + real server never does -- and every assertion passed against that fiction while + the protected subtree was, on a real server, only the top-level flow. So: + + sub-flow row displayName = the sub-flow's ALIAS, flowId, description, + authenticationFlow true, configurable false. NO `alias`, + NO providerId (basic-flow). + authenticator row displayName = the factory's DISPLAY TYPE (deliberately not + the provider id, so code reading it by mistake fails), + providerId, and -- only when a config is bound -- + authenticationConfig (the config id) and `alias` (the + CONFIG's alias, which is what the real server puts there). + + Null members are omitted, as Keycloak's NON_NULL serialisation omits them. + """ out = [] def walk(a, level): for i, ex in enumerate(sorted(state["flows"][a]["executions"], key=lambda e: e["priority"])): - is_flow = ex["kind"] == "flow" - out.append({ - "id": ex["id"], - "level": level, - "index": i, - "priority": ex["priority"], - "requirement": ex["requirement"], - "authenticationFlow": True if is_flow else None, - "alias": ex["name"] if is_flow else None, - "displayName": ex["name"], - "providerId": None if is_flow else ex["name"], - "flowId": ("flowid-" + ex["name"]) if is_flow else None, - "authenticationConfig": ex.get("configId"), - }) - if is_flow: + row = {"id": ex["id"], "level": level, "index": i, + "priority": ex["priority"], "requirement": ex["requirement"], + "requirementChoices": ["REQUIRED", "ALTERNATIVE", "DISABLED"]} + if ex["kind"] == "flow": + row.update({"authenticationFlow": True, "configurable": False, + "flowId": "flowid-" + ex["name"], + "description": ""}) + if not state["hide_display_name"]: + row["displayName"] = ex["name"] + out.append(row) walk(ex["name"], level + 1) + else: + row.update({"displayName": "Display: " + ex["name"], + "configurable": True, "providerId": ex["name"]}) + cid = ex.get("configId") + if cid: + row["authenticationConfig"] = cid + cfg = state["configs"].get(cid) or {} + if cfg.get("alias"): + row["alias"] = cfg["alias"] + out.append(row) walk(alias, 0) return out @@ -1932,6 +2032,8 @@ data: if budget(): return self._send(500, {"error": "stub build failure"}) rep = self._body() + # 26.0.6 resolves the execution REALM-WIDE by id and never checks it is + # in the path's flow -- modelled: find_exec searches every flow. ex = find_exec(rep.get("id")) if ex is None: return self._send(404, {"error": "Illegal execution"}) @@ -1993,6 +2095,21 @@ data: write_state() STATE_FILE = GOOD_STATE + BIND_PATH = os.path.join(tmpdir, "bind-browser-flow") + + def write_bind(value): + """git's bind-browser-flow. None removes the file (a ConfigMap without the key).""" + if value is None: + if os.path.exists(BIND_PATH): + os.unlink(BIND_PATH) + return + with open(BIND_PATH, "w", encoding="utf-8") as fh: + fh.write(value + "\n") + + # Sections 1-13 exercise the BIND path, so they run with git saying "true". The + # shipped default is "false"; section 14 covers it, and an absent file, explicitly. + write_bind("true") + BIND_FILE = BIND_PATH READY_ATTEMPTS, READY_SLEEP = 2, 0.01 os.environ.update({"KEYCLOAK_BASE_URL": base, "KEYCLOAK_REALM": "homelab", @@ -2074,6 +2191,7 @@ data: def reset(policy_drifted=True, tree=None, bound="browser-mfa"): state["lying_realm"] = False + state["hide_display_name"] = False state["fail_build_after"] = None state["sabotage"] = None state["ids"] = [0] @@ -2374,9 +2492,48 @@ data: "browser-mfa-abc123/executions/execution"), "a flow OUTSIDE the bound subtree must be writable -- that is what makes " "blue/green possible at all") + # An execution-id-addressed write needs BOTH: outside the bound subtree, AND inside + # the tree this run is building. Outside the bound subtree alone is not enough -- + # every other flow in the realm is outside it too. + expect(_refused("POST", "/admin/realms/homelab/authentication/executions/" + "ex-99/config"), + "binding a config to an execution that is NOT part of the tree being built " + "must be refused, even when it is outside the bound subtree") + _set_build_scope({"ex-99"}) expect(not _refused("POST", "/admin/realms/homelab/authentication/executions/" "ex-99/config"), - "an execution outside the bound subtree must be writable") + "an execution of the tree being built must be writable") + _set_build_scope(None) + + # 5b'. FINDING 3: PUT /flows//executions is screened on body["id"], not just + # the path. Keycloak's updateExecutions resolves the execution REALM-WIDE by id, + # so a candidate alias in the path says nothing about what is written. + EXEC_PUT = "/admin/realms/homelab/authentication/flows/browser-mfa-abc123/executions" + _set_build_scope({"ex-50", "ex-51"}) + expect(_refused("PUT", EXEC_PUT, + {"id": "ex-1", "requirement": "DISABLED", "priority": 0}), + "a requirement PUT whose path names the CANDIDATE flow but whose body id is an " + "execution of the BOUND flow must be refused -- updateExecutions never checks " + "the id belongs to the path's flow, so this would DISABLE a live login step") + expect(_refused("PUT", EXEC_PUT, + {"id": "ex-77", "requirement": "DISABLED", "priority": 0}), + "a requirement PUT whose body id is in NEITHER the bound subtree NOR the tree " + "being built must be refused -- it would edit some other flow in the realm") + expect(_refused("PUT", EXEC_PUT, {"requirement": "DISABLED", "priority": 0}), + "a requirement PUT with NO id must be refused") + expect(_refused("PUT", EXEC_PUT, + {"id": "ex-50", "requirement": "REQUIRED", "priority": 0, + "authenticationFlow": True, "flowId": "flowid-browser-mfa", + "displayName": "renamed"}), + "a requirement PUT carrying flowId/displayName must be refused -- the same " + "handler RENAMES whatever flow flowId names, bound or not") + expect(not _refused("PUT", EXEC_PUT, + {"id": "ex-50", "requirement": "REQUIRED", "priority": 3}), + "a requirement PUT on an execution of the tree being built must be allowed") + _set_build_scope(None) + expect(_refused("PUT", EXEC_PUT, + {"id": "ex-50", "requirement": "REQUIRED", "priority": 3}), + "with no build in progress, EVERY requirement PUT must be refused") _reset_protected() # 5e. THE BOUND FLOW CANNOT BE READ -> FAIL CLOSED, build nothing. Without the @@ -2617,8 +2774,10 @@ data: built = [a for a in state["flows"] if a.startswith("browser-mfa-") and a not in LEGACY_ALIASES] expect(built, "--no-bind must still BUILD the flow") - expect("NOT bound" in out and "one write" in out, - "--no-bind must print the one-write instruction to go live") + expect("WOULD BIND" in out, + "--no-bind must report the verified candidate it WOULD bind") + expect("overridden to NO BIND" in out, + "--no-bind overriding git's \"true\" must be stated in the log") # ========================================== 10. THE REQUIRED-ACTION BLAST RADIUS # 10a. the import-REPLACE damage: CONFIGURE_TOTP is gone. Loud, and NOT repaired. @@ -2802,6 +2961,149 @@ data: expect("browser" in state["flows"], "the previously bound built-in flow must NOT be deleted") + # ================== 14. FINDING 2: THE PROTECTED SUBTREE INCLUDES THE REAL SUB-FLOWS + # Discovered by a REAL run against the Keycloak-shaped listing (sub-flows named by + # displayName, no `alias`), not handed to _set_protected() by the test. + reset() + write_bind("false") + invoke() + expect(_PROTECTED_FLOW_ALIASES is not None + and set(LEGACY_ALIASES) <= set(_PROTECTED_FLOW_ALIASES), + "a real run must protect the bound flow AND every sub-flow beneath it, as " + "26.0.6 reports them (displayName on sub-flow rows), got %r" + % (sorted(_PROTECTED_FLOW_ALIASES or ()),)) + expect(_refused("POST", "/admin/realms/homelab/authentication/flows/" + "browser-mfa-forms/executions/execution"), + "after a real run, adding an execution to a protected SUB-flow " + "(browser-mfa-forms) must be REFUSED -- this is the write the alias-based " + "discovery let through") + expect(_refused("PUT", "/admin/realms/homelab/authentication/flows/" + "browser-mfa-conditional-otp/executions", + {"id": "ex-1", "requirement": "DISABLED", "priority": 0}), + "after a real run, a requirement write into the protected conditional-OTP " + "SUB-flow must be REFUSED") + expect(not [a for a in _PROTECTED_FLOW_ALIASES or () if a.startswith("Display: ")], + "an AUTHENTICATOR's displayName must never be mistaken for a sub-flow alias") + _reset_protected() + + # 14b. a sub-flow row WITHOUT a displayName: the subtree cannot be named, so the flow + # pass must stay FAIL-CLOSED -- build nothing, bind nothing -- rather than + # protect a smaller set than is live. + reset() + write_bind("true") + state["hide_display_name"] = True + before_flows = set(state["flows"]) + rc, out = invoke() + expect(rc == 1, "an unnameable sub-flow in the bound tree must FAIL the run") + expect("FAIL-CLOSED" in out, + "an unnameable sub-flow must leave the flow pass fail-closed, got tail=%r" + % out[-300:]) + expect(set(state["flows"]) == before_flows and state["bind_writes"] == [], + "with the bound subtree unnameable NOTHING may be built or bound, got new=%r " + "binds=%r" % (set(state["flows"]) - before_flows, state["bind_writes"])) + + # ===================== 15. FINDING 1: THE BIND SWITCH. "false" NEVER PUTs browserFlow + # 15a. first run with git saying "false": builds + verifies, binds NOTHING, exit 0. + reset() + write_bind("false") + rc, out = invoke() + expect(rc == 0, "bind=false must exit 0 -- an unbound verified candidate is the " + "DECLARED state, not a failure (tail=%r)" % out[-400:]) + expect(state["bind_writes"] == [], + "bind=false: NO realm PUT may change browserFlow, got %r" + % (state["bind_writes"],)) + expect(state["realm"]["browserFlow"] == "browser-mfa", + "bind=false: the realm must still be bound to the old flow, got %r" + % state["realm"]["browserFlow"]) + cand = [a for a in state["flows"] if a.startswith("browser-mfa-") + and a not in LEGACY_ALIASES and a.count("-") == 2] + expect(len(cand) == 1, "bind=false must still BUILD exactly one candidate, got %r" + % (cand,)) + expect("WOULD BIND" in out and (cand and cand[0] in out), + "bind=false must name the candidate it WOULD bind") + expect("KEYCLOAK_PASSKEY_RECONCILED_OK_PENDING_BIND" in out + and "KEYCLOAK_PASSKEY_RECONCILED_OK:" not in out, + "bind=false must NOT claim the bound flow matches git") + expect(state["realm"]["webAuthnPolicyPasswordlessRpId"] == RP_ID + and state["actions"][ENROL_ACTION]["enabled"] is True, + "bind=false gates ONLY the binding: passes 1 and 2 must still converge") + # 15b. and every later bind=false run is a no-op: reuse, zero writes of any kind. + flows_now = set(state["flows"]) + rc, out = invoke() + writes = [t for t in state["touched"] if t.split(" ")[0] in _WRITE_METHODS + and not t.endswith("/openid-connect/token")] + expect(rc == 0 and writes == [] and set(state["flows"]) == flows_now, + "a second bind=false run must make ZERO writes and build nothing, got " + "rc=%r writes=%r new=%r" % (rc, writes, set(state["flows"]) - flows_now)) + expect("reusing" in out and "WOULD BIND" in out, + "the second bind=false run must reuse the verified candidate and still report " + "what it would bind") + + # 15c. bind=false AFTER A PREVIOUS BIND: the operator's rollback STICKS. Bind with + # "true", then the operator flips the PR to "false" and rebinds the old flow. + reset() + write_bind("true") + invoke() + was_bound = state["realm"]["browserFlow"] + expect(was_bound != "browser-mfa", "precondition: the bind=true run must have bound") + write_bind("false") + state["realm"]["browserFlow"] = "browser-mfa" # the operator's rollback write + flows_now = set(state["flows"]) + for n in (1, 2, 3): + rc, out = invoke() + writes = [t for t in state["touched"] if t.split(" ")[0] in _WRITE_METHODS + and not t.endswith("/openid-connect/token")] + expect(state["bind_writes"] == [] and writes == [], + "ROLLBACK MUST STICK: run %d after bind=false must make NO write at all " + "(binds=%r writes=%r)" % (n, state["bind_writes"], writes)) + expect(state["realm"]["browserFlow"] == "browser-mfa", + "ROLLBACK MUST STICK: run %d rebound %r over the operator's rollback" + % (n, state["realm"]["browserFlow"])) + expect(rc == 0, "a rolled-back realm under bind=false must exit 0 (run %d)" % n) + expect(set(state["flows"]) == flows_now, + "a rolled-back realm under bind=false must not build anything, got %r" + % (set(state["flows"]) - flows_now,)) + + # 15d. bind=false and an operator HOTFIX inside the bound candidate: left alone -- + # no rebuild beside it (no -r2) and no rebind. + reset() + write_bind("true") + invoke() + live = state["realm"]["browserFlow"] + write_bind("false") + state["flows"][live]["executions"][0]["requirement"] = "DISABLED" # the hotfix + flows_now = set(state["flows"]) + rc, out = invoke() + expect(state["realm"]["browserFlow"] == live and state["bind_writes"] == [], + "bind=false: a hand-edited bound flow must stay bound, got %r binds=%r" + % (state["realm"]["browserFlow"], state["bind_writes"])) + expect(set(state["flows"]) == flows_now, + "bind=false: a hand-edited bound flow must not spawn a replacement build " + "(no -r2), got %r" % (set(state["flows"]) - flows_now,)) + expect("LEFT ALONE" in out, "the hand-edited bound flow must be reported as left alone") + expect(state["flows"][live]["executions"][0]["requirement"] == "DISABLED", + "the operator's hotfix must not be reverted") + + # 15e. an ABSENT bind file reads as "false" (a ConfigMap without the key binds + # nothing), and an INVALID one stops the whole run before any write. + reset() + write_bind(None) + rc, out = invoke() + expect(state["bind_writes"] == [] and state["realm"]["browserFlow"] == "browser-mfa", + "an ABSENT bind-browser-flow must mean NO BIND, got %r" + % (state["bind_writes"],)) + expect("bind-browser-flow (git): false" in out, + "the effective switch must be logged") + reset() + write_bind("yes") + rc, out = invoke() + expect(rc == 1 and state["realm_writes"] == [] and state["action_writes"] == [] + and not [t for t in state["touched"] if t.startswith("POST /admin")], + "an INVALID bind-browser-flow must stop the run before ANY write, got rc=%r " + "realm=%r" % (rc, state["realm_writes"])) + expect("NOTHING WAS WRITTEN" in out, "the invalid switch must be named as fatal") + write_bind("true") + srv.shutdown() print("") if failures: @@ -2809,8 +3111,10 @@ data: for f in failures: print(" FAIL %s" % f) return 1 - print("SELFTEST OK: %d assertions (blue/green build-verify-flip, the bound flow is " - "unwritable and fail-closed, a failed build never binds, verify-before-bind, " + print("SELFTEST OK: %d assertions (blue/green build-verify-flip, the git bind switch " + "and a rollback that sticks, the bound flow AND its 26.0.6-shaped sub-flows are " + "unwritable and fail-closed, body-id screening of the requirement PUT, a failed " + "build never binds, verify-before-bind, " "the conditional-OTP regression, idempotence with zero writes, recovery and a " "bounded alias budget, nothing is ever deleted, strict read-modify-write, the " "RpEntityName gate, required-action blast radius, fail-closed inputs)" @@ -2833,15 +3137,12 @@ data: # waiting for someone to notice that passkey login does not work. Runs are cheap: a # converged realm performs 5 GETs and ZERO writes. # -# A FAILING JOB IS THE ALARM, and unlike the first design of this component there is -# no longer any failure that is "expected by design" — the flow converges too, so a -# steady state here is a GREEN steady state. That makes a PrometheusRule on -# kube_job_status_failed{job_name=~"keycloak-passkey-reconciler.*"} both meaningful -# and worth having. It is NOT added here, and that is a real gap rather than a -# decision: this change is already large and the alert belongs with the rest of the -# keycloak alerting rules. Until it exists, a silently failing CronJob (image pull -# failure, a rotated admin password, a changed Secret key) is visible only to someone -# who looks at Jobs. Named explicitly so it is not mistaken for covered. +# A FAILING JOB IS THE ALARM. There is no failure that is "expected by design": with +# bind-browser-flow "false" an unbound, verified candidate is a SUCCESSFUL run (it is +# the declared state), so the steady state is GREEN either way. A failed Job raises +# KeycloakPasskeyReconcilerFailing (platform/monitoring/alerts.yaml, group +# platform-health.identity; runbooks/keycloak-passkey-reconciler.md), unit-tested in +# platform/monitoring/tests/keycloak-passkey-reconciler.test.yaml. apiVersion: batch/v1 kind: CronJob metadata: @@ -2858,13 +3159,17 @@ spec: concurrencyPolicy: Forbid startingDeadlineSeconds: 300 successfulJobsHistoryLimit: 1 - # Three, like the sibling: a flow divergence fails every run until an operator - # acts, and keeping the last three failures makes the Job log readable without - # letting completed pods accumulate. + # Three, like the sibling: keeping the last three failures makes the Job log + # readable without letting completed pods accumulate. failedJobsHistoryLimit: 3 jobTemplate: spec: - backoffLimit: 2 + # ZERO retries. Every failure this program reports is DETERMINISTIC within a tick + # (drift it may not repair, a refused write, a build that did not verify), so a + # retry repeats it -- and a retried BUILD failure consumes another of the + # MAX_BUILD_ATTEMPTS candidate aliases each time, burning the bounded budget in + # one tick instead of one per 15 minutes. The next scheduled run is the retry. + backoffLimit: 0 activeDeadlineSeconds: 600 ttlSecondsAfterFinished: 3600 template: @@ -2894,12 +3199,13 @@ spec: containers: - name: reconciler # Stock upstream python; the reconciler is pure standard library, so - # nothing is installed at runtime. Pinned exactly, and identical to the - # client-secret reconciler's image so the two share a layer. Renovate - # does not manage plain `image:` tags (renovate.json disables image-tag - # updates so it never fights Flux image-automation), so this is bumped - # by hand. - image: python:3.13.15-alpine3.23 + # nothing is installed at runtime. Same tag as the client-secret + # reconciler. PINNED BY DIGEST (the multi-arch index digest, read from + # Docker Hub 2026-10-29) because the official-image tag is MUTABLE: the + # sibling's pod on prod runs a different digest under this same tag. + # Renovate's `kubernetes` manager covers platform/** (renovate.json5), so + # it proposes tag and digest bumps for this line like any other image. + image: python:3.13.15-alpine3.23@sha256:6438599575cca0d1df94aeee0d2ae088d4d8846eab554b2ee7784a3a6df0d516 imagePullPolicy: IfNotPresent command: - python3 @@ -2935,7 +3241,9 @@ spec: mountPath: /opt/reconciler readOnly: true # The DESIRED STATE: the passwordless policy, the enrolment required - # action and the declared flow tree, GENERATED from + # action and the declared flow tree (passkey-desired-state.json), plus + # git's bind switch (bind-browser-flow) -- both keys of one ConfigMap, + # so both arrive through this one mount. The JSON is GENERATED from # keycloak-realm-import.yaml by ci/render-keycloak-passkey-policy.py # so the realm import stays the single source of truth. Mounted rather # than read through the Kubernetes API for the reason given above: this