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

[Identity] Evaluate expires_on field in AzureCliCredential #32952

Closed
scottaddie opened this issue Nov 3, 2023 · 4 comments · Fixed by #33947
Closed

[Identity] Evaluate expires_on field in AzureCliCredential #32952

scottaddie opened this issue Nov 3, 2023 · 4 comments · Fixed by #33947
Assignees
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library.
Milestone

Comments

@scottaddie
Copy link
Member

As of Azure CLI v2.54.0, the az account get-access-token command returns a new expires_on field containing a POSIX timestamp. This behavior is a departure from previous versions, in which the local datetime was returned in an expiresOn field. Update AzureCliCredential to also consider the new expires_on field.

Related links:

@scottaddie scottaddie added Client This issue points to a problem in the data-plane of the library. Azure.Identity labels Nov 3, 2023
@scottaddie scottaddie assigned pvaneck and xiangyan99 and unassigned pvaneck Nov 3, 2023
@scottaddie scottaddie added this to the 2024-02 milestone Nov 3, 2023
@scottaddie scottaddie moved this from Untriaged to Blocked in Azure Identity SDK Improvements Nov 3, 2023
@scottaddie
Copy link
Member Author

Marking as blocked until the az cli release ships.

@scottaddie scottaddie moved this from Blocked to Not Started in Azure Identity SDK Improvements Nov 14, 2023
@antkmsft
Copy link
Member

Marking as blocked until the az cli release ships.

Az CLI v.2.54.0 has shipped, I was able to obtain it via Winget.

@ahsonkhan
Copy link
Member

Office hour discussion conclusion across languages:
Azure/azure-sdk-for-cpp#5116 (comment)

@joshfree joshfree modified the milestones: 2024-02, 2024-03 Jan 8, 2024
@ahsonkhan
Copy link
Member

This change is addressed in C++, and similar changes can be made in other languages.

The gist is:

  • We want to parse and use the new expires_on field if it exists, as is, as a Posix time (seconds since UTC). In this case, we don't use the existing field at all.
  • If that new field doesn't exist, use the existing expiresOn field as a fallback, parsed as local time.
    • The existing expiresOn field deviates a bit from RFC 3339 and doesn't have the timezone info in the string to indicate the local time offset (no Z or +XX:XX), so we assume it is considered local time.

Test example:
https://github.com/Azure/azure-sdk-for-cpp/blob/cb73bbd62cb825171cd598bfc1936aea40ebbaa0/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp#L229-L277

PRs in C++ for inspiration, if relevant:
Azure/azure-sdk-for-cpp#5179
Azure/azure-sdk-for-cpp#5180

@pvaneck pvaneck self-assigned this Jan 22, 2024
@github-project-automation github-project-automation bot moved this from Not Started to Done in Azure Identity SDK Improvements Jan 23, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library.
Projects
Development

Successfully merging a pull request may close this issue.

6 participants