oauth2.Config captures ctx and re-uses it later

[vangent]

https://github.com/golang/oauth2/blob/master/oauth2.go#L239

For example, if I do something like this:

  ctx, cancel := context.WithCancel(context.Background())
  ts, err := google.DefaultTokenSource(ctx)
  cancel()
  tkn, err := ts.Token()

the last line will fail every time because the HTTP request isn't made until then, using the ctx I provided to DefaultTokenSource. Somewhere near here: https://github.com/golang/oauth2/blob/master/oauth2.go#L265

This is very surprising and should at least be documented clearly on DefaultTokenSource, FindDefaultCredentials, etc.

#262 is possibly related.

/cc @jba @zombiezen

[vangent]

Introduced in https://golang.org/cl/45370.

[vangent]

/cc @broady

[broady]

+1, this is certainly a problem (there's many problems with this package that I won't go into)

and +1 to #262, Token() should take a ctx, for sure. However, it makes ReuseTokenSource kind of awkward, because it becomes tempting to put stuff in the context that might change the type of token that's returned (e.g., something that changes per user), and if that varies between requests, then it's not valid to wrap it in ReuseTokenSource.

On Wed, Jun 26, 2019 at 10:40 AM Robert van Gent [email protected] wrote:

/cc @broady https://github.com/broady

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/golang/oauth2/issues/388?email_source=notifications&email_token=AAAGDFUV3XIZJSEN34JT7ODP4OSYRA5CNFSM4H3TPCRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYUJB5Y#issuecomment-505975031, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAGDFXYC7C2KWPILC56NFDP4OSYRANCNFSM4H3TPCRA .

[jacobsevart]

We just got bit by the same problem, causing all requests to fail once the initial token expired, since the context used to attempt the refresh had expired after application startup.

Suggestion: reuseTokenSource should refuse to construct if the underlying token source has a context deadline.

[jacobsevart]

Hi folks, I've uploaded a pull request to document this issue and report a more useful error.