-
Notifications
You must be signed in to change notification settings - Fork 1k
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
oauth2/google: fix the logic of sts 0 value of expires_in #687
Conversation
This PR (HEAD: 306f6b4) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/oauth2/+/545895. Important tips:
|
Message from Gopher Robot: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
306f6b4
to
00978f1
Compare
This PR (HEAD: 00978f1) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/oauth2/+/545895. Important tips:
|
Message from Jin Qin: Patch Set 2: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Shin Fan: Patch Set 2: Auto-Submit+1 Code-Review+2 Run-TryBot+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Gopher Robot: Patch Set 2: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Gopher Robot: Patch Set 2: TryBot-Result+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Leo Siracusa: Patch Set 2: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
This PR (HEAD: e7efdb4) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/oauth2/+/545895. Important tips:
|
This PR (HEAD: 22f8b75) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/oauth2/+/545895. Important tips:
|
Message from Jin Qin: Patch Set 3: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Cody Oss: Patch Set 4: Run-TryBot+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Gopher Robot: Patch Set 4: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Gopher Robot: Patch Set 4: TryBot-Result+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Jin Qin: Patch Set 6: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Cody Oss: Patch Set 6: Run-TryBot+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Jin Qin: Patch Set 7: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Jin Qin: Patch Set 7: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Cody Oss: Patch Set 7: Run-TryBot+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
This PR (HEAD: eeb1a08) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/oauth2/+/545895. Important tips:
|
Message from Jin Qin: Patch Set 7: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
Message from Cody Oss: Patch Set 8: (3 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
This PR (HEAD: a95c923) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/oauth2/+/545895. Important tips:
|
Message from Jin Qin: Patch Set 8: (4 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
The sts response contains an optional field of `expires_in` and the value can be any integer. https://github.com/golang/oauth2/blob/master/google/internal/externalaccount/basecredentials.go#L246-L248 In the case of less than `0`, we are going to throw an error. But in the case of equals to `0` practically it means "never expire" instead of "instantly expire" which doesn't make sense. So we need to not set the expiration value for Token object. The current else if greater or equal is wrong. It's never triggered only because we are sending positive `3600` in sts response. Change-Id: Id227ca71130855235572b65ab178681e80d0da3a GitHub-Last-Rev: a95c923 GitHub-Pull-Request: #687 Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/545895 Reviewed-by: Shin Fan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Cody Oss <[email protected]> Reviewed-by: Cody Oss <[email protected]>
Message from Cody Oss: Patch Set 9: Auto-Submit+1 Code-Review+1 Commit-Queue+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/545895. |
This PR is being closed because golang.org/cl/545895 has been merged. |
The sts response contains an optional field of `expires_in` and the value can be any integer. https://github.com/golang/oauth2/blob/master/google/internal/externalaccount/basecredentials.go#L246-L248 In the case of less than `0`, we are going to throw an error. But in the case of equals to `0` practically it means "never expire" instead of "instantly expire" which doesn't make sense. So we need to not set the expiration value for Token object. The current else if greater or equal is wrong. It's never triggered only because we are sending positive `3600` in sts response. Change-Id: Id227ca71130855235572b65ab178681e80d0da3a GitHub-Last-Rev: a95c923 GitHub-Pull-Request: golang#687 Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/545895 Reviewed-by: Shin Fan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Cody Oss <[email protected]> Reviewed-by: Cody Oss <[email protected]>
The sts response contains an optional field of
expires_in
and the value can be any integer.https://github.com/golang/oauth2/blob/master/google/internal/externalaccount/basecredentials.go#L246-L248
In the case of less than
0
, we are going to throw an error. But in the case of equals to0
practically it means "never expire" instead of "instantly expire" which doesn't make sense.So we need to not set the expiration value for Token object. The current else if greater or equal is wrong.
It's never triggered only because we are sending positive
3600
in sts response.