Skip to content

Commit 4a4b396

Browse files
committed
Added a check for matching OIDC discovery
1 parent d3464e6 commit 4a4b396

File tree

3 files changed

+417
-17
lines changed

3 files changed

+417
-17
lines changed

msal/authority.py

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,20 @@
3030
]
3131
_CIAM_DOMAIN_SUFFIX = ".ciamlogin.com"
3232

33+
# Trusted issuer hosts for OIDC issuer validation.
34+
# These are Microsoft's well-known identity provider hosts.
35+
TRUSTED_ISSUER_HOSTS = frozenset([
36+
AZURE_PUBLIC, # Microsoft Azure Worldwide
37+
"login.microsoft.com", # Microsoft Azure Worldwide
38+
"login.windows.net", # Microsoft Azure Worldwide (validation scenarios)
39+
"sts.windows.net", # Microsoft STS
40+
"login.partner.microsoftonline.cn", # Microsoft Azure China
41+
AZURE_CHINA, # Microsoft Azure China (legacy)
42+
"login.microsoftonline.de", # Microsoft Azure Germany
43+
"login-us.microsoftonline.com", # Microsoft Azure US Government (legacy)
44+
AZURE_US_GOVERNMENT, # Microsoft Azure US Government
45+
"login.usgovcloudapi.net", # Microsoft Azure US Government
46+
])
3347

3448
class AuthorityBuilder(object):
3549
def __init__(self, instance, tenant):
@@ -93,7 +107,8 @@ def __init__(
93107
.format(authority_url)
94108
) + " Also please double check your tenant name or GUID is correct."
95109
raise ValueError(error_message)
96-
openid_config.pop("issuer", None) # Not used in MSAL.py, so remove it therefore no need to validate it
110+
if oidc_authority_url:
111+
_validate_issuer(openid_config.get("issuer"), oidc_authority_url)
97112
logger.debug(
98113
'openid_config("%s") = %s', tenant_discovery_endpoint, openid_config)
99114
self.authorization_endpoint = openid_config['authorization_endpoint']
@@ -224,3 +239,49 @@ def tenant_discovery(tenant_discovery_endpoint, http_client, **kwargs):
224239
raise RuntimeError( # A fallback here, in case resp.raise_for_status() is no-op
225240
"Unable to complete OIDC Discovery: %d, %s" % (resp.status_code, resp.text))
226241

242+
def _validate_issuer(issuer, authority_url):
243+
"""Validate that the OIDC issuer matches the authority or is from a trusted source.
244+
245+
Per OIDC Discovery spec, the issuer returned MUST match the authority.
246+
We also allow issuers from well-known trusted Microsoft sources, including
247+
regional variants (e.g., westus2.login.microsoft.com).
248+
249+
:param issuer: The issuer claim from the OIDC discovery response.
250+
:param authority_url: The OIDC authority URL provided by the caller.
251+
:raises ValueError: If issuer is missing or not from authority/trusted sources.
252+
"""
253+
if not issuer:
254+
raise ValueError(
255+
"The OIDC discovery response from {} is missing the required 'issuer' claim."
256+
.format(authority_url))
257+
258+
# Normalize URLs for comparison (remove trailing slashes)
259+
normalized_issuer = issuer.rstrip("/")
260+
normalized_authority = authority_url.rstrip("/")
261+
262+
# Case 1: Exact match (most common case)
263+
if normalized_issuer == normalized_authority:
264+
return
265+
266+
# Case 2: Check if issuer is from a trusted source
267+
issuer_parsed = urlparse(issuer)
268+
issuer_host = issuer_parsed.hostname.lower() if issuer_parsed.hostname else None
269+
if issuer_host:
270+
# Direct lookup - O(1)
271+
if issuer_host in TRUSTED_ISSUER_HOSTS:
272+
return
273+
# Check for regional pattern: {region}.{trusted_base}
274+
# e.g., westus2.login.microsoft.com -> extract "login.microsoft.com"
275+
# Find the first dot and check if the remainder is a trusted host
276+
dot_index = issuer_host.find(".")
277+
if dot_index > 0:
278+
potential_base = issuer_host[dot_index + 1:] # e.g., "login.microsoft.com"
279+
region = issuer_host[:dot_index] # e.g., "westus2"
280+
# O(1) lookup instead of O(n) iteration
281+
if potential_base in TRUSTED_ISSUER_HOSTS and "." not in region:
282+
return
283+
284+
raise ValueError(
285+
"The issuer '{}' from the OIDC discovery response does not match "
286+
"the authority '{}' and is not from a trusted source."
287+
.format(issuer, authority_url))

tests/test_application.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
_OIDC_DISCOVERY = "msal.authority.tenant_discovery"
2424
_OIDC_DISCOVERY_MOCK = Mock(return_value={
25+
"issuer": "https://contoso.com/placeholder",
2526
"authorization_endpoint": "https://contoso.com/placeholder",
2627
"token_endpoint": "https://contoso.com/placeholder",
2728
})
@@ -794,6 +795,7 @@ def test_should_fallback_when_pymsalruntime_failed_to_initialize_broker(self):
794795

795796
@patch("sys.platform", new="darwin") # Pretend running on Mac.
796797
@patch("msal.authority.tenant_discovery", new=Mock(return_value={
798+
"issuer": "https://contoso.com/placeholder",
797799
"authorization_endpoint": "https://contoso.com/placeholder",
798800
"token_endpoint": "https://contoso.com/placeholder",
799801
}))
@@ -846,7 +848,7 @@ def test_should_use_broker_when_disabling_instance_discovery(self):
846848
def test_should_fallback_to_non_broker_when_using_oidc_authority(self):
847849
app = msal.PublicClientApplication(
848850
"client_id",
849-
oidc_authority="https://contoso.com/path",
851+
oidc_authority="https://contoso.com/placeholder",
850852
enable_broker_on_mac=True,
851853
)
852854
self.assertFalse(app._enable_broker)

0 commit comments

Comments
 (0)