From 5823d33ada7bfacfc063755f6e7148f60834aedf Mon Sep 17 00:00:00 2001 From: Oren Fromberg Date: Fri, 10 Jul 2026 10:13:54 -0400 Subject: [PATCH] feat(auth): support split-horizon OIDC for private-VPC clusters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #158. ``KeyCloakConfig.build()`` and the module-level ``configure()`` in ``00-gateway-auth.py`` derive all four OIDC URLs (authorize / token / userinfo / end_session) from a single ``issuer``. On private-VPC clusters where the external Keycloak hostname resolves ONLY outside the cluster (browser side, via a private DNS zone or external resolver), the hub cannot resolve ``issuer`` from inside the cluster and the token + userinfo backchannel legs fail. Deployers work around this by overriding the entire authenticator via ``hub.extraConfig`` — a ~40-line ``GenericOAuthenticator`` block with hardcoded split URLs. This PR adds first-class split-horizon support so that workaround retires. Values surface — two new opt-in fields, both default empty (no behaviour change on existing deploys): - ``keycloak.backchannelURL`` (top-level) — base URL used for backchannel legs. The realm suffix (``/realms/``) is appended by the helper. Typical value: ``http://keycloak-keycloakx-http.keycloak.svc.cluster.local:8080``. - ``jupyterhub.custom.keycloak-backchannel-issuer-url`` (explicit override) — full backchannel issuer URL INCLUDING ``/realms/``. Provided for the rare case where the ``backchannelURL`` + realm pattern cannot express the desired shape. Chart-side wiring: - New helper ``nebari-data-science-pack.keycloakBackchannelIssuerURL`` in ``_helpers.tpl``. Order of precedence: explicit override, then ``backchannelURL`` + realm, then empty (= no split-horizon). - ``templates/hub-config.yaml`` emits the helper's output into ``_CHART_DERIVED`` under key ``keycloak-backchannel-issuer-url``, so ``get_chart_config`` returns the deployer's value when set and the chart-rendered default otherwise (mirrors ``keycloak-token-url``). Python-side wiring in ``config/jupyterhub/00-gateway-auth.py``: - ``KeyCloakConfig.build()`` gains an optional ``backchannel_issuer`` kwarg. When non-empty, ``token_url`` and ``userdata_url`` are derived from it; ``authorize_url`` and ``end_session_url`` keep using ``issuer``. The dataclass's ``issuer`` field itself is unchanged — what downstream validators check against ``iss`` claims. Empty string is treated as "no split-horizon" and is byte-identical to the prior single-issuer signature (backward-compat guard). - ``configure()`` gains a matching optional kwarg and threads it through. - The module-level call site reads ``keycloak-backchannel-issuer-url`` via ``get_chart_config`` (which already handles the deployer-override + chart-default precedence) and passes it to ``configure``. Missing / empty is fine — hub keeps the historical single-issuer behaviour. Runtime contract with Keycloak: For the ``iss`` claim in issued tokens to keep matching the external ``issuer`` (so JWKS validators still work), run Keycloak with ``KC_HOSTNAME_BACKCHANNEL_DYNAMIC=true``. That flag makes KC dynamically choose the ``iss`` value based on the request's Host header, so a token minted via the backchannel URL still embeds the external hostname in ``iss``. This dependency is documented in the ``keycloak.backchannelURL`` docstring. Tests: 6 new assertions in ``tests/unit/test_keycloak_authenticator.py``: - ``KeyCloakConfig.build`` without backchannel — all URLs share issuer - ``KeyCloakConfig.build`` with backchannel — token + userinfo split, authorize + end_session unchanged, ``issuer`` field unchanged - Empty-string backchannel treated as no-split-horizon (footgun guard — Helm renders unset values as ``""`` and that must not produce ``//protocol/openid-connect/...`` URLs) - ``configure()`` wires backchannel through onto the authenticator class - ``configure()`` without the kwarg matches prior single-issuer shape - ``configure()`` with empty string treated as no-split-horizon Full ``test_keycloak_authenticator.py`` suite passes: 29/29. --- config/jupyterhub/00-gateway-auth.py | 56 ++++++++++-- templates/_helpers.tpl | 29 ++++++ templates/hub-config.yaml | 4 + tests/unit/test_keycloak_authenticator.py | 104 ++++++++++++++++++++++ values.yaml | 37 ++++++++ 5 files changed, 225 insertions(+), 5 deletions(-) diff --git a/config/jupyterhub/00-gateway-auth.py b/config/jupyterhub/00-gateway-auth.py index 0ce804b..c22e3d1 100644 --- a/config/jupyterhub/00-gateway-auth.py +++ b/config/jupyterhub/00-gateway-auth.py @@ -206,14 +206,39 @@ class KeyCloakConfig: post_logout_redirect_uri: str @classmethod - def build(cls, *, issuer: str, post_logout_redirect_uri: str) -> "KeyCloakConfig": - """Derive every KC endpoint URL from the realm issuer.""" + def build( + cls, + *, + issuer: str, + post_logout_redirect_uri: str, + backchannel_issuer: str = "", + ) -> "KeyCloakConfig": + """Derive every KC endpoint URL from the realm issuer. + + When ``backchannel_issuer`` is a non-empty string, ``token_url`` and + ``userdata_url`` are derived from it instead of ``issuer``, while + ``authorize_url`` and ``end_session_url`` keep using ``issuer``. + This split-horizon shape is needed on private-VPC clusters where + the external Keycloak hostname baked into ``issuer`` is not + resolvable from inside the cluster — the browser reaches the + external URL for the authorize + logout redirects, but the hub + talks to Keycloak via an in-cluster URL for the token + userinfo + legs. The ``iss`` claim in issued tokens still matches ``issuer`` + (Keycloak's ``KC_HOSTNAME_BACKCHANNEL_DYNAMIC=true`` reconciles + which URL a token comes back on with which one embeds in ``iss``), + so downstream validators unchanged. + + When empty (default), all four URLs use ``issuer`` — behaviour is + byte-identical to the prior single-issuer signature. Existing + callers do not need to change. + """ base = f"{issuer}/protocol/openid-connect" + bc_base = f"{(backchannel_issuer or issuer)}/protocol/openid-connect" return cls( issuer=issuer, authorize_url=f"{base}/auth", - token_url=f"{base}/token", - userdata_url=f"{base}/userinfo", + token_url=f"{bc_base}/token", + userdata_url=f"{bc_base}/userinfo", end_session_url=f"{base}/logout", post_logout_redirect_uri=post_logout_redirect_uri, ) @@ -502,6 +527,7 @@ def configure( realm_api_url: str = "", shared_mount_role_name: str = "allow-group-directory-creation-role", jupyterlab_profiles_role_name: str = "jupyterlab-profiles", + backchannel_issuer: str = "", ): """Wire KeyCloakOAuthenticator onto JupyterHub's `c` config object. @@ -513,9 +539,19 @@ def configure( KC client role whose ``profiles`` attribute lists the slugs a holder may select for ``access: keycloak`` profiles. Both default to the classic nebari names. + + ``backchannel_issuer`` enables split-horizon OIDC: when non-empty, the + hub uses this URL for the ``token_url`` and ``userdata_url`` legs while + the browser continues to use ``issuer`` for ``authorize_url`` and + ``end_session_url``. Needed on private-VPC clusters where in-cluster + CoreDNS cannot resolve the external Keycloak hostname. Empty (default) + means "no split-horizon" and all four URLs use ``issuer`` — behaviour + unchanged from the single-issuer signature. """ kc_config = KeyCloakConfig.build( - issuer=issuer, post_logout_redirect_uri=external_url, + issuer=issuer, + backchannel_issuer=backchannel_issuer, + post_logout_redirect_uri=external_url, ) c.JupyterHub.authenticator_class = KeyCloakOAuthenticator c.KeyCloakOAuthenticator.client_id = client_id @@ -633,6 +669,15 @@ def _resolve_oauth_urls() -> tuple[str, str] | None: os.environ.get("KC_REALM_API_URL") or _derive_realm_api_url(_issuer) ) + # Split-horizon OIDC: if set, ``token_url`` and ``userdata_url`` + # use this URL instead of ``issuer``. Sourced from the chart via + # ``get_chart_config`` (which reads ``custom.keycloak-backchannel- + # issuer-url`` first, then falls back to the chart-derived value + # baked in by ``00-chart-derived.py`` at Helm render time from + # ``keycloak.backchannelURL``). Empty string → no split-horizon. + _backchannel_issuer = get_chart_config( # noqa: F821 + "keycloak-backchannel-issuer-url", "", + ) configure( c, # noqa: F821 issuer=_issuer, @@ -641,6 +686,7 @@ def _resolve_oauth_urls() -> tuple[str, str] | None: callback_url=_callback_url, external_url=_external_url, realm_api_url=_realm_api_url, + backchannel_issuer=_backchannel_issuer, shared_mount_role_name=os.environ.get( "KC_SHARED_MOUNT_ROLE", "allow-group-directory-creation-role", diff --git a/templates/_helpers.tpl b/templates/_helpers.tpl index abc9bb8..c8f78a4 100644 --- a/templates/_helpers.tpl +++ b/templates/_helpers.tpl @@ -179,6 +179,35 @@ Nebari deployment ships with. {{- end -}} {{- end -}} +{{/* +Backchannel issuer URL for split-horizon OIDC. When non-empty, the hub uses +this URL as the base for ``token_url`` and ``userdata_url`` while the browser +continues to use the primary (``issuer``) URL for authorize + end_session. +Needed on private-VPC clusters where in-cluster CoreDNS cannot resolve the +external Keycloak hostname. + +Order of precedence: + 1. .Values.jupyterhub.custom.keycloak-backchannel-issuer-url (explicit + full issuer URL, e.g. http://keycloak-keycloakx-http.keycloak.svc.cluster.local:8080/realms/nebari) + 2. .Values.keycloak.backchannelURL + /realms/ (base URL only, + realm suffix added by the helper) + 3. Empty string — no split-horizon (the caller falls back to the primary + issuer for all four URLs) + +Emits WITHOUT a trailing ``/protocol/openid-connect`` — that suffix is +appended by the Python side in ``KeyCloakConfig.build()`` so the tokens +URL and userinfo URL sit next to each other under one base. +*/}} +{{- define "nebari-data-science-pack.keycloakBackchannelIssuerURL" -}} +{{- $explicit := index .Values.jupyterhub.custom "keycloak-backchannel-issuer-url" | default "" -}} +{{- $realm := .Values.keycloak.realm | default "nebari" -}} +{{- if $explicit -}} +{{- $explicit -}} +{{- else if .Values.keycloak.backchannelURL -}} +{{- printf "%s/realms/%s" .Values.keycloak.backchannelURL $realm -}} +{{- end -}} +{{- end -}} + {{/* OIDC client IDs follow nebari-operator's naming convention: jupyterhub-- (hub's own client) diff --git a/templates/hub-config.yaml b/templates/hub-config.yaml index 6ec8cbd..df166b6 100644 --- a/templates/hub-config.yaml +++ b/templates/hub-config.yaml @@ -43,6 +43,10 @@ data: "nebi-remote-url": {{ include "nebari-data-science-pack.nebiRemoteURL" . | quote }}, "nebi-internal-url": {{ include "nebari-data-science-pack.nebiInternalURL" . | quote }}, "keycloak-token-url": {{ include "nebari-data-science-pack.keycloakTokenURL" . | quote }}, + # Split-horizon OIDC backchannel URL. Empty when unset — the caller in + # 00-gateway-auth.py treats empty as "no split-horizon" and uses the + # primary issuer for all four URLs. See KeyCloakConfig.build(). + "keycloak-backchannel-issuer-url": {{ include "nebari-data-science-pack.keycloakBackchannelIssuerURL" . | quote }}, "nebi-client-id": {{ include "nebari-data-science-pack.nebiClientID" . | quote }}, "jupyterhub-client-id": {{ include "nebari-data-science-pack.hubClientID" . | quote }}, # Mirrors .Values.sharedStorage.enabled so deployers don't have to set diff --git a/tests/unit/test_keycloak_authenticator.py b/tests/unit/test_keycloak_authenticator.py index aa7d74f..d5810cb 100644 --- a/tests/unit/test_keycloak_authenticator.py +++ b/tests/unit/test_keycloak_authenticator.py @@ -294,3 +294,107 @@ def test_derive_realm_api_url_returns_empty_for_non_kc_url(): mod = load_config_module("00-gateway-auth.py") assert mod._derive_realm_api_url("https://example.test/no/realms/here") != "" assert mod._derive_realm_api_url("https://example.test/oidc") == "" + + +# --------------------------------------------------------------------------- +# Split-horizon OIDC (backchannel issuer) +# --------------------------------------------------------------------------- +# On private-VPC clusters the hub cannot resolve the external Keycloak +# hostname baked into ``issuer``. Passing a non-empty ``backchannel_issuer`` +# routes ``token_url`` + ``userdata_url`` through an in-cluster URL while +# ``authorize_url`` + ``end_session_url`` continue to use ``issuer`` (which +# the browser CAN resolve). The ``iss`` claim in tokens still matches +# ``issuer`` when Keycloak is run with ``KC_HOSTNAME_BACKCHANNEL_DYNAMIC``. + +BACKCHANNEL_ISSUER = "http://kc.svc.cluster.local:8080/realms/nebari" + + +def test_kc_config_build_without_backchannel_derives_all_from_issuer(): + """Default (backward-compatible) shape: single-issuer, all URLs share + the same base. Prior callers that don't pass ``backchannel_issuer`` + see exactly the pre-PR behaviour.""" + mod = load_config_module("00-gateway-auth.py") + cfg = mod.KeyCloakConfig.build( + issuer=ISSUER, post_logout_redirect_uri=EXTERNAL, + ) + base = f"{ISSUER}/protocol/openid-connect" + assert cfg.authorize_url == f"{base}/auth" + assert cfg.token_url == f"{base}/token" + assert cfg.userdata_url == f"{base}/userinfo" + assert cfg.end_session_url == f"{base}/logout" + + +def test_kc_config_build_with_backchannel_splits_urls(): + """Backchannel issuer set: token + userdata use backchannel; authorize + + end_session keep using primary issuer. Split-horizon contract.""" + mod = load_config_module("00-gateway-auth.py") + cfg = mod.KeyCloakConfig.build( + issuer=ISSUER, + backchannel_issuer=BACKCHANNEL_ISSUER, + post_logout_redirect_uri=EXTERNAL, + ) + primary_base = f"{ISSUER}/protocol/openid-connect" + bc_base = f"{BACKCHANNEL_ISSUER}/protocol/openid-connect" + # Browser legs use primary + assert cfg.authorize_url == f"{primary_base}/auth" + assert cfg.end_session_url == f"{primary_base}/logout" + # Backchannel legs use in-cluster + assert cfg.token_url == f"{bc_base}/token" + assert cfg.userdata_url == f"{bc_base}/userinfo" + # ``issuer`` field itself unchanged — this is what ``iss`` claims are + # matched against by downstream validators + assert cfg.issuer == ISSUER + + +def test_kc_config_build_with_empty_backchannel_matches_default(): + """Empty-string backchannel_issuer must be treated as "no split-horizon" + — same URLs as the default single-issuer signature. Guards against a + common footgun where an unset value flows through as ``""`` and would + otherwise produce ``//protocol/openid-connect/...`` URLs.""" + mod = load_config_module("00-gateway-auth.py") + default = mod.KeyCloakConfig.build( + issuer=ISSUER, post_logout_redirect_uri=EXTERNAL, + ) + explicit_empty = mod.KeyCloakConfig.build( + issuer=ISSUER, + backchannel_issuer="", + post_logout_redirect_uri=EXTERNAL, + ) + assert default == explicit_empty + + +def test_configure_wires_backchannel_issuer_onto_authenticator(): + """configure() must accept ``backchannel_issuer`` and thread it through + KeyCloakConfig.build() so the KeyCloakOAuthenticator ends up with the + split URLs on the class.""" + c, _ = _configure_with_defaults(backchannel_issuer=BACKCHANNEL_ISSUER) + kc = c.KeyCloakOAuthenticator + primary_base = f"{ISSUER}/protocol/openid-connect" + bc_base = f"{BACKCHANNEL_ISSUER}/protocol/openid-connect" + assert kc.authorize_url == f"{primary_base}/auth" + assert kc.token_url == f"{bc_base}/token" + assert kc.userdata_url == f"{bc_base}/userinfo" + + +def test_configure_without_backchannel_leaves_single_issuer_shape(): + """Backward-compat guard: when configure() is called without the new + kwarg (the common case), every existing test in this file still passes + — validated by the assertions in ``test_configure_derives_keycloak_urls_from_issuer`` + at the top of the file. Duplicated here so the split-horizon feature's + default-off behaviour is testable in isolation.""" + c, _ = _configure_with_defaults() # no backchannel_issuer passed + kc = c.KeyCloakOAuthenticator + base = f"{ISSUER}/protocol/openid-connect" + assert kc.token_url == f"{base}/token" + assert kc.userdata_url == f"{base}/userinfo" + + +def test_configure_treats_empty_backchannel_string_as_no_split_horizon(): + """Explicit empty string reaches configure() when the deployer leaves + ``keycloak.backchannelURL`` unset (Helm renders it as ``""``). Must be + treated as "no split-horizon", not as a broken URL.""" + c, _ = _configure_with_defaults(backchannel_issuer="") + kc = c.KeyCloakOAuthenticator + base = f"{ISSUER}/protocol/openid-connect" + assert kc.token_url == f"{base}/token" + assert kc.userdata_url == f"{base}/userinfo" diff --git a/values.yaml b/values.yaml index 2ba3062..0c2db3c 100644 --- a/values.yaml +++ b/values.yaml @@ -31,6 +31,30 @@ keycloak: # In-cluster bitnami/keycloakx service host:port. Used by the hub <-> nebi # token-exchange path when keycloak.hostname is empty. serviceHost: "keycloak-keycloakx-http.keycloak.svc.cluster.local:8080" + # OPTIONAL split-horizon OIDC. When set, the hub uses this base URL for the + # backchannel legs (``token_url`` and ``userdata_url``) while the browser + # continues to use ``hostname`` for authorize + end_session. The realm + # suffix (``/realms/``) is appended automatically. + # + # Needed on private-VPC clusters where in-cluster CoreDNS does NOT resolve + # the external Keycloak hostname — the hub pod can't reach ``hostname`` at + # all, so token exchange over the primary URL fails. Setting this to the + # in-cluster URL (e.g. ``http://keycloak-keycloakx-http.keycloak.svc.cluster.local:8080``) + # keeps the browser legs on ``hostname`` (where the browser can resolve it) + # while the backchannel legs go direct to the in-cluster Service (which + # doesn't need external DNS and skips the internal-NLB round-trip). + # + # Empty (default) means "no split-horizon" — all four URLs use the primary + # issuer read from the operator-provisioned Secret. Existing deploys are + # not affected by the addition of this field. + # + # Keycloak side: for the ``iss`` claim to still match ``hostname`` when + # tokens are minted via the backchannel URL, run Keycloak with + # ``KC_HOSTNAME_BACKCHANNEL_DYNAMIC=true`` so backchannel responses embed + # the browser-facing issuer rather than the in-cluster URL. Without that + # env var Keycloak will emit tokens whose ``iss`` claim matches the + # backchannel URL — validators keyed off the browser URL would reject. + backchannelURL: "" # Subdomain convention used to derive hub/nebi hostnames from # keycloak.hostname. Override per-deploy if your DNS doesn't follow this. @@ -355,6 +379,19 @@ jupyterhub: # If empty, derived from keycloak.hostname (https) or # keycloak.serviceHost (in-cluster http), with keycloak.realm. keycloak-token-url: "" + # OPTIONAL split-horizon OIDC — full backchannel issuer URL (INCLUDING + # ``/realms/``, e.g. ``http://keycloak-keycloakx-http.keycloak.svc.cluster.local:8080/realms/nebari``). + # When non-empty, the hub uses this as the base for ``token_url`` and + # ``userdata_url`` while the browser continues to use the primary + # (external) issuer for authorize + end_session. Empty (default) means + # no split-horizon. + # If empty, derived from top-level ``keycloak.backchannelURL`` + realm. + # Set this for the rare case where you need a URL shape that the + # ``keycloak.backchannelURL`` + realm pattern cannot express (e.g. a + # non-standard realm path). See ``00-gateway-auth.py`` for how it's + # consumed and the docstring on ``keycloak.backchannelURL`` at the top + # of this file for the motivation. + keycloak-backchannel-issuer-url: "" # OIDC client IDs (provisioned by nebari-operator's AuthReconciler). # If empty, derived from operator naming convention: # jupyterhub-- (this chart)