Add extra fields support to oAuth AccessTokenResponse by lo92fr · Pull Request #5355 · openhab/openhab-core

lo92fr · 2026-02-14T08:02:32Z

Add extra fields support to oAuth AccessTokenResponse

Description

This PR is about adding an extraFields maps in the OAuth AccessTokenResponse.
Some oAuth implementation return extra fields beside the accessToken/refreshToken/scope/state standard field.

With this change, this extraFields will be expose in the token response, giving a chance to use them.

The motivation beside this is for the new Smarthings Binding. Samsung oAuth implementation add en extra clientId field in the response, and I need to get this to handle smartthings stuff properly.

The changes are mainly in 3 files:

AccesTokenResponse.java to add the extraFields map.
A new AccessTokenResponseExtraFieldsAdapterFactory.java to handle decoding of extraFields from json response.
OAuthConnector to register the TypeAdapter on the gson object.

Signed-off-by: Laurent ARNAL <laurent@clae.net>

Copilot

Pull request overview

This pull request adds support for capturing extra (non-standard) fields from OAuth access token responses. This is necessary because some OAuth implementations, such as Samsung SmartThings, return additional fields beyond the RFC 6749 standard fields (access_token, token_type, expires_in, refresh_token, scope, state).

Changes:

Added an extraFields Map to AccessTokenResponse to store non-standard fields
Created AccessTokenResponseExtraFieldsAdapterFactory to handle custom JSON deserialization via Gson
Registered the new type adapter factory in OAuthConnector to enable extra field capture

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 11 comments.

File	Description
AccessTokenResponseExtraFieldsAdapterFactory.java	New Gson TypeAdapterFactory that extracts non-standard fields from OAuth JSON responses into a Map
AccessTokenResponse.java	Added extraFields Map field with getter/setter methods to store additional OAuth response fields
OAuthConnector.java	Registered the new AccessTokenResponseExtraFieldsAdapterFactory with the Gson builder

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.../org.openhab.core/src/main/java/org/openhab/core/auth/client/oauth2/AccessTokenResponse.java

...n/java/org/openhab/core/auth/client/oauth2/AccessTokenResponseExtraFieldsAdapterFactory.java

.../org.openhab.core/src/main/java/org/openhab/core/auth/client/oauth2/AccessTokenResponse.java

...n/java/org/openhab/core/auth/client/oauth2/AccessTokenResponseExtraFieldsAdapterFactory.java

.../org.openhab.core/src/main/java/org/openhab/core/auth/client/oauth2/AccessTokenResponse.java

...n/java/org/openhab/core/auth/client/oauth2/AccessTokenResponseExtraFieldsAdapterFactory.java

.../org.openhab.core/src/main/java/org/openhab/core/auth/client/oauth2/AccessTokenResponse.java

...n/java/org/openhab/core/auth/client/oauth2/AccessTokenResponseExtraFieldsAdapterFactory.java

.../org.openhab.core/src/main/java/org/openhab/core/auth/client/oauth2/AccessTokenResponse.java

Signed-off-by: Laurent ARNAL <laurent@clae.net>

lo92fr · 2026-02-26T15:41:20Z

Hello @wborn,

Thanks for the copilot review.
I've pushed the fixes for it.

Laurent.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.../org.openhab.core/src/main/java/org/openhab/core/auth/client/oauth2/AccessTokenResponse.java

...h.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthConnector.java

...test/java/org/openhab/core/auth/oauth2client/internal/AccessTokenResponseExtraFieldTest.java

...n/java/org/openhab/core/auth/client/oauth2/AccessTokenResponseExtraFieldsAdapterFactory.java

...test/java/org/openhab/core/auth/oauth2client/internal/AccessTokenResponseExtraFieldTest.java

...n/java/org/openhab/core/auth/client/oauth2/AccessTokenResponseExtraFieldsAdapterFactory.java

.../org.openhab.core/src/main/java/org/openhab/core/auth/client/oauth2/AccessTokenResponse.java

Signed-off-by: Laurent Arnal <laurent@clae.net>

lo92fr · 2026-03-05T14:20:18Z

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Push the fixes this morning about copilot review of 1/03.

Laurent

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...n/java/org/openhab/core/auth/client/oauth2/AccessTokenResponseExtraFieldsAdapterFactory.java

.../org.openhab.core/src/main/java/org/openhab/core/auth/client/oauth2/AccessTokenResponse.java

…nse::toString() Signed-off-by: Laurent Arnal <laurent@clae.net>

lo92fr · 2026-03-06T08:13:01Z

@kaikreuzer, @wborn,

I think this could now be merged.

Thanks,
Laurent.

kaikreuzer

Thanks @lo92fr!
A few final review comments NOT coming from a bot, but from a person for a change. 😉

kaikreuzer · 2026-03-06T21:36:08Z

.../org.openhab.core/src/main/java/org/openhab/core/auth/client/oauth2/AccessTokenResponse.java

+import java.util.Map;
 import java.util.Objects;

+import org.eclipse.jdt.annotation.NonNull;


If you want to introduce null annotations with the new fields/method, you should make the whole class consistently use null annotations - and this means you should annotate it with @NonNullByDefault and then rather add @Nullable annotations at the appropriate places.

kaikreuzer · 2026-03-06T21:37:12Z

.../org.openhab.core/src/main/java/org/openhab/core/auth/client/oauth2/AccessTokenResponse.java

+     * warning : the toString() function may returns sensitive information that should not go into the log.
+     *


Suggested change

* warning : the toString() function may returns sensitive information that should not go into the log.

*

* Warning: the toString() function may returns sensitive information that should not go into the log.

kaikreuzer · 2026-03-06T21:37:40Z

...n/java/org/openhab/core/auth/client/oauth2/AccessTokenResponseExtraFieldsAdapterFactory.java

+ * specification. All unknown JSON properties are collected into a map and exposed via {@code extraFields} on the
+ * {@link AccessTokenResponse}.
+ *
+ * @author Laurent Arnal


Suggested change

* @author Laurent Arnal

* @author Laurent Arnal - Initial contribution

Signed-off-by: Laurent ARNAL <laurent@clae.net>

lo92fr · 2026-03-10T17:17:35Z

Hello @kaikreuzer,

I've pushed the fixes.

Laurent.

kaikreuzer

Many thanks!

holgerfriedrich · 2026-03-11T15:58:38Z

This PR has changed Nullable annotations in class AccessTokenResponse. The change is currently breaking the add-ons build.

Maybe you could comment if there a strong reason why all those methods now are retuning Nullable stings? I might have overlooked it in the discussion above.

btw: There is also a small inconsistency here, setTokenType requires a non-null string as argument.

Before creating a fix either on core or add-ons side, I would like to understand why this was changed. In general, we try to stay with non-null by default (with exceptions where Nullable clearly makes sense).

lo92fr · 2026-03-11T16:09:07Z

This PR has changed Nullable annotations in class AccessTokenResponse. The change is currently breaking the add-ons build.

Maybe you could comment if there a strong reason why all those methods now are retuning Nullable stings? I might have overlooked it in the discussion above.
btw: There is also a small inconsistency here, `setTokenType` requires a non-null string as argument.
Before creating a fix either on core or add-ons side, I would like to understand why this was changed. In general, we try to stay with non-null by default (with exceptions where Nullable clearly makes sense).

Hello Holger,

Sorry for this.
This change was asked by @kaikreuzer because at first place, new extraFields was annotated with @nonnull

private Map<@nonnull String, @nonnull String> extraFields = Collections.emptyMap();

kaikreuzer
5 days ago
If you want to introduce null annotations with the new fields/method, you should make the whole class consistently use null annotations - and this means you should annotate it with @NonNullByDefault and then rather add @nullable annotations at the appropriate places.

So I add the @NonNullByDefault to the class, and add @nullable on old field to keep compatibility (I was thinking).
Let me look at compilation error so I try to find out what's wrong with that.

Laurent

holgerfriedrich · 2026-03-11T16:24:26Z

Hi Laurent, this happens from time to time and there is nothing to be sorry for.
Basically the change from no annotations to annotations is now making the binding code aware that there might be a missing null check.
Then it is probably good to fix that on add-ons side (and for consistency change the parameter of setTokenType to Nullable).

I worked in on an PR to remove all Null warnings in the past which basically changed AccessTokenResponse in a similar way you did. There is a draft PR for add-ons which could already contain the required changes. The first hit searching getAccessToken already shows what I had done to fix automower (currently the build fails there). https://github.com/openhab/openhab-addons/pull/18915/changes

lo92fr · 2026-03-11T16:25:18Z

he @NonNullByDefault to the class, and add @nullable on old field to keep compatibility (I was thinking).
Let me look at compilation error so I try to find out what's wrong with that.

Ok, I effectively don't see the side effect on other addons, that seems massive.
I think the better for now would be to create another PR to remove the @nullable change on AccessTokenResponse.
And perhaps come back later to fix more deeply the class.
Like this, it will enable a quick fix to make thing compile again.

Laurent.

lo92fr · 2026-03-11T16:28:32Z

We are just cross-posting at the same time !
Yes I was also looking at automower.

Yes I'm agree would be better to add the annotation.
But don't know how many time to fix all the addons.

So let me know, perhaps more simple to make it a 2 pass process:

First a quick PR to revert the nullable change, that could be ready in 1 hour.
Second a more deep PR to get back the @NonNullByDefault, and fix all addons.

Let me know !

holgerfriedrich · 2026-03-11T16:43:53Z

I think it is not too much to fix on add-ons side. I just searched the PR I mentioned above. Automower is the biggest change. A few more bindings need a new null check / exception blocks.

On core side, the setters in AccessTokenResponse which have corresponding getters with Nullable return value, should have Nullable parameters. Could you pls. create this first, then I can merge and start a CI build.

I can care for the add-ons side in the meantime.

lo92fr · 2026-03-11T17:03:37Z

ok, I prepare the change getters/setters

lo92fr · 2026-03-11T17:10:45Z

ok, I prepare the change getters/setters

@holgerfriedrich,

The PR for AccessTokenResponse is there:
#5417

lo92fr requested a review from a team as a code owner February 14, 2026 08:02

lo92fr mentioned this pull request Feb 15, 2026

[smartthings] Refactor binding to make use of new API openhab/openhab-addons#17855

Open

add extra fields support to oAuth AccessTokenResponse

5cc0f43

Signed-off-by: Laurent ARNAL <laurent@clae.net>

lo92fr force-pushed the features/oauth_token_extra_fields branch from 26292ff to 5cc0f43 Compare February 26, 2026 08:51

wborn requested a review from Copilot February 26, 2026 13:59

Copilot started reviewing on behalf of wborn February 26, 2026 14:00 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

lo92fr added 2 commits February 26, 2026 16:00

fixed for copilot review of 26/02

ddb46c5

Signed-off-by: Laurent ARNAL <laurent@clae.net>

add unit test

de56843

Signed-off-by: Laurent ARNAL <laurent@clae.net>

wborn requested a review from Copilot February 28, 2026 08:10

Copilot started reviewing on behalf of wborn February 28, 2026 08:10 View session

Copilot AI reviewed Feb 28, 2026

View reviewed changes

fixes for copilot review of 01/03

ded97e3

Signed-off-by: Laurent Arnal <laurent@clae.net>

wborn requested a review from Copilot March 5, 2026 16:39

Copilot AI reviewed Mar 5, 2026

View reviewed changes

wborn requested a review from Copilot March 6, 2026 07:51

Copilot started reviewing on behalf of wborn March 6, 2026 07:52 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

...n/java/org/openhab/core/auth/client/oauth2/AccessTokenResponseExtraFieldsAdapterFactory.java Show resolved Hide resolved

.../org.openhab.core/src/main/java/org/openhab/core/auth/client/oauth2/AccessTokenResponse.java Show resolved Hide resolved

add a javadoc warning about sensitive information on AccessTokenRespo…

0db0cd1

…nse::toString() Signed-off-by: Laurent Arnal <laurent@clae.net>

kaikreuzer requested changes Mar 6, 2026

View reviewed changes

lo92fr added 3 commits March 10, 2026 17:12

fix for @kai review of 6/03

733d3f7

Signed-off-by: Laurent ARNAL <laurent@clae.net>

change Nullable logic on AccessTokenResponse.java

79b7a82

Signed-off-by: Laurent ARNAL <laurent@clae.net>

fix error after @nullable changes

039f7f1

Signed-off-by: Laurent ARNAL <laurent@clae.net>

lo92fr requested a review from kaikreuzer March 10, 2026 17:17

kaikreuzer approved these changes Mar 10, 2026

View reviewed changes

kaikreuzer merged commit 926304f into openhab:main Mar 10, 2026
5 checks passed

kaikreuzer added the enhancement An enhancement or new feature of the Core label Mar 10, 2026

kaikreuzer added this to the 5.2 milestone Mar 10, 2026

holgerfriedrich mentioned this pull request Mar 11, 2026

Adapt to new Null annotations in AccessTokenResponse openhab/openhab-addons#20366

Merged

		* warning : the toString() function may returns sensitive information that should not go into the log.
		*

	* warning : the toString() function may returns sensitive information that should not go into the log.
	*
	* Warning: the toString() function may returns sensitive information that should not go into the log.

	* @author Laurent Arnal
	* @author Laurent Arnal - Initial contribution

Uh oh!

Conversation

lo92fr commented Feb 14, 2026

Add extra fields support to oAuth AccessTokenResponse

Description

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lo92fr commented Feb 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lo92fr commented Mar 5, 2026

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

lo92fr commented Mar 6, 2026

Uh oh!

kaikreuzer left a comment

Choose a reason for hiding this comment

Uh oh!

kaikreuzer Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

kaikreuzer Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

lo92fr Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

kaikreuzer Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

lo92fr Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

lo92fr commented Mar 10, 2026

Uh oh!

kaikreuzer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

holgerfriedrich commented Mar 11, 2026

Uh oh!

lo92fr commented Mar 11, 2026

Uh oh!

holgerfriedrich commented Mar 11, 2026

Uh oh!