From c401f35a43f25fbaf41b47521eaf732d5087be65 Mon Sep 17 00:00:00 2001 From: Andrew Sayre <6730289+andrewsayre@users.noreply.github.com> Date: Mon, 11 Mar 2019 07:33:25 -0500 Subject: [PATCH] Add cloudhook support to SmartThings component (#21905) * Add support for Nabu Casa cloudhooks * Added tests to cover cloudhook creation and removal * Remove cloud dependency --- .../components/smartthings/__init__.py | 13 +- .../components/smartthings/config_flow.py | 11 +- homeassistant/components/smartthings/const.py | 1 + .../components/smartthings/smartapp.py | 111 +++++++++++++++--- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/smartthings/conftest.py | 3 +- .../smartthings/test_config_flow.py | 36 +++++- tests/components/smartthings/test_init.py | 30 ++++- 9 files changed, 176 insertions(+), 33 deletions(-) diff --git a/homeassistant/components/smartthings/__init__.py b/homeassistant/components/smartthings/__init__.py index 3734baae6f4..e5226076f46 100644 --- a/homeassistant/components/smartthings/__init__.py +++ b/homeassistant/components/smartthings/__init__.py @@ -25,9 +25,10 @@ from .const import ( TOKEN_REFRESH_INTERVAL) from .smartapp import ( setup_smartapp, setup_smartapp_endpoint, smartapp_sync_subscriptions, - validate_installed_app) + unload_smartapp_endpoint, validate_installed_app, + validate_webhook_requirements) -REQUIREMENTS = ['pysmartapp==0.3.1', 'pysmartthings==0.6.7'] +REQUIREMENTS = ['pysmartapp==0.3.2', 'pysmartthings==0.6.7'] DEPENDENCIES = ['webhook'] _LOGGER = logging.getLogger(__name__) @@ -64,7 +65,7 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry): """Initialize config entry which represents an installed SmartApp.""" from pysmartthings import SmartThings - if not hass.config.api.base_url.lower().startswith('https://'): + if not validate_webhook_requirements(hass): _LOGGER.warning("The 'base_url' of the 'http' component must be " "configured and start with 'https://'") return False @@ -200,8 +201,9 @@ async def async_remove_entry( # Remove the app if not referenced by other entries, which if already # removed raises a 403 error. + all_entries = hass.config_entries.async_entries(DOMAIN) app_id = entry.data[CONF_APP_ID] - app_count = sum(1 for entry in hass.config_entries.async_entries(DOMAIN) + app_count = sum(1 for entry in all_entries if entry.data[CONF_APP_ID] == app_id) if app_count > 1: _LOGGER.debug("App %s was not removed because it is in use by other" @@ -218,6 +220,9 @@ async def async_remove_entry( raise _LOGGER.debug("Removed app %s", app_id) + if len(all_entries) == 1: + await unload_smartapp_endpoint(hass) + class DeviceBroker: """Manages an individual SmartThings config entry.""" diff --git a/homeassistant/components/smartthings/config_flow.py b/homeassistant/components/smartthings/config_flow.py index c290f0f8e55..da9b7c8854e 100644 --- a/homeassistant/components/smartthings/config_flow.py +++ b/homeassistant/components/smartthings/config_flow.py @@ -13,7 +13,8 @@ from .const import ( CONF_LOCATION_ID, CONF_OAUTH_CLIENT_ID, CONF_OAUTH_CLIENT_SECRET, DOMAIN, VAL_UID_MATCHER) from .smartapp import ( - create_app, find_app, setup_smartapp, setup_smartapp_endpoint, update_app) + create_app, find_app, setup_smartapp, setup_smartapp_endpoint, update_app, + validate_webhook_requirements) _LOGGER = logging.getLogger(__name__) @@ -56,10 +57,6 @@ class SmartThingsFlowHandler(config_entries.ConfigFlow): from pysmartthings import APIResponseError, AppOAuth, SmartThings errors = {} - if not self.hass.config.api.base_url.lower().startswith('https://'): - errors['base'] = "base_url_not_https" - return self._show_step_user(errors) - if user_input is None or CONF_ACCESS_TOKEN not in user_input: return self._show_step_user(errors) @@ -81,6 +78,10 @@ class SmartThingsFlowHandler(config_entries.ConfigFlow): # Setup end-point await setup_smartapp_endpoint(self.hass) + if not validate_webhook_requirements(self.hass): + errors['base'] = "base_url_not_https" + return self._show_step_user(errors) + try: app = await find_app(self.hass, self.api) if app: diff --git a/homeassistant/components/smartthings/const.py b/homeassistant/components/smartthings/const.py index 105c9760e12..aa7cba8e74c 100644 --- a/homeassistant/components/smartthings/const.py +++ b/homeassistant/components/smartthings/const.py @@ -8,6 +8,7 @@ APP_OAUTH_SCOPES = [ ] APP_NAME_PREFIX = 'homeassistant.' CONF_APP_ID = 'app_id' +CONF_CLOUDHOOK_URL = 'cloudhook_url' CONF_INSTALLED_APP_ID = 'installed_app_id' CONF_INSTALLED_APPS = 'installed_apps' CONF_INSTANCE_ID = 'instance_id' diff --git a/homeassistant/components/smartthings/smartapp.py b/homeassistant/components/smartthings/smartapp.py index 5527fda54f4..0b64bac5956 100644 --- a/homeassistant/components/smartthings/smartapp.py +++ b/homeassistant/components/smartthings/smartapp.py @@ -8,11 +8,12 @@ callbacks when device states change. import asyncio import functools import logging +from urllib.parse import urlparse from uuid import uuid4 from aiohttp import web -from homeassistant.components import webhook +from homeassistant.components import cloud, webhook from homeassistant.const import CONF_WEBHOOK_ID from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.dispatcher import ( @@ -21,9 +22,10 @@ from homeassistant.helpers.typing import HomeAssistantType from .const import ( APP_NAME_PREFIX, APP_OAUTH_CLIENT_NAME, APP_OAUTH_SCOPES, CONF_APP_ID, - CONF_INSTALLED_APP_ID, CONF_INSTALLED_APPS, CONF_INSTANCE_ID, - CONF_LOCATION_ID, CONF_REFRESH_TOKEN, DATA_BROKERS, DATA_MANAGER, DOMAIN, - SETTINGS_INSTANCE_ID, SIGNAL_SMARTAPP_PREFIX, STORAGE_KEY, STORAGE_VERSION) + CONF_CLOUDHOOK_URL, CONF_INSTALLED_APP_ID, CONF_INSTALLED_APPS, + CONF_INSTANCE_ID, CONF_LOCATION_ID, CONF_REFRESH_TOKEN, DATA_BROKERS, + DATA_MANAGER, DOMAIN, SETTINGS_INSTANCE_ID, SIGNAL_SMARTAPP_PREFIX, + STORAGE_KEY, STORAGE_VERSION) _LOGGER = logging.getLogger(__name__) @@ -59,15 +61,39 @@ async def validate_installed_app(api, installed_app_id: str): return installed_app +def validate_webhook_requirements(hass: HomeAssistantType) -> bool: + """Ensure HASS is setup properly to receive webhooks.""" + if cloud.async_active_subscription(hass): + return True + return get_webhook_url(hass).lower().startswith('https://') + + +def get_webhook_url(hass: HomeAssistantType) -> str: + """ + Get the URL of the webhook. + + Return the cloudhook if available, otherwise local webhook. + """ + cloudhook_url = hass.data[DOMAIN][CONF_CLOUDHOOK_URL] + if cloud.async_active_subscription(hass) and cloudhook_url is not None: + return cloudhook_url + return webhook.async_generate_url(hass, hass.data[DOMAIN][CONF_WEBHOOK_ID]) + + def _get_app_template(hass: HomeAssistantType): from pysmartthings import APP_TYPE_WEBHOOK, CLASSIFICATION_AUTOMATION + endpoint = "at " + hass.config.api.base_url + cloudhook_url = hass.data[DOMAIN][CONF_CLOUDHOOK_URL] + if cloudhook_url is not None: + endpoint = "via Nabu Casa" + description = "{} {}".format(hass.config.location_name, endpoint) + return { 'app_name': APP_NAME_PREFIX + str(uuid4()), 'display_name': 'Home Assistant', - 'description': "Home Assistant at " + hass.config.api.base_url, - 'webhook_target_url': webhook.async_generate_url( - hass, hass.data[DOMAIN][CONF_WEBHOOK_ID]), + 'description': description, + 'webhook_target_url': get_webhook_url(hass), 'app_type': APP_TYPE_WEBHOOK, 'single_instance': True, 'classifications': [CLASSIFICATION_AUTOMATION] @@ -162,33 +188,80 @@ async def setup_smartapp_endpoint(hass: HomeAssistantType): # Create config config = { CONF_INSTANCE_ID: str(uuid4()), - CONF_WEBHOOK_ID: webhook.generate_secret() + CONF_WEBHOOK_ID: webhook.generate_secret(), + CONF_CLOUDHOOK_URL: None } await store.async_save(config) + # Register webhook + webhook.async_register(hass, DOMAIN, 'SmartApp', + config[CONF_WEBHOOK_ID], smartapp_webhook) + + # Create webhook if eligible + cloudhook_url = config.get(CONF_CLOUDHOOK_URL) + if cloudhook_url is None \ + and cloud.async_active_subscription(hass) \ + and not hass.config_entries.async_entries(DOMAIN): + cloudhook_url = await cloud.async_create_cloudhook( + hass, config[CONF_WEBHOOK_ID]) + config[CONF_CLOUDHOOK_URL] = cloudhook_url + await store.async_save(config) + _LOGGER.debug("Created cloudhook '%s'", cloudhook_url) + # SmartAppManager uses a dispatcher to invoke callbacks when push events # occur. Use hass' implementation instead of the built-in one. dispatcher = Dispatcher( signal_prefix=SIGNAL_SMARTAPP_PREFIX, connect=functools.partial(async_dispatcher_connect, hass), send=functools.partial(async_dispatcher_send, hass)) - manager = SmartAppManager( - webhook.async_generate_path(config[CONF_WEBHOOK_ID]), - dispatcher=dispatcher) + # Path is used in digital signature validation + path = urlparse(cloudhook_url).path if cloudhook_url else \ + webhook.async_generate_path(config[CONF_WEBHOOK_ID]) + manager = SmartAppManager(path, dispatcher=dispatcher) manager.connect_install(functools.partial(smartapp_install, hass)) manager.connect_update(functools.partial(smartapp_update, hass)) manager.connect_uninstall(functools.partial(smartapp_uninstall, hass)) - webhook.async_register(hass, DOMAIN, 'SmartApp', - config[CONF_WEBHOOK_ID], smartapp_webhook) - hass.data[DOMAIN] = { DATA_MANAGER: manager, CONF_INSTANCE_ID: config[CONF_INSTANCE_ID], DATA_BROKERS: {}, CONF_WEBHOOK_ID: config[CONF_WEBHOOK_ID], + # Will not be present if not enabled + CONF_CLOUDHOOK_URL: config.get(CONF_CLOUDHOOK_URL), CONF_INSTALLED_APPS: [] } + _LOGGER.debug("Setup endpoint for %s", + cloudhook_url if cloudhook_url else + webhook.async_generate_url(hass, config[CONF_WEBHOOK_ID])) + + +async def unload_smartapp_endpoint(hass: HomeAssistantType): + """Tear down the component configuration.""" + if DOMAIN not in hass.data: + return + # Remove the cloudhook if it was created + cloudhook_url = hass.data[DOMAIN][CONF_CLOUDHOOK_URL] + if cloudhook_url and cloud.async_is_logged_in(hass): + await cloud.async_delete_cloudhook( + hass, hass.data[DOMAIN][CONF_WEBHOOK_ID]) + # Remove cloudhook from storage + store = hass.helpers.storage.Store(STORAGE_VERSION, STORAGE_KEY) + await store.async_save({ + CONF_INSTANCE_ID: hass.data[DOMAIN][CONF_INSTANCE_ID], + CONF_WEBHOOK_ID: hass.data[DOMAIN][CONF_WEBHOOK_ID], + CONF_CLOUDHOOK_URL: None + }) + _LOGGER.debug("Cloudhook '%s' was removed", cloudhook_url) + # Remove the webhook + webhook.async_unregister(hass, hass.data[DOMAIN][CONF_WEBHOOK_ID]) + # Disconnect all brokers + for broker in hass.data[DOMAIN][DATA_BROKERS].values(): + broker.disconnect() + # Remove all handlers from manager + hass.data[DOMAIN][DATA_MANAGER].dispatcher.disconnect_all() + # Remove the component data + hass.data.pop(DOMAIN) async def smartapp_sync_subscriptions( @@ -285,6 +358,9 @@ async def smartapp_install(hass: HomeAssistantType, req, resp, app): # Store the data where the flow can find it hass.data[DOMAIN][CONF_INSTALLED_APPS].append(install_data) + _LOGGER.debug("Installed SmartApp '%s' under parent app '%s'", + req.installed_app_id, app.app_id) + async def smartapp_update(hass: HomeAssistantType, req, resp, app): """ @@ -301,7 +377,7 @@ async def smartapp_update(hass: HomeAssistantType, req, resp, app): entry.data[CONF_REFRESH_TOKEN] = req.refresh_token hass.config_entries.async_update_entry(entry) - _LOGGER.debug("SmartApp '%s' under parent app '%s' was updated", + _LOGGER.debug("Updated SmartApp '%s' under parent app '%s'", req.installed_app_id, app.app_id) @@ -316,12 +392,13 @@ async def smartapp_uninstall(hass: HomeAssistantType, req, resp, app): req.installed_app_id), None) if entry: - _LOGGER.debug("SmartApp '%s' under parent app '%s' was removed", - req.installed_app_id, app.app_id) # Add as job not needed because the current coroutine was invoked # from the dispatcher and is not being awaited. await hass.config_entries.async_remove(entry.entry_id) + _LOGGER.debug("Uninstalled SmartApp '%s' under parent app '%s'", + req.installed_app_id, app.app_id) + async def smartapp_webhook(hass: HomeAssistantType, webhook_id: str, request): """ diff --git a/requirements_all.txt b/requirements_all.txt index d249ae12d4c..c574b02cbbc 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1268,7 +1268,7 @@ pysher==1.0.1 pysma==0.3.1 # homeassistant.components.smartthings -pysmartapp==0.3.1 +pysmartapp==0.3.2 # homeassistant.components.smartthings pysmartthings==0.6.7 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index b6d11228b93..84d103065e1 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -224,7 +224,7 @@ pyps4-homeassistant==0.4.8 pyqwikswitch==0.8 # homeassistant.components.smartthings -pysmartapp==0.3.1 +pysmartapp==0.3.2 # homeassistant.components.smartthings pysmartthings==0.6.7 diff --git a/tests/components/smartthings/conftest.py b/tests/components/smartthings/conftest.py index 67c35ba8232..3f346c9df0d 100644 --- a/tests/components/smartthings/conftest.py +++ b/tests/components/smartthings/conftest.py @@ -85,7 +85,8 @@ def app_fixture(hass, config_file): 'appType': 'WEBHOOK_SMART_APP', 'classifications': [CLASSIFICATION_AUTOMATION], 'displayName': 'Home Assistant', - 'description': "Home Assistant at " + hass.config.api.base_url, + 'description': + hass.config.location_name + " at " + hass.config.api.base_url, 'singleInstance': True, 'webhookSmartApp': { 'targetUrl': webhook.async_generate_url( diff --git a/tests/components/smartthings/test_config_flow.py b/tests/components/smartthings/test_config_flow.py index 28aa759a359..b79ab59a98a 100644 --- a/tests/components/smartthings/test_config_flow.py +++ b/tests/components/smartthings/test_config_flow.py @@ -6,6 +6,8 @@ from aiohttp import ClientResponseError from pysmartthings import APIResponseError from homeassistant import data_entry_flow +from homeassistant.components import cloud +from homeassistant.components.smartthings import smartapp from homeassistant.components.smartthings.config_flow import ( SmartThingsFlowHandler) from homeassistant.components.smartthings.const import ( @@ -41,7 +43,7 @@ async def test_base_url_not_https(hass): hass.config.api.base_url = 'http://0.0.0.0' flow = SmartThingsFlowHandler() flow.hass = hass - result = await flow.async_step_import() + result = await flow.async_step_user({'access_token': str(uuid4())}) assert result['type'] == data_entry_flow.RESULT_TYPE_FORM assert result['step_id'] == 'user' @@ -193,6 +195,38 @@ async def test_app_created_then_show_wait_form( assert result['step_id'] == 'wait_install' +async def test_cloudhook_app_created_then_show_wait_form( + hass, app, app_oauth_client, smartthings_mock): + """Test SmartApp is created with a cloudhoko and shows wait form.""" + # Unload the endpoint so we can reload it under the cloud. + await smartapp.unload_smartapp_endpoint(hass) + + mock_async_active_subscription = Mock(return_value=True) + mock_create_cloudhook = Mock(return_value=mock_coro( + return_value="http://cloud.test")) + with patch.object(cloud, 'async_active_subscription', + new=mock_async_active_subscription), \ + patch.object(cloud, 'async_create_cloudhook', + new=mock_create_cloudhook): + + await smartapp.setup_smartapp_endpoint(hass) + + flow = SmartThingsFlowHandler() + flow.hass = hass + smartthings = smartthings_mock.return_value + smartthings.apps.return_value = mock_coro(return_value=[]) + smartthings.create_app.return_value = \ + mock_coro(return_value=(app, app_oauth_client)) + smartthings.update_app_settings.return_value = mock_coro() + smartthings.update_app_oauth.return_value = mock_coro() + + result = await flow.async_step_user({'access_token': str(uuid4())}) + + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + assert result['step_id'] == 'wait_install' + assert mock_create_cloudhook.call_count == 1 + + async def test_app_updated_then_show_wait_form( hass, app, app_oauth_client, smartthings_mock): """Test SmartApp is updated when an existing is already created.""" diff --git a/tests/components/smartthings/test_init.py b/tests/components/smartthings/test_init.py index dfb596998b7..a5edc93fce6 100644 --- a/tests/components/smartthings/test_init.py +++ b/tests/components/smartthings/test_init.py @@ -6,10 +6,11 @@ from aiohttp import ClientConnectionError, ClientResponseError from pysmartthings import InstalledAppStatus import pytest -from homeassistant.components import smartthings +from homeassistant.components import cloud, smartthings from homeassistant.components.smartthings.const import ( - CONF_INSTALLED_APP_ID, CONF_REFRESH_TOKEN, DATA_BROKERS, DOMAIN, - EVENT_BUTTON, SIGNAL_SMARTTHINGS_UPDATE, SUPPORTED_PLATFORMS) + CONF_CLOUDHOOK_URL, CONF_INSTALLED_APP_ID, CONF_REFRESH_TOKEN, + DATA_BROKERS, DOMAIN, EVENT_BUTTON, SIGNAL_SMARTTHINGS_UPDATE, + SUPPORTED_PLATFORMS) from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers.dispatcher import async_dispatcher_connect @@ -224,6 +225,29 @@ async def test_remove_entry(hass, config_entry, smartthings_mock): assert api.delete_app.call_count == 1 +async def test_remove_entry_cloudhook(hass, config_entry, smartthings_mock): + """Test that the installed app, app, and cloudhook are removed up.""" + # Arrange + setattr(hass.config_entries, '_entries', [config_entry]) + hass.data[DOMAIN][CONF_CLOUDHOOK_URL] = "https://test.cloud" + api = smartthings_mock.return_value + api.delete_installed_app.side_effect = lambda _: mock_coro() + api.delete_app.side_effect = lambda _: mock_coro() + mock_async_is_logged_in = Mock(return_value=True) + mock_async_delete_cloudhook = Mock(return_value=mock_coro()) + # Act + with patch.object(cloud, 'async_is_logged_in', + new=mock_async_is_logged_in), \ + patch.object(cloud, 'async_delete_cloudhook', + new=mock_async_delete_cloudhook): + await smartthings.async_remove_entry(hass, config_entry) + # Assert + assert api.delete_installed_app.call_count == 1 + assert api.delete_app.call_count == 1 + assert mock_async_is_logged_in.call_count == 1 + assert mock_async_delete_cloudhook.call_count == 1 + + async def test_remove_entry_app_in_use(hass, config_entry, smartthings_mock): """Test app is not removed if in use by another config entry.""" # Arrange