Skip to content

Commit 0a8d2be

Browse files
CopilotbgavrilMS
andcommitted
Refactor: Use _is_oidc flag for proper authority classification
- Add _is_oidc flag to Authority class to distinguish OIDC generic authorities - Simplify thumbprint selection logic: use SHA256 for all except ADFS and OIDC - Authority classification now: * ADFS: authority.is_adfs → SHA1 * B2C: authority._is_b2c (not OIDC) → SHA256 * CIAM: authority._is_b2c (not OIDC) → SHA256 * OIDC generic: authority._is_oidc → SHA1 * AAD: everything else → SHA256 - Update tests to reflect new classification - Add test for unknown AAD authority (sovereign cloud) Co-authored-by: bgavrilMS <12273384+bgavrilMS@users.noreply.github.com>
1 parent 81240fb commit 0a8d2be

File tree

3 files changed

+56
-17
lines changed

3 files changed

+56
-17
lines changed

msal/application.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -851,18 +851,19 @@ def _build_client(self, client_credential, authority, skip_regional_client=False
851851
): # Then we treat the public_certificate value as PEM content
852852
headers["x5c"] = extract_certs(client_credential['public_certificate'])
853853
# Determine which thumbprint to use based on what's available and authority type
854-
# Based on the feature requirement:
855-
# - If both thumbprints are provided, use SHA256 for AAD authorities
856-
# (including B2C, CIAM), and SHA1 for ADFS and generic authorities
854+
# Authority classification:
855+
# - ADFS: authority.is_adfs
856+
# - B2C: authority._is_b2c (and not OIDC)
857+
# - CIAM: authority._is_b2c (and not OIDC)
858+
# - OIDC generic: authority._is_oidc
859+
# - AAD: everything else
860+
# Use SHA256 for AAD, B2C, CIAM; use SHA1 for ADFS and OIDC generic
857861
use_sha256 = False
858862
if sha256_thumbprint and sha1_thumbprint:
859863
# Both thumbprints provided - choose based on authority type
860-
# Use SHA256 for AAD (including B2C, CIAM), SHA1 for ADFS and generic
861-
from .authority import WELL_KNOWN_AUTHORITY_HOSTS
862-
is_known_aad = authority.instance in WELL_KNOWN_AUTHORITY_HOSTS
863-
is_b2c_or_ciam = getattr(authority, '_is_b2c', False)
864-
# Use SHA256 for known AAD, B2C, or CIAM; SHA1 for ADFS and generic
865-
use_sha256 = (is_known_aad or is_b2c_or_ciam) and not authority.is_adfs
864+
is_oidc = getattr(authority, '_is_oidc', False)
865+
# Use SHA1 for ADFS and OIDC generic; SHA256 for everything else (AAD, B2C, CIAM)
866+
use_sha256 = not authority.is_adfs and not is_oidc
866867
elif sha256_thumbprint:
867868
# Only SHA256 provided
868869
use_sha256 = True

msal/authority.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ def _initialize_oidc_authority(self, oidc_authority_url):
107107
self._is_b2c = True # Not exactly true, but
108108
# OIDC Authority was designed for CIAM which is the next gen of B2C.
109109
# Besides, application.py uses this to bypass broker.
110+
self._is_oidc = True # Track that this is a generic OIDC authority
110111
self._is_known_to_developer = True # Not really relevant, but application.py uses this to bypass authority validation
111112
return oidc_authority_url + "/.well-known/openid-configuration"
112113

@@ -126,6 +127,7 @@ def _initialize_entra_authority(
126127
self._is_b2c = any(
127128
self.instance.endswith("." + d) for d in WELL_KNOWN_B2C_HOSTS
128129
) or (len(parts) == 3 and parts[2].lower().startswith("b2c_"))
130+
self._is_oidc = False # This is not a generic OIDC authority
129131
self._is_known_to_developer = self.is_adfs or self._is_b2c or not validate_authority
130132
is_known_to_microsoft = self.instance in WELL_KNOWN_AUTHORITY_HOSTS
131133
instance_discovery_endpoint = 'https://{}/common/discovery/instance'.format( # Note: This URL seemingly returns V1 endpoint only

tests/test_optional_thumbprint.py

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ def test_pem_with_certificate_only_uses_sha256(
9797
self, mock_extract, mock_load_cert, mock_jwt_creator_class, mock_authority_class):
9898
"""Test that providing only public_certificate (no thumbprint) uses SHA-256"""
9999
authority = "https://login.microsoftonline.com/common"
100-
self._setup_mocks(mock_authority_class, authority)
100+
mock_authority = self._setup_mocks(mock_authority_class, authority)
101+
mock_authority._is_oidc = False # AAD is not OIDC generic
101102
self._setup_certificate_mocks(mock_extract, mock_load_cert)
102103

103104
# Create app with certificate credential WITHOUT thumbprint
@@ -243,7 +244,8 @@ def test_pem_with_both_thumbprints_aad_uses_sha256(
243244
self, mock_jwt_creator_class, mock_authority_class):
244245
"""Test that with both thumbprints, AAD authority uses SHA-256"""
245246
authority = "https://login.microsoftonline.com/common"
246-
self._setup_mocks(mock_authority_class, authority)
247+
mock_authority = self._setup_mocks(mock_authority_class, authority)
248+
mock_authority._is_oidc = False # AAD is not OIDC generic
247249

248250
# Create app with BOTH thumbprints for AAD
249251
app = ConfidentialClientApplication(
@@ -295,6 +297,7 @@ def test_pem_with_both_thumbprints_b2c_uses_sha256(
295297
authority = "https://contoso.b2clogin.com/contoso.onmicrosoft.com/B2C_1_susi"
296298
mock_authority = self._setup_mocks(mock_authority_class, authority)
297299
mock_authority._is_b2c = True # Manually set _is_b2c to True for this B2C authority
300+
mock_authority._is_oidc = False # B2C is not OIDC generic
298301

299302
# Create app with BOTH thumbprints for B2C
300303
app = ConfidentialClientApplication(
@@ -320,6 +323,8 @@ def test_pem_with_both_thumbprints_ciam_uses_sha256(
320323
"""Test that with both thumbprints, CIAM authority uses SHA-256"""
321324
authority = "https://contoso.ciamlogin.com/contoso.onmicrosoft.com"
322325
mock_authority = self._setup_mocks(mock_authority_class, authority)
326+
mock_authority._is_b2c = True # CIAM sets _is_b2c to True
327+
mock_authority._is_oidc = False # CIAM is not OIDC generic
323328

324329
# Create app with BOTH thumbprints for CIAM
325330
app = ConfidentialClientApplication(
@@ -342,15 +347,16 @@ def test_pem_with_both_thumbprints_ciam_uses_sha256(
342347

343348
def test_pem_with_both_thumbprints_generic_uses_sha1(
344349
self, mock_jwt_creator_class, mock_authority_class):
345-
"""Test that with both thumbprints, generic authority uses SHA-1"""
346-
authority = "https://custom.authority.com/tenant"
350+
"""Test that with both thumbprints, OIDC generic authority uses SHA-1"""
351+
authority = "https://custom.oidc.authority.com/tenant"
347352
mock_authority = self._setup_mocks(mock_authority_class, authority)
348353

349-
# Set up as a generic authority (not ADFS, not B2C, not in known hosts)
354+
# Set up as an OIDC generic authority
350355
mock_authority.is_adfs = False
351-
mock_authority._is_b2c = False
356+
mock_authority._is_b2c = True # OIDC sets this but it's not truly B2C
357+
mock_authority._is_oidc = True # This distinguishes OIDC from B2C/CIAM
352358

353-
# Create app with BOTH thumbprints for generic authority
359+
# Create app with BOTH thumbprints for OIDC generic authority
354360
app = ConfidentialClientApplication(
355361
client_id="my_client_id",
356362
client_credential={
@@ -361,14 +367,44 @@ def test_pem_with_both_thumbprints_generic_uses_sha1(
361367
authority=authority
362368
)
363369

364-
# For generic authorities, should use SHA-1 when both are provided
370+
# For OIDC generic authorities, should use SHA-1 when both are provided
365371
self._verify_assertion_params(
366372
mock_jwt_creator_class,
367373
expected_algorithm='RS256',
368374
expected_thumbprint_type='sha1',
369375
expected_thumbprint_value=self.test_sha1_thumbprint
370376
)
371377

378+
def test_pem_with_both_thumbprints_unknown_aad_uses_sha256(
379+
self, mock_jwt_creator_class, mock_authority_class):
380+
"""Test that with both thumbprints, unknown AAD authority (e.g., sovereign cloud) uses SHA-256"""
381+
authority = "https://login.microsoftonline.de/tenant" # Example of sovereign cloud not in known list
382+
mock_authority = self._setup_mocks(mock_authority_class, authority)
383+
384+
# Set up as an AAD authority (not ADFS, not B2C, not OIDC)
385+
mock_authority.is_adfs = False
386+
mock_authority._is_b2c = False
387+
mock_authority._is_oidc = False
388+
389+
# Create app with BOTH thumbprints for unknown AAD authority
390+
app = ConfidentialClientApplication(
391+
client_id="my_client_id",
392+
client_credential={
393+
"private_key": self.test_private_key,
394+
"thumbprint": self.test_sha1_thumbprint,
395+
"thumbprint_sha256": self.test_sha256_thumbprint,
396+
},
397+
authority=authority
398+
)
399+
400+
# For AAD authorities (even unknown ones), should use SHA-256 when both are provided
401+
self._verify_assertion_params(
402+
mock_jwt_creator_class,
403+
expected_algorithm='PS256',
404+
expected_thumbprint_type='sha256',
405+
expected_thumbprint_value=self.test_sha256_thumbprint
406+
)
407+
372408

373409
if __name__ == "__main__":
374410
unittest.main()

0 commit comments

Comments
 (0)