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

feat(keycloak): reconcile client redirect URIs, not just client secrets

Browse Source 
The realm import is a one-shot that never re-runs, so a redirect URI changed in git
reaches the realm never. Prod's stored URIs were measured still on the pre-migration
hostname while git declared the new one -- after a BASE_DOMAIN flip every OIDC login
fails with a redirect_uri mismatch, with the CR still reading Done=True and nothing
alerting. Same failure class as the literal-placeholder secrets, same cure.

EXTENDS the existing reconciler rather than adding a sibling. Decisive reason: a URL
write needs NO new endpoint. It is the same minimal client PUT, and the current URLs
already come back in the allow-listed client lookup -- so the allow-list is widened by
nothing and the users/groups/roles/authentication guarantee is preserved unchanged
rather than re-declared in a second copy. The ConfigMap also carries exactly one
script key by L0 contract, so a sibling would have had to fork ~300 lines of
safety-critical code that the header explicitly forbids forking.

Reconciles redirectUris, webOrigins, rootUrl, baseUrl, adminUrl -- only the fields a
client actually declares, and on a write only the fields that actually drifted. A
field the desired state omits is UNMANAGED and left untouched; absence never means
force-to-empty.

THE TWO PASSES ARE INDEPENDENT. Each reads its own desired state and fails closed for
its own fields only. A malformed URL ConfigMap cannot stop client-SECRET repair, which
would otherwise make credential healing depend on a hostname file.

List fields compare order-insensitively (sorted, not set -- duplicates are still
drift), because Keycloak does not preserve the authored order of redirectUris and an
order-sensitive compare would rewrite every client forever while reporting success.

--selftest grows to 84 assertions covering the domain flip, a client gaining a URI, a
client losing a stale one, a public client's URLs being managed while its secret is
not, per-pass fail-closed, order-insensitivity, and a server that accepts a PUT
without applying it.

Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
jakman85andClaude Opus 5 committed 2026-09-12 21:25:34 -05:00
1 parent 51d8676f90
commit 23ea72d01e
2 files changed
+642 -67
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

conformance/tests/l2-behavioral/keycloak-client-secrets-match-git/chainsaw-test.yaml
+1 -1
View File
@@ -119,7 +119,7 @@ spec:
image: python:3.13.15-alpine3.23
command:
- python3
- /opt/reconciler/reconcile-client-secrets.py
- /opt/reconciler/reconcile-keycloak-clients.py
# READ-ONLY. --audit never issues a write on any path; the
# script's --selftest proves that (L0 gate).
- --audit
platform/keycloak/client-secret-reconciler.yaml
+641 -66
View File
@@ -1,6 +1,27 @@
---
# =============================================================================
# Keycloak CLIENT-SECRET RECONCILER (R-IDP-03, R-IDP-04)
# Keycloak CLIENT RECONCILER (R-IDP-03, R-IDP-04)
#
# TWO PASSES over the realm's OIDC clients, both closing the SAME failure class --
# the realm import is a one-shot that never re-runs, so anything it got wrong (or
# anything git changed afterwards) stays wrong forever:
#
# pass 1 CLIENT SECRETS, against the mounted SOPS keycloak-realm-clients Secret.
# The 2026-09-06 incident: all 12 clients stored the literal
# `(env:...)` placeholder and every OIDC login failed.
# pass 2 ENVIRONMENT-DERIVED CLIENT URLS (redirectUris, webOrigins, rootUrl,
# baseUrl, adminUrl), against the mounted keycloak-client-urls ConfigMap
# that ci/render-keycloak-client-urls.py generates from the realm import.
# The failure: prod's stored redirect URIs still named the PRE-migration
# hostname while git declared the new one, so after a BASE_DOMAIN flip
# every OIDC login fails with a redirect_uri mismatch. Nothing re-ran the
# import to fix it, and nothing alerted.
#
# THE TWO PASSES ARE INDEPENDENT. Each reads its OWN desired state and fails closed
# for its OWN fields only; neither can disable the other. That matters: if a
# malformed URL ConfigMap could abort the run, a typo in a hostname would stop
# client-SECRET repair, making credential healing depend on a file that has nothing
# to do with credentials. Proved by --selftest cases 6a/6b.
#
# CARRIED BY platform-config (dependsOn platform, wait:true), so it applies only
# after the `platform` tier has applied the SOPS-encrypted keycloak-realm-clients
@@ -48,6 +69,18 @@
# platform-admins / viewers, and they must survive every run. It never logs a
# secret, and it writes only where the stored value differs from git.
#
# ADDING PASS 2 WIDENED THE ALLOW-LIST BY NOTHING AT ALL. A URL write is the SAME
# minimal `PUT /admin/realms/<realm>/clients/<uuid>` the secret pass already used,
# and the current URLs come back in the client LOOKUP that was already allow-listed
# -- so pass 2 needed no new endpoint and the users/groups/roles guarantee is
# unchanged, not merely re-argued. --selftest asserts the refusal of /users,
# /groups, /roles, /authentication and the whole-realm endpoint directly.
#
# WHAT PASS 2 MAY WRITE is narrower still: only the five URL fields named above,
# and on any given client only the ones that actually drifted. A field the desired
# state does not declare is UNMANAGED and left exactly as it is -- absence never
# means "force to empty".
#
# PROOF: the script's own `--selftest` drives these real code paths against an
# in-process stub Keycloak and asserts compare-then-write, idempotence (a second
# run over a converged realm issues ZERO writes), no-secret-logging, the endpoint
@@ -70,12 +103,12 @@ data:
# rendered by a Flux Kustomization with postBuild.substituteFrom, so an
# unescaped dollar-brace expansion here would be rewritten before any container
# ran (the trap documented at length in apps/wave-1/nextcloud.yaml).
reconcile-client-secrets.py: |
reconcile-keycloak-clients.py: |
#!/usr/bin/env python3
# =============================================================================
# reconcile-client-secrets.py -- make every Keycloak OIDC client's STORED secret
# equal the value git declares for it, idempotently, without touching anything
# else in the realm.
# reconcile-keycloak-clients.py -- make every Keycloak OIDC client's STORED secret
# AND its environment-derived URLs equal what git declares, idempotently, without
# touching anything else in the realm.
#
# WHY THIS EXISTS (the incident it prevents)
# ------------------------------------------
@@ -94,8 +127,8 @@ data:
# So the import is a ONE-SHOT that fails OPEN. This is the level-triggered
# reconciler that closes it.
#
# CONTRACT
# --------
# CONTRACT -- PASS 1, CLIENT SECRETS
# ---------------------------------
# Desired state files in SECRETS_DIR named `<clientId>-client-secret`, each
# holding exactly the secret for that clientId (this is the
# `keycloak-realm-clients` Secret, mounted).
@@ -103,6 +136,31 @@ data:
# Action only where they differ: PUT /admin/realms/<realm>/clients/<uuid>
# with the MINIMAL representation {"clientId":..,"secret":..}.
#
# CONTRACT -- PASS 2, ENVIRONMENT-DERIVED CLIENT URLS
# --------------------------------------------------
# Desired state URLS_FILE, a JSON object {clientId: {field: value}} covering only
# redirectUris / webOrigins / rootUrl / baseUrl / adminUrl. It is the
# mounted `keycloak-client-urls` ConfigMap, GENERATED from
# keycloak-realm-import.yaml by ci/render-keycloak-client-urls.py and
# gated by the L0 check, so the realm import stays the single source
# of truth for every hostname. A field absent from it is UNMANAGED.
# Actual state the client LOOKUP response (which already carries these fields --
# no extra endpoint, no extra request).
# Action only where they differ: the same minimal client PUT, carrying
# clientId plus ONLY the fields that drifted.
#
# WHY THIS PASS EXISTS: the realm import is templated per environment, so a
# BASE_DOMAIN change re-renders every declared redirect URI in git -- and reaches
# the realm never, because the import will not re-run. Measured on prod: stored
# redirect URIs on the PRE-migration hostname while git declared the new one. Every
# OIDC login would fail with a redirect_uri mismatch, with nothing alerting and the
# CR still reading Done=True.
#
# LIST FIELDS ARE COMPARED ORDER-INSENSITIVELY (sorted, not set: duplicates are
# still drift). Keycloak does not preserve the authored order of redirectUris, so an
# order-sensitive comparison would rewrite every client on every run forever while
# reporting success.
#
# WHY MINIMAL: Keycloak merges non-null fields of a client representation, so a
# two-field PUT rewrites the secret and nothing else. PUTting the full
# round-tripped representation back has a far larger blast radius and is in fact
@@ -121,7 +179,18 @@ data:
# structurally unreachable from here -- prod's `homelab` realm carries a
# real user and the groups media-users / platform-admins / viewers, and they
# MUST survive.
# 5. public clients (e.g. `kubernetes`: PKCE, no secret) are skipped.
# 5. public clients (e.g. `kubernetes`: PKCE, no secret) are skipped BY PASS 1.
# Pass 2 does NOT skip them: a public client's redirect URIs are exactly what
# kubectl logins land on, and a stale one widens what the realm will redirect to.
# 6. the two passes are INDEPENDENT. Each reads its own desired state and fails
# closed for its own fields; an unreadable URL ConfigMap cannot stop secret
# repair, and an unmounted Secret cannot stop URL repair.
# 7. pass 2 writes ONLY the five URL fields, and on a given client only the ones
# that drifted. An undeclared field is never written at all.
#
# URLs ARE LOGGED IN FULL; SECRETS ARE NEVER LOGGED. Redirect URIs are public
# hostnames, and naming the exact before/after is what makes a domain migration
# auditable from the Job log. Property 3 is unchanged for secret values.
#
# MODES
# (default) reconcile: compare, and write only what differs.
@@ -161,6 +230,20 @@ data:
READY_ATTEMPTS = int(os.environ.get("READY_ATTEMPTS", "24"))
READY_SLEEP = float(os.environ.get("READY_SLEEP", "5"))
# ---- pass 2 input: the environment-derived client URLs -------------------------
# A GENERATED ConfigMap, rendered from keycloak-realm-import.yaml by
# ci/render-keycloak-client-urls.py and gated by the L0 check, mounted read-only.
# See the header for why this is a mounted file and not a read of the CR.
URLS_FILE = os.environ.get("CLIENT_URLS_FILE",
"/etc/keycloak-client-urls/client-urls.json")
# The ONLY client fields this program may write, besides the secret. Each is an
# environment-derived URL that changes when the base domain changes. Anything not
# named here is not writable by this program at all: an unknown field in the
# desired-state file is a hard error, not something to pass through to Keycloak.
URL_LIST_FIELDS = ("redirectUris", "webOrigins")
URL_STR_FIELDS = ("rootUrl", "baseUrl", "adminUrl")
URL_FIELDS = URL_LIST_FIELDS + URL_STR_FIELDS
# Endpoint allow-list (safety property 4). fullmatch-ed against the path of every
# URL _http() is asked to open; anything else raises before a socket exists.
_ALLOWED = (
@@ -223,6 +306,80 @@ data:
return out
def declared_urls(path):
"""{clientId: {field: value}} from the mounted, generated client-URL ConfigMap.
Validates the shape STRICTLY and raises on anything unexpected. This file decides
what gets written into the realm, so a malformed one must stop the URL pass dead
rather than be half-understood: an unknown field name here would otherwise be
forwarded to Keycloak as a client-representation field, which is exactly the
wide-blast-radius write the minimal-PUT rule exists to prevent.
"""
with open(path, "r", encoding="utf-8") as fh:
raw = json.load(fh)
if not isinstance(raw, dict) or not raw:
raise ValueError("%s does not hold a non-empty JSON object of clients" % path)
out = {}
for cid, entry in raw.items():
if not isinstance(cid, str) or not cid:
raise ValueError("%s has a client with an empty clientId" % path)
if not isinstance(entry, dict) or not entry:
raise ValueError("%s: client %r has no declared URL fields" % (path, cid))
clean = {}
for field, value in entry.items():
if field not in URL_FIELDS:
raise ValueError(
"%s: client %r declares field %r, which this reconciler is not "
"allowed to write. Writable fields are: %s"
% (path, cid, field, ", ".join(URL_FIELDS)))
if field in URL_LIST_FIELDS:
if (not isinstance(value, list)
or not all(isinstance(x, str) and x for x in value)):
raise ValueError("%s: client %r field %s must be a list of "
"non-empty strings" % (path, cid, field))
clean[field] = list(value)
else:
if not isinstance(value, str):
raise ValueError("%s: client %r field %s must be a string"
% (path, cid, field))
clean[field] = value
out[cid] = clean
return out
def url_drift(client, wanted):
"""{field: (stored, declared)} for every declared URL field that differs.
LIST FIELDS ARE COMPARED AS SORTED SEQUENCES, not in the order Keycloak happens to
return them. Keycloak does not preserve the authored order of redirectUris, so an
order-sensitive comparison would report drift on every run and rewrite the client
forever -- the exact opposite of "a converged realm produces zero writes". Sorting
rather than set()-ing keeps it duplicate-sensitive, so a realm holding the same URI
twice is still drift.
A field ABSENT from `wanted` is UNMANAGED and never inspected: the desired-state
file only carries what the realm import actually declares, and absence must mean
"leave it alone", never "force it empty". Keycloak reports an unset string field as
null or absent; both read as "" so an unset rootUrl matching a declared "" is not
perpetual drift.
"""
drift = {}
for field, declared_value in sorted(wanted.items()):
if field in URL_LIST_FIELDS:
stored = client.get(field) or []
if not isinstance(stored, list):
stored = []
if sorted(stored) != sorted(declared_value):
drift[field] = (sorted(stored), sorted(declared_value))
else:
stored = client.get(field) or ""
if not isinstance(stored, str):
stored = ""
if stored != declared_value:
drift[field] = (stored, declared_value)
return drift
def admin_token(base, user, password, log):
"""Bearer token for admin-cli in the master realm, with a bounded readiness wait."""
last = ""
@@ -315,6 +472,86 @@ data:
return changed, problems
def reconcile_url_fields(base, realm, token, want, audit, log):
"""PASS 2 -- make every declared client URL match git. Return (changed, problems).
Independent of the secret pass by construction: it shares only the bearer token and
the endpoint allow-list, reads a different input file, and writes a disjoint set of
client fields. Neither pass can disable the other (see run()).
Unlike the secret pass this does NOT skip public clients. A public client's redirect
URIs matter just as much -- `kubernetes` is PKCE-public and its localhost callbacks
are what kubectl logins land on -- and a stale URI left behind on any client widens
what the realm will redirect to.
URL VALUES ARE LOGGED IN FULL, deliberately. They are public hostnames, not
secrets, and naming the exact before/after is what makes a domain migration
auditable from the Job log. The secret pass still never prints a value.
"""
hdrs = {"Authorization": "Bearer " + token}
changed = 0
problems = []
for cid in sorted(want):
st, text = _http("GET", base, "/admin/realms/%s/clients" % realm,
query={"clientId": cid}, headers=hdrs)
if st != 200:
problems.append("%s: client lookup failed (http %s)" % (cid, st))
continue
found = json.loads(text)
if not isinstance(found, list) or not found:
problems.append("%s: NOT FOUND in realm %s -- git declares URLs for it but "
"the realm has no such client (the operator runs the realm "
"import exactly once)" % (cid, realm))
continue
client = found[0]
uuid = client.get("id")
drift = url_drift(client, want[cid])
if not drift:
log("%-14s urls ok (already correct, no write)" % cid)
continue
for field in sorted(drift):
stored, declared_value = drift[field]
if audit:
problems.append("%s: %s DRIFT -- realm has %r, git declares %r"
% (cid, field, stored, declared_value))
else:
log("%-14s %s DRIFT -- realm has %r, git declares %r"
% (cid, field, stored, declared_value))
if audit:
continue
# MINIMAL representation, same rule as the secret pass: clientId plus ONLY the
# fields that actually drifted. Keycloak merges non-null fields, so the client's
# secret, protocol mappers, scopes, flags and every realm-level object are
# untouched by this write. A full round-tripped representation is rejected
# outright (400 "Cannot parse the JSON") and is modelled as such in --selftest.
rep = {"clientId": client.get("clientId")}
for field in sorted(drift):
rep[field] = want[cid][field]
st, text = _http("PUT", base, "/admin/realms/%s/clients/%s" % (realm, uuid),
headers=hdrs, body=rep)
if not 200 <= st < 300:
problems.append("%s: url PUT failed (http %s) %s" % (cid, st, text[:160]))
continue
# Readback through the same allow-listed lookup: a 204 only says the request
# parsed, not that the realm now holds what git declares.
st, text = _http("GET", base, "/admin/realms/%s/clients" % realm,
query={"clientId": cid}, headers=hdrs)
after = json.loads(text) if st == 200 else []
if not isinstance(after, list) or not after:
problems.append("%s: url PUT was accepted but the client could not be "
"re-read to verify it" % cid)
continue
still = url_drift(after[0], want[cid])
if still:
problems.append("%s: url PUT was accepted but %s still does not match git"
% (cid, ", ".join(sorted(still))))
continue
changed += 1
log("%-14s urls REPAIRED (%s) and verified by readback"
% (cid, ", ".join(sorted(drift))))
return changed, problems
def run(argv, log):
audit = "--audit" in argv
base = os.environ.get("KEYCLOAK_BASE_URL", "").strip()
@@ -329,45 +566,129 @@ data:
return 1
log("mode=%s realm=%s base=%s"
% ("audit (read-only)" if audit else "reconcile", realm, base))
# ---- READ BOTH DESIRED STATES FIRST, AND INDEPENDENTLY ---------------------
# Each pass fails closed for ITS OWN fields and MUST NOT be able to disable the
# other. If the URL ConfigMap were allowed to gate the whole run, an unapplied or
# malformed ConfigMap would stop client-SECRET repair -- making SSO credential
# healing depend on a file that has nothing to do with credentials. That would be
# a strictly worse posture than before this pass existed, so the two inputs are
# read separately and a failure in one is recorded, not propagated.
want_secrets = None
secret_input_fail = ""
try:
want = declared(SECRETS_DIR)
want_secrets = declared(SECRETS_DIR)
except OSError as e:
log("FATAL: cannot read %s (%s)" % (SECRETS_DIR, e.__class__.__name__))
secret_input_fail = ("cannot read %s (%s)"
% (SECRETS_DIR, e.__class__.__name__))
if want_secrets is not None and not want_secrets:
want_secrets = None
secret_input_fail = (
"no `<clientId>%s` keys found in %s -- the keycloak-realm-clients Secret "
"is missing or empty, and reporting success here would repeat the very "
"fail-open the realm import already has" % (KEY_SUFFIX, SECRETS_DIR))
want_urls = None
url_input_fail = ""
try:
want_urls = declared_urls(URLS_FILE)
except OSError as e:
url_input_fail = ("cannot read %s (%s) -- the keycloak-client-urls ConfigMap "
"is not mounted" % (URLS_FILE, e.__class__.__name__))
except (ValueError, TypeError) as e:
url_input_fail = "%s is unusable: %s" % (URLS_FILE, e)
if want_secrets is None and want_urls is None:
log("FATAL: neither desired state could be read, so nothing can be "
"reconciled and NOTHING WAS WRITTEN.")
log("FATAL: secrets: %s" % secret_input_fail)
log("FATAL: urls: %s" % url_input_fail)
return 1
if not want:
log("FATAL: no `<clientId>%s` keys found in %s -- the keycloak-realm-clients "
"Secret is missing or empty, and reporting success here would repeat the "
"very fail-open the realm import already has"
% (KEY_SUFFIX, SECRETS_DIR))
return 1
log("declared clients (%d): %s" % (len(want), ", ".join(sorted(want))))
if want_secrets is not None:
log("declared client secrets (%d): %s"
% (len(want_secrets), ", ".join(sorted(want_secrets))))
if want_urls is not None:
log("declared client urls (%d): %s"
% (len(want_urls), ", ".join(sorted(want_urls))))
try:
token = admin_token(base, user, password, log)
changed, problems = reconcile(base, realm, token, want, audit, log)
except Refused as e:
log("FATAL: %s" % e)
return 1
except (RuntimeError, OSError, ValueError) as e:
log("FATAL: %s: %s" % (e.__class__.__name__, e))
return 1
rc = 0
problems = []
# ---- PASS 1: client secrets ------------------------------------------------
if want_secrets is None:
log("")
log("SECRETS PASS SKIPPED -- FAILED CLOSED, no secret was written: %s"
% secret_input_fail)
rc = 1
else:
log("")
log("-- pass 1: client secrets --")
try:
changed, found = reconcile(base, realm, token, want_secrets, audit, log)
except Refused as e:
log("FATAL: %s" % e)
return 1
except (RuntimeError, OSError, ValueError) as e:
log("FATAL: %s: %s" % (e.__class__.__name__, e))
return 1
problems.extend(found)
if found:
rc = 1
elif audit:
log("KEYCLOAK_CLIENT_SECRETS_AUDIT_OK: all %d declared client secret(s) "
"match git" % len(want_secrets))
else:
log("KEYCLOAK_CLIENT_SECRETS_RECONCILED_OK: %d client(s) checked, %d "
"repaired" % (len(want_secrets), changed))
# ---- PASS 2: environment-derived client URLs -------------------------------
if want_urls is None:
log("")
log("URL PASS SKIPPED -- FAILED CLOSED, no URL was written: %s"
% url_input_fail)
rc = 1
else:
log("")
log("-- pass 2: client urls --")
try:
changed, found = reconcile_url_fields(base, realm, token, want_urls,
audit, log)
except Refused as e:
log("FATAL: %s" % e)
return 1
except (RuntimeError, OSError, ValueError) as e:
log("FATAL: %s: %s" % (e.__class__.__name__, e))
return 1
problems.extend(found)
if found:
rc = 1
elif audit:
log("KEYCLOAK_CLIENT_URLS_AUDIT_OK: all %d declared client URL set(s) "
"match git" % len(want_urls))
else:
log("KEYCLOAK_CLIENT_URLS_RECONCILED_OK: %d client(s) checked, %d "
"repaired" % (len(want_urls), changed))
if problems:
log("")
for p in problems:
log("PROBLEM %s" % p)
log("")
if audit:
log("AUDIT FAIL: %d client secret(s) do not match git" % len(problems))
log("AUDIT FAIL: %d client field(s) do not match git" % len(problems))
else:
log("RECONCILE FAIL: %d client secret(s) could not be reconciled"
log("RECONCILE FAIL: %d client field(s) could not be reconciled"
% len(problems))
return 1
if audit:
log("KEYCLOAK_CLIENT_SECRETS_AUDIT_OK: all %d declared client secret(s) match "
"git" % len(want))
else:
log("KEYCLOAK_CLIENT_SECRETS_RECONCILED_OK: %d client(s) checked, %d repaired"
% (len(want), changed))
return 0
return rc
# =============================================================================
@@ -392,7 +713,7 @@ data:
import tempfile
import threading
global SECRETS_DIR, READY_ATTEMPTS, READY_SLEEP
global SECRETS_DIR, URLS_FILE, READY_ATTEMPTS, READY_SLEEP
SECRET_A = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
SECRET_B = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
@@ -403,7 +724,11 @@ data:
# spelled this way so the file stays free of the character envsubst reacts to.
LITERAL = chr(36) + "(env:PIHOLE_CLIENT_SECRET)"
state = {"clients": {}, "writes": [], "touched": []}
# "lying" holds clientIds for which the stub ACCEPTS a URL PUT (204) and then does
# NOT apply it -- a server that reports success without converging. Only the
# readback catches that, so it is what proves the readback is load-bearing.
state = {"clients": {}, "secret_writes": [], "url_writes": [],
"touched": [], "lying": set()}
class Stub(http.server.BaseHTTPRequestHandler):
def log_message(self, *a):
@@ -438,15 +763,32 @@ data:
if m and method == "PUT":
raw = self.rfile.read(int(self.headers.get("Content-Length") or 0))
rep = json.loads(raw or b"{}")
keys = set(rep)
# The live server rejects a full round-tripped representation with
# 400 "Cannot parse the JSON"; modelled so a regression to the fat
# PUT fails here rather than against a real realm.
if set(rep) != {"clientId", "secret"}:
# PUT fails here rather than against a real realm. Exactly two minimal
# shapes are accepted: the secret write, and a URL write carrying only
# clientId plus writable URL fields.
is_secret_put = keys == {"clientId", "secret"}
rest = keys - {"clientId"}
is_url_put = ("clientId" in keys and rest
and rest.issubset(set(URL_FIELDS)))
if not (is_secret_put or is_url_put):
return self._send(400, {
"error": "unknown_error",
"error_description": "Cannot parse the JSON"})
state["writes"].append(m.group(1))
state["clients"][m.group(1)]["secret"] = rep["secret"]
target = state["clients"][m.group(1)]
if is_secret_put:
state["secret_writes"].append(m.group(1))
target["secret"] = rep["secret"]
else:
# Record WHICH fields were written: the proof that a URL write
# never carries a secret and never widens past the drifted field.
state["url_writes"].append(
(m.group(1), tuple(sorted(rest))))
if m.group(1) not in state["lying"]:
for f in rest:
target[f] = rep[f]
return self._send(204, {})
return self._send(500, {"error": "an endpoint the reconciler must not touch"})
@@ -472,6 +814,41 @@ data:
os.mkdir(os.path.join(tmp, "..data"))
SECRETS_DIR = tmp
# ---- pass-2 fixture: THE DOMAIN MIGRATION, modelled -------------------------
# git declares the NEW apex hostnames; the realm below still holds the OLD
# pre-migration ones, which is exactly what was measured on prod (stored
# grafana redirectUri on `grafana.prod.home.` while git declared `grafana.home.`).
NEW = "home.keithjackson.dev"
OLD = "prod.home.keithjackson.dev"
WANT_URLS = {
# drifted: the domain flip. Also proves a MULTI-FIELD write.
"grafana": {
"redirectUris": ["https://grafana." + NEW + "/login/generic_oauth"],
"webOrigins": ["https://grafana." + NEW],
},
# already converged: must take ZERO writes even though its secret drifts.
"pihole": {
"redirectUris": ["https://pihole." + NEW + "/oauth2/callback"],
},
# a PUBLIC client still has its URLs managed -- kubectl logins land on these.
# Declares TWO URIs where the realm holds one: proves GAINING a URI.
"kubernetes": {
"redirectUris": ["http://localhost:8000", "http://localhost:18000"],
},
# declares ONE URI where the realm holds two: proves LOSING a URI. A stale
# wildcard left behind is a real widening of what the realm will redirect to.
"gitea": {
"redirectUris": ["https://gitea." + NEW + "/user/oauth2/keycloak/callback"],
"rootUrl": "https://gitea." + NEW,
},
}
urls_dir = tempfile.mkdtemp()
URLS_GOOD = os.path.join(urls_dir, "client-urls.json")
with open(URLS_GOOD, "w", encoding="utf-8") as fh:
json.dump(WANT_URLS, fh)
URLS_FILE = URLS_GOOD
READY_ATTEMPTS, READY_SLEEP = 2, 0.01
os.environ.update({"KEYCLOAK_BASE_URL": base, "KEYCLOAK_REALM": "homelab",
"KEYCLOAK_ADMIN_USER": "admin",
@@ -486,73 +863,187 @@ data:
failures.append(what)
def invoke(*argv):
state["writes"] = []
state["secret_writes"] = []
state["url_writes"] = []
state["touched"] = []
buf = io.StringIO()
with contextlib.redirect_stdout(buf):
rc = run(list(argv), print)
return rc, buf.getvalue()
def url_writes():
"""[(clientId, (fields written, ...))] sorted -- the URL blast radius."""
return sorted(state["url_writes"])
def reset():
state["lying"] = set()
state["clients"] = {
# already correct
# secret already correct; URLs on the OLD domain (the migration drift)
"grafana": {"clientId": "grafana", "publicClient": False,
"secret": SECRET_A},
# THE INCIDENT: the literal, unresolved import placeholder
"pihole": {"clientId": "pihole", "publicClient": False, "secret": LITERAL},
# public client: no secret, must be skipped
"kubernetes": {"clientId": "kubernetes", "publicClient": True},
# ordinary drift (rotated in git, stale in the realm)
"gitea": {"clientId": "gitea", "publicClient": False, "secret": STALE},
"secret": SECRET_A,
"redirectUris": ["https://grafana." + OLD
+ "/login/generic_oauth"],
"webOrigins": ["https://grafana." + OLD]},
# THE INCIDENT: the literal, unresolved import placeholder.
# URLs are ALREADY CONVERGED -- proves the two passes are independent:
# a secret repair here must not drag a URL write along with it.
# adminUrl is set in the realm and NOT declared in git: an UNMANAGED field.
# It must survive untouched. "Not declared" must mean "leave it alone",
# never "force it empty" -- an operator-set adminUrl or a field a future
# Keycloak version populates itself must not be wiped by this reconciler.
"pihole": {"clientId": "pihole", "publicClient": False, "secret": LITERAL,
"redirectUris": ["https://pihole." + NEW + "/oauth2/callback"],
"adminUrl": "https://pihole." + NEW + "/admin"},
# public client: no secret (skipped by pass 1) but URLs ARE managed.
# Holds one URI where git declares two => must GAIN one.
"kubernetes": {"clientId": "kubernetes", "publicClient": True,
"redirectUris": ["http://localhost:8000"]},
# ordinary secret drift, plus a STALE EXTRA redirect URI that must be
# REMOVED, plus an unset rootUrl that must be set.
"gitea": {"clientId": "gitea", "publicClient": False, "secret": STALE,
"redirectUris": [
"https://gitea." + NEW + "/user/oauth2/keycloak/callback",
"https://gitea." + OLD + "/user/oauth2/keycloak/callback"]},
}
def leaked(text):
return [s for s in (SECRET_A, SECRET_B, SECRET_C, STALE, ADMIN_PW)
if s in text]
# -- 1. audit of a drifted realm: fails, names the placeholder, writes NOTHING
# -- 1. audit of a drifted realm: fails, names the drift, writes NOTHING
reset()
rc, out = invoke("--audit")
expect(rc == 1, "audit must exit 1 when a stored secret differs from git")
expect(state["writes"] == [],
"audit mode must never write (writes=%r)" % state["writes"])
expect(rc == 1, "audit must exit 1 when a stored value differs from git")
expect(state["secret_writes"] == [] and state["url_writes"] == [],
"audit mode must never write (secrets=%r urls=%r)"
% (state["secret_writes"], state["url_writes"]))
expect("UNRESOLVED IMPORT PLACEHOLDER" in out,
"audit must name the unresolved-placeholder case explicitly")
expect("pihole: DRIFT" in out and "gitea: DRIFT" in out,
"audit must report BOTH drifted clients, not stop at the first")
"audit must report BOTH drifted client secrets, not stop at the first")
expect(re.search(r"grafana\s+ok \(already correct", out),
"an already-correct client must be reported as such")
"an already-correct client secret must be reported as such")
expect(re.search(r"kubernetes\s+skipped \(public client", out),
"a public client must be skipped, not treated as drift")
"a public client must be skipped BY THE SECRET PASS, not treated as drift")
expect(not leaked(out), "audit output leaked a secret: %r" % leaked(out))
# the URL half of the same audit
expect("grafana: redirectUris DRIFT" in out and "grafana: webOrigins DRIFT" in out,
"audit must report EVERY drifted URL field of a client, not just the first")
expect("kubernetes: redirectUris DRIFT" in out,
"audit must report URL drift on a PUBLIC client -- its URLs are managed "
"even though its secret is not")
expect(re.search(r"pihole\s+urls ok \(already correct", out),
"a client whose URLs are already converged must be reported as such")
expect(OLD in out,
"a URL audit must print the actual stored value -- URLs are public "
"hostnames, and naming the before/after is what makes a migration auditable")
# -- 2. reconcile: repairs exactly the drifted clients, verified by readback
# -- 2. reconcile: repairs exactly what drifted, in both passes, readback-verified
reset()
rc, out = invoke()
expect(rc == 0, "reconcile must exit 0 after repairing (tail=%r)" % out[-300:])
expect(sorted(state["writes"]) == ["gitea", "pihole"],
"reconcile must write exactly the drifted clients, got %r" % state["writes"])
expect("grafana" not in state["writes"],
"compare-then-write: an already-correct client must not be written")
expect("kubernetes" not in state["writes"], "a public client must not be written")
expect(rc == 0, "reconcile must exit 0 after repairing (tail=%r)" % out[-400:])
expect(sorted(state["secret_writes"]) == ["gitea", "pihole"],
"the secret pass must write exactly the secret-drifted clients, got %r"
% state["secret_writes"])
expect("grafana" not in state["secret_writes"],
"compare-then-write: an already-correct secret must not be written")
expect("kubernetes" not in state["secret_writes"],
"a public client must not be written by the secret pass")
expect("REPAIRED and verified by readback" in out,
"every repair must be readback-verified")
expect("2 repaired" in out, "the summary must count the repairs")
expect(not leaked(out), "reconcile output leaked a secret: %r" % leaked(out))
# -- 2a. THE URL BLAST RADIUS, field by field. This is the whole point: a write
# carries ONLY the fields that drifted, for ONLY the clients that drifted.
expect(url_writes() == [
("gitea", ("redirectUris", "rootUrl")),
("grafana", ("redirectUris", "webOrigins")),
("kubernetes", ("redirectUris",)),
],
"the URL pass must write exactly the drifted FIELDS of exactly the drifted "
"clients, got %r" % (url_writes(),))
expect(not [w for w in state["url_writes"] if w[0] == "pihole"],
"INDEPENDENCE: pihole's secret was repaired but its URLs were already "
"converged, so it must take NO url write")
expect(not [w for w in state["url_writes"] if "secret" in w[1]],
"a URL write must never carry the `secret` field")
# the corrections actually landed, including the ones that are not simple swaps
expect(state["clients"]["grafana"]["redirectUris"]
== ["https://grafana." + NEW + "/login/generic_oauth"],
"the drifted redirectUri must be CORRECTED to the declared value, got %r"
% (state["clients"]["grafana"]["redirectUris"],))
expect(sorted(state["clients"]["kubernetes"]["redirectUris"])
== sorted(["http://localhost:8000", "http://localhost:18000"]),
"a client GAINING a redirect URI must end up with both, got %r"
% (state["clients"]["kubernetes"]["redirectUris"],))
expect(state["clients"]["gitea"]["redirectUris"]
== ["https://gitea." + NEW + "/user/oauth2/keycloak/callback"],
"a client LOSING a redirect URI must have the stale one REMOVED (a stale "
"URI left behind widens what the realm will redirect to), got %r"
% (state["clients"]["gitea"]["redirectUris"],))
expect(state["clients"]["gitea"].get("rootUrl") == "https://gitea." + NEW,
"a previously-unset rootUrl must be set to the declared value")
expect("urls REPAIRED" in out, "a URL repair must be logged as such")
# UNMANAGED MEANS UNTOUCHED. pihole holds an adminUrl that git does not declare.
# Absence from the desired state must never be read as "force this to empty".
expect(state["clients"]["pihole"].get("adminUrl")
== "https://pihole." + NEW + "/admin",
"a field the desired state does NOT declare must be left exactly as it "
"was, not wiped, got %r" % (state["clients"]["pihole"].get("adminUrl"),))
# -- 2b. READBACK IS LOAD-BEARING. A server that accepts the PUT with 204 and does
# not apply it must be reported as a FAILURE, not as a repair. Without the
# readback this is indistinguishable from success -- and "reported success
# while the realm never converged" is the entire failure class being closed.
reset()
state["lying"] = {"grafana"}
rc, out = invoke()
expect(rc == 1, "a PUT accepted but not applied must fail the run, not pass it")
expect("still does not match git" in out,
"the readback must name the client whose write did not take effect")
expect(not [ln for ln in out.splitlines()
if "urls REPAIRED" in ln and "grafana" in ln],
"a client whose write did not take effect must NOT be logged as repaired")
# Re-converge before the idempotence check below, which requires a converged realm.
state["lying"] = set()
invoke()
# -- 3. IDEMPOTENCE: re-running against the converged realm writes nothing
rc, out = invoke()
expect(rc == 0, "the second reconcile must exit 0")
expect(state["writes"] == [],
"IDEMPOTENCE: a converged realm must take ZERO writes, got %r"
% state["writes"])
expect(state["secret_writes"] == [] and state["url_writes"] == [],
"IDEMPOTENCE: a converged realm must take ZERO writes, got secrets=%r "
"urls=%r" % (state["secret_writes"], state["url_writes"]))
expect("0 repaired" in out, "the second run must report 0 repaired")
rc, out = invoke("--audit")
expect(rc == 0, "audit must pass once the realm has converged")
expect("KEYCLOAK_CLIENT_SECRETS_AUDIT_OK" in out,
"a clean audit must emit the sentinel the L2 conformance test greps for")
"a clean audit must emit the secret sentinel the L2 test greps for")
expect("KEYCLOAK_CLIENT_URLS_AUDIT_OK" in out,
"a clean audit must emit the URL sentinel the L2 test greps for")
# -- 4. BLAST RADIUS: only the allow-listed endpoint shapes are ever touched
# -- 3a. ORDER-INSENSITIVITY. Keycloak does not preserve the authored order of
# redirectUris. If the comparison were order-sensitive, a converged realm
# would be rewritten on EVERY run -- a permanent write loop against the
# IdP database that still reports success.
state["clients"]["kubernetes"]["redirectUris"] = [
"http://localhost:18000", "http://localhost:8000"]
rc, out = invoke()
expect(rc == 0, "a reordered-but-equal URI list must still exit 0")
expect(state["url_writes"] == [],
"ORDER-INSENSITIVITY: a realm holding the declared URIs in a different "
"order is CONVERGED and must take zero writes, got %r"
% (state["url_writes"],))
# ... but a DUPLICATE is still drift: sorted(), not set().
state["clients"]["kubernetes"]["redirectUris"] = [
"http://localhost:8000", "http://localhost:8000", "http://localhost:18000"]
rc, out = invoke()
expect(url_writes() == [("kubernetes", ("redirectUris",))],
"a duplicated redirect URI is still drift and must be corrected, got %r"
% (url_writes(),))
# -- 4. BLAST RADIUS: only the allow-listed endpoint shapes are ever touched.
# The URL pass added NO endpoint: it reuses the client lookup and the same
# minimal client PUT, so users/groups/roles stay structurally unreachable.
reset()
invoke()
for touched in state["touched"]:
@@ -569,6 +1060,10 @@ data:
"the allow-list must refuse a users endpoint before opening a socket")
expect(_refused("/admin/realms/homelab/groups"),
"the allow-list must refuse a groups endpoint before opening a socket")
expect(_refused("/admin/realms/homelab/roles"),
"the allow-list must refuse a roles endpoint before opening a socket")
expect(_refused("/admin/realms/homelab/authentication/flows"),
"the allow-list must refuse an authentication-flow endpoint")
expect(_refused("/admin/realms/homelab"),
"the allow-list must refuse a whole-realm update endpoint")
@@ -579,13 +1074,77 @@ data:
expect(rc == 1, "a declared client missing from the realm must fail the audit")
expect("NOT FOUND" in out, "the missing client must be named in the output")
# -- 6. an empty / unmounted Secret must fail closed, never report success
# -- 6. FAIL CLOSED, AND ONLY FOR THE AFFECTED PASS.
# 6a. an empty / unmounted Secret: no secret is written, and the URL pass still
# runs. Before pass 2 existed a missing Secret aborted the run; it must not
# now be able to abort URL reconciliation either, nor vice versa (6b).
reset()
SECRETS_DIR = tempfile.mkdtemp()
rc, out = invoke()
expect(rc == 1, "an empty secrets dir must fail closed")
expect("keys found in" in out, "the empty-secret failure must say what is missing")
expect(state["secret_writes"] == [],
"FAIL CLOSED: an unreadable secret desired-state must write NO secret, "
"got %r" % (state["secret_writes"],))
expect(state["url_writes"] != [],
"INDEPENDENCE: an unreadable SECRET input must not disable the URL pass -- "
"SSO URL healing cannot depend on the credentials file")
SECRETS_DIR = tmp
# 6b. the mirror image: an unmounted / malformed URL ConfigMap must write no URL
# and must NOT stop client-secret repair, which is the more critical half.
reset()
URLS_FILE = os.path.join(urls_dir, "does-not-exist.json")
rc, out = invoke()
expect(rc == 1, "a missing URL desired-state file must fail closed")
expect(state["url_writes"] == [],
"FAIL CLOSED: an unreadable URL desired-state must write NO url, got %r"
% (state["url_writes"],))
expect(sorted(state["secret_writes"]) == ["gitea", "pihole"],
"INDEPENDENCE: a missing URL ConfigMap must not disable client-SECRET "
"repair, got %r" % (state["secret_writes"],))
expect("FAILED CLOSED" in out, "the fail-closed path must say so explicitly")
# 6c. a desired-state file naming a field this program may not write is REFUSED
# wholesale -- it must never be forwarded to Keycloak as a client field.
reset()
bad = os.path.join(urls_dir, "bad-field.json")
with open(bad, "w", encoding="utf-8") as fh:
json.dump({"grafana": {"serviceAccountsEnabled": True}}, fh)
URLS_FILE = bad
rc, out = invoke()
expect(rc == 1, "a desired-state file with a non-writable field must fail closed")
expect(state["url_writes"] == [],
"a non-writable field must produce NO write at all, got %r"
% (state["url_writes"],))
expect("not allowed to write" in out,
"the refusal must name the problem as a non-writable field")
# 6d. an empty desired state is fail-closed, not "everything is unmanaged"
reset()
empty = os.path.join(urls_dir, "empty.json")
with open(empty, "w", encoding="utf-8") as fh:
fh.write("{}")
URLS_FILE = empty
rc, out = invoke()
expect(rc == 1, "an empty URL desired state must fail closed, not silently pass")
expect(state["url_writes"] == [], "an empty desired state must write nothing")
URLS_FILE = URLS_GOOD
# -- 7. BOTH desired states unreadable: nothing is written at all.
reset()
SECRETS_DIR = tempfile.mkdtemp()
URLS_FILE = os.path.join(urls_dir, "also-missing.json")
rc, out = invoke()
expect(rc == 1, "with neither input readable the run must fail")
expect(state["secret_writes"] == [] and state["url_writes"] == [],
"with neither input readable NOTHING may be written, got secrets=%r urls=%r"
% (state["secret_writes"], state["url_writes"]))
expect("NOTHING WAS WRITTEN" in out,
"the total-failure path must state plainly that nothing was written")
SECRETS_DIR = tmp
URLS_FILE = URLS_GOOD
srv.shutdown()
print("")
if failures:
@@ -594,7 +1153,8 @@ data:
print(" FAIL %s" % f)
return 1
print("SELFTEST OK: %d assertions (compare-then-write, idempotence, "
"no-secret-logging, endpoint allow-list, fail-closed)" % checks[0])
"order-insensitivity, per-pass fail-closed, minimal-write blast radius, "
"no-secret-logging, endpoint allow-list)" % checks[0])
return 0
@@ -668,7 +1228,7 @@ spec:
imagePullPolicy: IfNotPresent
command:
- python3
- /opt/reconciler/reconcile-client-secrets.py
- /opt/reconciler/reconcile-keycloak-clients.py
env:
- name: KEYCLOAK_BASE_URL
value: http://keycloak-service.keycloak.svc.cluster.local:8080
@@ -703,6 +1263,14 @@ spec:
- name: client-secrets
mountPath: /etc/keycloak-realm-clients
readOnly: true
# The DESIRED state for pass 2: every environment-derived client URL,
# GENERATED from keycloak-realm-import.yaml (ci/render-keycloak-client-
# urls.py) so there is one source of truth for the hostnames. Mounted
# for the same reason as the Secret above: this pod has no Kubernetes
# API access, so it cannot read the KeycloakRealmImport CR itself.
- name: client-urls
mountPath: /etc/keycloak-client-urls
readOnly: true
securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: true
@@ -727,3 +1295,10 @@ spec:
# sets. 0400 is owner-only and the owner is root, not the 65532 the container
# runs as. See the fsGroup comment above.
defaultMode: 0440
# The URL desired state. 0444 is fine where the Secret needs 0440: these are
# public hostnames, not credentials. fsGroup 65532 (set above) makes it readable
# either way.
- name: client-urls
configMap:
name: keycloak-client-urls
defaultMode: 0444