bogosj / tesla

Provides a wrapper around the API to easily query and command a Telsa car.
Other
22 stars 18 forks source link

Update client.go to handle new OAuth #8

Closed bogosj closed 3 years ago

bogosj commented 3 years ago

Currently I use the library by creating a http.Client outside of the library and manually creating a struct: https://github.com/bogosj/go-tesla-go/blob/0734ca75a1aaca030779f96ebdee98c1483b03e0/setup.go#L100-L108

This is done by using https://github.com/uhthomas/tesla/blob/master/cmd/login/main.go, saving the resulting JSON dictionary to disk and reading those files back.

This was done in https://github.com/bogosj/go-tesla-go/commit/0734ca75a1aaca030779f96ebdee98c1483b03e0

It seems sub-optimal to rely on another binary to bootstrap usage. We could bring uthomas' code into this project and create a method to persist the result of a login to a single token file, but there are likely other options worth considering.

@michaelharo

andig commented 3 years ago

I‘m doing the same without the separate binary in https://github.com/andig/evcc/blob/master/vehicle/tesla/client.go. We had discussed if the uthomas api could be further abstracted.

michaelharo commented 3 years ago

Regarding getting the token, I did something similar to your client.go but I promptthe user (me) to go to a URL and provide the redirect URL back to the program so that MFA works. This would probably be easier to use if it MITMd the session but I'm unclear if that's a good thing to contribute to the library.

andig commented 3 years ago

For a library it should be user‘s choice. In my case- server app- I cannot redirect the user. Could be eased by supplementing the lib with something in cmd at least as example.

michaelharo commented 3 years ago

Lets leave login out of scope for this issue.

If we agree that we should switch to the oauth2 library (I think we do?) then whatever the interface for NewClient ends up we can remove a bunch of the code in client. I have a locally modified client.go that does this:

var Endpoint = oauth2.Endpoint{
  AuthURL:  "https://auth.tesla.com/oauth2/v3/authorize",
  TokenURL: "https://auth.tesla.com/oauth2/v3/token",
}

func NewClientWithOAuth2Token(ctx context.Context, auth *Auth, tok *oauth2.Token) (*Client, error) {
  conf := &oauth2.Config{
    ClientID:     auth.ClientID,
    ClientSecret: auth.ClientSecret,
    Endpoint:     Endpoint,
  }
  client := &Client{
    Auth:         auth,
    HTTP:         conf.Client(ctx, tok),
    BaseURL:      BaseURL,
    StreamingURL: StreamingURL,
  }
  return client, nil
}

I'm unclear if it's better to require passing in an oauth2 token or an http client that was created by the oauth2 library. I'm leaning towards the model above because it ensures that oauth2 is in use. I could also see being required to pass in an oauth2.Config instead of Auth since they're basically the same thing at this point if you ignore email address and password. How important is it for the tesla library to know email address and password? It looks like password isn't used and email is only used once by streaming.

I propose we delete all the existing NewClient functions, rip out a bunch of the oauth code that's no longer needed and replace with

// Endpoint is the oauth2 endpoint used by the Tesla API.
var Endpoint = oauth2.Endpoint{
  AuthURL:  "https://auth.tesla.com/oauth2/v3/authorize",
  TokenURL: "https://auth.tesla.com/oauth2/v3/token",
}

// NewConfig populates a partial oauth2 configuration that may be good enough depending on how it's used.
// For example if using it to login with config.AuthURL(...), RedirectURL should be set. 
func NewConfig(clientID, clientSecret string) *oauth2.Config {
  return &oauth2.Config{
    ClientID:     clientID,
    ClientSecret: clientSecret,
    Endpoint:     Endpoint,
    Scopes:       []string{"openid", "email", "offline_access"},
  }
}

func NewClient(ctx context.Context, conf *oauth2.Config, tok *oauth2.Token) (*Client, error) {
  client := &Client{
    http:         conf.Client(ctx, tok),
    baseURL:      BaseURL,
    streamingURL: StreamingURL,
  }
  return client, nil
 }

Users of this API could use it as such

conf := tesla.NewConfig("clientID", "secret")  // or whatever they want to do to create an oauth2 client
tok, err := LoadToken(tokenPath) // reads existing token json from disk (external to this library)
client, err := tesla.NewClient(ctx, conf, tok)

@andig has a modified version of this library that uses oauth2.TokenSource. I'm unclear why TokenSource instead of Token.

Thoughts?

andig commented 3 years ago

I'm unclear why TokenSource instead of Token.

Good morning. The TokenSource handles the Token refresh which we can then strip from this library, too. All that needs be done is using https://pkg.go.dev/golang.org/x/oauth2#NewClient together with the provided context. The context allows the consumer to inject their own HTTP client (e.g. for logging). Thus we can also get rid of the custom client.

Do we even need the Auth object then?

michaelharo commented 3 years ago

Token also refreshes (https://pkg.go.dev/golang.org/x/oauth2#Config.Client). I guess the difference is that with TokenSource you don't need to provide an oauth2.Config and instead it's up to the author to implement the TokenSource interface?

I don't think we need the Auth object, that was one my my questions in my last post.