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

[nest] Fix for missing refresh token after reauthorization #12711

Merged
merged 1 commit into from
May 15, 2022

Conversation

mhilbush
Copy link
Contributor

By default, the Google OAuth API returns the refresh token only on the first authorization. On subsequent authorizations using the same Client ID and Secret, Google does not return the refresh token in the access token response. This causes the binding to fail on the first refresh of the access token after a reauthorization.

This affects the SDM and Pub/Sub APIs. However, the failure is more spectacular with the Pub/Sub API as it causes the binding to repeatedly do a pub/sub request against the Google API every 20-40 msec.

This PR adds a new query parameter (prompt=consent) to the URL used to get the authorization code. This parameter causes Google to always return the refresh token in the authorization response (via oAuthService.getAccessTokenResponseByAuthorizationCode).

In addition, this PR adds checks to make sure the refresh token in present in the access token response in order to fail more quickly and gracefully if the refresh token is missing.

Further details of my analysis are here.
https://community.openhab.org/t/nest-binding-throwing-pub-sub-errors-repeatedly/135721

I know @wborn is pretty time-constrained right now, but I'm hopeful he can find a few minutes to review. 😉

Signed-off-by: Mark Hilbush mark@hilbush.com

Signed-off-by: Mark Hilbush <mark@hilbush.com>
@mhilbush mhilbush added the bug An unexpected problem or unintended behavior of an add-on label May 10, 2022
@mhilbush mhilbush requested a review from wborn as a code owner May 10, 2022 14:30
@lolodomo
Copy link
Contributor

I do not see in the code the addition of the prompt query parameter.

@mhilbush
Copy link
Contributor Author

That's correct. It's not in the code, but only in the URL in the documentation. You need to copy/paste the URL from the documentation into your browser's address bar, then replace the <ProjectID> and <ClientID> in the URL with your specific values. It's a completely manual process.

Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM

@lolodomo
Copy link
Contributor

I let a chance to @wborn to review.

Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@wborn wborn merged commit 049fd17 into openhab:main May 15, 2022
@wborn wborn added this to the 3.3 milestone May 15, 2022
@mhilbush mhilbush deleted the nest-refresh-token branch May 16, 2022 21:12
@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/openhab-3-3-release-discussion/136925/60

andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
…2711)

Signed-off-by: Mark Hilbush <mark@hilbush.com>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
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

Successfully merging this pull request may close these issues.

4 participants