diff --git a/api/auth.py b/api/auth.py index b6a30940..846c9945 100644 --- a/api/auth.py +++ b/api/auth.py @@ -105,6 +105,12 @@ _TRUSTED_AUTH_HEADER_ENV = 'HERMES_WEBUI_TRUSTED_AUTH_HEADER' _TRUSTED_GROUPS_HEADER_ENV = 'HERMES_WEBUI_TRUSTED_GROUPS_HEADER' _TRUSTED_GROUP_PROFILE_MAP_ENV = 'HERMES_WEBUI_GROUP_PROFILE_MAP' _TRUSTED_AUTH_LOGOUT_URL_ENV = 'HERMES_WEBUI_TRUSTED_AUTH_LOGOUT_URL' +# Opt-in: also treat '|' as a group separator in the trusted-groups header. +# Off by default so an existing deployment whose group NAME legitimately +# contains a literal '|' is never silently re-split into two groups (which +# could change its profile binding). Set to 1/true/yes/on for identity +# providers (some Authentik outpost configs) that emit "admins|developpeur". +_TRUSTED_GROUPS_PIPE_SEPARATOR_ENV = 'HERMES_WEBUI_TRUSTED_GROUPS_PIPE_SEPARATOR' _TRUSTED_AUTH_WARNINGS_EMITTED: set[str] = set() @@ -722,15 +728,19 @@ def _trusted_groups_header_value(handler) -> list[str]: if not raw: return [] values = [] - # Authentik's outpost typically joins multiple group names with a comma, - # but this deployment's proxy provider/property mapping emits a - # pipe-separated list instead (e.g. "admins|developpeur"). A bare comma - # split silently treats a multi-group pipe-joined value as one unmatched - # group name, dropping the session to the unbound "default" profile even - # though the identity is a legitimate member of a mapped group. Accept - # comma, pipe, and newline as equivalent separators so either format - # works regardless of which the outpost happens to send. - normalized = str(raw).replace('\n', ',').replace('|', ',') + # Authentik's outpost typically joins multiple group names with a comma or + # newline; parse those as separators by default. Some proxy provider / + # property-mapping configs instead emit a pipe-separated list (e.g. + # "admins|developpeur"), which a comma-only split would treat as one + # unmatched group name — silently dropping the session to the unbound + # "default" profile despite a legitimate mapped membership. Pipe splitting + # is therefore available but OPT-IN (HERMES_WEBUI_TRUSTED_GROUPS_PIPE_SEPARATOR), + # because a group NAME can legitimately contain a literal '|' and must not be + # re-split by default — doing so unconditionally could change an existing + # deployment's profile binding. + normalized = str(raw).replace('\n', ',') + if str(os.getenv(_TRUSTED_GROUPS_PIPE_SEPARATOR_ENV, '')).strip().lower() in ('1', 'true', 'yes', 'on'): + normalized = normalized.replace('|', ',') for part in normalized.split(','): part = part.strip() if part: diff --git a/api/passkeys.py b/api/passkeys.py index 220d1484..1ddb2c75 100644 --- a/api/passkeys.py +++ b/api/passkeys.py @@ -208,7 +208,11 @@ def rp_context(handler) -> tuple[str, str]: from urllib.parse import urlparse parsed = urlparse(browser_origin.strip()) if parsed.scheme in ("http", "https") and parsed.hostname: - netloc = parsed.hostname if parsed.port is None else f"{parsed.hostname}:{parsed.port}" + # Re-bracket IPv6 literals: parsed.hostname strips the [] that the + # browser's clientDataJSON.origin carries, so an unbracketed + # "http://::1:8787" would never match the stored origin. + host_part = f"[{parsed.hostname}]" if ":" in parsed.hostname else parsed.hostname + netloc = host_part if parsed.port is None else f"{host_part}:{parsed.port}" return parsed.hostname, f"{parsed.scheme}://{netloc}" except Exception: pass diff --git a/tests/test_passkey_auth.py b/tests/test_passkey_auth.py index 5f914964..81d7d853 100644 --- a/tests/test_passkey_auth.py +++ b/tests/test_passkey_auth.py @@ -395,3 +395,73 @@ def test_passwordless_settings_and_last_passkey_guard_are_wired(): assert "id=\"btnGoPasswordless\"" in index assert "async function goPasswordless" in panels assert "prompt(" not in panels + + +# ── rp_context: RPID/origin derivation (Origin-first, IPv6-safe) — #6772 ────── + +class _HeaderHandler: + """Minimal stand-in for the HTTP handler rp_context() reads headers from.""" + + def __init__(self, headers): + # case-insensitive get, like http.client.HTTPMessage + self._h = {k.lower(): v for k, v in headers.items()} + + class _H: + def __init__(self, h): + self._h = h + + def get(self, name, default=""): + return self._h.get(name.lower(), default) + + @property + def headers(self): + return _HeaderHandler._H(self._h) + + +def test_rp_context_prefers_wellformed_origin_over_host(): + from api import passkeys + + h = _HeaderHandler({"Origin": "https://app.example.com", "Host": "backend.internal:9000"}) + rp_id, origin = passkeys.rp_context(h) + assert rp_id == "app.example.com" + assert origin == "https://app.example.com" + + +def test_rp_context_preserves_origin_port(): + from api import passkeys + + h = _HeaderHandler({"Origin": "http://localhost:8787", "Host": "localhost:8787"}) + rp_id, origin = passkeys.rp_context(h) + assert rp_id == "localhost" + assert origin == "http://localhost:8787" + + +def test_rp_context_ipv6_origin_is_bracketed_to_match_clientdata(): + """A browser's clientDataJSON.origin brackets an IPv6 host ('http://[::1]:8787'). + rp_context must store the same bracketed form, or _client_data()'s origin + comparison (and thus passkey login over IPv6) would always fail.""" + from api import passkeys + + h = _HeaderHandler({"Origin": "http://[::1]:8787", "Host": "[::1]:8787"}) + rp_id, origin = passkeys.rp_context(h) + assert rp_id == "::1" + assert origin == "http://[::1]:8787" + + +def test_rp_context_falls_back_to_host_on_malformed_origin(): + from api import passkeys + + for bad in ("javascript:alert(1)", "file:///etc/passwd", "not-a-url", ""): + h = _HeaderHandler({"Origin": bad, "Host": "real.example.com"}) + rp_id, _origin = passkeys.rp_context(h) + assert rp_id == "real.example.com", f"expected Host fallback for Origin={bad!r}" + + +def test_rp_context_strips_origin_userinfo(): + """A userinfo-bearing Origin must not be stored verbatim; the hostname wins.""" + from api import passkeys + + h = _HeaderHandler({"Origin": "https://user:pw@evil.example.com", "Host": "real.example.com"}) + rp_id, origin = passkeys.rp_context(h) + assert rp_id == "evil.example.com" + assert "@" not in origin and origin == "https://evil.example.com" diff --git a/tests/test_trusted_header_auth.py b/tests/test_trusted_header_auth.py index 5ebe65c0..baf35692 100644 --- a/tests/test_trusted_header_auth.py +++ b/tests/test_trusted_header_auth.py @@ -219,18 +219,12 @@ def test_group_map_prefers_mapping_order_over_header_order(monkeypatch): ("admins", ["admins"]), # Comma-separated (the format this parser originally supported). ("admins,developpeur", ["admins", "developpeur"]), - # Pipe-separated (this deployment's Authentik outpost format). - ("admins|developpeur", ["admins", "developpeur"]), # Newline-separated (already supported before this fix). ("admins\ndeveloppeur", ["admins", "developpeur"]), - # Mixed separators in the same value. - ("admins,developpeur|it\nops", ["admins", "developpeur", "it", "ops"]), - # Repeated/adjacent separators collapse instead of producing + # Repeated/adjacent comma separators collapse instead of producing # empty group names. - ("admins||developpeur", ["admins", "developpeur"]), ("admins,,developpeur", ["admins", "developpeur"]), # Surrounding and interior whitespace is trimmed per group. - (" admins | developpeur ", ["admins", "developpeur"]), ("admins, developpeur", ["admins", "developpeur"]), ], ) @@ -238,6 +232,45 @@ def test_trusted_groups_header_value_accepts_separator_variants( monkeypatch, raw_header, expected ): _trusted_env(monkeypatch, groups_header="Remote-Groups") + monkeypatch.delenv("HERMES_WEBUI_TRUSTED_GROUPS_PIPE_SEPARATOR", raising=False) + handler = _Handler(headers={"Remote-User": "alice", "Remote-Groups": raw_header}) + + assert auth._trusted_groups_header_value(handler) == expected + + +def test_trusted_groups_pipe_is_literal_by_default(monkeypatch): + """Without the opt-in, a '|' is part of the group NAME, not a separator. + + This is the backward-compatibility guarantee: an existing deployment whose + group name legitimately contains a '|' must not be silently re-split into + two groups (which could change its profile binding).""" + _trusted_env(monkeypatch, groups_header="Remote-Groups") + monkeypatch.delenv("HERMES_WEBUI_TRUSTED_GROUPS_PIPE_SEPARATOR", raising=False) + handler = _Handler( + headers={"Remote-User": "alice", "Remote-Groups": "admins|developpeur"} + ) + + assert auth._trusted_groups_header_value(handler) == ["admins|developpeur"] + + +@pytest.mark.parametrize( + "raw_header, expected", + [ + # Pipe-separated (this deployment's Authentik outpost format). + ("admins|developpeur", ["admins", "developpeur"]), + # Mixed separators in the same value. + ("admins,developpeur|it\nops", ["admins", "developpeur", "it", "ops"]), + # Repeated/adjacent pipe separators collapse instead of producing + # empty group names. + ("admins||developpeur", ["admins", "developpeur"]), + # Surrounding and interior whitespace is trimmed per group. + (" admins | developpeur ", ["admins", "developpeur"]), + ], +) +def test_trusted_groups_pipe_separator_opt_in(monkeypatch, raw_header, expected): + """With HERMES_WEBUI_TRUSTED_GROUPS_PIPE_SEPARATOR set, '|' also splits.""" + _trusted_env(monkeypatch, groups_header="Remote-Groups") + monkeypatch.setenv("HERMES_WEBUI_TRUSTED_GROUPS_PIPE_SEPARATOR", "1") handler = _Handler(headers={"Remote-User": "alice", "Remote-Groups": raw_header}) assert auth._trusted_groups_header_value(handler) == expected @@ -250,6 +283,7 @@ def test_group_map_accepts_pipe_separated_header_value(monkeypatch): # of a mapped group: a lone-membership identity works fine (no # separator to parse), but a second group membership starts producing a # pipe-joined header value that a comma-only split can't see through. + # Pipe splitting is opt-in, so this exercises the env flag explicitly. # # NOTE: this only covers the concrete-mapped-group case. The # wildcard-mapped-group case (a "*" mapping value dominating a concrete @@ -261,6 +295,7 @@ def test_group_map_accepts_pipe_separated_header_value(monkeypatch): groups_header="Remote-Groups", group_map={"hermes_devops": "devops"}, ) + monkeypatch.setenv("HERMES_WEBUI_TRUSTED_GROUPS_PIPE_SEPARATOR", "1") handler = _Handler( headers={"Remote-User": "alice", "Remote-Groups": "hermes_devops|other_group"} )