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

Unmarshal expires_in outside of oauth2/internal #484

Open
andig opened this issue Mar 14, 2021 · 6 comments
Open

Unmarshal expires_in outside of oauth2/internal #484

andig opened this issue Mar 14, 2021 · 6 comments

Comments

@andig
Copy link
Contributor

andig commented Mar 14, 2021

Some almost-OAuth2 apis return expires_in as part of their token response. Unfortunately, unmarshaling into an oauth2.Token does not populate it's Expiry field. Hence, the token structure needs be duplicated/embedded to provide this logic. It would be nice if oauth2.Token.UnmarshalJSON populated the Expiry field when expires_in is populated.

@zak905
Copy link

zak905 commented May 12, 2021

Good point. I mentionned something about it in my latest post: http://www.zakariaamine.com/2021-03-23/leveraging_oauth2_package_golang

This logic here https://github.com/golang/oauth2/blob/master/internal/token.go#L262 needs to be duplicated:

		e := vals.Get("expires_in")
		expires, _ := strconv.Atoi(e)
		if expires != 0 {
			token.Expiry = time.Now().Add(time.Duration(expires) * time.Second)
		}

However, nothing garantees that the time you (as a user) unmarshall a token, is the exact time of the token obtention. This can lead to Expiry field being inaccurate. I guess this is why they kept it internally, because they populate Expiry right after receiving the token, and therefore the Valid() check is correct.

@andig
Copy link
Contributor Author

andig commented May 12, 2021

Imho that‘s a smaller concern, addressable via docs, and already true with the internal handling today.

@andig
Copy link
Contributor Author

andig commented May 2, 2022

This is one of the patterns that seems to come up a lot: https://cs.github.com/?scopeName=All+repos&scope=&q=language%3Agolang+ExpiresIn+int#.

An alternative way of achieving the same result could be using

func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL string, v url.Values, authStyle AuthStyle) (*Token, error) {
.

@zak905
Copy link

zak905 commented May 3, 2022

unforutnately anything under /internal/ cannot be exported and used by other librairies (as per Go specs)

@hickford
Copy link
Contributor

Fixing this would surely break backwards compatibility? Nevertheless easy marshalling between oauth2.Token and RFC 6749 Access Token Response JSON is desirable. Maybe this has to wait until the next major version?

@andig
Copy link
Contributor Author

andig commented Jul 18, 2023

Migrated to golang/go#61417

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

No branches or pull requests

3 participants