-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Description
Migrated from golang/oauth2#484, refs #56402 (comment)
There are a number of OAuth2 token uses outside of the oauth2 library with the token structure being the common denominator. Unfortunately, unmarshaling JSON into an oauth2.Token
does not populate it's Expiry
field. Hence, the token structure needs be duplicated/embedded to provide this logic as it currently lives in oauth2/internal
.
Proposed solution:
Allow unmarshaling an oauth2.Token from its usual json representation, i.e. containing the expires_in
field.
An implementation choice might be to add a Token.UnmarshalJSON
method though that might imply that (later) adding Token.MarshalJSON
may not make sense given the nature of expires_in
being relative to the current moment.
Consequence of not implementing:
Duplicated code like https://cs.github.com/?scopeName=All+repos&scope=&q=language%3Agolang+ExpiresIn+int#.
Alternatives:
- Duplicated code like https://github.com/evcc-io/evcc/blob/master/util/oauth/token.go#L13
- expose
internal/RetrieveToken
to avoid new methods onToken
like Make RetrieveToken usable by client packages oauth2#354 - providing other means of obtaining tokens in a configurable way from
oauth2.Config
like custom exchange request attribute/header oauth2#533, Config: Refresh Token Request with Custom Parameters oauth2#521 or Add additional headers during token refresh oauth2#483
Metadata
Metadata
Assignees
Type
Projects
Status
Activity
rsc commentedon Jul 26, 2023
This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group
rsc commentedon Aug 2, 2023
Is the specific proposal here to change Token.UnmarshalJSON to set the Expiry field based on expires_in?
andig commentedon Aug 2, 2023
Yes.
Token.UnmarshalJSON
is not exposed today. If it gets exposed it should set theExpiry
field based onexpires_in
. If exposing/addingToken.UnmarshalJSON
is not feasible the proposal would be to provide another method to the same result. Thank you!rsc commentedon Aug 9, 2023
I'm very confused, since Token.UnmarshalJSON does not exist and yet the top proposal above mentions it. What is the specific API change we are discussing? Or do you have a link to an implementation of the change?
andig commentedon Aug 9, 2023
I'm sorry for the confusion. The request here is to allow unmarshaling an
oauth2.Token
from its usual json representation, i.e. containing theexpires_in
field. Doing so by addingUnmarshalJson
or exposing an existing internal function is irrelevant. I've updated the proposal.rsc commentedon Aug 16, 2023
What is the specific API that we should add?
andig commentedon Aug 17, 2023
@rsc I'm proposing to add the ability of unmarshaling tokens from JSON. I'm in no position to propose a specific api. As written before options I see are:
Token.UnmarshalJSON
oauth2.TokenFromJSON
method based oninternal.tokenJSON
I feel the weekly round trips on this proposal do not really help to move it forward as apparently I cannot make the required contribution. Is there any chance one of the oauth2 owners joins here to improve the contribution?
77 remaining items
gopherbot commentedon Aug 16, 2024
Change https://go.dev/cl/605955 mentions this issue:
x/oauth2: add Token.ExpiresIn
andig commentedon Aug 16, 2024
Sorry, I don't get it right:
still points to oauth2.
magical commentedon Aug 16, 2024
Odd. That's the right syntax; i'm not sure why gerrit's autolinker isn't parsing it correctly. Maybe something misconfigured? Here's another x/oauth2 CL showing the same problem (https://golang.org/cl/450155), whereas a recent x/tools CL shows a correct link (https://golang.org/cl/605015).
In any case, it should work correctly when the commit is eventually merged.
x/oauth2: add Token.ExpiresIn
mvdan commentedon Sep 19, 2024
Thank you all for this proposal! Even in its minimal form, it's already saving me some amount of copy pasting: https://review.gerrithub.io/c/cue-lang/cue/+/1201519
x/oauth2: apply the expires_in value returned by the server to Token.…
gopherbot commentedon Oct 20, 2024
Change https://go.dev/cl/621215 mentions this issue:
x/oauth2: apply the expires_in value returned by the server to Token.ExpiresIn
x/oauth2: clarify that ExpiresIn is not automatically populated by th…
x/oauth2: apply the expires_in value returned by the server to Token.…
x/oauth2: clarify that ExpiresIn is not automatically populated by th…