Skip to content

Commit 2c50ec4

Browse files
committed
Cleaner implementation and precise docs on SHA256
1 parent bbeb494 commit 2c50ec4

File tree

3 files changed

+258
-32
lines changed

3 files changed

+258
-32
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: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -66,16 +66,21 @@ def _str2bytes(raw):
6666
except:
6767
return raw
6868

69-
def _extract_cert_and_thumbprints(private_key, cert):
69+
def _extract_cert_and_thumbprints(cert):
7070
# Cert concepts https://security.stackexchange.com/a/226758/125264
7171
from cryptography.hazmat.primitives import hashes, serialization
72-
cert_pem = cert.public_bytes(encoding=serialization.Encoding.PEM).decode()
72+
cert_pem = cert.public_bytes( # Requires cryptography 1.0+
73+
encoding=serialization.Encoding.PEM).decode()
7374
x5c = [
74-
'\n'.join(cert_pem.splitlines()[1:-1])
75+
'\n'.join(
76+
cert_pem.splitlines()
77+
[1:-1] # Strip the "--- header ---" and "--- footer ---"
78+
)
7579
]
76-
sha256_thumbprint = cert.fingerprint(hashes.SHA256()).hex()
77-
sha1_thumbprint = cert.fingerprint(hashes.SHA1()).hex()
78-
return private_key, sha256_thumbprint, sha1_thumbprint, x5c
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
7984

8085
def _parse_pfx(pfx_path, passphrase_bytes):
8186
# Cert concepts https://security.stackexchange.com/a/226758/125264
@@ -86,7 +91,8 @@ def _parse_pfx(pfx_path, passphrase_bytes):
8691
f.read(), passphrase_bytes)
8792
if not (private_key and cert):
8893
raise ValueError("Your PFX file shall contain both private key and cert")
89-
return _extract_cert_and_thumbprints(private_key, cert)
94+
sha256_thumbprint, sha1_thumbprint, x5c = _extract_cert_and_thumbprints(cert)
95+
return private_key, sha256_thumbprint, sha1_thumbprint, x5c
9096

9197

9298
def _load_private_key_from_pem_str(private_key_pem_str, passphrase_bytes):
@@ -290,7 +296,10 @@ def __init__(
290296
291297
{
292298
"private_key": "...-----BEGIN PRIVATE KEY-----... in PEM format",
293-
"thumbprint": "An SHA-1 thumbprint such as A1B2C3D4E5F6...", # Optional since version 1.35.0
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.",
294303
"passphrase": "Needed if the private_key is encrypted (Added in version 1.6.0)",
295304
"public_certificate": "...-----BEGIN CERTIFICATE-----...", # Needed if you use Subject Name/Issuer auth. Added in version 0.5.0.
296305
}
@@ -805,31 +814,30 @@ def _build_client(self, client_credential, authority, skip_regional_client=False
805814
passphrase_bytes)
806815
if client_credential.get("public_certificate") is True and x5c:
807816
headers["x5c"] = x5c
808-
elif (client_credential.get("private_key")
809-
and client_credential.get("public_certificate")
810-
and not client_credential.get("thumbprint")): #in case user does not pass thumbprint but only certificate and private key
811-
if passphrase_bytes: # PEM with passphrase
812-
private_key = _load_private_key_from_pem_str(
813-
client_credential['private_key'], passphrase_bytes)
814-
else: # PEM without passphrase
815-
private_key = client_credential['private_key']
816-
817-
private_key, sha256_thumbprint, sha1_thumbprint, x5c =(
818-
_extract_cert_and_thumbprints(
819-
private_key,
820-
client_credential['public_certificate']))
821-
if x5c:
822-
headers["x5c"] = x5c
823-
824-
elif (
825-
client_credential.get("private_key") # PEM blob
826-
and client_credential.get("thumbprint")):
827-
sha1_thumbprint = client_credential["thumbprint"]
828-
if passphrase_bytes:
829-
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(
830820
client_credential['private_key'], passphrase_bytes)
831-
else: # PEM without passphrase
832-
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.")
833841
else:
834842
raise ValueError(
835843
"client_credential needs to follow this format "

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)