Skip to content

Commit ba84c3c

Browse files
CopilotbgavrilMS
Copilot
and
bgavrilMS
committed
Refactor: Use authority._is_b2c and extract test constants
Co-authored-by: bgavrilMS <12273384+bgavrilMS@users.noreply.github.com>
1 parent dd43694 commit ba84c3c

File tree

2 files changed

+24
-35
lines changed

2 files changed

+24
-35
lines changed

‎msal/application.py‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -858,12 +858,9 @@ def _build_client(self, client_credential, authority, skip_regional_client=False
858858
if sha256_thumbprint and sha1_thumbprint:
859859
# Both thumbprints provided - choose based on authority type
860860
# Use SHA256 for AAD (including B2C, CIAM), SHA1 for ADFS and generic
861-
from .authority import WELL_KNOWN_AUTHORITY_HOSTS, WELL_KNOWN_B2C_HOSTS, _CIAM_DOMAIN_SUFFIX
861+
from .authority import WELL_KNOWN_AUTHORITY_HOSTS
862862
is_known_aad = authority.instance in WELL_KNOWN_AUTHORITY_HOSTS
863-
is_b2c_or_ciam = (
864-
authority.instance.endswith(_CIAM_DOMAIN_SUFFIX) or
865-
any(authority.instance.endswith("." + d) for d in WELL_KNOWN_B2C_HOSTS)
866-
)
863+
is_b2c_or_ciam = getattr(authority, '_is_b2c', False)
867864
# Use SHA256 for known AAD, B2C, or CIAM; SHA1 for ADFS and generic
868865
use_sha256 = (is_known_aad or is_b2c_or_ciam) and not authority.is_adfs
869866
elif sha256_thumbprint:

‎tests/test_optional_thumbprint.py‎

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ class TestClientCredentialWithOptionalThumbprint(unittest.TestCase):
2121
BAMMC0V4YW1wbGUgQ0EwHhcNMjQwMTAxMDAwMDAwWhcNMjUwMTAxMDAwMDAwWjAW
2222
-----END CERTIFICATE-----"""
2323

24+
# Test thumbprint values
25+
test_sha1_thumbprint = "A1B2C3D4E5F6"
26+
test_sha256_thumbprint = "A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2"
27+
28+
2429
def _setup_mocks(self, mock_authority_class, authority="https://login.microsoftonline.com/common"):
2530
"""Helper to setup Authority mock"""
2631
# Setup Authority mock
@@ -119,12 +124,11 @@ def test_pem_with_manual_thumbprint_uses_sha1(
119124
self._setup_mocks(mock_authority_class, authority)
120125

121126
# Create app with manual thumbprint (legacy approach)
122-
manual_thumbprint = "A1B2C3D4E5F6"
123127
app = ConfidentialClientApplication(
124128
client_id="my_client_id",
125129
client_credential={
126130
"private_key": self.test_private_key,
127-
"thumbprint": manual_thumbprint,
131+
"thumbprint": self.test_sha1_thumbprint,
128132
# Note: NO public_certificate provided
129133
},
130134
authority=authority
@@ -135,7 +139,7 @@ def test_pem_with_manual_thumbprint_uses_sha1(
135139
mock_jwt_creator_class,
136140
expected_algorithm='RS256',
137141
expected_thumbprint_type='sha1',
138-
expected_thumbprint_value=manual_thumbprint
142+
expected_thumbprint_value=self.test_sha1_thumbprint
139143
)
140144

141145
def test_pem_with_both_uses_manual_thumbprint_as_sha1(
@@ -145,12 +149,11 @@ def test_pem_with_both_uses_manual_thumbprint_as_sha1(
145149
self._setup_mocks(mock_authority_class, authority)
146150

147151
# Create app with BOTH thumbprint and certificate
148-
manual_thumbprint = "A1B2C3D4E5F6"
149152
app = ConfidentialClientApplication(
150153
client_id="my_client_id",
151154
client_credential={
152155
"private_key": self.test_private_key,
153-
"thumbprint": manual_thumbprint,
156+
"thumbprint": self.test_sha1_thumbprint,
154157
"public_certificate": self.test_certificate,
155158
},
156159
authority=authority
@@ -161,7 +164,7 @@ def test_pem_with_both_uses_manual_thumbprint_as_sha1(
161164
mock_jwt_creator_class,
162165
expected_algorithm='RS256',
163166
expected_thumbprint_type='sha1',
164-
expected_thumbprint_value=manual_thumbprint,
167+
expected_thumbprint_value=self.test_sha1_thumbprint,
165168
has_x5c=True # x5c should still be present
166169
)
167170

@@ -217,12 +220,11 @@ def test_pem_with_thumbprint_sha256_only_uses_sha256(
217220
self._setup_mocks(mock_authority_class, authority)
218221

219222
# Create app with only SHA256 thumbprint
220-
sha256_thumbprint = "A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2"
221223
app = ConfidentialClientApplication(
222224
client_id="my_client_id",
223225
client_credential={
224226
"private_key": self.test_private_key,
225-
"thumbprint_sha256": sha256_thumbprint,
227+
"thumbprint_sha256": self.test_sha256_thumbprint,
226228
},
227229
authority=authority
228230
)
@@ -241,14 +243,12 @@ def test_pem_with_both_thumbprints_aad_uses_sha256(
241243
self._setup_mocks(mock_authority_class, authority)
242244

243245
# Create app with BOTH thumbprints for AAD
244-
sha1_thumbprint = "A1B2C3D4E5F6"
245-
sha256_thumbprint = "A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2"
246246
app = ConfidentialClientApplication(
247247
client_id="my_client_id",
248248
client_credential={
249249
"private_key": self.test_private_key,
250-
"thumbprint": sha1_thumbprint,
251-
"thumbprint_sha256": sha256_thumbprint,
250+
"thumbprint": self.test_sha1_thumbprint,
251+
"thumbprint_sha256": self.test_sha256_thumbprint,
252252
},
253253
authority=authority
254254
)
@@ -267,14 +267,12 @@ def test_pem_with_both_thumbprints_adfs_uses_sha1(
267267
self._setup_mocks(mock_authority_class, authority)
268268

269269
# Create app with BOTH thumbprints for ADFS
270-
sha1_thumbprint = "A1B2C3D4E5F6"
271-
sha256_thumbprint = "A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2"
272270
app = ConfidentialClientApplication(
273271
client_id="my_client_id",
274272
client_credential={
275273
"private_key": self.test_private_key,
276-
"thumbprint": sha1_thumbprint,
277-
"thumbprint_sha256": sha256_thumbprint,
274+
"thumbprint": self.test_sha1_thumbprint,
275+
"thumbprint_sha256": self.test_sha256_thumbprint,
278276
},
279277
authority=authority
280278
)
@@ -284,7 +282,7 @@ def test_pem_with_both_thumbprints_adfs_uses_sha1(
284282
mock_jwt_creator_class,
285283
expected_algorithm='RS256',
286284
expected_thumbprint_type='sha1',
287-
expected_thumbprint_value=sha1_thumbprint
285+
expected_thumbprint_value=self.test_sha1_thumbprint
288286
)
289287

290288
def test_pem_with_both_thumbprints_b2c_uses_sha256(
@@ -297,14 +295,12 @@ def test_pem_with_both_thumbprints_b2c_uses_sha256(
297295
mock_authority._is_b2c = True
298296

299297
# Create app with BOTH thumbprints for B2C
300-
sha1_thumbprint = "A1B2C3D4E5F6"
301-
sha256_thumbprint = "A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2"
302298
app = ConfidentialClientApplication(
303299
client_id="my_client_id",
304300
client_credential={
305301
"private_key": self.test_private_key,
306-
"thumbprint": sha1_thumbprint,
307-
"thumbprint_sha256": sha256_thumbprint,
302+
"thumbprint": self.test_sha1_thumbprint,
303+
"thumbprint_sha256": self.test_sha256_thumbprint,
308304
},
309305
authority=authority
310306
)
@@ -323,14 +319,12 @@ def test_pem_with_both_thumbprints_ciam_uses_sha256(
323319
mock_authority = self._setup_mocks(mock_authority_class, authority)
324320

325321
# Create app with BOTH thumbprints for CIAM
326-
sha1_thumbprint = "A1B2C3D4E5F6"
327-
sha256_thumbprint = "A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2"
328322
app = ConfidentialClientApplication(
329323
client_id="my_client_id",
330324
client_credential={
331325
"private_key": self.test_private_key,
332-
"thumbprint": sha1_thumbprint,
333-
"thumbprint_sha256": sha256_thumbprint,
326+
"thumbprint": self.test_sha1_thumbprint,
327+
"thumbprint_sha256": self.test_sha256_thumbprint,
334328
},
335329
authority=authority
336330
)
@@ -353,14 +347,12 @@ def test_pem_with_both_thumbprints_generic_uses_sha1(
353347
mock_authority._is_b2c = False
354348

355349
# Create app with BOTH thumbprints for generic authority
356-
sha1_thumbprint = "A1B2C3D4E5F6"
357-
sha256_thumbprint = "A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4E5F6A1B2"
358350
app = ConfidentialClientApplication(
359351
client_id="my_client_id",
360352
client_credential={
361353
"private_key": self.test_private_key,
362-
"thumbprint": sha1_thumbprint,
363-
"thumbprint_sha256": sha256_thumbprint,
354+
"thumbprint": self.test_sha1_thumbprint,
355+
"thumbprint_sha256": self.test_sha256_thumbprint,
364356
},
365357
authority=authority
366358
)
@@ -370,7 +362,7 @@ def test_pem_with_both_thumbprints_generic_uses_sha1(
370362
mock_jwt_creator_class,
371363
expected_algorithm='RS256',
372364
expected_thumbprint_type='sha1',
373-
expected_thumbprint_value=sha1_thumbprint
365+
expected_thumbprint_value=self.test_sha1_thumbprint
374366
)
375367

376368

0 commit comments

Comments
 (0)