Skip to content

Commit 2a7c4f6

Browse files
Address PR review comments (#897)
- Fix ManagedIdentityError: use single descriptive message instead of two args - Fix _try_parse_cert_not_after: update return type to float (always returns fallback) - Fix _der_oid: allow second component >= 40 for OIDs starting with 2.x - Fix _cert_cache_key: include ManagedIdentityIdType to prevent cross-identity collisions - Fix setup.cfg: exclude tests from msal-key-attestation wheel - Remove egg-info build artifacts from git, add to .gitignore - Fix remaining CodeQL clear-text logging alerts in sample (use print instead of logger) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 00d923b commit 2a7c4f6

File tree

11 files changed

+29
-152
lines changed

11 files changed

+29
-152
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ __pycache__/
1515
# Result of running python setup.py install/pip install -e
1616
/build/
1717
/msal.egg-info/
18+
/msal-key-attestation/msal_key_attestation.egg-info/
1819

1920
# Test results
2021
/TestResults/

msal-key-attestation/msal_key_attestation.egg-info/PKG-INFO

Lines changed: 0 additions & 118 deletions
This file was deleted.

msal-key-attestation/msal_key_attestation.egg-info/SOURCES.txt

Lines changed: 0 additions & 12 deletions
This file was deleted.

msal-key-attestation/msal_key_attestation.egg-info/dependency_links.txt

Lines changed: 0 additions & 1 deletion
This file was deleted.

msal-key-attestation/msal_key_attestation.egg-info/requires.txt

Lines changed: 0 additions & 1 deletion
This file was deleted.

msal-key-attestation/msal_key_attestation.egg-info/top_level.txt

Lines changed: 0 additions & 2 deletions
This file was deleted.

msal-key-attestation/setup.cfg

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,8 @@ packages = find:
3030
python_requires = >=3.8
3131
install_requires =
3232
msal>=1.32.0
33+
34+
[options.packages.find]
35+
exclude =
36+
tests
37+
tests.*

msal/managed_identity.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -327,9 +327,8 @@ def acquire_token_for_client(
327327

328328
if with_attestation_support and not mtls_proof_of_possession:
329329
raise ManagedIdentityError(
330-
"attestation_requires_pop",
331-
"with_attestation_support=True requires "
332-
"mtls_proof_of_possession=True (mTLS PoP).")
330+
"attestation_requires_pop: with_attestation_support=True "
331+
"requires mtls_proof_of_possession=True (mTLS PoP).")
333332

334333
if use_msi_v2:
335334
# Auto-discover attestation provider from msal-key-attestation

msal/msi_v2.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,15 @@ def is_expired(self, now: Optional[float] = None) -> bool:
121121

122122
def _cert_cache_key(managed_identity: Optional[Dict[str, Any]],
123123
attested: bool) -> str:
124-
"""Build a cache key from managed identity + attestation flag."""
124+
"""Build a cache key from managed identity + identifier type + attestation flag."""
125+
mi_id_type = "SYSTEM_ASSIGNED"
125126
mi_id = "SYSTEM_ASSIGNED"
126127
if isinstance(managed_identity, dict):
128+
mi_id_type = str(
129+
managed_identity.get("ManagedIdentityIdType") or "SYSTEM_ASSIGNED")
127130
mi_id = str(managed_identity.get("Id") or "SYSTEM_ASSIGNED")
128131
tag = "#att=1" if attested else "#att=0"
129-
return mi_id + tag
132+
return mi_id_type + ":" + mi_id + tag
130133

131134

132135
def _cert_cache_get(key: str) -> Optional[_CertCacheEntry]:
@@ -231,16 +234,15 @@ def _der_to_pem(der_bytes: bytes) -> str:
231234
+ "\n-----END CERTIFICATE-----")
232235

233236

234-
def _try_parse_cert_not_after(der_bytes: bytes) -> Optional[float]:
237+
def _try_parse_cert_not_after(der_bytes: bytes) -> float:
235238
"""
236239
Best-effort extraction of notAfter from a DER X.509 certificate.
237-
Returns epoch seconds, or None on failure.
240+
Returns epoch seconds. Falls back to now + 8 hours on any failure.
238241
"""
239242
try:
240243
from cryptography import x509
241244
from cryptography.hazmat.backends import default_backend
242245
cert = x509.load_der_x509_certificate(der_bytes, default_backend())
243-
import datetime
244246
na = cert.not_valid_after_utc if hasattr(
245247
cert, "not_valid_after_utc") else cert.not_valid_after
246248
if na.tzinfo is None:
@@ -653,7 +655,7 @@ def _der_integer(value: int) -> bytes:
653655

654656
def _der_oid(oid: str) -> bytes:
655657
parts = [int(x) for x in oid.split(".")]
656-
if len(parts) < 2 or parts[0] > 2 or parts[1] >= 40:
658+
if len(parts) < 2 or parts[0] > 2 or (parts[0] < 2 and parts[1] >= 40):
657659
raise ValueError(f"Invalid OID: {oid}")
658660
first = 40 * parts[0] + parts[1]
659661
out = bytearray([first])

sample/msi_v2_sample.py

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -79,22 +79,20 @@ def main():
7979
)
8080

8181
if "access_token" not in result:
82-
# Only log error code/description — never log tokens or secrets
83-
logger.error("Token acquisition failed: %s - %s",
84-
result.get("error", "unknown"),
85-
result.get("error_description", "no description"))
82+
print("ERROR: Token acquisition failed:",
83+
result.get("error", "unknown"), "-",
84+
result.get("error_description", "no description"))
8685
sys.exit(1)
8786

8887
token_type = result.get("token_type", "unknown")
8988
expires_in = result.get("expires_in", 0)
9089

91-
logger.info("Token acquired successfully!")
92-
logger.info(" token_type: %s", token_type)
93-
logger.info(" expires_in: %s seconds", expires_in)
90+
print("Token acquired successfully!")
91+
print(f" token_type: {token_type}")
92+
print(f" expires_in: {expires_in} seconds")
9493

9594
if token_type != "mtls_pop":
96-
logger.warning(
97-
"Expected token_type='mtls_pop' but got '%s'.", token_type)
95+
print(f"WARNING: Expected token_type='mtls_pop' but got '{token_type}'.")
9896

9997
# --- Verify binding ---
10098
from msal.msi_v2 import verify_cnf_binding

0 commit comments

Comments
 (0)