golang / oauth2

Go OAuth2
https://golang.org/x/oauth2
BSD 3-Clause "New" or "Revised" License
5.29k stars 979 forks source link

Cache token / transport confusion #84

Open billmccord opened 9 years ago

billmccord commented 9 years ago

Hi, I'm having a really hard time figuring out how to accomplish what I want to do. Essentially, I would like to cache the access and refresh tokens so that I don't have to ask the user to authenticate every time I want to create a new client. It seems like the Transport may have been added for this purpose, but an example would make things much clearer. Thanks for the awesome library and any help you can provide!

kke commented 2 years ago

Agree with @KoduIsGreat and @blueForestIcarus - I've been trying to understand and implement this or to find a project using this package that would have a clean token cache implementation but haven't found one.

sonmaximum commented 2 years ago

I also wound up here looking for documentation on how to solve this problem (caching/saving tokens when refresh token is invalidated/updated upon use) and was surprised to find there doesn't seem to be a clear answer.

It would be helpful for users to have some clarity on this.

manojmalik20 commented 2 years ago

I tried using the method suggested by @ibudiallo above and I'm getting this error from the Token method -

Post "": unsupported protocol scheme ""

Does anyone know the reason behind this and how to fix this?

j0hnsmith commented 2 years ago

@manojmalik20 usually that happens when you have a URI without http/https

Blasikov commented 2 years ago

@ibudiallo - You saved my bacon. Thank you.

My workflow's target system refresh token is always unlimited, so this is perfect.

Would have taken me ages, if ever, to understand this gem:

newToken, err := src.Token() // this actually goes and renews the tokens

tonimelisma commented 1 year ago

Am I to understand correctly that for 7 years no one has fixed this bug? Would a PR be accepted, or would it be better to fork the library? EDIT: I just realized there's over 70 open pull requests and over 110 open issues. So it looks like Google is no longer maintaining the official oauth2 library? Would you be open to handing over maintenance to volunteers who have more time?

tonimelisma commented 1 year ago

A simple way to cache refreshed token (for very simple use cases):

tokenSource := conf.TokenSource(oauth2.NoContext, token)
newToken, err := tokenSource.Token()
if err != nil {
    log.Fatalln(err)
}

if newToken.AccessToken != token.AccessToken {
    SaveToken(newToken)
    log.Println("Saved new token:", newToken.AccessToken)
}

client := oauth2.NewClient(oauth2.NoContext, tokenSource)
resp, err := client.Get(...)

This only works for the initial loading of the token. As I understand it, each time you call the client to make an API call it might update the token silently. So you have to make another check after each API call to see if the token changed.

dnesting commented 1 year ago

Something like this solution works for me:

type cachingTokenSource struct {
  base oauth2.TokenSource
  filename string
}

func (c *cachingTokenSource) saveToken(tok *oauth2.Token) error // save tok as JSON to c.filename
func (c *cachingTokenSource) loadToken() (*oauth2.Token, error) // load JSON token from c.filename

func (c *cachingTokenSource) Token() (tok *oauth2.Token, err error) {
  tok, _ = t.loadToken()
  if tok != nil && tok.IsValid() {
    return tok, nil
  }

  if tok, err = c.base.Token(); err != nil {
    return nil, err
  }

  err2 := c.saveToken(tok)
  if err2 != nil {
    log.Error("cache token:", err2)  // or return it
  }

  return tok, err
}

func NewCachingTokenSource(filename string, config *oauth2.Config, tok *oauth2.Token) oauth2.TokenSource {
  orig := config.TokenSource(context.Background(), tok)
  return oauth2.ReuseTokenSource(nil, &cachingTokenSource{
    filename: filename,
    base: orig,
  })
}

So long as this is the TokenSource you use everywhere, it's guaranteed to get the cache updated when a token refresh occurs, so it seems to address all of the requirements discussed in this thread.

pmrt commented 1 year ago

Something like this solution works for me:

type cachingTokenSource struct {
  base oauth2.TokenSource
  filename string
}

func (c *cachingTokenSource) saveToken(tok *oauth2.Token) error // save tok as JSON to c.filename
func (c *cachingTokenSource) loadToken() (*oauth2.Token, error) // load JSON token from c.filename

func (c *cachingTokenSource) Token() (tok *oauth2.Token, err error) {
  tok, _ = t.loadToken()
  if tok != nil && tok.IsValid() {
    return tok, nil
  }

  if tok, err = c.base.Token(); err != nil {
    return nil, err
  }

  err2 := c.saveToken(tok)
  if err2 != nil {
    log.Error("cache token:", err2)  // or return it
  }

  return tok, err
}

func NewCachingTokenSource(filename string, config *oauth2.Config, tok *oauth2.Token) oauth2.TokenSource {
  orig := config.TokenSource(context.Background(), tok)
  return oauth2.ReuseTokenSource(nil, &cachingTokenSource{
    filename: filename,
    base: orig,
  })
}

So long as this is the TokenSource you use everywhere, it's guaranteed to get the cache updated when a token refresh occurs, so it seems to address all of the requirements discussed in this thread.

Just to clarify, your workaround is to never use oauth2.Config.Client() (ie. use your own http client) and handle the refresh by your own, retrieving the token from cachingTokenSource.Token() before each request until the oauth2 http client library implements a token cache interface, right? Because if you later use oauth2.Config.Client() to make requests your tokens could be silently refreshed during the requests.

On the other hand, this is not thread safe so beware of race conditions in your files, if your storage layer is a cookie or a database this may not be a problem (the scope of cookies is per user-agent and databases generally handle race conditions well) but if you're writing to files as in your example, two different requests could try to write to the same file at the same time, so use singleflight or mutexes.

dnesting commented 1 year ago

your workaround is to never use oauth2.Config.Client() (ie. use your own http client)

Sort of. The implementation of oauth2/Config.Client() is trivial:

func (c *Config) Client(ctx context.Context, t *Token) *http.Client {
    return NewClient(ctx, c.TokenSource(ctx, t))
}

So if you wish to insert yourself in the process somewhere in order to cache the last token for reuse later, the TokenSource is the logical place to do that. To obtain an http.Client you then just need to oauth2.NewClient(ctx, myCachingTokenSource).

handle the refresh by your own

Sort of.

  orig := config.TokenSource(context.Background(), tok)
  return oauth2.ReuseTokenSource(nil, &cachingTokenSource{
    filename: filename,
    base: orig,
  })

What this code does is instantiate a TokenSource that will handle the refreshes automatically, and the logic implemented above will call it when needed if the cached token isn't valid. What I'm doing here is just inserting myself in between the existing TokenSource and the http.Client produced by oauth2.NewClient.

Probably one difference worth noting is that I'm using context.Background() instead of passing the original context around, so if you were relying on that context being preserved for the token refreshes you'd need to modify accordingly.

this is not thread safe so beware of race conditions in your files

It should be concurrency-safe. Because I wrap my caching token source with ReuseTokenSource, this ensures that calls to Token are serialized with a mutex. You would only have concurrency issues if you instantiate multiple caching token sources pointing to the same file.

tonimelisma commented 1 year ago

Thank you very much for these workarounds. However, they're so complex it seems to me like this library has significant architectural issues. It would be great to get maintainer views on whether they foresee anyone ever fixing things or even approving PRs.

pmrt commented 1 year ago

Noted, thank you so much @dnesting That was the part of the puzzle I was missing.

In my use case apart from updating the token pair in the database I also want to update the pair in an encrypted cookie, which are obviously per user-agent so my idea was instead of using a cacheTokenSource, to use something more like a notifyTokenSource that takes a callback that gets executed when the token is refreshed. Implementing it would be very similar to your workaround but I have doubts about the usage, take this pseudocode as an example where we would use it within an http handler:

type SomeHandler struct {
    db     *sql.DB
    config *oauth2.Config
}

func (h *SomeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    // recreate *oauth.Token from cookie
    tok := tokenFromCookie(r)
        // create a new tokenSource for our current request
    ts := NewNotifyTokenSource(h.config, tok, func(newTok *oauth2.Token) {
        // save new token in the db
        saveToken(newTok, db)
        // update the encrypted cookie
        saveCookie(newTok, w)
    })
    c := oauth2.NewClient(context.Background(), ts)

    // make requests with my client
    c.Do(...)
    // etc
}

Notice how here we would create an instance of tokenSource and http.Client per request because we have 1 token per 1 request in this case (also we use the same instance of oauth2.Config across requests).

My guess is that it is concurrent safe because we're using 1 instance per 1 request so it shouldn't even need mutex. And even if within a request we spawn multiple goroutines that try to use the same client with that token source, both the original config.TokingSource() and our custom tokenSource are returning a tokenRefresher wrapped with a reuseTokenSource which is guarded with mutex. From what you've mentioned this should be okay and safe to use, but I'm not sure, can someone confirm this please?

And yeah, I agree with @tonimelisma. We need an official and clean way to hook into this client which doesn't feel so hacky.

tonimelisma commented 9 months ago

Am I to understand correctly that for 7 years no one has fixed this bug? Would a PR be accepted, or would it be better to fork the library? EDIT: I just realized there's over 70 open pull requests and over 110 open issues. So it looks like Google is no longer maintaining the official oauth2 library? Would you be open to handing over maintenance to volunteers who have more time?

Bumping this. Looks like there isn't a popular fork of this library and creating one would be difficult as this one is maintained by Google, would the maintainers be willing to move ownership of this library to more active community maintainers who could approve PRs and fix some of the bugs?

Not sure who the right people are, but perhaps @quartzmo @codyoss @BigTailWolf @rolandshoemaker @dmitshur @bcmills could comment?

andrewhowdencom commented 7 months ago

(similar to @dnesting's approach): A caching token provider:

I struggled with seeding the cache, which is why the cache instantiation is so long. The seed is also in the same project.