Support for Wink oauth application authorization #8208
Conversation
|
@w1ll1am23, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff, @balloob and @pvizeli to be potential reviewers. |
homeassistant/components/wink.py
Outdated
| ATTR_CLIENT_ID: self.config_file["client_id"], | ||
| ATTR_CLIENT_SECRET: self.config_file["client_secret"] | ||
| } | ||
| _write_config_file(hass.config.path(WINK_CONFIG_FILE), config_contents) |
There was a problem hiding this comment.
line too long (83 > 79 characters)
homeassistant/components/wink.py
Outdated
| You can close this window now!""" | ||
|
|
||
| if data.get('code') is not None: | ||
| response = self.request_token(data.get('code'), self.config_file["client_secret"]) |
There was a problem hiding this comment.
line too long (94 > 79 characters)
homeassistant/components/wink.py
Outdated
| else: | ||
| _LOGGER.info("Getting a new Wink token.") | ||
| _get_wink_token_from_web() | ||
| user_agent = USER_AGENT % (MINOR_VERSION, MAJOR_VERSION, PATCH_VERSION, time.time()) |
There was a problem hiding this comment.
line too long (92 > 79 characters)
homeassistant/components/wink.py
Outdated
|
|
||
| wink_auth_start_url = pywink.get_authorization_url(config_file.get(ATTR_CLIENT_ID), redirect_uri) | ||
| hass.http.register_redirect(WINK_AUTH_START, wink_auth_start_url) | ||
| hass.http.register_view(WinkAuthCallbackView(config, config_file, pywink.request_token)) |
There was a problem hiding this comment.
line too long (100 > 79 characters)
homeassistant/components/wink.py
Outdated
| redirect_uri = '{}{}'.format(hass.config.api.base_url, | ||
| WINK_AUTH_CALLBACK_PATH) | ||
|
|
||
| wink_auth_start_url = pywink.get_authorization_url(config_file.get(ATTR_CLIENT_ID), redirect_uri) |
There was a problem hiding this comment.
line too long (109 > 79 characters)
homeassistant/components/wink.py
Outdated
| config_file.get(ATTR_CLIENT_SECRET), | ||
| access_token=access_token, | ||
| refresh_token=refresh_token) | ||
| # This is called to create the redirect so the user can Authorize Home-Assistant |
There was a problem hiding this comment.
line too long (88 > 79 characters)
homeassistant/components/wink.py
Outdated
| return True | ||
|
|
||
| if "wink" in _CONFIGURING: | ||
| get_component('configurator').request_done(_CONFIGURING.pop("wink")) |
There was a problem hiding this comment.
line too long (80 > 79 characters)
homeassistant/components/wink.py
Outdated
|
|
||
| access_token = config[DOMAIN].get(CONF_ACCESS_TOKEN) | ||
| client_id = config[DOMAIN].get('client_id') | ||
| user_agent = USER_AGENT % (MINOR_VERSION, MAJOR_VERSION, PATCH_VERSION, time.time()) |
There was a problem hiding this comment.
line too long (88 > 79 characters)
homeassistant/components/wink.py
Outdated
| else: | ||
| setup(hass, config) | ||
| else: | ||
| # Shouldn't get here, but if we do, call setup again and create default config again |
There was a problem hiding this comment.
line too long (96 > 79 characters)
homeassistant/components/wink.py
Outdated
| raise IOError("Saving Wink config file failed") | ||
| return config | ||
|
|
||
| def _read_config_file(file_path): |
|
@balloob For some reason this stopped working for me if I have a password set. I get a 401 after the authentication when Wink redirects back to Hass. If I remove my password it works fine. Any idea what would cause that? @rpitera tested this and didn't have the issue, and like I said I tested this yesterday and didn't have a problem either. Tried different browsers on multiple devices all result in a 401 from HA and a persistent notification pops up saying failed login attempt. |
|
Using Chrome and serving Hass over https ? Try opening the dev tools -> application -> clear storage -> select all and click clear site data. |
|
Nope still the same thing. I tried rebooting everything an clearing all the data I know of, still get a 401. Wink's API sends a redirect URI of http://IP_ADDRESS:8123/auth/wink/callback?code=SOME_RANDOM_CODE?state= if I append that url with my password like so. http://IP_ADDRESS:8123/auth/wink/callback?code=SOME_RANDOM_CODE&state=&api_password=MY_PASSWORD it works like it should/like it did a few days ago. I am not sure what the deal is. I am serving Hass over http to my nginx reverse proxy, but I am not going through the proxy I am connecting directly to HA via the same computer that HA is running on using an IP address. |
|
Just as an FYI, the only way I got this to work (suggested by @w1ll1am23 in testing) was to run it in a GUI session in the browser on my Pi which is hosting the HA server. |
|
Hey guys, I have been very busy lately and not keeping up on the changes @w1ll1am23 is doing. Would like to have my wink devices working for more then a few hours with HASS. Can someone direct me on how to use the latest wink.py. I do have a Wink application with Oauth Settings and Oauth URLs. |
|
@Dayve67 No one has reported that issue in production in several releases. Can you confirm that you are still having this problem with the newest production release 0.47? |
|
@balloob I, too, get a 401 after the authentication when Wink redirects back to Hass. After I removed my Hass password I was able to finish the configuration process. Once finished, I added back the Hass password and everything still appears to be working. |
79bd839 to
9ae19f5
Compare
|
@w1ll1am23 make sure to remove |
|
@balloob will do, pretty sure the code is good. I was hoping to get one other person to test from the forums. I'll remove the |
homeassistant/components/wink.py
Outdated
|
|
||
| _LOGGER = logging.getLogger(__name__) | ||
|
|
||
| CHANNELS = [] | ||
|
|
||
| DOMAIN = 'wink' | ||
|
|
||
| _CONFIGURING = {} |
There was a problem hiding this comment.
Please don't use a global but store your things in hass.data
homeassistant/components/wink.py
Outdated
| def wink_configuration_callback(callback_data): | ||
| """Handle configuration updates.""" | ||
| _config_path = hass.config.path(WINK_CONFIG_FILE) | ||
| if os.path.isfile(_config_path): |
There was a problem hiding this comment.
To make the code more readable, consider adopting guard-clause pattern.
if not os.path.isfile(_config_path):
setup(hass, config)
return
config_file = _read_config_file(_config_path)
…
homeassistant/components/wink.py
Outdated
| start_url = "{}{}".format(hass.config.api.base_url, | ||
| WINK_AUTH_CALLBACK_PATH) | ||
|
|
||
| description = """If you haven't done so already. |
There was a problem hiding this comment.
I think that you can skip the first sentence.
homeassistant/components/wink.py
Outdated
| Then come back here and hit the below button. | ||
| """.format(start_url, config_path) | ||
|
|
||
| submit = "I have saved my Client ID and Client Secret into wink.conf." |
There was a problem hiding this comment.
Why not have people enter it via the configurator UI ? You can add fields to request_config. Example: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/media_player/gpmdp.py#L98-L105
There was a problem hiding this comment.
👍 Didn't know about this, that is awesome and way more user friendly.
homeassistant/components/wink.py
Outdated
|
|
||
| return web.Response(text=html_response, content_type='text/html') | ||
| else: | ||
| _LOGGER.error("No code returned from Wink API") |
There was a problem hiding this comment.
This should also be returned to the user. Consider using a guard clause again.
There was a problem hiding this comment.
By this do you mean return to the browser? See update.
9ae19f5 to
f5a0bb5
Compare
homeassistant/components/wink.py
Outdated
|
|
||
| # Call subscribe after the user sets up wink via the configurator | ||
| # All other methods will complete setup before EVENT_HOMEASSISTANT_START | ||
| # is called meaning they will call subscribe via the method below. (start_subscription) |
homeassistant/components/wink.py
Outdated
|
|
||
| pywink.set_user_agent(USER_AGENT) |
homeassistant/components/wink.py
Outdated
| return True | ||
|
|
||
| if DOMAIN in hass.data[DOMAIN]['configuring']: | ||
| get_component('configurator').request_done(hass.data[DOMAIN]['configuring'].pop( |
homeassistant/components/wink.py
Outdated
| configurator = get_component('configurator') | ||
| if DOMAIN in hass.data[DOMAIN]['configuring']: | ||
| configurator.notify_errors( | ||
| hass.data[DOMAIN]['configuring'][DOMAIN], "Failed to register, please try again.") |
homeassistant/components/wink.py
Outdated
| description=description, submit_caption="submit", | ||
| description_image="/static/images/config_wink.png", | ||
| fields=[{'id': 'client_id', 'name': 'Client ID', 'type': 'string'}, | ||
| {'id': 'client_secret', 'name': 'Client secret', 'type': 'string'}] |
homeassistant/components/wink.py
Outdated
| https://developer.wink.com. | ||
| Add a Redirect URI of {}. | ||
| They will provide you a Client ID and secret | ||
| after reviewing your request. (This can take several days). |
homeassistant/components/wink.py
Outdated
| return | ||
| else: | ||
| error_msg = ("Your input was invalid. Please try again.") | ||
| configurator.notify_errors(hass.data[DOMAIN]['configuring'][DOMAIN], error_msg) |
homeassistant/components/wink.py
Outdated
| client_secret = callback_data.get('client_secret') | ||
| if None not in (client_id, client_secret): | ||
| _write_config_file(_config_path, {ATTR_CLIENT_ID: client_id, | ||
| ATTR_CLIENT_SECRET: client_secret}) |
b9507cd to
38b5454
Compare
|
@balloob This is ready for another look. I believe I have address all of your concerns. |
|
Confirm that this has worked for me.... |
|
I just updated to HASS 0.50.1 and trying to get the WINK api working again. What do I need to put in the wink.conf file for information? And do I need to use the quotes around the client_id and client_secret info? |
|
You don't need to edit the wink.conf all you need is wink: in your main config. If that exists you will get a wink configurator in the frontend which will guide you through the setup. |
Description:
Wink added a developer portal recently and client_id and client_secrets obtained from there do not support the grant_type of password like the old client_id and client_secrets do. So... this adds support for authorizing your application (HomeAssistant) like the fitbit component does. I copied most of that code.
Users will only be able to use this new method for oauth meaning they will need to update their configs and follow the new flow. People without client_id and client_secrets can still use the username and password method which uses the Home-Assistant.io form to get an access token.Both methods of oauth will continue to work to support older client_id and client_secrets.Manually assigning
access_tokenin the config is been removed. All users should be moved over to at least the username/password authentication which uses the form on the .io site.Also still need to update the image for what the redirect URL should be, set to the fitbit one currently.Fixes #7546
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#3025
Example entry for
configuration.yaml(if applicable):wink:Things that still need fixed:
Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py.