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

[mielecloud] Handle used for OAuth2 storage should contain binding context #14783

Open
jlaur opened this issue Apr 11, 2023 · 2 comments
Open
Labels
bug An unexpected problem or unintended behavior of an add-on

Comments

@jlaur
Copy link
Contributor

jlaur commented Apr 11, 2023

While working with OAuth2 in context of #14745 and #14780 I noticed in my StorageHandler.For.OAuthClientService.json file that this binding uses only the e-mail address as handle:

private String getOAuthServiceHandle() {
return getConfig().get(MieleCloudBindingConstants.CONFIG_PARAM_EMAIL).toString();
}

This is currently not a problem since no other bindings use e-mail address as handle. However, it seems undesired, because if multiple bindings would do this, and users would use the same e-mail address for multiple accounts, there would be a clash.

Expected Behavior

Handle should at least be unique for the binding. Probably it should also be unique for the bridge thing, so it would be possible to have more than one bridge with different handles, so the OAuth authorization flow could be tested with one of them while the other one remains authorized.

Current Behavior

Raw e-mail address is used for creating the handle without any prefixes or postfixes.

Possible Solution

It seems that this.getThing().getUID().getAsString() is often used to create the handle.

Migration might be possible by using importAccessTokenResponse and deleteServiceAndAccessToken

Steps to Reproduce

Go through the authorization flow and check the file StorageHandler.For.OAuthClientService.json afterwards.

@jlaur jlaur added the bug An unexpected problem or unintended behavior of an add-on label Apr 11, 2023
@lolodomo
Copy link
Contributor

lolodomo commented May 6, 2023

@jlaur : do you have a clear idea how to implement the change of handle id and will you be able to test it ?
Note that it could then be also changed in hydrawise and myq bindings (issue #14939).

@jlaur
Copy link
Contributor Author

jlaur commented May 6, 2023

@lolodomo - changing the handle should be straight-forward, but I didn't check closely if my proposal for backwards compatibility will hold.

I don't use this binding normally, but I do have connected Miele appliances, so I will be able to test.

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
Projects
None yet
Development

No branches or pull requests

2 participants