Updated the trusted host list and added region check logic

Author: 4gust

msal/authority.py

@@ -21,6 +21,26 @@
     'login-us.microsoftonline.com',
     AZURE_US_GOVERNMENT,
     ])
+
+# Trusted issuer hosts for OIDC issuer validation
+# Includes all well-known Microsoft identity provider hosts and national clouds
+TRUSTED_ISSUER_HOSTS = frozenset([
+    # Global/Public cloud
+    "login.microsoftonline.com",
+    "login.microsoft.com",
+    "login.windows.net",
+    "sts.windows.net",
+    # China cloud
+    "login.chinacloudapi.cn",
+    "login.partner.microsoftonline.cn",
+    # Germany cloud (legacy)
+    "login.microsoftonline.de",
+    # US Government clouds
+    "login.microsoftonline.us",
+    "login.usgovcloudapi.net",
+    "login-us.microsoftonline.com",
+])
+
 WELL_KNOWN_B2C_HOSTS = [
     "b2clogin.com",
     "b2clogin.cn",
@@ -186,47 +206,59 @@ def user_realm_discovery(self, username, correlation_id=None, response=None):
             self.__class__._domains_without_user_realm_discovery.add(self.instance)
         return {}  # This can guide the caller to fall back normal ROPC flow
 
-    def has_valid_issuer(self) -> bool:
+    def has_valid_issuer(self):
         """
-            Returns True if the issuer from OIDC discovery is valid for this authority.
-
-            An issuer is valid if one of the following is true:
-            - It exactly matches the authority URL
-            - It has the same scheme and host as the authority (path can be different)
-            - For CIAM, the issuer follows the pattern of {tenant}.ciamlogin.com (tenant comes from the authority)
-            """
-        if self._oidc_authority_url == self._issuer:
-            # The issuer matches the authority URL exactly
-            return True
+        Returns True if the issuer from OIDC discovery is valid for this authority.
 
-        issuer = urlparse(self._issuer) if self._issuer else None
-        if not issuer:
+        An issuer is valid if one of the following is true:
+        - It exactly matches the authority URL (with/without trailing slash)
+        - It has the same scheme and host as the authority (path can be different)
+        - The issuer host is a well-known Microsoft authority host
+        - The issuer host is a regional variant of a well-known host (e.g., westus2.login.microsoft.com)
+        - For CIAM, the issuer follows the pattern of {tenant}.ciamlogin.com
+        """
+        if not self._issuer or not self._oidc_authority_url:
             return False
 
-        # Check if issuer has the same scheme and host as the authority
-        authority = urlparse(self._oidc_authority_url)
-        if authority.scheme == issuer.scheme and authority.netloc == issuer.netloc:
+        # Case 1: Exact match (most common case, normalized for trailing slashes)
+        if self._issuer.rstrip("/") == self._oidc_authority_url.rstrip("/"):
             return True
 
-        # Check CIAM scenario: issuer follows the pattern {tenant}.ciamlogin.com
-        # Extract tenant from authority URL - could be in the host or path
-        tenant = None
-        if authority.hostname.endswith(_CIAM_DOMAIN_SUFFIX):
-            # Normal CIAM host: {tenant}.ciamlogin.com
-            tenant = authority.hostname.rsplit(_CIAM_DOMAIN_SUFFIX, 1)[0]
-        else:
-            # Custom CIAM host: extract tenant from path
-            parts = authority.path.split('/')
-            if len(parts) >= 2 and parts[1]:
-                tenant = parts[1]  # First path segment after the initial '/'
+        issuer_parsed = urlparse(self._issuer)
+        authority_parsed = urlparse(self._oidc_authority_url)
+        issuer_host = issuer_parsed.hostname.lower() if issuer_parsed.hostname else None
 
-        if tenant and issuer.hostname.endswith(_CIAM_DOMAIN_SUFFIX):
-            # Check if issuer follows the pattern {tenant}.ciamlogin.com
-            expected_issuer_host = f"{tenant}{_CIAM_DOMAIN_SUFFIX}"
-            if issuer.hostname == expected_issuer_host:
+        if not issuer_host:
+            return False
+        
+        # Case 2: Issuer is from a trusted Microsoft host - O(1) lookup
+        if issuer_host in TRUSTED_ISSUER_HOSTS:
+            return True
+
+        # Case 3: Regional variant check - O(1) lookup
+        # e.g., westus2.login.microsoft.com -> extract "login.microsoft.com"
+        dot_index = issuer_host.find(".")
+        if dot_index > 0:
+            potential_base = issuer_host[dot_index + 1:]
+            if potential_base in TRUSTED_ISSUER_HOSTS and "." not in issuer_host[:dot_index]:
                 return True
 
-        # None of the conditions matched
+        # Case 4: Same scheme and host (path can differ)
+        if (authority_parsed.scheme == issuer_parsed.scheme and 
+            authority_parsed.netloc == issuer_parsed.netloc):
+            return True
+
+        # Case 5: CIAM scenario - issuer follows pattern {tenant}.ciamlogin.com
+        if issuer_host.endswith(_CIAM_DOMAIN_SUFFIX):
+            authority_host = authority_parsed.hostname.lower() if authority_parsed.hostname else ""
+            if authority_host.endswith(_CIAM_DOMAIN_SUFFIX):
+                tenant = authority_host[:-len(_CIAM_DOMAIN_SUFFIX)]
+            else:
+                parts = authority_parsed.path.split('/')
+                tenant = parts[1] if len(parts) >= 2 and parts[1] else None
+            
+            if tenant and issuer_host == tenant + _CIAM_DOMAIN_SUFFIX:
+                return True
         return False
 
 def canonicalize(authority_or_auth_endpoint):

tests/test_authority.py

@@ -6,7 +6,7 @@
 
 import msal
 from msal.authority import *
-from msal.authority import _CIAM_DOMAIN_SUFFIX  # Explicitly import the private constant
+from msal.authority import _CIAM_DOMAIN_SUFFIX, TRUSTED_ISSUER_HOSTS  # Explicitly import private/new constants
 from tests import unittest
 from tests.http_client import MinimalHttpClient
 
@@ -372,43 +372,122 @@ def test_invalid_issuer(self, tenant_discovery_mock):
 
     @patch("msal.authority.tenant_discovery")
     def test_custom_authority_with_microsoft_issuer(self, tenant_discovery_mock):
-        """Test when custom authority is used with a known Microsoft issuer (should fail)"""
+        """Test when custom authority is used with a known Microsoft issuer (should succeed)"""
         authority_url = "https://custom-domain.com/tenant"
         issuer = f"https://{WORLD_WIDE}/tenant"
+        authority = self._create_authority_with_issuer(authority_url, issuer, tenant_discovery_mock)
+        self.assertTrue(authority.has_valid_issuer(),
+            "Issuer from trusted Microsoft host should be valid even with custom authority")
+
+    @patch("msal.authority.tenant_discovery")
+    def test_known_authority_with_non_matching_issuer(self, tenant_discovery_mock):
+        """Test when known authority is used with an issuer that doesn't match (should fail)"""
+        # Known Microsoft authority URLs
+        authority_url = f"https://{WORLD_WIDE}/tenant"
+        issuer = "https://custom-domain.com/tenant"
 
         tenant_discovery_mock.return_value = {
             "authorization_endpoint": "https://example.com/oauth2/authorize",
             "token_endpoint": "https://example.com/oauth2/token",
             "issuer": issuer,
         }
 
-        # Since initialization now checks for valid issuer and we removed the check for known hosts,
-        # we expect it to raise ValueError because the hosts don't match
+        # We expect it to raise ValueError because the paths don't match
+        # and we're now checking for exact matches
         with self.assertRaises(ValueError) as context:
             Authority(None, self.http_client, oidc_authority_url=authority_url)
 
         self.assertIn("issuer", str(context.exception).lower())
         self.assertIn(issuer, str(context.exception))
         self.assertIn(authority_url, str(context.exception))
 
+    # Regional pattern tests
     @patch("msal.authority.tenant_discovery")
-    def test_known_authority_with_non_matching_issuer(self, tenant_discovery_mock):
-        """Test when known authority is used with an issuer that doesn't match (should fail)"""
-        # Known Microsoft authority URLs
-        authority_url = f"https://{WORLD_WIDE}/tenant"
-        issuer = "https://custom-domain.com/tenant"
+    def test_regional_issuer_westus2_login_microsoft(self, tenant_discovery_mock):
+        """Test regional variant: westus2.login.microsoft.com"""
+        authority_url = "https://custom-authority.com/tenant"
+        issuer = "https://westus2.login.microsoftonline.com/tenant"
+        authority = self._create_authority_with_issuer(authority_url, issuer, tenant_discovery_mock)
+        self.assertTrue(authority.has_valid_issuer(), 
+            "Regional issuer westus2.login.microsoftonline.com should be valid")
 
+    @patch("msal.authority.tenant_discovery")
+    def test_regional_issuer_eastus_login_microsoftonline(self, tenant_discovery_mock):
+        """Test regional variant: eastus.login.microsoftonline.com"""
+        authority_url = "https://custom-authority.com/tenant"
+        issuer = "https://eastus.login.microsoftonline.com/tenant"
+        authority = self._create_authority_with_issuer(authority_url, issuer, tenant_discovery_mock)
+        self.assertTrue(authority.has_valid_issuer(),
+            "Regional issuer eastus.login.microsoftonline.com should be valid")
+
+    @patch("msal.authority.tenant_discovery")
+    def test_regional_issuer_for_china_cloud(self, tenant_discovery_mock):
+        """Test regional variant for China cloud: region.login.chinacloudapi.cn"""
+        authority_url = "https://custom-authority.com/tenant"
+        issuer = "https://chinanorth.login.chinacloudapi.cn/tenant"
+        authority = self._create_authority_with_issuer(authority_url, issuer, tenant_discovery_mock)
+        self.assertTrue(authority.has_valid_issuer(),
+            "Regional issuer for China cloud should be valid")
+
+    @patch("msal.authority.tenant_discovery")
+    def test_regional_issuer_for_us_government(self, tenant_discovery_mock):
+        """Test regional variant for US Government: region.login.microsoftonline.us"""
+        authority_url = "https://custom-authority.com/tenant"
+        issuer = "https://usgovvirginia.login.microsoftonline.us/tenant"
+        authority = self._create_authority_with_issuer(authority_url, issuer, tenant_discovery_mock)
+        self.assertTrue(authority.has_valid_issuer(),
+            "Regional issuer for US Government should be valid")
+
+    @patch("msal.authority.tenant_discovery")
+    def test_invalid_regional_pattern_with_dots_in_region(self, tenant_discovery_mock):
+        """Test that region with dots is rejected: west.us.2.login.microsoftonline.com"""
+        authority_url = "https://custom-authority.com/tenant"
+        issuer = "https://west.us.2.login.microsoftonline.com/tenant"
         tenant_discovery_mock.return_value = {
             "authorization_endpoint": "https://example.com/oauth2/authorize",
             "token_endpoint": "https://example.com/oauth2/token",
             "issuer": issuer,
         }
+        with self.assertRaises(ValueError):
+            Authority(None, self.http_client, oidc_authority_url=authority_url)
 
-        # We expect it to raise ValueError because the paths don't match
-        # and we're now checking for exact matches
-        with self.assertRaises(ValueError) as context:
+    @patch("msal.authority.tenant_discovery")
+    def test_invalid_regional_pattern_untrusted_base(self, tenant_discovery_mock):
+        """Test that regional pattern with untrusted base is rejected"""
+        authority_url = "https://custom-authority.com/tenant"
+        issuer = "https://westus2.login.evil.com/tenant"  # evil.com is not trusted
+        tenant_discovery_mock.return_value = {
+            "authorization_endpoint": "https://example.com/oauth2/authorize",
+            "token_endpoint": "https://example.com/oauth2/token",
+            "issuer": issuer,
+        }
+        with self.assertRaises(ValueError):
             Authority(None, self.http_client, oidc_authority_url=authority_url)
 
-        self.assertIn("issuer", str(context.exception).lower())
-        self.assertIn(issuer, str(context.exception))
-        self.assertIn(authority_url, str(context.exception))
+    @patch("msal.authority.tenant_discovery")
+    def test_well_known_host_issuer_directly(self, tenant_discovery_mock):
+        """Test issuer from well-known Microsoft host directly (not regional)"""
+        authority_url = "https://custom-authority.com/tenant"
+        issuer = f"https://{WORLD_WIDE}/tenant"
+        authority = self._create_authority_with_issuer(authority_url, issuer, tenant_discovery_mock)
+        self.assertTrue(authority.has_valid_issuer(),
+            "Issuer from well-known Microsoft host should be valid")
+
+    @patch("msal.authority.tenant_discovery")
+    def test_issuer_with_trailing_slash_match(self, tenant_discovery_mock):
+        """Test issuer validation handles trailing slashes"""
+        authority_url = "https://example.com/tenant/"
+        issuer = "https://example.com/tenant"  # No trailing slash
+        authority = self._create_authority_with_issuer(authority_url, issuer, tenant_discovery_mock)
+        self.assertTrue(authority.has_valid_issuer(),
+            "Trailing slash difference should not affect exact match")
+
+    @patch("msal.authority.tenant_discovery")
+    def test_issuer_case_sensitivity_host(self, tenant_discovery_mock):
+        """Test that host comparison is case-insensitive for regional check"""
+        authority_url = "https://custom-authority.com/tenant"
+        issuer = "https://WESTUS2.LOGIN.MICROSOFTONLINE.COM/tenant"  # Uppercase
+        authority = self._create_authority_with_issuer(authority_url, issuer, tenant_discovery_mock)
+        self.assertTrue(authority.has_valid_issuer(),
+            "Host comparison should be case-insensitive")
+