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

Handle expires_on in int format #698

Merged
merged 6 commits into from May 19, 2022

Conversation

jhendrixMSFT
Copy link
Member

Unmarshal the value into an interface{} and perform the proper
conversion depending on the underlying type.

Thank you for your contribution to Go-AutoRest! We will triage and review it as soon as we can.

As part of submitting, please make sure you can make the following assertions:

  • I've tested my changes, adding unit tests if applicable.
  • I've added Apache 2.0 Headers to the top of any new source files.

Unmarshal the value into an interface{} and perform the proper
conversion depending on the underlying type.
@jhendrixMSFT
Copy link
Member Author

Fixes #696

chlowell
chlowell previously approved these changes May 19, 2022
autorest/adal/token_test.go Outdated Show resolved Hide resolved
@jhendrixMSFT
Copy link
Member Author

Looking at the related issue, it looks to me that expires_on, at least in that example, isn't the number of seconds from now but probably from the Unix epoch. I need to take a closer look.

@jhendrixMSFT jhendrixMSFT marked this pull request as draft May 19, 2022 17:34
@chlowell
Copy link

Everywhere except App Service, expires_on is epoch seconds, either as a number or a string.

@jhendrixMSFT
Copy link
Member Author

And App Service is as a time-stamp correct?

@jhendrixMSFT
Copy link
Member Author

OK I did a little digging. Token.Expires() already treats ExpiresOn as Unix time. It does mean though that our handling of expires_on in date-time format is incorrect at present.

@jhendrixMSFT jhendrixMSFT marked this pull request as ready for review May 19, 2022 18:56
@jhendrixMSFT jhendrixMSFT dismissed chlowell’s stale review May 19, 2022 19:01

need updated review

autorest/adal/token_test.go Outdated Show resolved Hide resolved
autorest/adal/token_test.go Show resolved Hide resolved
@jhendrixMSFT jhendrixMSFT merged commit 7dd32b6 into Azure:master May 19, 2022
@jhendrixMSFT jhendrixMSFT deleted the adal_expireson branch May 19, 2022 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants