Skip to content

Commit f0dec59

Browse files
rayluorayluo
authored
Merge pull request #835 from vi7us/vitcurda/20250624/cert-thumbprint-made-optional
Thumbprint for certificate made optional
2 parents 14bcad3 + 2c50ec4 commit f0dec59

File tree

3 files changed

+264
-20
lines changed

3 files changed

+264
-20
lines changed

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,3 +62,6 @@ msal_cache.bin
6262

6363
.env
6464
.perf.baseline
65+
66+
*.pfx
67+
.vscode/settings.json

‎msal/application.py‎

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -66,24 +66,32 @@ def _str2bytes(raw):
6666
except:
6767
return raw
6868

69+
def _extract_cert_and_thumbprints(cert):
70+
# Cert concepts https://security.stackexchange.com/a/226758/125264
71+
from cryptography.hazmat.primitives import hashes, serialization
72+
cert_pem = cert.public_bytes( # Requires cryptography 1.0+
73+
encoding=serialization.Encoding.PEM).decode()
74+
x5c = [
75+
'\n'.join(
76+
cert_pem.splitlines()
77+
[1:-1] # Strip the "--- header ---" and "--- footer ---"
78+
)
79+
]
80+
# https://cryptography.io/en/latest/x509/reference/#x-509-certificate-object
81+
sha256_thumbprint = cert.fingerprint(hashes.SHA256()).hex() # Requires cryptography 0.7+
82+
sha1_thumbprint = cert.fingerprint(hashes.SHA1()).hex() # Requires cryptography 0.7+
83+
return sha256_thumbprint, sha1_thumbprint, x5c
6984

7085
def _parse_pfx(pfx_path, passphrase_bytes):
7186
# Cert concepts https://security.stackexchange.com/a/226758/125264
72-
from cryptography.hazmat.primitives import hashes, serialization
7387
from cryptography.hazmat.primitives.serialization import pkcs12
7488
with open(pfx_path, 'rb') as f:
7589
private_key, cert, _ = pkcs12.load_key_and_certificates( # cryptography 2.5+
7690
# https://cryptography.io/en/latest/hazmat/primitives/asymmetric/serialization/#cryptography.hazmat.primitives.serialization.pkcs12.load_key_and_certificates
7791
f.read(), passphrase_bytes)
7892
if not (private_key and cert):
7993
raise ValueError("Your PFX file shall contain both private key and cert")
80-
cert_pem = cert.public_bytes(encoding=serialization.Encoding.PEM).decode() # cryptography 1.0+
81-
x5c = [
82-
'\n'.join(cert_pem.splitlines()[1:-1]) # Strip the "--- header ---" and "--- footer ---"
83-
]
84-
sha256_thumbprint = cert.fingerprint(hashes.SHA256()).hex() # cryptography 0.7+
85-
sha1_thumbprint = cert.fingerprint(hashes.SHA1()).hex() # cryptography 0.7+
86-
# https://cryptography.io/en/latest/x509/reference/#x-509-certificate-object
94+
sha256_thumbprint, sha1_thumbprint, x5c = _extract_cert_and_thumbprints(cert)
8795
return private_key, sha256_thumbprint, sha1_thumbprint, x5c
8896

8997

@@ -288,7 +296,10 @@ def __init__(
288296
289297
{
290298
"private_key": "...-----BEGIN PRIVATE KEY-----... in PEM format",
291-
"thumbprint": "An SHA-1 thumbprint such as A1B2C3D4E5F6...",
299+
"thumbprint": "An SHA-1 thumbprint such as A1B2C3D4E5F6..."
300+
"Changed in version 1.35.0, if thumbprint is absent"
301+
"and a public_certificate is present, MSAL will"
302+
"automatically calculate an SHA-256 thumbprint instead.",
292303
"passphrase": "Needed if the private_key is encrypted (Added in version 1.6.0)",
293304
"public_certificate": "...-----BEGIN CERTIFICATE-----...", # Needed if you use Subject Name/Issuer auth. Added in version 0.5.0.
294305
}
@@ -803,15 +814,30 @@ def _build_client(self, client_credential, authority, skip_regional_client=False
803814
passphrase_bytes)
804815
if client_credential.get("public_certificate") is True and x5c:
805816
headers["x5c"] = x5c
806-
elif (
807-
client_credential.get("private_key") # PEM blob
808-
and client_credential.get("thumbprint")):
809-
sha1_thumbprint = client_credential["thumbprint"]
810-
if passphrase_bytes:
811-
private_key = _load_private_key_from_pem_str(
817+
elif client_credential.get("private_key"): # PEM blob
818+
private_key = ( # handles both encrypted and unencrypted
819+
_load_private_key_from_pem_str(
812820
client_credential['private_key'], passphrase_bytes)
813-
else: # PEM without passphrase
814-
private_key = client_credential['private_key']
821+
if passphrase_bytes
822+
else client_credential['private_key']
823+
)
824+
825+
# Determine thumbprints based on what's provided
826+
if client_credential.get("thumbprint"):
827+
# User provided a thumbprint - use it as SHA-1 (legacy/manual approach)
828+
sha1_thumbprint = client_credential["thumbprint"]
829+
sha256_thumbprint = None
830+
elif isinstance(client_credential.get('public_certificate'), str):
831+
# No thumbprint provided, but we have a certificate to calculate thumbprints
832+
from cryptography import x509
833+
cert = x509.load_pem_x509_certificate(
834+
_str2bytes(client_credential['public_certificate']))
835+
sha256_thumbprint, sha1_thumbprint, headers["x5c"] = (
836+
_extract_cert_and_thumbprints(cert))
837+
else:
838+
raise ValueError(
839+
"You must provide either 'thumbprint' or 'public_certificate' "
840+
"from which the thumbprint can be calculated.")
815841
else:
816842
raise ValueError(
817843
"client_credential needs to follow this format "
@@ -1828,9 +1854,9 @@ def acquire_token_by_username_password(
18281854
18291855
- A successful response would contain "access_token" key,
18301856
- an error response would contain "error" and usually "error_description".
1831-
1832-
[Deprecated] This API is deprecated for public client flows and will be
1833-
removed in a future release. Use a more secure flow instead.
1857+
1858+
[Deprecated] This API is deprecated for public client flows and will be
1859+
removed in a future release. Use a more secure flow instead.
18341860
Migration guide: https://aka.ms/msal-ropc-migration
18351861
18361862
"""

‎tests/test_optional_thumbprint.py‎

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
import unittest
2+
from unittest.mock import Mock, patch
3+
from msal.application import ConfidentialClientApplication
4+
5+
6+
@patch('msal.application.Authority')
7+
@patch('msal.application.JwtAssertionCreator', new_callable=lambda: Mock(
8+
return_value=Mock(create_regenerative_assertion=Mock(return_value="mock_jwt_assertion"))))
9+
class TestClientCredentialWithOptionalThumbprint(unittest.TestCase):
10+
"""Test that thumbprint is optional when public_certificate is provided"""
11+
12+
# Sample test certificate and private key (PEM format)
13+
# These are minimal valid PEM structures for testing
14+
test_private_key = """-----BEGIN PRIVATE KEY-----
15+
MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC7VJTUt9Us8cKj
16+
MzEfYyjiWA4R4/M2bS1+fWIcPm15j7uo6xKvRr4PNx5bKMDFqMdW6/xfqFWX0nZK
17+
-----END PRIVATE KEY-----"""
18+
19+
test_certificate = """-----BEGIN CERTIFICATE-----
20+
MIIC5jCCAc6gAwIBAgIJALdYQVsVsNZHMA0GCSqGSIb3DQEBCwUAMBYxFDASBgNV
21+
BAMMC0V4YW1wbGUgQ0EwHhcNMjQwMTAxMDAwMDAwWhcNMjUwMTAxMDAwMDAwWjAW
22+
-----END CERTIFICATE-----"""
23+
24+
def _setup_mocks(self, mock_authority_class, authority="https://login.microsoftonline.com/common"):
25+
"""Helper to setup Authority mock"""
26+
# Setup Authority mock
27+
mock_authority = Mock()
28+
mock_authority.is_adfs = "adfs" in authority.lower()
29+
30+
# Extract instance from authority URL
31+
if mock_authority.is_adfs:
32+
# For ADFS: https://adfs.contoso.com/adfs -> adfs.contoso.com
33+
mock_authority.instance = authority.split("//")[1].split("/")[0]
34+
mock_authority.token_endpoint = f"https://{mock_authority.instance}/adfs/oauth2/token"
35+
mock_authority.authorization_endpoint = f"https://{mock_authority.instance}/adfs/oauth2/authorize"
36+
else:
37+
# For AAD: https://login.microsoftonline.com/common -> login.microsoftonline.com
38+
mock_authority.instance = authority.split("//")[1].split("/")[0]
39+
mock_authority.token_endpoint = f"https://{mock_authority.instance}/common/oauth2/v2.0/token"
40+
mock_authority.authorization_endpoint = f"https://{mock_authority.instance}/common/oauth2/v2.0/authorize"
41+
42+
mock_authority.device_authorization_endpoint = None
43+
mock_authority_class.return_value = mock_authority
44+
45+
return mock_authority
46+
47+
def _setup_certificate_mocks(self, mock_extract, mock_load_cert):
48+
"""Helper to setup certificate parsing mocks"""
49+
# Mock certificate loading
50+
mock_cert = Mock()
51+
mock_load_cert.return_value = mock_cert
52+
53+
# Mock _extract_cert_and_thumbprints to return thumbprints
54+
mock_extract.return_value = (
55+
"mock_sha256_thumbprint", # sha256_thumbprint
56+
"mock_sha1_thumbprint", # sha1_thumbprint
57+
["mock_x5c_value"] # x5c
58+
)
59+
60+
def _verify_assertion_params(self, mock_jwt_creator_class, expected_algorithm,
61+
expected_thumbprint_type, expected_thumbprint_value=None,
62+
has_x5c=False):
63+
"""Helper to verify JwtAssertionCreator was called with correct params"""
64+
mock_jwt_creator_class.assert_called_once()
65+
call_args = mock_jwt_creator_class.call_args
66+
67+
# Verify algorithm
68+
self.assertEqual(call_args[1]['algorithm'], expected_algorithm)
69+
70+
# Verify thumbprint type
71+
if expected_thumbprint_type == 'sha256':
72+
self.assertIn('sha256_thumbprint', call_args[1])
73+
self.assertNotIn('sha1_thumbprint', call_args[1])
74+
elif expected_thumbprint_type == 'sha1':
75+
self.assertIn('sha1_thumbprint', call_args[1])
76+
self.assertNotIn('sha256_thumbprint', call_args[1])
77+
if expected_thumbprint_value:
78+
self.assertEqual(call_args[1]['sha1_thumbprint'], expected_thumbprint_value)
79+
80+
# Verify x5c header if expected
81+
if has_x5c:
82+
self.assertIn('headers', call_args[1])
83+
self.assertIn('x5c', call_args[1]['headers'])
84+
85+
return call_args
86+
87+
@patch('cryptography.x509.load_pem_x509_certificate')
88+
@patch('msal.application._extract_cert_and_thumbprints')
89+
def test_pem_with_certificate_only_uses_sha256(
90+
self, mock_extract, mock_load_cert, mock_jwt_creator_class, mock_authority_class):
91+
"""Test that providing only public_certificate (no thumbprint) uses SHA-256"""
92+
authority = "https://login.microsoftonline.com/common"
93+
self._setup_mocks(mock_authority_class, authority)
94+
self._setup_certificate_mocks(mock_extract, mock_load_cert)
95+
96+
# Create app with certificate credential WITHOUT thumbprint
97+
app = ConfidentialClientApplication(
98+
client_id="my_client_id",
99+
client_credential={
100+
"private_key": self.test_private_key,
101+
"public_certificate": self.test_certificate,
102+
# Note: NO thumbprint provided
103+
},
104+
authority=authority
105+
)
106+
107+
# Verify SHA-256 with PS256 algorithm is used
108+
self._verify_assertion_params(
109+
mock_jwt_creator_class,
110+
expected_algorithm='PS256',
111+
expected_thumbprint_type='sha256',
112+
has_x5c=True
113+
)
114+
115+
def test_pem_with_manual_thumbprint_uses_sha1(
116+
self, mock_jwt_creator_class, mock_authority_class):
117+
"""Test that providing manual thumbprint (no certificate) uses SHA-1"""
118+
authority = "https://login.microsoftonline.com/common"
119+
self._setup_mocks(mock_authority_class, authority)
120+
121+
# Create app with manual thumbprint (legacy approach)
122+
manual_thumbprint = "A1B2C3D4E5F6"
123+
app = ConfidentialClientApplication(
124+
client_id="my_client_id",
125+
client_credential={
126+
"private_key": self.test_private_key,
127+
"thumbprint": manual_thumbprint,
128+
# Note: NO public_certificate provided
129+
},
130+
authority=authority
131+
)
132+
133+
# Verify SHA-1 with RS256 algorithm is used
134+
self._verify_assertion_params(
135+
mock_jwt_creator_class,
136+
expected_algorithm='RS256',
137+
expected_thumbprint_type='sha1',
138+
expected_thumbprint_value=manual_thumbprint
139+
)
140+
141+
def test_pem_with_both_uses_manual_thumbprint_as_sha1(
142+
self, mock_jwt_creator_class, mock_authority_class):
143+
"""Test that providing both thumbprint and certificate prefers manual thumbprint (SHA-1)"""
144+
authority = "https://login.microsoftonline.com/common"
145+
self._setup_mocks(mock_authority_class, authority)
146+
147+
# Create app with BOTH thumbprint and certificate
148+
manual_thumbprint = "A1B2C3D4E5F6"
149+
app = ConfidentialClientApplication(
150+
client_id="my_client_id",
151+
client_credential={
152+
"private_key": self.test_private_key,
153+
"thumbprint": manual_thumbprint,
154+
"public_certificate": self.test_certificate,
155+
},
156+
authority=authority
157+
)
158+
159+
# Verify manual thumbprint takes precedence (backward compatibility)
160+
self._verify_assertion_params(
161+
mock_jwt_creator_class,
162+
expected_algorithm='RS256',
163+
expected_thumbprint_type='sha1',
164+
expected_thumbprint_value=manual_thumbprint,
165+
has_x5c=True # x5c should still be present
166+
)
167+
168+
@patch('cryptography.x509.load_pem_x509_certificate')
169+
@patch('msal.application._extract_cert_and_thumbprints')
170+
def test_pem_with_adfs_uses_sha1(
171+
self, mock_extract, mock_load_cert, mock_jwt_creator_class, mock_authority_class):
172+
"""Test that ADFS authority uses SHA-1 even with SHA-256 thumbprint"""
173+
authority = "https://adfs.contoso.com/adfs"
174+
self._setup_mocks(mock_authority_class, authority)
175+
self._setup_certificate_mocks(mock_extract, mock_load_cert)
176+
177+
# Create app with certificate on ADFS
178+
app = ConfidentialClientApplication(
179+
client_id="my_client_id",
180+
client_credential={
181+
"private_key": self.test_private_key,
182+
"public_certificate": self.test_certificate,
183+
},
184+
authority=authority
185+
)
186+
187+
# ADFS should force SHA-1 with RS256 even though SHA-256 would be calculated
188+
self._verify_assertion_params(
189+
mock_jwt_creator_class,
190+
expected_algorithm='RS256',
191+
expected_thumbprint_type='sha1'
192+
)
193+
194+
def test_pem_with_neither_raises_error(self, mock_jwt_creator_class, mock_authority_class):
195+
"""Test that providing neither thumbprint nor certificate raises ValueError"""
196+
authority = "https://login.microsoftonline.com/common"
197+
self._setup_mocks(mock_authority_class, authority)
198+
199+
# Should raise ValueError when neither thumbprint nor certificate provided
200+
with self.assertRaises(ValueError) as context:
201+
app = ConfidentialClientApplication(
202+
client_id="my_client_id",
203+
client_credential={
204+
"private_key": self.test_private_key,
205+
# Note: NO thumbprint and NO public_certificate
206+
},
207+
authority=authority
208+
)
209+
210+
self.assertIn("thumbprint", str(context.exception).lower())
211+
self.assertIn("public_certificate", str(context.exception).lower())
212+
213+
214+
if __name__ == "__main__":
215+
unittest.main()

0 commit comments

Comments
 (0)