oauth2 icon indicating copy to clipboard operation
oauth2 copied to clipboard

Unmarshal expires_in outside of oauth2/internal

Open andig opened this issue 3 years ago • 6 comments

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.

andig avatar Mar 14 '21 19:03 andig

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.

zak905 avatar May 12 '21 12:05 zak905

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

andig avatar May 12 '21 17:05 andig

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 https://github.com/golang/oauth2/blob/9780585627b5122c8cc9c6a378ac9861507e7551/internal/token.go#L188.

andig avatar May 02 '22 09:05 andig

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

zak905 avatar May 03 '22 16:05 zak905

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?

hickford avatar Jan 29 '23 22:01 hickford

Migrated to https://github.com/golang/go/issues/61417

andig avatar Jul 18 '23 19:07 andig