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

[Tesla] Add SSO handler to authenticate against Tesla SSO service #10259

Merged
merged 6 commits into from
Mar 1, 2021

Conversation

cguedel
Copy link
Contributor

@cguedel cguedel commented Feb 28, 2021

Signed-off-by: Christian Güdel [email protected]

Fixes #10016

Adds authentication against the Tesla SSO service as described here https://tesla-api.timdorr.com/api-basics/authentication

@kaikreuzer
Copy link
Member

Looks like a productive weekend, thanks!
Will try to (start to) test & review tonight.

@kaikreuzer kaikreuzer added the bug An unexpected problem or unintended behavior of an add-on label Feb 28, 2021
@kaikreuzer kaikreuzer self-requested a review February 28, 2021 16:09
@kaikreuzer
Copy link
Member

First feedback: Creating an account thing (without filling in credentials or token) in the UI immediately results in two errors in the log:

17:26:27.024 [-thingHandler-1] ERROR o.o.b.t.i.handler.TeslaSSOHandler:253 - An exception occurred while invoking a HTTP request: 'java.lang.IllegalArgumentException: Buffering capacity 16384 exceeded'
17:26:27.024 [-thingHandler-1] ERROR o.o.b.t.i.handler.TeslaSSOHandler:178 - Failed to obtain code from SSO login page when submitting form, response status code: no response

@kaikreuzer
Copy link
Member

Second feedback: Vehicle discovery crashes with an exception:

17:33:47.439 [-thingHandler-3] WARN  o.o.c.i.c.WrappedScheduledExecutorService:67 - Scheduled runnable ended with an exception: 
javax.ws.rs.ProcessingException: java.io.IOException: IOException invoking https://owner-api.teslamotors.com/api/1/vehicles: HTTPS hostname wrong:  should be <owner-api.teslamotors.com>
	at org.apache.cxf.jaxrs.client.AbstractClient.checkClientException(AbstractClient.java:633)
	at org.apache.cxf.jaxrs.client.AbstractClient.preProcessResult(AbstractClient.java:607)
	at org.apache.cxf.jaxrs.client.WebClient.doResponse(WebClient.java:1145)
	at org.apache.cxf.jaxrs.client.WebClient.doChainedInvocation(WebClient.java:1082)
	at org.apache.cxf.jaxrs.client.WebClient.doInvoke(WebClient.java:927)
	at org.apache.cxf.jaxrs.client.WebClient.doInvoke(WebClient.java:896)
	at org.apache.cxf.jaxrs.client.WebClient.invoke(WebClient.java:461)
	at org.apache.cxf.jaxrs.client.SyncInvokerImpl.method(SyncInvokerImpl.java:135)
	at org.apache.cxf.jaxrs.client.SyncInvokerImpl.method(SyncInvokerImpl.java:130)
	at org.apache.cxf.jaxrs.client.SyncInvokerImpl.get(SyncInvokerImpl.java:50)
	at org.apache.cxf.jaxrs.client.spec.InvocationBuilderImpl.get(InvocationBuilderImpl.java:88)
	at org.openhab.binding.tesla.internal.handler.TeslaAccountHandler.queryVehicles(TeslaAccountHandler.java:209)
	at org.openhab.binding.tesla.internal.handler.TeslaAccountHandler.lambda$1(TeslaAccountHandler.java:155)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: java.io.IOException: IOException invoking https://owner-api.teslamotors.com/api/1/vehicles: HTTPS hostname wrong:  should be <owner-api.teslamotors.com>
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
	at org.apache.cxf.transport.http.HTTPConduit$WrappedOutputStream.mapException(HTTPConduit.java:1402)
	at org.apache.cxf.transport.http.HTTPConduit$WrappedOutputStream.close(HTTPConduit.java:1386)
	at org.apache.cxf.io.AbstractWrappedOutputStream.close(AbstractWrappedOutputStream.java:77)
	at org.apache.cxf.transport.AbstractConduit.close(AbstractConduit.java:56)
	at org.apache.cxf.transport.http.HTTPConduit.close(HTTPConduit.java:673)
	at org.apache.cxf.interceptor.MessageSenderInterceptor$MessageSenderEndingInterceptor.handleMessage(MessageSenderInterceptor.java:63)
	at org.apache.cxf.phase.PhaseInterceptorChain.doIntercept(PhaseInterceptorChain.java:308)
	at org.apache.cxf.jaxrs.client.AbstractClient.doRunInterceptorChain(AbstractClient.java:705)
	at org.apache.cxf.jaxrs.client.WebClient.doChainedInvocation(WebClient.java:1081)
	... 15 common frames omitted
Caused by: java.io.IOException: HTTPS hostname wrong:  should be <owner-api.teslamotors.com>
	at java.base/sun.net.www.protocol.https.HttpsClient.checkURLSpoofing(HttpsClient.java:648)
	at java.base/sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:581)
	at java.base/sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:185)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1581)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1509)
	at java.base/java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:527)
	at java.base/sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:329)
	at org.apache.cxf.transport.http.URLConnectionHTTPConduit$URLConnectionWrappedOutputStream$2.run(URLConnectionHTTPConduit.java:377)
	at org.apache.cxf.transport.http.URLConnectionHTTPConduit$URLConnectionWrappedOutputStream$2.run(URLConnectionHTTPConduit.java:373)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at org.apache.cxf.transport.http.URLConnectionHTTPConduit$URLConnectionWrappedOutputStream.getResponseCode(URLConnectionHTTPConduit.java:373)
	at org.apache.cxf.transport.http.HTTPConduit$WrappedOutputStream.doProcessResponseCode(HTTPConduit.java:1599)
	at org.apache.cxf.transport.http.HTTPConduit$WrappedOutputStream.handleResponseInternal(HTTPConduit.java:1627)
	at org.apache.cxf.transport.http.HTTPConduit$WrappedOutputStream.handleResponse(HTTPConduit.java:1572)
	at org.apache.cxf.transport.http.HTTPConduit$WrappedOutputStream.close(HTTPConduit.java:1373)
	... 22 common frames omitted

@cguedel
Copy link
Contributor Author

cguedel commented Feb 28, 2021

Interesting... could be that you get throttled by Tesla's WAF and the logon page now looks different. Will need to see if I can reproduce that.

The second thing looks very odd and not really related to the changes, more like a certificate issue?

@kaikreuzer
Copy link
Member

Will try to investigate myself as well. FTR: I am running the binding from within the IDE.

@cguedel
Copy link
Contributor Author

cguedel commented Feb 28, 2021

I was doing the same, Eclipse on Windows with Java 11, FWIW.

@cguedel
Copy link
Contributor Author

cguedel commented Feb 28, 2021

I know there is still one problem with the SSO handler. In case you successfully logged in on the SSO endpoint using username/password and try this again, it directly skips the sign in step upon fetching the login form. This is currently not (yet) handled by the handler.

But never saw those two issues/exceptions you just posted

@kaikreuzer
Copy link
Member

The exceptions are not fully reproducible. Now the discovery starts and I can see in the debugger that it has the correct vehicleArray, but the call in this line results in an HTTP 408, which again makes the discovery return no results. Pretty weird...

@cguedel
Copy link
Contributor Author

cguedel commented Feb 28, 2021

Ah, the 408 means that the vehicle is asleep and you need to wake it up first. That's a trap that was in there before and I also fell into it the first time the discovery of the account actually worked :)

@kaikreuzer
Copy link
Member

Thanks - indeed, waking up the vehicle made it being discovered. Maybe something that could be improved, but not within this PR.
So good news is that account+vehicle things are now online. Will see if it stays like that and will meanwhile do the code review!

@kaikreuzer
Copy link
Member

Another exception that asks to be fixed:

19:18:47.000 [-thingHandler-3] ERROR o.o.b.t.i.h.TeslaAccountHandler:472 - An exception occurred while executing a request to the vehicle: 'null'
java.lang.NullPointerException: null
	at org.openhab.binding.tesla.internal.handler.TeslaVehicleHandler.getVehicleId(TeslaVehicleHandler.java:227)
	at org.openhab.binding.tesla.internal.handler.TeslaAccountHandler$Request.run(TeslaAccountHandler.java:466)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at org.openhab.binding.tesla.internal.throttler.QueueChannelThrottler.lambda$0(QueueChannelThrottler.java:45)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code wise it looks excellent! Looking forward to more contributions from you ;-)
A few small change requests below.

cguedel added 2 commits March 1, 2021 08:05
Skip the form submit in this case and directly proceed to exchange the
received authorization code for an access token

Signed-off-by: Christian Güdel <[email protected]>
Replace random string generation with pure java

Signed-off-by: Christian Güdel <[email protected]>
@cguedel
Copy link
Contributor Author

cguedel commented Mar 1, 2021

@kaikreuzer Thanks for the quick review! I added the changes requested. Not sure what to do about the exceptions though. The very first one with the buffer capacity error seems very weird. The others I would say were already in :)

Annotate TeslaSSOHandler and methods
Change most of logging calls to debug
Documentation changes

Signed-off-by: Christian Güdel <[email protected]>
@cguedel cguedel force-pushed the 10016-tesla-sso-auth branch from 0be21b0 to 9dc1762 Compare March 1, 2021 07:28
@kaikreuzer
Copy link
Member

The very first one with the buffer capacity error seems very weird.

I haven't seen it ever again, so let's ignore it.
The "HTTPS hostname wrong" occurs quite regular for me, though, and it makes the vehicle Thing go OFFLINE for a while.
I traced it down to HttpsClient.checkURLSpoofing(), where there are no peer certificates available and the session is marked as invalidated. No idea how this happens, though.

I am fine to merge it like it is, though, and we can see what testing by others bring up. I might do a follow-up PR for handling this exception gracefully then.

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks, let's get that into the SNAPSHOT distro and have people test it!

@kaikreuzer kaikreuzer merged commit 62e829f into openhab:main Mar 1, 2021
@kaikreuzer kaikreuzer added this to the 3.1 milestone Mar 1, 2021
@kaikreuzer
Copy link
Member

FTR: The fix is now included in distro build 2230 - would be great if everyone could test it!

@kaikreuzer
Copy link
Member

@cguedel For the hostname exception, I have created #10264 for further tracking. It also occurs on my production system, so it wasn't due to IDE or something like that.

Checking my log file, it occurs as often as every 4 minutes, so I guess we'll need to do something about it...

@cguedel
Copy link
Contributor Author

cguedel commented Mar 1, 2021

I'll check if I can see the same on my side

@phuongpham
Copy link

kaikreuzer pushed a commit that referenced this pull request Mar 2, 2021
@kaikreuzer kaikreuzer added the patch A PR that has been cherry-picked to a patch release branch label Mar 2, 2021
@kaikreuzer
Copy link
Member

Cherry-picked it to 3.0.x.

@mrphil81
Copy link

mrphil81 commented Mar 3, 2021

@cguedel and @kaikreuzer Thank you very much for your effort! I installed the snapshot jar file on my openhab version 3.0.1. Works like a charm again since 2 days. I also recognized the 408 exception (timeout due to sleeping vehicle), but it wakes up within a short time. I use it mainly to start/stop charging and display some vehicle information.
Thanks again, highly appreciated!

themillhousegroup pushed a commit to themillhousegroup/openhab2-addons that referenced this pull request May 10, 2021
computergeek1507 pushed a commit to computergeek1507/openhab-addons that referenced this pull request Jul 13, 2021
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
@ScanxTaz
Copy link

ScanxTaz commented Jan 6, 2022

Stupid question, but is there any update on this binding ?

marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on patch A PR that has been cherry-picked to a patch release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tesla] Authentication no longer working / getting blocked
5 participants