Skip to content

Commit 984899b

Browse files
committed
Instance discovery remains cloud local on known clouds
1 parent d7e0e11 commit 984899b

File tree

2 files changed

+109
-34
lines changed

2 files changed

+109
-34
lines changed

msal/authority.py

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,38 +11,28 @@
1111
# Endpoints were copied from here
1212
# https://docs.microsoft.com/en-us/azure/active-directory/develop/authentication-national-cloud#azure-ad-authentication-endpoints
1313
AZURE_US_GOVERNMENT = "login.microsoftonline.us"
14-
AZURE_CHINA = "login.chinacloudapi.cn"
14+
DEPRECATED_AZURE_CHINA = "login.chinacloudapi.cn"
1515
AZURE_PUBLIC = "login.microsoftonline.com"
16+
AZURE_GOV_FR = "login.sovcloud-identity.fr"
17+
AZURE_GOV_DE = "login.sovcloud-identity.de"
18+
AZURE_GOV_SG = "login.sovcloud-identity.sg"
1619

1720
WORLD_WIDE = 'login.microsoftonline.com' # There was an alias login.windows.net
18-
WELL_KNOWN_AUTHORITY_HOSTS = set([
21+
WELL_KNOWN_AUTHORITY_HOSTS = frozenset([
1922
WORLD_WIDE,
20-
AZURE_CHINA,
21-
'login-us.microsoftonline.com',
22-
AZURE_US_GOVERNMENT,
23-
])
24-
25-
# Trusted issuer hosts for OIDC issuer validation
26-
# Includes all well-known Microsoft identity provider hosts and national clouds
27-
TRUSTED_ISSUER_HOSTS = frozenset([
28-
# Global/Public cloud
29-
"login.microsoftonline.com",
3023
"login.microsoft.com",
3124
"login.windows.net",
3225
"sts.windows.net",
33-
# China cloud
34-
"login.chinacloudapi.cn",
26+
DEPRECATED_AZURE_CHINA,
3527
"login.partner.microsoftonline.cn",
36-
# Germany cloud (legacy)
37-
"login.microsoftonline.de",
38-
# US Government clouds
39-
"login.microsoftonline.us",
28+
"login.microsoftonline.de", # deprecated
29+
'login-us.microsoftonline.com',
30+
AZURE_US_GOVERNMENT,
4031
"login.usgovcloudapi.net",
41-
"login-us.microsoftonline.com",
42-
"https://login.sovcloud-identity.fr", # AzureBleu
43-
"https://login.sovcloud-identity.de", # AzureDelos
44-
"https://login.sovcloud-identity.sg", # AzureGovSG
45-
])
32+
AZURE_GOV_FR,
33+
AZURE_GOV_DE,
34+
AZURE_GOV_SG,
35+
])
4636

4737
WELL_KNOWN_B2C_HOSTS = [
4838
"b2clogin.com",
@@ -162,10 +152,10 @@ def _initialize_entra_authority(
162152
) or (len(parts) == 3 and parts[2].lower().startswith("b2c_"))
163153
self._is_known_to_developer = self.is_adfs or self._is_b2c or not validate_authority
164154
is_known_to_microsoft = self.instance in WELL_KNOWN_AUTHORITY_HOSTS
155+
instance_discovery_host = (
156+
self.instance if self.instance in WELL_KNOWN_AUTHORITY_HOSTS else WORLD_WIDE)
165157
instance_discovery_endpoint = 'https://{}/common/discovery/instance'.format( # Note: This URL seemingly returns V1 endpoint only
166-
WORLD_WIDE # Historically using WORLD_WIDE. Could use self.instance too
167-
# See https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/4.0.0/src/Microsoft.Identity.Client/Instance/AadInstanceDiscovery.cs#L101-L103
168-
# and https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/4.0.0/src/Microsoft.Identity.Client/Instance/AadAuthority.cs#L19-L33
158+
instance_discovery_host
169159
) if instance_discovery in (None, True) else instance_discovery
170160
if instance_discovery_endpoint and not (
171161
is_known_to_microsoft or self._is_known_to_developer):
@@ -177,8 +167,8 @@ def _initialize_entra_authority(
177167
if payload.get("error") == "invalid_instance":
178168
raise ValueError(
179169
"invalid_instance: "
180-
"The authority you provided, %s, is not whitelisted. "
181-
"If it is indeed your legit customized domain name, "
170+
"The authority you provided, %s, is not known. "
171+
"If it is a valid domain name known to you, "
182172
"you can turn off this check by passing in "
183173
"instance_discovery=False"
184174
% authority_url)
@@ -235,7 +225,7 @@ def has_valid_issuer(self):
235225
return False
236226

237227
# Case 2: Issuer is from a trusted Microsoft host - O(1) lookup
238-
if issuer_host in TRUSTED_ISSUER_HOSTS:
228+
if issuer_host in WELL_KNOWN_AUTHORITY_HOSTS:
239229
return True
240230

241231
# Case 3: Regional variant check - O(1) lookup
@@ -245,7 +235,7 @@ def has_valid_issuer(self):
245235
potential_base = issuer_host[dot_index + 1:]
246236
if "." not in issuer_host[:dot_index]:
247237
# 3a: Base host is a trusted Microsoft host
248-
if potential_base in TRUSTED_ISSUER_HOSTS:
238+
if potential_base in WELL_KNOWN_AUTHORITY_HOSTS:
249239
return True
250240
# 3b: Issuer has a region prefix on the authority host
251241
# e.g. issuer=us.someweb.com, authority=someweb.com

tests/test_authority.py

Lines changed: 89 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import msal
88
from msal.authority import *
9-
from msal.authority import _CIAM_DOMAIN_SUFFIX, TRUSTED_ISSUER_HOSTS # Explicitly import private/new constants
9+
from msal.authority import _CIAM_DOMAIN_SUFFIX
1010
from tests import unittest
1111
from tests.http_client import MinimalHttpClient
1212

@@ -37,10 +37,95 @@ def _test_authority_builder(self, host, tenant):
3737
c.close()
3838

3939
def test_wellknown_host_and_tenant(self):
40-
# Assert all well known authority hosts are using their own "common" tenant
40+
# This test makes real HTTP calls to authority endpoints.
41+
# It is intentionally network-based to validate reachable hosts end-to-end.
42+
excluded_hosts = {
43+
DEPRECATED_AZURE_CHINA,
44+
"login.microsoftonline.de", # deprecated
45+
"login.microsoft.com", # issuer-only in this test context
46+
"login.windows.net", # issuer-only in this test context
47+
"sts.windows.net", # issuer-only in this test context
48+
"login.partner.microsoftonline.cn", # issuer-only in this test context
49+
"login.usgovcloudapi.net", # issuer-only in this test context
50+
AZURE_GOV_FR, # currently unreachable in this environment
51+
AZURE_GOV_DE, # currently unreachable in this environment
52+
AZURE_GOV_SG, # currently unreachable in this environment
53+
}
4154
for host in WELL_KNOWN_AUTHORITY_HOSTS:
42-
if host != AZURE_CHINA: # It is prone to ConnectionError
43-
self._test_given_host_and_tenant(host, "common")
55+
if host in excluded_hosts:
56+
continue
57+
self._test_given_host_and_tenant(host, "common")
58+
59+
def test_new_sovereign_hosts_should_be_known_authorities(self):
60+
self.assertIn(AZURE_GOV_FR, WELL_KNOWN_AUTHORITY_HOSTS)
61+
self.assertIn(AZURE_GOV_DE, WELL_KNOWN_AUTHORITY_HOSTS)
62+
self.assertIn(AZURE_GOV_SG, WELL_KNOWN_AUTHORITY_HOSTS)
63+
64+
@patch("msal.authority._instance_discovery")
65+
@patch("msal.authority.tenant_discovery")
66+
def test_new_sovereign_hosts_should_build_authority_endpoints(
67+
self, tenant_discovery_mock, instance_discovery_mock):
68+
for host in (AZURE_GOV_FR, AZURE_GOV_DE, AZURE_GOV_SG):
69+
tenant_discovery_mock.return_value = {
70+
"authorization_endpoint": "https://{}/common/oauth2/v2.0/authorize".format(host),
71+
"token_endpoint": "https://{}/common/oauth2/v2.0/token".format(host),
72+
"issuer": "https://{}/common/v2.0".format(host),
73+
}
74+
instance_discovery_mock.return_value = {
75+
"tenant_discovery_endpoint": (
76+
"https://{}/common/v2.0/.well-known/openid-configuration".format(host)
77+
),
78+
}
79+
c = MinimalHttpClient()
80+
a = Authority(AuthorityBuilder(host, "common"), c)
81+
self.assertEqual(
82+
a.authorization_endpoint,
83+
"https://{}/common/oauth2/v2.0/authorize".format(host))
84+
self.assertEqual(
85+
a.token_endpoint,
86+
"https://{}/common/oauth2/v2.0/token".format(host))
87+
c.close()
88+
89+
@patch("msal.authority._instance_discovery")
90+
@patch("msal.authority.tenant_discovery")
91+
def test_known_authority_should_use_same_host_and_skip_instance_discovery(
92+
self, tenant_discovery_mock, instance_discovery_mock):
93+
host = AZURE_US_GOVERNMENT
94+
tenant_discovery_mock.return_value = {
95+
"authorization_endpoint": "https://{}/common/oauth2/v2.0/authorize".format(host),
96+
"token_endpoint": "https://{}/common/oauth2/v2.0/token".format(host),
97+
"issuer": "https://{}/common/v2.0".format(host),
98+
}
99+
c = MinimalHttpClient()
100+
Authority("https://{}/common".format(host), c)
101+
c.close()
102+
103+
instance_discovery_mock.assert_not_called()
104+
tenant_discovery_endpoint = tenant_discovery_mock.call_args[0][0]
105+
self.assertTrue(
106+
tenant_discovery_endpoint.startswith(
107+
"https://{}/common/v2.0/.well-known/openid-configuration".format(host)))
108+
109+
@patch("msal.authority._instance_discovery")
110+
@patch("msal.authority.tenant_discovery")
111+
def test_unknown_authority_should_use_world_wide_instance_discovery_endpoint(
112+
self, tenant_discovery_mock, instance_discovery_mock):
113+
tenant_discovery_mock.return_value = {
114+
"authorization_endpoint": "https://example.com/tenant/oauth2/v2.0/authorize",
115+
"token_endpoint": "https://example.com/tenant/oauth2/v2.0/token",
116+
"issuer": "https://example.com/tenant/v2.0",
117+
}
118+
instance_discovery_mock.return_value = {
119+
"tenant_discovery_endpoint": "https://example.com/tenant/v2.0/.well-known/openid-configuration",
120+
}
121+
122+
c = MinimalHttpClient()
123+
Authority("https://example.com/tenant", c)
124+
c.close()
125+
126+
self.assertEqual(
127+
"https://{}/common/discovery/instance".format(WORLD_WIDE),
128+
instance_discovery_mock.call_args[0][2])
44129

45130
def test_wellknown_host_and_tenant_using_new_authority_builder(self):
46131
self._test_authority_builder(AZURE_PUBLIC, "consumers")

0 commit comments

Comments
 (0)