Cache token / transport confusion #84
Comments
|
After digging into the code a bit more, it seems like perhaps when I'm finished using an OAuth2 Client and want to cache the token away for later reuse I could:
|
|
You could also just cache the Token from created from the Config.Exchange() func so that you only cache Token once because the refresh token never expires. |
|
Every time I refresh tokens on the OAuth library I'm using I get a new
|
|
Token.RefreshToken stays the same. Token.AccessToken changes. See if the following gist explains it any better. |
|
Jim, thanks for posting the gist. That was helpful, but unless I'm misunderstanding something I think there are still cases where the token could be lost. Consider the following and let me know if this is a possible scenario:
In this scenario, I would have no way of knowing the token was modified unless I followed my process above of recaching the token or, at the very least, verifying that the token didn't change while I was using the Client. This is because the Client automatically refreshes the Token for you as a convenience and, unfortunately, doesn't tell you that it did it via a callback. Does that make sense or am I still missing something? |
|
The RefreshToken does not change. so you don't have to worry about the background refresh. The tokenRefresher struct stores the RefreshToken in the oldToken field and this field is not updated again. *note that the RefreshToken field of the returned Token pointer is updated by the passed refresh token. The tokenRefresher is not updated by the returned Token. |
|
Jim, unfortunately, this does not seem to be the case. I built a test based on the tests included with oauth2 here: You can see from my gist that if the token expires after being used to create an oauth2.Client, then another request will be made in the background to obtain a new token (new access_token and new refresh_token) before proceeding with the request. Unfortunately, at this point the refresh_token is lost because the ORIGINAL_REFRESH_TOKEN is no longer valid after the NEW_REFRESH_TOKEN is generated according to the OAuth2 spec. Therefore, it is possible that the token that I stored per your suggestion will become invalid and the only way I can see to ensure that I have the right tokens is to always get the Token again from the Client after use and ensure that a) it hasn't changed from what I have stored or b) update what I have stored with the changed Token I got from the Client. |
|
I think I see where we might be talking past each other and that an actual bug exists. I tested code using Google's client apis and their implementation of oauth2. According to Google's oauth documentation , refresh tokens do not expire but must be revoked. In my tests, a new refresh token is only generated during a code exchange (i.e. Config.Exchange() ). In a token refresh call (tokenRefresher.Token() ) the refresh_token field is omitted/left blank. This is allowed in the oauth2 spec as sending a new refresh_token during a refresh operation is optional (oauth2 doc section 1.5), not required. You have found a bug. If a new refresh_token is generated during a refresh, the token source refresh token is not updated. See test code for an example. Back to your original question of how to cache a Token each time it is refreshed. A hook would need to be added to Config and ReuseTokenSource(). |
|
Thanks for validating this. The OAuth provider I'm calling uses a library that always generates a new refresh_token and there isn't an option to not do this yet. Since it is optional, I agree, that a hook would be necessary to get the updated Token in cases where it is renewed. Since that hook doesn't exist yet, I suppose that my workaround of getting the Token from the client when I'm finished using it is probably the only option? e.g. It isn't pretty, but it seems like a reasonable workaround for now. |
|
Does the provider return a different refresh token each time you ask for a new access token? |
|
Yes. You can see here that issue_refresh_token is hard-coded to true for the refresh_token grant_type: |
|
Their API is not complaint with the OAuth 2.0 spec, and we have no intention to support provider-specific non-spec features. |
|
Two points:
|
|
https://tools.ietf.org/html/rfc6749#section-6 The authorization server MAY issue a new refresh token, in which case https://tools.ietf.org/html/rfc6749#section-10.4 For example, the authorization server could employ refresh token |
|
Jim, thanks, that seems to validate that the bug lies with this project because it doesn't provide a mechanism for obtaining the new refresh_token in situations where the refresh_token is replaced during the Refreshing an Access Token operation, correct? If so, I can file a close this out and create a more specific issue OR we can just use this to track. |
|
Bill, @rakyll are you ok with calling this a bug? Should we start a new issue to discuss caching difficulties? |
|
@jfcote87 It looks like a bug to me. I think we should start a new issue to discuss caching strategies. |
Fixes bug documented in Issue #84 (#84 (comment)). During a refresh request, a new refresh token MAY be returned by the authorization server. When this occurs, tokenRefesher.Token() fails to capture the new refresh token leaving it with an invalid refresh token for future calls. Change-Id: I33b18fdbb750549174865f75eddf85b9725cf281 Reviewed-on: https://go-review.googlesource.com/4151 Reviewed-by: Andrew Gerrand <[email protected]>
|
@adg is there open issue about caching strategies? |
|
Not that I know of. On 4 May 2015 at 04:53, Miroslav Genov [email protected] wrote:
|
|
For reference, there used to be support for token caching but it was removed for some reason: 93ad3f4 |
|
So is this fixed, then ? |
|
@johnl, the trivial cache implementation is removed because we were not able to come up with a generic and useful cacher interface. I think a wrapper cacher RoundTripper is the perfect solution although it is not documented anywhere. |
|
If the token has been restored, i can access a resource. But token is not auto-refreshed. Why? sourceToken := oauth2.ReuseTokenSource(nil, &fileTokenSource{accessToken, refreshToken, expiry})
client := &http.Client{
Transport: &oauth2.Transport{
Base: ContextTransport(oauth2.NoContext), // from internal/transport.go
Source: oauth2.ReuseTokenSource(nil, sourceToken),
},
}
client.Get("...")
// access the resource
// long time
// token expired
client.Get("...")
// and not auto-refreshed |
|
When your mind is fresh... no problems sourceToken := oauth2.ReuseTokenSource(nil, &fileTokenSource{accessToken, refreshToken, expiry})
t, _ := sourceToken.Token()
client = conf.Client(oauth2.NoContext, t)
client.Get("...")
// token is auto-updated |
|
@rakyll |
|
Is there currently any workaround for this problem? |
|
Ok, I see bug with refreshing token got fixed: cc2494a right? However, original question remains - what's currently proper way to save and restore token? Is there any kind of event when token get's changed? Any example how to achieve that? |
While this is admirably simple, I realized it unfortunately has a failure mode:
|
|
@lpar in the same solution mentioned by @ibudiallo, how about saving the token with a TTL with little less time (say 5 minutes) than the expiry time, this way we're refreshing the token ahead of time. |
|
I came here to say, yes, this is very confusing. It took a whole hour of searching just to figure out that CacheFile used to be supported but was removed. It is strange this isnt a solved issue since I expect it is a common usecase, I'd think that most of the time application developers would want to cache the token. Maybe there is something I don't understand about oauth2, but if so it is not obvious from this thread or this repo. My understanding is:
Most of the solutions here seem to ignore the 4th point, or like @gouthampc above, require extra time based plumbing. I think either something needs to be added to the library to facilitate having a CacheFile that updates when the token is refreshed, or at least an example needs to be written for how to do it with existing plumbing. |
|
What about utilizing "golang.org/x/sync/singleflight" to prevent unnecessary renewal of access tokens? |
|
@lpar , @ibudiallo - You could set the Token.Expiry to 0 to disable the "auto-refresh", as per the documentation here: https://pkg.go.dev/golang.org/x/oauth2#Token This means that then you can manage refreshing manually yourself. It defeats the purpose of auto-refresh functionality though. It really should be simpler than this, to save a refreshed token. |
|
I've read this thread, multiple times and am still confused about what, if any of these solutions actually work, or if there is a more documented way to solve this problem. |
|
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. |
|
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. |
|
I tried using the method suggested by @ibudiallo above and I'm getting this error from the Does anyone know the reason behind this and how to fix this? |
|
@manojmalik20 usually that happens when you have a URI without |
|
@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:
|
|
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 |
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. |
|
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. |
Sort of. The implementation of 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
Sort of. orig := config.TokenSource(context.Background(), tok)
return oauth2.ReuseTokenSource(nil, &cachingTokenSource{
filename: filename,
base: orig,
})What this code does is instantiate a Probably one difference worth noting is that I'm using
It should be concurrency-safe. Because I wrap my caching token source with |
|
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. |
|
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. |
…nSource When used with tokens issued by a server supporting refresh token rotation it is unsafe to continuing using the token provided to ReuseTokenSource (including via the Client method of Config) outside of the returned TokenSource and/or Client as it leads to a race condition when the first renewal happens: * If ReuseTokenSource renews its token first, the original token's RefreshToken is now invalid (revoked) and any use/renewal attempt will fail. * If the original token renews its token first, the ReuseTokenSource holds the invliad RefreshToken and will fail on the next usage attempt. golang#84 has extensive discussion of related risks and complications when trying to cache or store the RefreshToken, but the generic risk of race conditions exists regardless of whether any caching or storage is being attempted and API users must be warned of this possibility.
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? |
|
(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. |
@manojmalik20, you got this error because your oauth2.Config was missing the Auth and/or Token URL endpoints. @ibudiallo's solution is barebones to the point of being unusable as-is. I got this error, added my endpoints, then got an "invalid_client" error, again because I hadn't properly configured the Config: it was missing the client ID and secret. |
|
Just throwing this in here in case others are interested--it var ctx = context.Background()
var conf = &oauth2.Config{
// your oauth2 config here
}
func main() {
token := loadToken()
if token == nil {
// get the initial token (possibly with redirect+conf.Exchange(..))
}
// example usage
client := oauth2.NewClient(ctx, newTokenPersister(token))
client.Get("https://some-authenticated-api.com")
}
type tokenPersister struct {
next oauth2.TokenSource // wrapped token source
expiry time.Time
}
func newTokenPersister(token *oauth2.Token) *tokenPersister {
return &tokenPersister{
next: conf.TokenSource(ctx, token),
expiry: token.Expiry,
}
}
func (p *tokenPersister) Token() (*oauth2.Token, error) {
token, err := p.next.Token()
if err != nil {
return nil, err
}
// can use something other than the expiry for change-detection, but it's good enough for me.
if p.expiry != token.Expiry {
if err := saveToken(token); err != nil {
log.Println("error saving token:", err)
// Still, just return the token without the error (good enough for me).
// If you want, you can add extra fail safety here, but probably shouldn't
// return an error as you successfully got the token, it just didn't persist.
return token, nil
} else {
log.Println("intercepted token refresh, saved new token")
}
p.expiry = token.Expiry
}
return token, nil
}
func saveToken(token *oauth2.Token) error {
// save the token somewhere
}
func loadToken() *oauth2.Token {
// load the token from somewhere
} |
and others
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!
The text was updated successfully, but these errors were encountered: