From 6a182db5dc9c8cd54f295308f5cfe7266733fc4f Mon Sep 17 00:00:00 2001 From: Hardik Kaurani Date: Thu, 22 Oct 2026 11:05:21 +0530 Subject: [PATCH] fix(abstractions): normalize allowed hosts, reject userinfo, and validate URL structure --- .../authentication/allowed_hosts_validator.py | 47 ++++++++++--------- .../test_allowed_hosts_validator.py | 46 ++++++++++++++++-- 2 files changed, 67 insertions(+), 26 deletions(-) diff --git a/packages/abstractions/kiota_abstractions/authentication/allowed_hosts_validator.py b/packages/abstractions/kiota_abstractions/authentication/allowed_hosts_validator.py index 77670b68..afa3a44d 100644 --- a/packages/abstractions/kiota_abstractions/authentication/allowed_hosts_validator.py +++ b/packages/abstractions/kiota_abstractions/authentication/allowed_hosts_validator.py @@ -13,14 +13,8 @@ def __init__(self, allowed_hosts: list[str]) -> None: allowed_hosts (list[str]): A list of valid hosts. If the list is empty, all hosts are valid. """ - if not isinstance(allowed_hosts, list): - raise TypeError("Allowed hosts must be a list of strings") - - for host in allowed_hosts: - if host.startswith("https://") or host.startswith("http://"): - raise ValueError("Allowed host value cannot contain 'https://' or 'http://' prefix") - - self.allowed_hosts: set[str] = {x.lower() for x in allowed_hosts} + self.allowed_hosts: set[str] = set() + self.set_allowed_hosts(allowed_hosts) def get_allowed_hosts(self) -> list[str]: """Gets the list of valid hosts. If the list is empty, all hosts are valid. @@ -40,11 +34,18 @@ def set_allowed_hosts(self, allowed_hosts: list[str]) -> None: if not isinstance(allowed_hosts, list): raise TypeError("Allowed hosts must be a list of strings") + normalized_hosts: set[str] = set() for host in allowed_hosts: - if host.startswith("https://") or host.startswith("http://"): + if not isinstance(host, str): + raise TypeError("Allowed host value must be a string") + cleaned_host = host.strip() + lower_host = cleaned_host.lower() + if lower_host.startswith("https://") or lower_host.startswith("http://"): raise ValueError("Allowed host value cannot contain 'https://' or 'http://' prefix") + if cleaned_host: + normalized_hosts.add(lower_host) - self.allowed_hosts = {x.lower() for x in allowed_hosts} + self.allowed_hosts = normalized_hosts def is_url_host_valid(self, url: str) -> bool: """Checks whether the provided host is valid. @@ -53,25 +54,25 @@ def is_url_host_valid(self, url: str) -> bool: url (str): The url to check. Returns: - bool: [description] + bool: True if the host is valid, False otherwise. """ - if not url: + if not url or not isinstance(url, str): return False - if not self.get_allowed_hosts(): - return True + cleaned_url = url.strip() + if not cleaned_url: + return False + # Format: urlparse("scheme://netloc/path;parameters?query#fragment") # Returns: ParseResult(scheme='scheme', netloc='netloc', path='/path;parameters', params='', # query='query', fragment='fragment') - o = urlparse(url) - if not all([o.scheme, o.netloc]): + o = urlparse(cleaned_url) + if not (o.scheme and o.netloc and o.hostname) or o.username or o.password: return False - if not o.hostname: - return False - hostname = o.hostname.lower() - # Existing exact match - if hostname in self.allowed_hosts: + + if not self.allowed_hosts: return True - # Suffix match for entries starting with "." - return any( + + hostname = o.hostname.lower() + return hostname in self.allowed_hosts or any( suffix.startswith(".") and hostname.endswith(suffix) for suffix in self.allowed_hosts ) diff --git a/packages/abstractions/tests/authentication/test_allowed_hosts_validator.py b/packages/abstractions/tests/authentication/test_allowed_hosts_validator.py index d07942a7..1b3696c0 100644 --- a/packages/abstractions/tests/authentication/test_allowed_hosts_validator.py +++ b/packages/abstractions/tests/authentication/test_allowed_hosts_validator.py @@ -12,9 +12,17 @@ def test_lowercases_hosts(self): validator = AllowedHostsValidator(["Example.COM", "API.Example.com"]) assert set(validator.get_allowed_hosts()) == {"example.com", "api.example.com"} + def test_trims_whitespace_from_hosts(self): + validator = AllowedHostsValidator([" example.com ", " api.example.com "]) + assert set(validator.get_allowed_hosts()) == {"example.com", "api.example.com"} + def test_raises_on_non_list(self): with pytest.raises(TypeError): - AllowedHostsValidator("example.com") + AllowedHostsValidator("example.com") # type: ignore + + def test_raises_on_non_string_element(self): + with pytest.raises(TypeError): + AllowedHostsValidator(["example.com", 123]) # type: ignore def test_raises_on_http_prefix(self): with pytest.raises(ValueError): @@ -24,6 +32,16 @@ def test_raises_on_https_prefix(self): with pytest.raises(ValueError): AllowedHostsValidator(["https://example.com"]) + def test_raises_on_uppercase_prefix(self): + with pytest.raises(ValueError): + AllowedHostsValidator(["HTTP://example.com"]) + with pytest.raises(ValueError): + AllowedHostsValidator(["HTTPS://example.com"]) + + def test_raises_on_prefix_with_whitespace(self): + with pytest.raises(ValueError): + AllowedHostsValidator([" http://example.com "]) + class TestIsUrlHostValid: def test_returns_false_for_empty_url(self): @@ -32,12 +50,30 @@ def test_returns_false_for_empty_url(self): def test_returns_false_for_none_url(self): validator = AllowedHostsValidator(["example.com"]) - assert validator.is_url_host_valid(None) is False + assert validator.is_url_host_valid(None) is False # type: ignore + + def test_returns_false_for_non_string_url(self): + validator = AllowedHostsValidator(["example.com"]) + assert validator.is_url_host_valid(123) is False # type: ignore def test_returns_true_when_allowed_hosts_empty(self): validator = AllowedHostsValidator([]) assert validator.is_url_host_valid("https://anything.com/path") is True + def test_returns_false_for_malformed_url_even_when_allowed_hosts_empty(self): + validator = AllowedHostsValidator([]) + assert validator.is_url_host_valid("not-a-valid-url") is False + assert validator.is_url_host_valid("https://") is False + + def test_returns_false_for_userinfo_even_when_allowed_hosts_empty(self): + validator = AllowedHostsValidator([]) + assert validator.is_url_host_valid("https://user:password@example.com/path") is False + + def test_returns_false_for_userinfo_url(self): + validator = AllowedHostsValidator(["example.com"]) + assert validator.is_url_host_valid("https://user:password@example.com/path") is False + assert validator.is_url_host_valid("https://user@example.com/path") is False + def test_returns_true_for_allowed_host(self): validator = AllowedHostsValidator(["example.com"]) assert validator.is_url_host_valid("https://example.com/path") is True @@ -93,6 +129,10 @@ def test_handles_url_with_query_and_fragment(self): validator = AllowedHostsValidator(["example.com"]) assert validator.is_url_host_valid("https://example.com/path?q=1#frag") is True + def test_handles_url_with_whitespace(self): + validator = AllowedHostsValidator(["example.com"]) + assert validator.is_url_host_valid(" https://example.com/path ") is True + class TestSetAllowedHosts: def test_updates_allowed_hosts(self): @@ -104,7 +144,7 @@ def test_updates_allowed_hosts(self): def test_raises_on_non_list(self): validator = AllowedHostsValidator(["example.com"]) with pytest.raises(TypeError): - validator.set_allowed_hosts("example.com") + validator.set_allowed_hosts("example.com") # type: ignore def test_raises_on_http_prefix(self): validator = AllowedHostsValidator(["example.com"])