From 52aba1774076f58fbb87b2c735a98a06496987a0 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Mon, 4 May 2026 02:49:00 -0700 Subject: [PATCH] fix(oidc): require TURNSTONE_OIDC_REDIRECT_BASE; drop Host-header fallback (sec-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _build_oidc_redirect_uri previously fell back to the request Host header when redirect_base was unset. With a permissive reverse proxy or direct backend access, a spoofed Host minted an authorize URL pointing to attacker-controlled host — combined with a permissive IdP redirect_uri allowlist this enables auth-code interception. There is no production scenario where a Host-derived redirect_uri is correct, so this fails closed: - initialize_oidc_state checks redirect_base after discovery succeeds and disables OIDC (with an explicit error log naming the env var) if it's empty. Runs before fetch_jwks so a misconfigured deploy doesn't make a wasted JWKS call. - _build_oidc_redirect_uri simplifies to f"{redirect_base}/v1/api/auth/oidc/callback". request parameter dropped; both call sites (handle_oidc_authorize, handle_oidc_callback) updated. - docs/oidc.md promotes TURNSTONE_OIDC_REDIRECT_BASE from "Recommended" to "Required" with the security rationale. --- docs/oidc.md | 23 ++++++++----- tests/test_oidc.py | 65 +++++++++++++++++-------------------- tests/test_oidc_handlers.py | 1 + turnstone/core/auth.py | 23 +++++++------ turnstone/core/oidc.py | 11 +++++++ 5 files changed, 66 insertions(+), 57 deletions(-) diff --git a/docs/oidc.md b/docs/oidc.md index f902ac17..d8c55166 100644 --- a/docs/oidc.md +++ b/docs/oidc.md @@ -39,18 +39,18 @@ are set. | `TURNSTONE_OIDC_ROLE_CLAIM` | No | — | ID token claim containing role/group values (see [Role Mapping](#role-mapping)) | | `TURNSTONE_OIDC_ROLE_MAP` | No | — | Mapping from claim values to Turnstone role IDs (see [Role Mapping](#role-mapping)) | | `TURNSTONE_OIDC_PASSWORD_ENABLED` | No | `true` | Set to `false` to hide the password form and block all username/password logins (including admin). API tokens continue to work. | -| `TURNSTONE_OIDC_REDIRECT_BASE` | No | — | Externally-reachable origin for the OIDC redirect URI (e.g. `https://app.example.com`). Recommended when running behind a reverse proxy. When unset, derived from the request Host header. | +| `TURNSTONE_OIDC_REDIRECT_BASE` | Yes | — | Externally-reachable origin for the OIDC redirect URI (e.g. `https://app.example.com`). Without this, OIDC will refuse to start. The previous Host-header fallback was unsafe under permissive reverse proxies. | -OIDC is enabled when all three required fields (issuer, client ID, client -secret) are non-empty. If any is missing, OIDC is silently disabled and -the login screen shows only the password form. +All four required fields — issuer, client ID, client secret, and +`TURNSTONE_OIDC_REDIRECT_BASE` — must be set. If any are missing OIDC +is disabled at startup (an error is logged when only `redirect_base` +is missing) and the login screen shows only the password form. -### Reverse Proxy / Load Balancer +### Redirect base (required) -When Turnstone runs behind a reverse proxy, the internal `Host` header may -not match the externally-reachable URL. Set `TURNSTONE_OIDC_REDIRECT_BASE` -to the public origin so the redirect URI sent to the identity provider is -correct: +`TURNSTONE_OIDC_REDIRECT_BASE` pins the redirect URI sent to the identity +provider to a known externally-visible origin. Set it to the public origin +of your Turnstone deployment: ```bash TURNSTONE_OIDC_REDIRECT_BASE=https://app.example.com @@ -60,6 +60,11 @@ The resulting callback URL will be `https://app.example.com/v1/api/auth/oidc/callback` — register this as the authorized redirect URI in your identity provider. +OIDC will refuse to start when this variable is unset. There is no +Host-header fallback: a permissive reverse proxy or direct backend access +would otherwise let an attacker spoof `Host` and steer the IdP redirect +to a callback origin they control. + ### config.toml alternative ```toml diff --git a/tests/test_oidc.py b/tests/test_oidc.py index e9cdaa9c..c6a647b9 100644 --- a/tests/test_oidc.py +++ b/tests/test_oidc.py @@ -920,42 +920,13 @@ class TestValidateDiscoveredEndpoint: class TestBuildOIDCRedirectURI: """Tests for ``_build_oidc_redirect_uri`` in auth.py.""" - def _make_request(self, host="app.example.com", scheme="https", forwarded_proto=""): - """Build a minimal mock Starlette Request.""" - req = MagicMock() - headers = {"host": host} - if forwarded_proto: - headers["x-forwarded-proto"] = forwarded_proto - req.headers = headers - req.url.scheme = scheme - return req - - def test_pinned_redirect_base(self): - """When redirect_base is set, Host header is ignored.""" + def test_build_redirect_uri_uses_redirect_base_only(self): + """The redirect URI is built solely from ``redirect_base``.""" from turnstone.core.auth import _build_oidc_redirect_uri - config = _make_config(redirect_base="https://public.example.com") - req = self._make_request(host="internal-host:8080", scheme="http") - result = _build_oidc_redirect_uri(req, config) - assert result == "https://public.example.com/v1/api/auth/oidc/callback" - - def test_fallback_to_host_header(self): - """When redirect_base is empty, redirect URI uses Host header.""" - from turnstone.core.auth import _build_oidc_redirect_uri - - config = _make_config(redirect_base="") - req = self._make_request(host="app.example.com", scheme="https") - result = _build_oidc_redirect_uri(req, config) - assert result == "https://app.example.com/v1/api/auth/oidc/callback" - - def test_fallback_x_forwarded_proto(self): - """When redirect_base is empty and X-Forwarded-Proto is https, scheme is https.""" - from turnstone.core.auth import _build_oidc_redirect_uri - - config = _make_config(redirect_base="") - req = self._make_request(host="app.example.com", scheme="http", forwarded_proto="https") - result = _build_oidc_redirect_uri(req, config) - assert result == "https://app.example.com/v1/api/auth/oidc/callback" + config = _make_config(redirect_base="https://example.com") + result = _build_oidc_redirect_uri(config) + assert result == "https://example.com/v1/api/auth/oidc/callback" # --------------------------------------------------------------------------- @@ -1673,9 +1644,31 @@ class TestInitializeOIDCState: assert state.oidc_config.enabled is False assert state.jwks_data is None + def test_initialize_disables_when_redirect_base_unset(self, caplog): + """Discovery succeeds but redirect_base is empty -> disable + log error.""" + cfg = _make_config(redirect_base="") + state = types.SimpleNamespace(oidc_config=cfg, jwks_data=None) + + async def _ok(c): + return c + + async def _jwks_unexpected(_uri): + raise AssertionError("fetch_jwks must not be called when redirect_base is empty") + + with ( + patch("turnstone.core.oidc.discover_oidc", side_effect=_ok), + patch("turnstone.core.oidc.fetch_jwks", side_effect=_jwks_unexpected), + caplog.at_level("ERROR", logger="turnstone.core.oidc"), + ): + asyncio.run(initialize_oidc_state(state)) + + assert state.oidc_config.enabled is False + assert state.jwks_data is None + assert any("TURNSTONE_OIDC_REDIRECT_BASE" in record.message for record in caplog.records) + def test_initialize_keeps_enabled_but_no_jwks_on_jwks_failure(self): """JWKS fetch failure preserves enabled=True for lazy retry.""" - cfg = _make_config() + cfg = _make_config(redirect_base="https://app.example.com") state = types.SimpleNamespace(oidc_config=cfg, jwks_data=None) async def _ok(c): @@ -1696,7 +1689,7 @@ class TestInitializeOIDCState: def test_initialize_success(self): """Both discovery and JWKS prefetch succeed.""" - cfg = _make_config() + cfg = _make_config(redirect_base="https://app.example.com") state = types.SimpleNamespace(oidc_config=cfg, jwks_data=None) jwks = {"keys": [{"kid": "k1", "kty": "RSA"}]} diff --git a/tests/test_oidc_handlers.py b/tests/test_oidc_handlers.py index 0567af8b..5a5c720a 100644 --- a/tests/test_oidc_handlers.py +++ b/tests/test_oidc_handlers.py @@ -52,6 +52,7 @@ def _make_oidc_config(**overrides: Any) -> OIDCConfig: "role_claim": "", "role_map": {}, "password_enabled": True, + "redirect_base": "https://app.example.com", "authorization_endpoint": "https://idp.example.com/authorize", "token_endpoint": "https://idp.example.com/token", "userinfo_endpoint": "https://idp.example.com/userinfo", diff --git a/turnstone/core/auth.py b/turnstone/core/auth.py index 7d83af17..8ccca34a 100644 --- a/turnstone/core/auth.py +++ b/turnstone/core/auth.py @@ -1385,17 +1385,16 @@ async def handle_auth_refresh(request: Request, audience: str) -> Response: return response -def _build_oidc_redirect_uri(request: Request, oidc_config: OIDCConfig) -> str: - """Build the OIDC callback redirect URI. +def _build_oidc_redirect_uri(oidc_config: OIDCConfig) -> str: + """Build the OIDC callback redirect URI from the pinned ``redirect_base``. - Uses ``redirect_base`` from OIDC config when set (recommended for - reverse-proxy deployments), otherwise falls back to the request Host header. + ``initialize_oidc_state`` refuses to enable OIDC unless ``redirect_base`` + is set, so any caller reaching this point may assume it is non-empty. + A previous Host-header fallback was removed because a permissive front + proxy could let an attacker spoof ``Host`` and mint an authorize URL + pointing to an attacker-controlled callback origin. """ - if oidc_config.redirect_base: - return f"{oidc_config.redirect_base}/v1/api/auth/oidc/callback" - scheme = "https" if is_secure_request(dict(request.headers), request.url.scheme) else "http" - host = request.headers.get("host", "localhost") - return f"{scheme}://{host}/v1/api/auth/oidc/callback" + return f"{oidc_config.redirect_base}/v1/api/auth/oidc/callback" async def handle_oidc_authorize(request: Request, audience: str) -> Response: @@ -1439,8 +1438,8 @@ async def handle_oidc_authorize(request: Request, audience: str) -> Response: # Store pending state in database storage.create_oidc_pending_state(state, nonce, code_verifier, audience) - # Build redirect URI (pinned by TURNSTONE_OIDC_REDIRECT_BASE when set) - redirect_uri = _build_oidc_redirect_uri(request, oidc_config) + # Build redirect URI (pinned by TURNSTONE_OIDC_REDIRECT_BASE) + redirect_uri = _build_oidc_redirect_uri(oidc_config) url = build_authorize_url(oidc_config, redirect_uri, state, nonce, code_verifier) return RedirectResponse(url, status_code=302) @@ -1493,7 +1492,7 @@ async def handle_oidc_callback(request: Request, audience: str) -> Response: return RedirectResponse("/?oidc_error=Login+session+expired", status_code=302) # Build redirect URI (must match what was sent in authorize) - redirect_uri = _build_oidc_redirect_uri(request, oidc_config) + redirect_uri = _build_oidc_redirect_uri(oidc_config) try: from turnstone.core.oidc import ( diff --git a/turnstone/core/oidc.py b/turnstone/core/oidc.py index 41792c30..fc66f53c 100644 --- a/turnstone/core/oidc.py +++ b/turnstone/core/oidc.py @@ -538,6 +538,17 @@ async def initialize_oidc_state(app_state: Any) -> None: app_state.jwks_data = None return + if not cfg.redirect_base: + log.error( + "OIDC enabled but TURNSTONE_OIDC_REDIRECT_BASE is unset. " + "This is required to prevent Host-header-derived redirect_uri spoofing. " + "Set it to your service's externally-visible URL " + "(e.g. https://idp.example.com). OIDC will be disabled." + ) + app_state.oidc_config = dataclasses.replace(cfg, enabled=False) + app_state.jwks_data = None + return + try: jwks_data = await fetch_jwks(cfg.jwks_uri) except OIDCError: