feat: Add RemoteOAuth Token helper to refresh access_token from cloud environment#1866
feat: Add RemoteOAuth Token helper to refresh access_token from cloud environment#1866kodiakhq[bot] merged 17 commits intomainfrom
access_token from cloud environment#1866Conversation
⏱️ Benchmark results
|
access_token from cloud environmentaccess_token from cloud environment
erezrokah
left a comment
There was a problem hiding this comment.
Nice helper to have, added a few comments
| "golang.org/x/oauth2" | ||
| ) | ||
|
|
||
| type Token struct { |
There was a problem hiding this comment.
oauth2 has a ReuseTokenSource (or ReuseTokenSourceWithExpiry) where you can pass the initial token and a interface implementation of Token().
That will save us from implementing all the locking involved, valid and expiry logic
There was a problem hiding this comment.
All token-refreshing methods in oauth2 require client credentials to work and they use a refresh_token to do so. They are not usable for this use case on their own.
There was a problem hiding this comment.
As another point, we want to default to the client-provided token only if cloud env is not set (because plugins don't have the expires_at field in their spec as they assume the user-provided token would be valid for the lifetime of the plugin). That's why the remoteToken exists, if cloud env is set we override the spec-specified token with a cloud one and can have the actual expiry timestamp.
There was a problem hiding this comment.
... I'll check if something can be done within the limitations of the things I've outlined in the above two comments.
There was a problem hiding this comment.
I don't think it saves us from anything, the code is simple and about the same as the official implementation. Wrapping it in another ReuseTokenSource will just make it complicated. Furthermore, removing expired will remove timeNow access and will make testing harder or slower.
There was a problem hiding this comment.
All token-refreshing methods in
oauth2require client credentials to work and they use arefresh_tokento do so. They are not usable for this use case on their own.
Looks like ReuseTokenSource doesn't use a refresh token it calls .Token() on the provided src interface https://github.com/golang/oauth2/blob/b52af7d5b4e39d5bb1ee067d8aa110fcce9e4cc7/oauth2.go#L310
You can even pass the initial token as nil if you'd like, e.g. ReuseTokenSource(nil, customTokenSourceThatGetsNewTokenFromCloud)
There was a problem hiding this comment.
Refresh token logic is here https://github.com/golang/oauth2/blob/b52af7d5b4e39d5bb1ee067d8aa110fcce9e4cc7/oauth2.go#L250, it creates a tokenRefresher (which is an implementation of TokenSource interface).
What I'm suggesting is that we do the same with our own TokenSource interface implementation
There was a problem hiding this comment.
if cloud env is not set (because plugins don't have the
expires_atfield in their spec as they assume the user-provided token would be valid for the lifetime of the plugin)
We can:
- Set the expiry to a very long time in the future not in a cloud environment
- Have our own
TokenSource interfacealways return the client provided token not in a cloud environment
🤖 I have created a release *beep* *boop* --- ## [4.60.0](v4.59.0...v4.60.0) (2024-08-12) ### Features * Add RemoteOAuth Token helper to refresh `access_token` from cloud environment ([#1866](#1866)) ([bcd9081](bcd9081)) ### Bug Fixes * **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.22.0 ([#1864](#1864)) ([382f980](382f980)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Implements https://github.com/cloudquery/cloudquery-issues/issues/1978 (internal issue)