mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-25 05:14:47 -06:00
fix(oidc): require TURNSTONE_OIDC_REDIRECT_BASE; drop Host-header fallback (sec-2)
_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.
(cherry picked from commit 52aba17740)
This commit is contained in:
+14
-9
@@ -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
|
||||
|
||||
+29
-36
@@ -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"}]}
|
||||
|
||||
@@ -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",
|
||||
|
||||
+11
-12
@@ -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 (
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user