Support ISO8601-ish formats for expires_on in Managed Identity#900
Support ISO8601-ish formats for expires_on in Managed Identity#900
expires_on in Managed Identity#900Conversation
Agent-Logs-Url: https://github.com/AzureAD/microsoft-authentication-library-for-python/sessions/988c4f0a-9dee-4865-9c61-1dccdfd9ea49 Co-authored-by: gladjohn <90415114+gladjohn@users.noreply.github.com>
expires_on in Managed Identity
There was a problem hiding this comment.
Pull request overview
This PR improves Managed Identity token handling by making expires_on parsing resilient to non-epoch formats returned by some MI endpoints (e.g., App Service, Azure ML), preventing failures when int(expires_on) would previously raise.
Changes:
- Added
_parse_expires_on(raw: str) -> intwith a cascading parsing strategy (epoch → ISO8601-ish → Azure SDK-style strptime formats). - Updated App Service and Azure ML MI flows to compute
expires_inusing_parse_expires_on(...). - Added unit tests covering the supported
expires_onformats.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
msal/managed_identity.py |
Adds _parse_expires_on() and switches App Service/Azure ML to use it when normalizing expires_in. |
tests/test_mi.py |
Adds coverage for multiple expires_on string formats and expected epoch conversions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import logging | ||
| import os | ||
| import re | ||
| import socket |
There was a problem hiding this comment.
socket is imported but not used anywhere in this module (only referenced in a comment). Please remove the unused import to avoid lint/maintenance noise.
| import socket |
| for format in ( | ||
| "%m/%d/%Y %H:%M:%S %z", # Support "06/20/2019 02:57:58 +00:00" | ||
| # Derived from https://github.com/Azure/azure-sdk-for-python/blob/azure-identity_1.21.0/sdk/identity/azure-identity/azure/identity/_credentials/azure_ml.py#L52 | ||
| "%m/%d/%Y %I:%M:%S %p %z", # Support "1/16/2020 12:0:12 AM +00:00" | ||
| # Derived from https://github.com/Azure/azure-sdk-for-python/blob/azure-identity_1.21.0/sdk/identity/azure-identity/azure/identity/_credentials/azure_ml.py#L51 | ||
| ): | ||
| try: | ||
| return calendar.timegm(time.strptime(raw, format)) |
There was a problem hiding this comment.
The loop variable name format shadows Python’s built-in format() function. Consider renaming it (e.g., fmt) to avoid shadowing and improve readability.
| for format in ( | |
| "%m/%d/%Y %H:%M:%S %z", # Support "06/20/2019 02:57:58 +00:00" | |
| # Derived from https://github.com/Azure/azure-sdk-for-python/blob/azure-identity_1.21.0/sdk/identity/azure-identity/azure/identity/_credentials/azure_ml.py#L52 | |
| "%m/%d/%Y %I:%M:%S %p %z", # Support "1/16/2020 12:0:12 AM +00:00" | |
| # Derived from https://github.com/Azure/azure-sdk-for-python/blob/azure-identity_1.21.0/sdk/identity/azure-identity/azure/identity/_credentials/azure_ml.py#L51 | |
| ): | |
| try: | |
| return calendar.timegm(time.strptime(raw, format)) | |
| for fmt in ( | |
| "%m/%d/%Y %H:%M:%S %z", # Support "06/20/2019 02:57:58 +00:00" | |
| # Derived from https://github.com/Azure/azure-sdk-for-python/blob/azure-identity_1.21.0/sdk/identity/azure-identity/azure/identity/_credentials/azure_ml.py#L52 | |
| "%m/%d/%Y %I:%M:%S %p %z", # Support "1/16/2020 12:0:12 AM +00:00" | |
| # Derived from https://github.com/Azure/azure-sdk-for-python/blob/azure-identity_1.21.0/sdk/identity/azure-identity/azure/identity/_credentials/azure_ml.py#L51 | |
| ): | |
| try: | |
| return calendar.timegm(time.strptime(raw, fmt)) |
| # Derived from https://github.com/Azure/azure-sdk-for-python/blob/azure-identity_1.21.0/sdk/identity/azure-identity/azure/identity/_credentials/azure_ml.py#L51 | ||
| ): | ||
| try: | ||
| return calendar.timegm(time.strptime(raw, format)) |
There was a problem hiding this comment.
calendar.timegm(time.strptime(...)) ignores the parsed %z offset (time tuples don’t apply the timezone offset when converted this way). If expires_on ever includes a non-+00:00 offset, this will compute the wrong epoch and thus wrong expires_in. Prefer parsing into a timezone-aware datetime (e.g., datetime.datetime.strptime(...).timestamp()) so offsets are honored.
| return calendar.timegm(time.strptime(raw, format)) | |
| return int(datetime.datetime.strptime(raw, format).timestamp()) |
| for input, epoch in { | ||
| "1234567890": 1234567890, | ||
| "1970-01-01T00:00:12.0000000+00:00": 12, |
There was a problem hiding this comment.
The variable name input shadows Python’s built-in input() function. Rename it (e.g., raw/expires_on) to avoid shadowing and make the test intent clearer.
| return calendar.timegm(time.strptime(raw, format)) | ||
| except ValueError: | ||
| pass | ||
| raise ManagedIdentityError(f"Cannot parse expires_on: {raw}") |
There was a problem hiding this comment.
raw is mutated during ISO preprocessing, and the final exception reports the possibly-normalized value rather than the original input. Preserve the original string (e.g., original_raw = raw) and include that in the ManagedIdentityError message to make troubleshooting easier.
Managed Identity endpoints (particularly App Service and Machine Learning) can return
expires_onin non-epoch formats (e.g.,2024-10-18T19:51:37.0000000+00:00as seen in the dotnet MSAL issue tracker), causingint()parsing to fail. This recreates PR #804 (original author left the team, CLA blocked merge).Changes
msal/managed_identity.py: Adds_parse_expires_on(raw: str) -> intwith a cascading parse strategy:datetime.fromisoformat(), with regex pre-processing to strip 7-digit microseconds on Python < 3.11 and normalizeZsuffix%m/%d/%Y %H:%M:%S %zand%m/%d/%Y %I:%M:%S %p %zManagedIdentityErrorif all attempts failReplaces bare
int(payload["expires_on"])calls in_obtain_token_on_app_service()and_obtain_token_on_machine_learning()with_parse_expires_on().tests/test_mi.py: AddsExpiresOnTestCasecovering all supported formats:Original prompt
This pull request was created from Copilot chat.