Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add v3 token refresh + v2 long-lived token generation (optional) #38

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

one4many
Copy link

In the light of timdorr/tesla-api#260 (former ticket timdorr/tesla-api#215) and issue #31 I created this patch which deprecates email/password logins for now and introduces Tesla's oauth V3 token handling including very short-lived 'code' based tokens.
More but not very precise information can be found here: https://tesla-api.timdorr.com/api-basics/authentication#step-2-obtain-an-authorization-code

V3 access tokens are only valid for 300 seconds which is not suitable for most server applications, although they get renewed automatically I an added the option short_lived_v3token which can be set to False (default True) in order to created a 'old' V2 token which is valid for the usual 45 days.

…n of creating long lived v2 tokens. Add code based token retrieval option
@Hellowlol
Copy link

Hellowlol commented Jan 30, 2021

Why remove support for login using username and password? It a much simpler solution then the one you provided. I just skimmed the code as I don’t really know that much about oath but replacing it with something that ppl has to manually create seems a very bad solution imo.

With that being said, it’s great for ppl has activated two factor.

@one4many
Copy link
Author

email/username and password is deprecated because Tesla deprecated in their API a while ago (around the time I opened this issue) and they enforce it since yesterday (29.01.21).
When you try to login in that way and create a token you will get:
400: {"response": "endpoint_deprecated:_please_update_your_app."}

@Hellowlol
Copy link

Oh i havnt noticed a error yet. Ok, thanks explains it.

@Dreamsorcerer
Copy link
Collaborator

Dreamsorcerer commented Jan 30, 2021

I'll try and test it out tomorrow and do a proper review. But, initial thoughts on first glance:
Drop the user/password arguments if they are no longer useful, no point in raising an exception (we'll probably make it a major version bump).

We aim to minimise the amount of work and knowledge needed for a developer. So, they should be able to just pass the url/form to the user and have it work automatically. Which means automatically converting to a 45 day access token. The developer shouldn't need to know anything about different token versions and codes.

I'm a bit concerned you mention the refresh token no longer working. I'd be surprised if there's no way to refresh now. If we really can't refresh a long-lived token, then we'll need to try and think of some ways to work around this (like maybe having an option to use a saved user/password and handling the form in the library in order to refresh).

@timdorr
Copy link

timdorr commented Jan 30, 2021

I would note that email/password login still works. You go through the authorize endpoint to convert that to an authorization code, which you then exchange for a short-lived bearer token and the eventual Owner API access token. It's just an HTML response instead of JSON. No magic involved, just parsing out some bits with regex and POSTing that to the right endpoint.

@Dreamsorcerer
Copy link
Collaborator

Thanks, we'll have to make sure that's still working in this PR then.

@Dreamsorcerer
Copy link
Collaborator

Regarding token refresh: timdorr/tesla-api#260 (comment)

If this results in us needing to handle 2 tokens in the client, these should be stored in a single object, so the developer's application can continue to just store the object we pass to it and not care about the details.

@one4many
Copy link
Author

@timdorr interesting point. I have MFA enabled which makes it hard to implement in the library unless we play around with server side JS libs like pyppeteer (puppeteer port for python). Without MFA it might work in a stable way unless Tesla keeps changing the HTML on the auth pages in question. However Authentication without a 2nd factor makes the lib unsafe to use (at least in my eyes). Storing the MFA secret in any kind of app/lib (unless it is the token generating helper app itself) is out of the question as well.

@Dreamsorcerer Unfortunately there is no expire timestamp in the (new) V3 refresh_token. It might be the case that the V3 refresh token expires before the V2 (long-lived) access token does. Which means the user has to re-authenticate after 45 days anyway.

One way forward could be to use the V2 access token for the lib/app to interact with the Tesla API servers. While simultaneously refreshing the V3 tokens (access and refresh) on a daily basis (or later/longer depending on the actual lifetime of the V3 refresh token, once we found out), just to keep a valid pair around for creating a new V2 access token, coming close to 45 days. Unless refreshing V3 tokens invalidates derived V2 access tokens (needs to tested).

Btw, I choose to add the 'webcode' option because the code can be generated in an app / browser and sent to the server (running this library) in order to generate V2/3 tokens. This is at least how we do at www.otrky.com

@timdorr
Copy link

timdorr commented Jan 30, 2021

Refresh tokens don't have any expiration typically. The spec doesn't mention having them expire at least, and implementing them as expiring is not typical, at least with any OAuth provider I've ever used.

You do, however, need to bootstrap the refresh token lifecycle with an Authorization Code flow, which involves the email/password. I would not recommend using a full browser engine like Puppeteer/Chromium, since that won't operate in all environments. I would just parse out the HTML and keep track of the session cookie for the first two requests.

Also, nothing has changed in terms of needing to store the login credentials for the account to be able to use the API. That's how it's been for the last 7 years, so I don't think that's going to change any time soon. It's certainly non-ideal, but it's currently the only way to use it without some accommodation by Tesla.

@Dreamsorcerer
Copy link
Collaborator

Refresh tokens don't have any expiration typically. The spec doesn't mention having them expire at least, and implementing them as expiring is not typical, at least with any OAuth provider I've ever used.

I thought the refresh tokens typically expired after 2x the token expiry time (i.e. 90 days in the old version). That's why I thought the new ones might expire after only 10 mins.

Also, nothing has changed in terms of needing to store the login credentials for the account to be able to use the API. That's how it's been for the last 7 years, so I don't think that's going to change any time soon. It's certainly non-ideal, but it's currently the only way to use it without some accommodation by Tesla.

I'm not sure I follow. The current implementation doesn't store credentials, it just saves the token and uses the refresh token to keep that alive indefinitely (refreshing after 45 days has expired). I'm hoping we can still do this.

@Dreamsorcerer
Copy link
Collaborator

@Dreamsorcerer Unfortunately there is no expire timestamp in the (new) V3 refresh_token. It might be the case that the V3 refresh token expires before the V2 (long-lived) access token does. Which means the user has to re-authenticate after 45 days anyway.

I think at the moment we should assume the refresh token is long lived, in order to keep the current behaviour. If we find out it's not, we can adapt the code later.

@one4many
Copy link
Author

I think at the moment we should assume the refresh token is long lived, in order to keep the current behaviour. If we find out it's not, we can adapt the code later.

I totally agree

@timdorr
Copy link

timdorr commented Jan 31, 2021

I thought the refresh tokens typically expired after 2x the token expiry time (i.e. 90 days in the old version). That's why I thought the new ones might expire after only 10 mins.

Nope, there's no definition of expiration of refresh tokens in the spec. As such, it's not typical to do so. I'm not aware of an expiration on Tesla's API, but I'm not sure many folks have tested it, given the timeframe involved.

I'm not sure I follow. The current implementation doesn't store credentials, it just saves the token and uses the refresh token to keep that alive indefinitely (refreshing after 45 days has expired). I'm hoping we can still do this.

Since we don't know if Tesla may invalidate credentials in the future, storing the raw email/password combo is the only way to ensure automated recovery in the case of some mass expiry event (as was the case when they switched this API over). It's also the only reasonable way to get users to provide credentials to the API, as the particulars of OAuth usually go over the heads of most folks. As such, I would highly recommend keeping functionality for automating email/password auth in this library.

@Dreamsorcerer
Copy link
Collaborator

Since we don't know if Tesla may invalidate credentials in the future, storing the raw email/password combo is the only way to ensure automated recovery in the case of some mass expiry event (as was the case when they switched this API over).

True, but there are security issues with storing the credentials (something many online services try to guarantee they are not doing). Most applications can deal with the odd expiry as long as it's only likely to happen every year or less, requiring users to reauthorise on such an occasion.

@Dreamsorcerer
Copy link
Collaborator

And if MFA is enabled, then storing the credentials isn't going to help anyway. Best if we can have an approach that works as well with both approaches.

@Dreamsorcerer
Copy link
Collaborator

Nope, there's no definition of expiration of refresh tokens in the spec. As such, it's not typical to do so. I'm not aware of an expiration on Tesla's API, but I'm not sure many folks have tested it, given the timeframe involved.

This seems highly questionable if true. What's the point of an expiry on the token in the first place, if you can always renew that token after any amount of time (just remove the expiry on the access token...). A quick search suggests than many implementations do expire refresh tokens though.

@Dreamsorcerer
Copy link
Collaborator

I've created a new PR to replace this one, which keeps the old interface working. #39

If you've got some time, please try it out.

I still need to clean it up, handle refresh tokens and ensure that old tokens are handled gracefully.

@Dreamsorcerer
Copy link
Collaborator

Also worth noting that I've apparently locked myself out of my account by testing the error handling of wrong login details. So, be careful not to use your own email if you are testing that.

@one4many one4many changed the title Remove email and password logins. Add v3 token refresh with the optio… Add v3 token refresh + v2 long-lived token generation (optional) Jan 31, 2021
one4many referenced this pull request in dracoventions/TWCManager Feb 2, 2021
@Hellowlol
Copy link

@one4many lol, i have done the same? How long time did it go before you got access again? Oh well, atleast it worked before i got blocked. I added support for mfa, refresh handling, long lived token and refreshing them. Only thing i havnt done so far was a bit of error handling and figuring out what to do with token in the constructer. I doesnt really hit anymore as with that one along it isnt possible to handle anything else. What the use case for keeping the token?

@fracai fracai mentioned this pull request Aug 30, 2021
…entTypeError exception, because Tesla servers sometimes have upstream errors and answer with the incorrect content type. Add ability to change charging amps of vehilces.
@one4many
Copy link
Author

Removing V2 token code completely since they have finally retired.
Added ability to control charging amps.
Added ability to force a access token refresh
Catch content-type exception as Tesla's server tend to return with a text/plain from time to time screwing this up
Added ability to store addition info the token-renewal-callback function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants