Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Profile] az account get-access-token: Return expires_on as POSIX timestamp #27476

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

jiasli
Copy link
Member

@jiasli jiasli commented Sep 27, 2023

Close #19700

Related command
az account get-access-token

Description
Returning expiresOn as a local datetime is a legacy from the ADAL age. It causes lots of unnecessary complexities. See #19700 for detailed explanation.

This PR makes az account get-access-token return expires_on as POSIX timestamp which has been supported long ago by Azure CLI Core during the MSAL migration (#19853).

However, as shown in #19700 (comment), downstream applications that only support 32-bit integer precision timestamp will encounter error in year 2038, but that's not something we can control.

Testing Guide

az account get-access-token

Before:

{
  "accessToken": "...",
  "expiresOn": "2023-10-31 21:59:10.000000",
  "subscription": "...",
  "tenant": "...",
  "tokenType": "Bearer"
}

After:

{
  "accessToken": "...",
  "expiresOn": "2023-10-31 21:59:10.000000",
  "expires_on": 1698760750,
  "subscription": "...",
  "tenant": "...",
  "tokenType": "Bearer"
}

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Sep 27, 2023

🔄AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.11
️✔️3.9
️✔️ams
️✔️latest
️✔️3.11
️✔️3.9
️✔️apim
️✔️latest
️✔️3.11
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.11
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.11
️✔️3.9
️✔️aro
️✔️latest
️✔️3.11
️✔️3.9
️✔️backup
️✔️latest
️✔️3.11
️✔️3.9
️✔️batch
️✔️latest
️✔️3.11
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.11
️✔️3.9
️✔️billing
️✔️latest
️✔️3.11
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.11
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.11
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.11
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.11
️✔️3.9
️✔️config
️✔️latest
️✔️3.11
️✔️3.9
️✔️configure
️✔️latest
️✔️3.11
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.11
️✔️3.9
️✔️container
️✔️latest
️✔️3.11
️✔️3.9
🔄containerapp
🔄latest
🔄3.11
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.11
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️dla
️✔️latest
️✔️3.11
️✔️3.9
️✔️dls
️✔️latest
️✔️3.11
️✔️3.9
️✔️dms
️✔️latest
️✔️3.11
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.11
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.11
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.11
️✔️3.9
️✔️find
️✔️latest
️✔️3.11
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.11
️✔️3.9
️✔️identity
️✔️latest
️✔️3.11
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️kusto
️✔️latest
️✔️3.11
️✔️3.9
️✔️lab
️✔️latest
️✔️3.11
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.11
️✔️3.9
️✔️maps
️✔️latest
️✔️3.11
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.11
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.11
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.11
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.11
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.11
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.11
️✔️3.9
️✔️profile
️✔️latest
️✔️3.11
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.11
️✔️3.9
️✔️redis
️✔️latest
️✔️3.11
️✔️3.9
️✔️relay
️✔️latest
️✔️3.11
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️role
️✔️latest
️✔️3.11
️✔️3.9
️✔️search
️✔️latest
️✔️3.11
️✔️3.9
️✔️security
️✔️latest
️✔️3.11
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.11
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.11
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.11
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.11
️✔️3.9
️✔️sql
️✔️latest
️✔️3.11
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.11
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.11
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️util
️✔️latest
️✔️3.11
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Sep 27, 2023

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@yonzhan
Copy link
Collaborator

yonzhan commented Sep 27, 2023

improve expires_on

@microsoft-github-policy-service microsoft-github-policy-service bot added the Auto-Assign Auto assign by bot label Sep 27, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the ARM az resource/group/lock/tag/deployment/policy/managementapp/account management-group label Sep 27, 2023
@jiasli jiasli assigned jiasli and unassigned zhoxing-ms Sep 27, 2023
@jiasli jiasli added Account az login/account and removed ARM az resource/group/lock/tag/deployment/policy/managementapp/account management-group labels Sep 27, 2023
yonzhan
yonzhan previously approved these changes Sep 27, 2023
demoray pushed a commit to demoray/azure-sdk-for-rust that referenced this pull request Oct 3, 2023
As indicated in Azure#1371, there are numerous issues with handling the
az-cli's output of an access token's expiration date when it comes to
time zones.

An update to the Azure CLI is in progress that will add a _new_ field
that will handle the conversion, currently tracked as Azure/azure-cli#27476.

We should make use of this new feature once it's available, but we
should also keep this functionality as a fallback for supporting older
Azure CLIs.
demoray added a commit to Azure/azure-sdk-for-rust that referenced this pull request Oct 3, 2023
As indicated in #1371, there are numerous issues with handling the
az-cli's output of an access token's expiration date when it comes to
time zones.

An update to the Azure CLI is in progress that will add a _new_ field
that will handle the conversion, currently tracked as Azure/azure-cli#27476.

We should make use of this new feature once it's available, but we
should also keep this functionality as a fallback for supporting older
Azure CLIs.
Copy link
Contributor

@dbradish-microsoft dbradish-microsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested change: Add the PEP 495 link if it aligns with engineering URL best practices.

src/azure-cli/azure/cli/command_modules/profile/_help.py Outdated Show resolved Hide resolved
evelyn-ys
evelyn-ys previously approved these changes Oct 31, 2023
@jiasli jiasli dismissed stale reviews from evelyn-ys and yonzhan via 16a22b9 November 1, 2023 01:48
@jiasli
Copy link
Member Author

jiasli commented Dec 1, 2023

This PR has been released in Azure CLI 2.54.0.

Whether using expiresOn causes failure really depends on how the downstream program uses the access token and expiresOn returned by Azure CLI.

One thing is for sure: The access token returned by az account get-access-token is guaranteed to be valid at the time of invocation. Downstream program can use it without validating expiresOn. expiresOn is used by downstream program that has a caching mechanism.

expiresOn faces Daylight Saving Time problem (#19700 (comment)):

  1. expiresOn can only express the first hour when Daylight Saving Time ends, not the folded(second) hour. For example, 1699172100/2023-11-05 01:15:00-07:00/2023-11-05 08:15:00 UTC is expressed as 2023-11-05 01:15:00.
  2. When spacetime treads into the folded hour (1699175700/2023-11-05 01:15:00-08:00/2023-11-05 09:15:00 UTC), the expiresOn still represents a time 1 hour earlier (2023-11-05 01:15:00) due to the lack of time zone and fold information. Downstream program will think the token has expired and request a new one.
  3. However, the expiresOn of the new token returned by Azure CLI is still 1 hour earlier (2023-11-05 01:15:00), downstream program will think the new token has also expired. It may either fail or go into an infinite loop.

If you want better compatibility with old versions of Azure CLI, you may allow falling back to expiresOn, but the downstream program would suffer from the Daylight Saving Time problem. If you want robustness and code simplicity, only use expires_on and err out when it is missing.

Copy link

@Moazzem-Hossain-pixel Moazzem-Hossain-pixel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Account az login/account Auto-Assign Auto assign by bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

az account get-access-token: Use epoch expiresOn/expires_on
8 participants