Skip to content

handle null expireson#22899

Closed
xiangyan99 wants to merge 1 commit intomainfrom
identity_null_expireson
Closed

handle null expireson#22899
xiangyan99 wants to merge 1 commit intomainfrom
identity_null_expireson

Conversation

@xiangyan99
Copy link
Member

@xiangyan99 xiangyan99 requested a review from chlowell February 3, 2022 18:08
@ghost ghost added the Azure.Identity label Feb 3, 2022
dt = datetime.strptime(token["expiresOn"], "%Y-%m-%d %H:%M:%S.%f")
expires_on_in_token = token["expiresOn"]
if not expires_on_in_token:
expires_on_in_token = (datetime.now()+timedelta(days=1)).strftime("%Y-%m-%d %H:%M:%S.%f")
Copy link
Member

Choose a reason for hiding this comment

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

The token's lifetime depends on the tenant's configuration. We don't know anything about that, so we can't safely guess when the token expires. Underestimating degrades performance; overestimating causes the auth policy to send an expired token. Really this is a CLI bug that should be fixed in the CLI. I think we have two options for handling it here:

  1. use the token only once (we can assume it's valid right now)
  2. raise AuthenticationFailedError

Option 1 makes the application terribly slow; option 2 maybe crashes it 😄

@jiasli
Copy link
Member

jiasli commented Feb 9, 2022

This issue has long been fixed (Azure/azure-cli#20219). Azure CLI guarantees the existence of expiresOn.

@xiangyan99 xiangyan99 closed this Feb 9, 2022
@xiangyan99 xiangyan99 deleted the identity_null_expireson branch March 2, 2022 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants