fix(abstractions): normalize allowed hosts, reject userinfo, and validate URL structure

packages/abstractions/kiota_abstractions/authentication/allowed_hosts_validator.py

@@ -13,14 +13,8 @@
             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 @@
         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 @@
             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
         )

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"])