-
Notifications
You must be signed in to change notification settings - Fork 175
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
v11.0.0 #326
v11.0.0 #326
Conversation
1924a39
to
cc7d4d2
Compare
Fixes #325 |
ExpiresIn string `json:"expires_in"` | ||
ExpiresOn string `json:"expires_on"` | ||
NotBefore string `json:"not_before"` | ||
ExpiresIn json.Number `json:"expires_in"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be a good moment to add some documentation about the expected contents of these values as well.
For example, I believe this is a whole number of seconds. We should say so :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a link to the docs for this type.
autorest/adal/token_test.go
Outdated
@@ -868,7 +862,7 @@ func expireToken(t *Token) *Token { | |||
|
|||
func setTokenToExpireAt(t *Token, expireAt time.Time) *Token { | |||
t.ExpiresIn = "3600" | |||
t.ExpiresOn = strconv.Itoa(int(expireAt.Sub(date.UnixEpoch()).Seconds())) | |||
t.ExpiresOn = json.Number(strconv.Itoa(int(expireAt.Sub(date.UnixEpoch()).Seconds()))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd change this to
t.ExpiresOn = json.Number(fmt.Sprintf(int64(expireAt.Sub(date.UnixEpoch()) / time.Second)))
While it's not any more readable, it has two advantages IMHO:
- Not calling
.Seconds()
prevents the conversion from anint
type to afloat
type. - Using
fmt.Sprintf()
instead ofstrconv.Itoa()
prevents us from casting down from 64 bits to 32 bits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Errr, I see this is just a test file so don't see this comment as too blocking or anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tweaked this a bit.
Changed a few fields in adal.Token from string to json.Number to handle differences between AAD and ADFS in how they send data over the wire. Added auth.NewAuthorizerFromFileWithResource to create an authorizer from the config file with the specified resource. Setting a client's PollingDuration to zero will use the provided context to control a LRO's polling duration.
38aa960
to
063a7bf
Compare
Fixes #314 |
Changed a few fields in adal.Token from string to json.Number to handle
differences between AAD and ADFS in how they send data over the wire.
Added auth.NewAuthorizerFromFileWithResource to create an authorizer
from the config file with the specified resource.
Setting a client's PollingDuration to zero will use the provided context
to control a LRO's polling duration.
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:
dev
branch, except in the case of urgent bug fixes warranting their own release.master
, I've updated CHANGELOG.md to address the changes I'm making.