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

[oauth] Fix access token serialization/deserialization #3083

Merged
merged 1 commit into from
Sep 15, 2022

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Sep 12, 2022

This fixes a regression of #3066 where serialization and deserialization to and from storage was broken after migrating from LocalDateTime to Instant:

2022-09-11 22:18:41.210 [ERROR] [lient.internal.OAuthStoreHandlerImpl] - Unable to deserialize json, discarding AccessTokenResponse.  Please check json against standard or with oauth provider. json:
{
  "accessToken": "xxx",
  "tokenType": "Bearer",
  "expiresIn": 2592000,
  "refreshToken": "xxx",
  "createdOn": "2022-08-14T21:21:05.568991"
}
com.google.gson.JsonSyntaxException: java.lang.IllegalStateException: Expected BEGIN_OBJECT but was STRING at line 6 column 17 path $.createdOn
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:226) ~[bundleFile:?]
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$1.read(ReflectiveTypeAdapterFactory.java:131) ~[bundleFile:?]
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:222) ~[bundleFile:?]
	at com.google.gson.Gson.fromJson(Gson.java:963) ~[bundleFile:?]
	at com.google.gson.Gson.fromJson(Gson.java:928) ~[bundleFile:?]
	at com.google.gson.Gson.fromJson(Gson.java:877) ~[bundleFile:?]
	at com.google.gson.Gson.fromJson(Gson.java:848) ~[bundleFile:?]
	at org.openhab.core.auth.oauth2client.internal.OAuthStoreHandlerImpl$StorageFacade.get(OAuthStoreHandlerImpl.java:283) [bundleFile:?]
	at org.openhab.core.auth.oauth2client.internal.OAuthStoreHandlerImpl.loadAccessTokenResponse(OAuthStoreHandlerImpl.java:119) [bundleFile:?]
	at org.openhab.core.auth.oauth2client.internal.OAuthClientServiceImpl.getAccessTokenResponse(OAuthClientServiceImpl.java:326) [bundleFile:?]
	at org.openhab.binding.mielecloud.internal.auth.OpenHabOAuthTokenRefresher.getAccessTokenFromStorage(OpenHabOAuthTokenRefresher.java:121) [bundleFile:?]
	at org.openhab.binding.mielecloud.internal.handler.MieleBridgeHandler.tryInitializeWebservice(MieleBridgeHandler.java:232) [bundleFile:?]
	at org.openhab.binding.mielecloud.internal.handler.MieleBridgeHandler.initializeWebservice(MieleBridgeHandler.java:143) [bundleFile:?]
	at org.openhab.binding.mielecloud.internal.handler.MieleBridgeHandler.initialize(MieleBridgeHandler.java:120) [bundleFile:?]
	at jdk.internal.reflect.GeneratedMethodAccessor113.invoke(Unknown Source) ~[?:?]
	at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:?]
	at java.lang.reflect.Method.invoke(Method.java:566) ~[?:?]
	at org.openhab.core.internal.common.AbstractInvocationHandler.invokeDirect(AbstractInvocationHandler.java:154) [bundleFile:?]
	at org.openhab.core.internal.common.Invocation.call(Invocation.java:52) [bundleFile:?]
	at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
	at java.lang.Thread.run(Thread.java:829) [?:?]
Caused by: java.lang.IllegalStateException: Expected BEGIN_OBJECT but was STRING at line 6 column 17 path $.createdOn
	at com.google.gson.stream.JsonReader.beginObject(JsonReader.java:384) ~[bundleFile:?]
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:215) ~[bundleFile:?]
	... 22 more

Value will now correctly be serialized as Instant into the store, and deserialization supports both Instant and LocalDateTime (for backwards compatibility).

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur force-pushed the 3066-oauth-fix-deserialization branch from 691bb59 to 25b3d84 Compare September 12, 2022 19:59
@lolodomo
Copy link
Contributor

I believe there is a kind of urgency to review & merge this fix.

@J-N-K
Copy link
Member

J-N-K commented Sep 15, 2022

Thanks.

@J-N-K J-N-K merged commit 27c2887 into openhab:main Sep 15, 2022
@J-N-K J-N-K added this to the 3.4 milestone Sep 15, 2022
@J-N-K J-N-K added the bug An unexpected problem or unintended behavior of the Core label Sep 15, 2022
@lolodomo
Copy link
Contributor

@jlaur : do you know if the bug was included in a 3.4 milestone?

@jlaur
Copy link
Contributor Author

jlaur commented Sep 15, 2022

@lolodomo - it was unfortunately introduced in 3.4M2.

@lolodomo
Copy link
Contributor

Ok, we know what to say in case the problem would be reported in the community forum.

I saw myself an exception the last time I installed a snapshot.

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/mercedes-me-binding/136852/55

@lolodomo
Copy link
Contributor

lolodomo commented Sep 20, 2022

@jlaur : I just installed snaspshot 3089 generated yesterday and at startup, I still see that error:

Edit: it could be an old data from the JSON DB. I try to delete it and restart.
Edit2: seems OK finally (I suppressed the file from the main directory but not in the backup subdirectory).

@jlaur
Copy link
Contributor Author

jlaur commented Sep 20, 2022

I just installed snaspshot 3089 generated yesterday and at startup, I still see that error:

Edit: it could be an old data from the JSON DB. I try to delete it and restart. Edit2: seems OK finally (I suppressed the file from the main directory but not in the backup subdirectory).

I'm wondering how that data was created, and with which version:

"createdOn": {
    "seconds": 1663660427,
    "nanos": 311625000
  }

That timestamp is 20.09.2022 09:53:47 CET (with DST), i.e. this morning. Current snapshot version:

.registerTypeAdapter(Instant.class,
(JsonSerializer<Instant>) (date, type,
jsonSerializationContext) -> new JsonPrimitive(date.toString()))

uses Instant.toString() for serialization, which should generate something like 2022-09-20T07:53:47.311625Z.

Since you installed the snapshot yesterday and this JSON seems to have been generated today, I don't have a good feeling about this. Which binding do you use, and is it possible somehow to reproduce? EDIT: Okay, now I saw that it's homeconnect. Is that also current shapshot version?

@lolodomo
Copy link
Contributor

Since you installed the snapshot yesterday and this JSON seems to have been generated today, I don't have a good feeling about this. Which binding do you use, and is it possible somehow to reproduce? EDIT: Okay, now I saw that it's homeconnect. Is that also current shapshot version?

Sorry, don't loose your time, I think there is no problem, it was my file from the previous version which was read. The new version has just been installed one hour ago (while the home-connect token was probably updated this morning by my old snapshot).
After removing all files containing oAuth2 tokens, restarting OH, checking my 2 bindings using oAuth2 work, and then restarting OH again, I see no errors.

@jlaur
Copy link
Contributor Author

jlaur commented Sep 20, 2022

Sorry, don't loose your time, I think there is no problem, it was my file from the previous version which was read. The new version has just been installed one hour ago (while the home-connect token was probably updated this morning by my old snapshot).
After removing all files containing oAuth2 tokens, restarting OH, checking my 2 bindings using oAuth2 work, and then restarting OH again, I see no errors.

Okay, thanks. Still, I'm wondering how this was created, and if this scenario has to be supported also. Serialized tokens generated before #3066 shouldn't look like this, and they should also be correctly deserialized now. Tokens generated after this PR should also not look like this, and should also be correctly deserialized. Perhaps some permutation of pre/post #3066/#3083 vs. core/addons can cause this.

@lolodomo
Copy link
Contributor

Here is how it looks now in the DB:

  "ecowatt:signals:monecowatt.AccessTokenResponse": {
    "class": "java.lang.String",
    "value": "{\n  \"accessToken\": \"xxx\",\n  \"tokenType\": \"Bearer\",\n  \"expiresIn\": 7200,\n  \"createdOn\": \"2022-09-20T18:24:22.923094Z\"\n}"
  },
...
  "homeconnect:api_bridge:bridge.AccessTokenResponse": {
    "class": "java.lang.String",
    "value": "{\n  \"accessToken\": \"yyy\",\n  \"tokenType\": \"Bearer\",\n  \"expiresIn\": 86400,\n  \"refreshToken\": \"zzz\",\n  \"scope\": \"Washer-Control Settings IdentifyAppliance Hood-Control Monitor WasherDryer-Control Dishwasher-Control Dryer-Control CleaningRobot-Control Oven-Control CoffeeMaker-Control\",\n  \"createdOn\": \"2022-09-20T18:26:24.357219Z\"\n}"
  },

@jlaur jlaur deleted the 3066-oauth-fix-deserialization branch September 27, 2022 20:43
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/oh4-0-0-snapshot-and-m1-google-tts-exception/145046/6

splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
GitOrigin-RevId: 27c2887
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 the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants