Skip to content

Commit d7e0e11

Browse files
Avery-Dunn4gust
andauthored
Add OIDC issuer validation (#840)
* Add OIDC issuer validation and related tests * Remove check on known hosts and adjust error message towards regular authority API * Updated the trusted host list and added region check logic * Update test_authority.py * Updated the version for crypto, as 48 is deperecated * Updated one conditions and added issuer to all tests * Update authority.py updated the checking on host checking * adding issuer for some missing tests * Update authority.py * updated with new regional cloud URL's --------- Co-authored-by: Nilesh Choudhary <nichoudhary@microsoft.com>
1 parent 58cf073 commit d7e0e11

File tree

5 files changed

+446
-26
lines changed

5 files changed

+446
-26
lines changed

msal/authority.py

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,29 @@
2121
'login-us.microsoftonline.com',
2222
AZURE_US_GOVERNMENT,
2323
])
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",
30+
"login.microsoft.com",
31+
"login.windows.net",
32+
"sts.windows.net",
33+
# China cloud
34+
"login.chinacloudapi.cn",
35+
"login.partner.microsoftonline.cn",
36+
# Germany cloud (legacy)
37+
"login.microsoftonline.de",
38+
# US Government clouds
39+
"login.microsoftonline.us",
40+
"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+
])
46+
2447
WELL_KNOWN_B2C_HOSTS = [
2548
"b2clogin.com",
2649
"b2clogin.cn",
@@ -67,6 +90,7 @@ def __init__(
6790
performed.
6891
"""
6992
self._http_client = http_client
93+
self._oidc_authority_url = oidc_authority_url
7094
if oidc_authority_url:
7195
logger.debug("Initializing with OIDC authority: %s", oidc_authority_url)
7296
tenant_discovery_endpoint = self._initialize_oidc_authority(
@@ -93,14 +117,24 @@ def __init__(
93117
.format(authority_url)
94118
) + " Also please double check your tenant name or GUID is correct."
95119
raise ValueError(error_message)
96-
openid_config.pop("issuer", None) # Not used in MSAL.py, so remove it therefore no need to validate it
97120
logger.debug(
98121
'openid_config("%s") = %s', tenant_discovery_endpoint, openid_config)
122+
self._issuer = openid_config.get('issuer')
99123
self.authorization_endpoint = openid_config['authorization_endpoint']
100124
self.token_endpoint = openid_config['token_endpoint']
101125
self.device_authorization_endpoint = openid_config.get('device_authorization_endpoint')
102126
_, _, self.tenant = canonicalize(self.token_endpoint) # Usually a GUID
103127

128+
# Validate the issuer if using OIDC authority
129+
if self._oidc_authority_url and not self.has_valid_issuer():
130+
raise ValueError((
131+
"The issuer '{iss}' does not match the authority '{auth}' or a known pattern. "
132+
"When using the 'oidc_authority' parameter in ClientApplication, the authority "
133+
"will be validated against the issuer from {auth}/.well-known/openid-configuration ."
134+
"If using a known Entra authority (e.g. login.microsoftonline.com) the "
135+
"'authority' parameter should be used instead of 'oidc_authority'. "
136+
""
137+
).format(iss=self._issuer, auth=oidc_authority_url))
104138
def _initialize_oidc_authority(self, oidc_authority_url):
105139
authority, self.instance, tenant = canonicalize(oidc_authority_url)
106140
self.is_adfs = tenant.lower() == 'adfs' # As a convention
@@ -175,6 +209,60 @@ def user_realm_discovery(self, username, correlation_id=None, response=None):
175209
self.__class__._domains_without_user_realm_discovery.add(self.instance)
176210
return {} # This can guide the caller to fall back normal ROPC flow
177211

212+
def has_valid_issuer(self):
213+
"""
214+
Returns True if the issuer from OIDC discovery is valid for this authority.
215+
216+
An issuer is valid if one of the following is true:
217+
- It exactly matches the authority URL (with/without trailing slash)
218+
- It has the same scheme and host as the authority (path can be different)
219+
- The issuer host is a well-known Microsoft authority host
220+
- The issuer host is a regional variant of a well-known host (e.g., westus2.login.microsoft.com)
221+
- For CIAM, hosts that end with well-known B2C hosts (e.g., tenant.b2clogin.com) are accepted as valid issuers
222+
"""
223+
if not self._issuer or not self._oidc_authority_url:
224+
return False
225+
226+
# Case 1: Exact match (most common case, normalized for trailing slashes)
227+
if self._issuer.rstrip("/") == self._oidc_authority_url.rstrip("/"):
228+
return True
229+
230+
issuer_parsed = urlparse(self._issuer)
231+
authority_parsed = urlparse(self._oidc_authority_url)
232+
issuer_host = issuer_parsed.hostname.lower() if issuer_parsed.hostname else None
233+
234+
if not issuer_host:
235+
return False
236+
237+
# Case 2: Issuer is from a trusted Microsoft host - O(1) lookup
238+
if issuer_host in TRUSTED_ISSUER_HOSTS:
239+
return True
240+
241+
# Case 3: Regional variant check - O(1) lookup
242+
# e.g., westus2.login.microsoft.com -> extract "login.microsoft.com"
243+
dot_index = issuer_host.find(".")
244+
if dot_index > 0:
245+
potential_base = issuer_host[dot_index + 1:]
246+
if "." not in issuer_host[:dot_index]:
247+
# 3a: Base host is a trusted Microsoft host
248+
if potential_base in TRUSTED_ISSUER_HOSTS:
249+
return True
250+
# 3b: Issuer has a region prefix on the authority host
251+
# e.g. issuer=us.someweb.com, authority=someweb.com
252+
authority_host = authority_parsed.hostname.lower() if authority_parsed.hostname else ""
253+
if potential_base == authority_host:
254+
return True
255+
256+
# Case 4: Same scheme and host (path can differ)
257+
if (authority_parsed.scheme == issuer_parsed.scheme and
258+
authority_parsed.netloc == issuer_parsed.netloc):
259+
return True
260+
261+
# Case 5: Check if issuer host ends with any well-known B2C host (e.g., tenant.b2clogin.com)
262+
if any(issuer_host.endswith(h) for h in WELL_KNOWN_B2C_HOSTS):
263+
return True
264+
265+
return False
178266

179267
def canonicalize(authority_or_auth_endpoint):
180268
# Returns (url_parsed_result, hostname_in_lowercase, tenant)
@@ -223,4 +311,3 @@ def tenant_discovery(tenant_discovery_endpoint, http_client, **kwargs):
223311
resp.raise_for_status()
224312
raise RuntimeError( # A fallback here, in case resp.raise_for_status() is no-op
225313
"Unable to complete OIDC Discovery: %d, %s" % (resp.status_code, resp.text))
226-

tests/simulator.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ def __init__(self, number_of_tenants=1, tokens_per_tenant=1, cache_hit=False):
3131
with patch.object(msal.authority, "tenant_discovery", return_value={
3232
"authorization_endpoint": "https://contoso.com/placeholder",
3333
"token_endpoint": "https://contoso.com/placeholder",
34+
"issuer": "https://contoso.com/placeholder",
3435
}) as _: # Otherwise it would fail on OIDC discovery
3536
self.apps = [ # In MSAL Python, each CCA binds to one tenant only
3637
msal.ConfidentialClientApplication(

tests/test_account_source.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ def _mock_post(url, headers=None, *args, **kwargs):
2626
@patch.object(msal.authority, "tenant_discovery", return_value={
2727
"authorization_endpoint": "https://contoso.com/placeholder",
2828
"token_endpoint": "https://contoso.com/placeholder",
29+
"issuer": "https://contoso.com/placeholder",
2930
}) # Otherwise it would fail on OIDC discovery
3031
class TestAccountSourceBehavior(unittest.TestCase):
3132

tests/test_application.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
_OIDC_DISCOVERY_MOCK = Mock(return_value={
2525
"authorization_endpoint": "https://contoso.com/placeholder",
2626
"token_endpoint": "https://contoso.com/placeholder",
27+
"issuer": "https://contoso.com/tenant",
2728
})
2829

2930

@@ -690,6 +691,7 @@ def mock_post(url, headers=None, *args, **kwargs):
690691
@patch(_OIDC_DISCOVERY, new=Mock(return_value={
691692
"authorization_endpoint": "https://contoso.com/common",
692693
"token_endpoint": "https://contoso.com/common",
694+
"issuer": "https://contoso.com/common",
693695
}))
694696
def test_common_authority_should_emit_warning(self):
695697
self._test_certain_authority_should_emit_warning(
@@ -698,6 +700,7 @@ def test_common_authority_should_emit_warning(self):
698700
@patch(_OIDC_DISCOVERY, new=Mock(return_value={
699701
"authorization_endpoint": "https://contoso.com/organizations",
700702
"token_endpoint": "https://contoso.com/organizations",
703+
"issuer": "https://contoso.com/organizations",
701704
}))
702705
def test_organizations_authority_should_emit_warning(self):
703706
self._test_certain_authority_should_emit_warning(
@@ -755,6 +758,7 @@ def test_client_id_should_be_a_valid_scope(self):
755758
@patch("msal.authority.tenant_discovery", new=Mock(return_value={
756759
"authorization_endpoint": "https://contoso.com/placeholder",
757760
"token_endpoint": "https://contoso.com/placeholder",
761+
"issuer": "https://contoso.com/placeholder",
758762
}))
759763
class TestMsalBehaviorWithoutPyMsalRuntimeOrBroker(unittest.TestCase):
760764

@@ -796,6 +800,7 @@ def test_should_fallback_when_pymsalruntime_failed_to_initialize_broker(self):
796800
@patch("msal.authority.tenant_discovery", new=Mock(return_value={
797801
"authorization_endpoint": "https://contoso.com/placeholder",
798802
"token_endpoint": "https://contoso.com/placeholder",
803+
"issuer": "https://contoso.com/placeholder",
799804
}))
800805
@patch("msal.application._init_broker", new=Mock()) # Pretend pymsalruntime installed and working
801806
class TestBrokerFallbackWithDifferentAuthorities(unittest.TestCase):

0 commit comments

Comments
 (0)