Use token source instead of non-refreshable token #151
Conversation
There was a problem hiding this comment.
This looks awesome, thanks!
If you want to take this one step further, one could also replace the token source by an http client and leave the entire setup to the consumer to also address #23.
this would be a great next step afterwards
|
Thanks for the PR. Specifying the http.Client will have other useful applications. APIs that require the Client to handle OAuth need to document this requirement. |
| @@ -114,7 +114,11 @@ func (p *oauthPayload) domain() string { | |||
|
|
|||
| // New returns an [Account] that can be used to fetch a [vehicle.Vehicle]. | |||
| // Optional userAgent can be passed in - otherwise it will be generated from code | |||
| func New(oauthToken, userAgent string) (*Account, error) { | |||
| func New(ts oauth2.TokenSource, userAgent string) (*Account, error) { | |||
There was a problem hiding this comment.
Currently tokens are invalidated immediately after another refresh token get issued. I'm afraid that building this functionality is going to create downstream errors for folks who manages their tokens cross app/manually
There was a problem hiding this comment.
To be fair, this PR is from before that change. Actually, I think this makes this PR even more important. How you build your token source is up to you.
|
@sethterashima remaining compile errors are in config/example. I really wonder how to handle config. Specifying a short-lived token one time makes even less sense now than before. If this PR is a good idea depends on how you look at accounts: if that's a short-lived thing that should be regenerated all the time or something long-lived that requires a source of refreshed tokens? |
|
|
||
| wakeLock sync.Mutex | ||
| lastPoke time.Time | ||
| } | ||
|
|
||
| // NewConnection creates a Connection. | ||
| func NewConnection(vin string, authHeader, serverURL, userAgent string) *Connection { | ||
| func NewConnection(vin string, client *http.Client, serverURL, userAgent string) *Connection { |
There was a problem hiding this comment.
The requirements for client need to be clearly documented, ideally with an example of setting up a token source. Alternatively, we could create a NewConnectionWithClient(client *http.Client, vin string ...) *Connection function (with documentation + example).
This could be mirrored in a account.NewWithClient(...) function.
|
The initial release was focused around the proxy, with the aim of creating a drop-in replacement for existing 3P apps, and this informed the API design around tokens. I think having An extra, reverse-compatible RefreshTokenName field in Config should work. |
Description
Authenticate using oauth2 token sources instead of tokens. Authentication is handled using an
http.Transportrather than passingauthHeadersaround. Creation of the token source could be further simplified by implementing #24. Happy to provide a PR, too.If you want to take this one step further, one could also replace the token source by an http client and leave the entire setup to the consumer to also address #23.
Fixes #29
Type of change
Please select all options that apply to this change:
Checklist:
Confirm you have completed the following steps: