Enhance SmartThings component subscription (#21124)

* Move to config v2 to store SmartApp oauth keys

* Add migration functionality.

* Regenerate refresh token on periodic basis

* Fix regenerate and misc. optimizations

* Review feedback

* Subscription sync logic now performs a difference operation

* Removed config entry reloading.

homeassistant/components/smartthings/__init__.py

@@ -14,16 +14,20 @@ from homeassistant.helpers.aiohttp_client import async_get_clientsession
 from homeassistant.helpers.dispatcher import (
     async_dispatcher_connect, async_dispatcher_send)
 from homeassistant.helpers.entity import Entity
+from homeassistant.helpers.event import async_track_time_interval
 from homeassistant.helpers.typing import ConfigType, HomeAssistantType
 
 from .config_flow import SmartThingsFlowHandler  # noqa
 from .const import (
-    CONF_APP_ID, CONF_INSTALLED_APP_ID, DATA_BROKERS, DATA_MANAGER, DOMAIN,
-    EVENT_BUTTON, SIGNAL_SMARTTHINGS_UPDATE, SUPPORTED_PLATFORMS)
+    CONF_APP_ID, CONF_INSTALLED_APP_ID, CONF_OAUTH_CLIENT_ID,
+    CONF_OAUTH_CLIENT_SECRET, CONF_REFRESH_TOKEN, DATA_BROKERS, DATA_MANAGER,
+    DOMAIN, EVENT_BUTTON, SIGNAL_SMARTTHINGS_UPDATE, SUPPORTED_PLATFORMS,
+    TOKEN_REFRESH_INTERVAL)
 from .smartapp import (
-    setup_smartapp, setup_smartapp_endpoint, validate_installed_app)
+    setup_smartapp, setup_smartapp_endpoint, smartapp_sync_subscriptions,
+    validate_installed_app)
 
-REQUIREMENTS = ['pysmartapp==0.3.0', 'pysmartthings==0.6.2']
+REQUIREMENTS = ['pysmartapp==0.3.0', 'pysmartthings==0.6.3']
 DEPENDENCIES = ['webhook']
 
 _LOGGER = logging.getLogger(__name__)
@@ -35,6 +39,33 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType):
     return True
 
 
+async def async_migrate_entry(hass: HomeAssistantType, entry: ConfigEntry):
+    """Handle migration of a previous version config entry.
+
+    A config entry created under a previous version must go through the
+    integration setup again so we can properly retrieve the needed data
+    elements. Force this by removing the entry and triggering a new flow.
+    """
+    from pysmartthings import SmartThings
+
+    # Delete the installed app
+    api = SmartThings(async_get_clientsession(hass),
+                      entry.data[CONF_ACCESS_TOKEN])
+    await api.delete_installed_app(entry.data[CONF_INSTALLED_APP_ID])
+    # Delete the entry
+    hass.async_create_task(
+        hass.config_entries.async_remove(entry.entry_id))
+    # only create new flow if there isn't a pending one for SmartThings.
+    flows = hass.config_entries.flow.async_progress()
+    if not [flow for flow in flows if flow['handler'] == DOMAIN]:
+        hass.async_create_task(
+            hass.config_entries.flow.async_init(
+                DOMAIN, context={'source': 'import'}))
+
+    # Return False because it could not be migrated.
+    return False
+
+
 async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry):
     """Initialize config entry which represents an installed SmartApp."""
     from pysmartthings import SmartThings
@@ -62,6 +93,14 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry):
         installed_app = await validate_installed_app(
             api, entry.data[CONF_INSTALLED_APP_ID])
 
+        # Get SmartApp token to sync subscriptions
+        token = await api.generate_tokens(
+            entry.data[CONF_OAUTH_CLIENT_ID],
+            entry.data[CONF_OAUTH_CLIENT_SECRET],
+            entry.data[CONF_REFRESH_TOKEN])
+        entry.data[CONF_REFRESH_TOKEN] = token.refresh_token
+        hass.config_entries.async_update_entry(entry)
+
         # Get devices and their current status
         devices = await api.devices(
             location_ids=[installed_app.location_id])
@@ -71,18 +110,21 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry):
                 await device.status.refresh()
             except ClientResponseError:
                 _LOGGER.debug("Unable to update status for device: %s (%s), "
-                              "the device will be ignored",
+                              "the device will be excluded",
                               device.label, device.device_id, exc_info=True)
                 devices.remove(device)
 
         await asyncio.gather(*[retrieve_device_status(d)
                                for d in devices.copy()])
 
+        # Sync device subscriptions
+        await smartapp_sync_subscriptions(
+            hass, token.access_token, installed_app.location_id,
+            installed_app.installed_app_id, devices)
+
         # Setup device broker
-        broker = DeviceBroker(hass, devices,
-                              installed_app.installed_app_id)
-        broker.event_handler_disconnect = \
-            smart_app.connect_event(broker.event_handler)
+        broker = DeviceBroker(hass, entry, token, smart_app, devices)
+        broker.connect()
         hass.data[DOMAIN][DATA_BROKERS][entry.entry_id] = broker
 
     except ClientResponseError as ex:
@@ -117,8 +159,8 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry):
 async def async_unload_entry(hass: HomeAssistantType, entry: ConfigEntry):
     """Unload a config entry."""
     broker = hass.data[DOMAIN][DATA_BROKERS].pop(entry.entry_id, None)
-    if broker and broker.event_handler_disconnect:
-        broker.event_handler_disconnect()
+    if broker:
+        broker.disconnect()
 
     tasks = [hass.config_entries.async_forward_entry_unload(entry, component)
              for component in SUPPORTED_PLATFORMS]
@@ -128,14 +170,18 @@ async def async_unload_entry(hass: HomeAssistantType, entry: ConfigEntry):
 class DeviceBroker:
     """Manages an individual SmartThings config entry."""
 
-    def __init__(self, hass: HomeAssistantType, devices: Iterable,
-                 installed_app_id: str):
+    def __init__(self, hass: HomeAssistantType, entry: ConfigEntry,
+                 token, smart_app, devices: Iterable):
         """Create a new instance of the DeviceBroker."""
         self._hass = hass
-        self._installed_app_id = installed_app_id
-        self.assignments = self._assign_capabilities(devices)
+        self._entry = entry
+        self._installed_app_id = entry.data[CONF_INSTALLED_APP_ID]
+        self._smart_app = smart_app
+        self._token = token
+        self._event_disconnect = None
+        self._regenerate_token_remove = None
+        self._assignments = self._assign_capabilities(devices)
         self.devices = {device.device_id: device for device in devices}
-        self.event_handler_disconnect = None
 
     def _assign_capabilities(self, devices: Iterable):
         """Assign platforms to capabilities."""
@@ -158,17 +204,45 @@ class DeviceBroker:
             assignments[device.device_id] = slots
         return assignments
 
+    def connect(self):
+        """Connect handlers/listeners for device/lifecycle events."""
+        # Setup interval to regenerate the refresh token on a periodic basis.
+        # Tokens expire in 30 days and once expired, cannot be recovered.
+        async def regenerate_refresh_token(now):
+            """Generate a new refresh token and update the config entry."""
+            await self._token.refresh(
+                self._entry.data[CONF_OAUTH_CLIENT_ID],
+                self._entry.data[CONF_OAUTH_CLIENT_SECRET])
+            self._entry.data[CONF_REFRESH_TOKEN] = self._token.refresh_token
+            self._hass.config_entries.async_update_entry(self._entry)
+            _LOGGER.debug('Regenerated refresh token for installed app: %s',
+                          self._installed_app_id)
+
+        self._regenerate_token_remove = async_track_time_interval(
+            self._hass, regenerate_refresh_token, TOKEN_REFRESH_INTERVAL)
+
+        # Connect handler to incoming device events
+        self._event_disconnect = \
+            self._smart_app.connect_event(self._event_handler)
+
+    def disconnect(self):
+        """Disconnects handlers/listeners for device/lifecycle events."""
+        if self._regenerate_token_remove:
+            self._regenerate_token_remove()
+        if self._event_disconnect:
+            self._event_disconnect()
+
     def get_assigned(self, device_id: str, platform: str):
         """Get the capabilities assigned to the platform."""
-        slots = self.assignments.get(device_id, {})
+        slots = self._assignments.get(device_id, {})
         return [key for key, value in slots.items() if value == platform]
 
     def any_assigned(self, device_id: str, platform: str):
         """Return True if the platform has any assigned capabilities."""
-        slots = self.assignments.get(device_id, {})
+        slots = self._assignments.get(device_id, {})
         return any(value for value in slots.values() if value == platform)
 
-    async def event_handler(self, req, resp, app):
+    async def _event_handler(self, req, resp, app):
         """Broker for incoming events."""
         from pysmartapp.event import EVENT_TYPE_DEVICE
         from pysmartthings import Capability, Attribute

homeassistant/components/smartthings/config_flow.py

@@ -9,7 +9,8 @@ from homeassistant.const import CONF_ACCESS_TOKEN
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 
 from .const import (
-    CONF_APP_ID, CONF_INSTALLED_APP_ID, CONF_LOCATION_ID, DOMAIN,
+    APP_OAUTH_CLIENT_NAME, APP_OAUTH_SCOPES, CONF_APP_ID, CONF_INSTALLED_APPS,
+    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)
@@ -35,7 +36,7 @@ class SmartThingsFlowHandler(config_entries.ConfigFlow):
         b) Config entries setup for all installations
     """
 
-    VERSION = 1
+    VERSION = 2
     CONNECTION_CLASS = config_entries.CONN_CLASS_CLOUD_PUSH
 
     def __init__(self):
@@ -43,6 +44,8 @@ class SmartThingsFlowHandler(config_entries.ConfigFlow):
         self.access_token = None
         self.app_id = None
         self.api = None
+        self.oauth_client_secret = None
+        self.oauth_client_id = None
 
     async def async_step_import(self, user_input=None):
         """Occurs when a previously entry setup fails and is re-initiated."""
@@ -50,7 +53,7 @@ class SmartThingsFlowHandler(config_entries.ConfigFlow):
 
     async def async_step_user(self, user_input=None):
         """Get access token and validate it."""
-        from pysmartthings import APIResponseError, SmartThings
+        from pysmartthings import APIResponseError, AppOAuth, SmartThings
 
         errors = {}
         if not self.hass.config.api.base_url.lower().startswith('https://'):
@@ -83,10 +86,18 @@ class SmartThingsFlowHandler(config_entries.ConfigFlow):
             if app:
                 await app.refresh()  # load all attributes
                 await update_app(self.hass, app)
+                # Get oauth client id/secret by regenerating it
+                app_oauth = AppOAuth(app.app_id)
+                app_oauth.client_name = APP_OAUTH_CLIENT_NAME
+                app_oauth.scope.extend(APP_OAUTH_SCOPES)
+                client = await self.api.generate_app_oauth(app_oauth)
             else:
-                app = await create_app(self.hass, self.api)
+                app, client = await create_app(self.hass, self.api)
             setup_smartapp(self.hass, app)
             self.app_id = app.app_id
+            self.oauth_client_secret = client.client_secret
+            self.oauth_client_id = client.client_id
+
         except APIResponseError as ex:
             if ex.is_target_error():
                 errors['base'] = 'webhook_error'
@@ -113,19 +124,23 @@ class SmartThingsFlowHandler(config_entries.ConfigFlow):
 
     async def async_step_wait_install(self, user_input=None):
         """Wait for SmartApp installation."""
-        from pysmartthings import InstalledAppStatus
-
         errors = {}
         if user_input is None:
             return self._show_step_wait_install(errors)
 
         # Find installed apps that were authorized
-        installed_apps = [app for app in await self.api.installed_apps(
-            installed_app_status=InstalledAppStatus.AUTHORIZED)
-                          if app.app_id == self.app_id]
+        installed_apps = self.hass.data[DOMAIN][CONF_INSTALLED_APPS].copy()
         if not installed_apps:
             errors['base'] = 'app_not_installed'
             return self._show_step_wait_install(errors)
+        self.hass.data[DOMAIN][CONF_INSTALLED_APPS].clear()
+
+        # Enrich the data
+        for installed_app in installed_apps:
+            installed_app[CONF_APP_ID] = self.app_id
+            installed_app[CONF_ACCESS_TOKEN] = self.access_token
+            installed_app[CONF_OAUTH_CLIENT_ID] = self.oauth_client_id
+            installed_app[CONF_OAUTH_CLIENT_SECRET] = self.oauth_client_secret
 
         # User may have installed the SmartApp in more than one SmartThings
         # location. Config flows are created for the additional installations
@@ -133,21 +148,10 @@ class SmartThingsFlowHandler(config_entries.ConfigFlow):
             self.hass.async_create_task(
                 self.hass.config_entries.flow.async_init(
                     DOMAIN, context={'source': 'install'},
-                    data={
-                        CONF_APP_ID: installed_app.app_id,
-                        CONF_INSTALLED_APP_ID: installed_app.installed_app_id,
-                        CONF_LOCATION_ID: installed_app.location_id,
-                        CONF_ACCESS_TOKEN: self.access_token
-                    }))
+                    data=installed_app))
 
-        # return entity for the first one.
-        installed_app = installed_apps[0]
-        return await self.async_step_install({
-            CONF_APP_ID: installed_app.app_id,
-            CONF_INSTALLED_APP_ID: installed_app.installed_app_id,
-            CONF_LOCATION_ID: installed_app.location_id,
-            CONF_ACCESS_TOKEN: self.access_token
-        })
+        # Create config entity for the first one.
+        return await self.async_step_install(installed_apps[0])
 
     def _show_step_user(self, errors):
         return self.async_show_form(

homeassistant/components/smartthings/const.py

@@ -1,14 +1,20 @@
 """Constants used by the SmartThings component and platforms."""
+from datetime import timedelta
 import re
 
+APP_OAUTH_CLIENT_NAME = "Home Assistant"
 APP_OAUTH_SCOPES = [
     'r:devices:*'
 ]
 APP_NAME_PREFIX = 'homeassistant.'
 CONF_APP_ID = 'app_id'
 CONF_INSTALLED_APP_ID = 'installed_app_id'
+CONF_INSTALLED_APPS = 'installed_apps'
 CONF_INSTANCE_ID = 'instance_id'
 CONF_LOCATION_ID = 'location_id'
+CONF_OAUTH_CLIENT_ID = 'client_id'
+CONF_OAUTH_CLIENT_SECRET = 'client_secret'
+CONF_REFRESH_TOKEN = 'refresh_token'
 DATA_MANAGER = 'manager'
 DATA_BROKERS = 'brokers'
 DOMAIN = 'smartthings'
@@ -29,6 +35,7 @@ SUPPORTED_PLATFORMS = [
     'binary_sensor',
     'sensor'
 ]
+TOKEN_REFRESH_INTERVAL = timedelta(days=14)
 VAL_UID = "^(?:([0-9a-fA-F]{32})|([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]" \
           "{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}))$"
 VAL_UID_MATCHER = re.compile(VAL_UID)

homeassistant/components/smartthings/smartapp.py

@@ -13,15 +13,16 @@ from uuid import uuid4
 from aiohttp import web
 
 from homeassistant.components import webhook
-from homeassistant.const import CONF_ACCESS_TOKEN, CONF_WEBHOOK_ID
+from homeassistant.const import CONF_WEBHOOK_ID
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 from homeassistant.helpers.dispatcher import (
     async_dispatcher_connect, async_dispatcher_send)
 from homeassistant.helpers.typing import HomeAssistantType
 
 from .const import (
-    APP_NAME_PREFIX, APP_OAUTH_SCOPES, CONF_APP_ID, CONF_INSTALLED_APP_ID,
-    CONF_INSTANCE_ID, CONF_LOCATION_ID, DATA_BROKERS, DATA_MANAGER, DOMAIN,
+    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)
 
 _LOGGER = logging.getLogger(__name__)
@@ -83,7 +84,7 @@ async def create_app(hass: HomeAssistantType, api):
     app = App()
     for key, value in template.items():
         setattr(app, key, value)
-    app = (await api.create_app(app))[0]
+    app, client = await api.create_app(app)
     _LOGGER.debug("Created SmartApp '%s' (%s)", app.app_name, app.app_id)
 
     # Set unique hass id in settings
@@ -97,12 +98,12 @@ async def create_app(hass: HomeAssistantType, api):
 
     # Set oauth scopes
     oauth = AppOAuth(app.app_id)
-    oauth.client_name = 'Home Assistant'
+    oauth.client_name = APP_OAUTH_CLIENT_NAME
     oauth.scope.extend(APP_OAUTH_SCOPES)
     await api.update_app_oauth(oauth)
     _LOGGER.debug("Updated App OAuth for SmartApp '%s' (%s)",
                   app.app_name, app.app_id)
-    return app
+    return app, client
 
 
 async def update_app(hass: HomeAssistantType, app):
@@ -185,32 +186,24 @@ async def setup_smartapp_endpoint(hass: HomeAssistantType):
         DATA_MANAGER: manager,
         CONF_INSTANCE_ID: config[CONF_INSTANCE_ID],
         DATA_BROKERS: {},
-        CONF_WEBHOOK_ID: config[CONF_WEBHOOK_ID]
+        CONF_WEBHOOK_ID: config[CONF_WEBHOOK_ID],
+        CONF_INSTALLED_APPS: []
     }
 
 
 async def smartapp_sync_subscriptions(
         hass: HomeAssistantType, auth_token: str, location_id: str,
-        installed_app_id: str, *, skip_delete=False):
+        installed_app_id: str, devices):
     """Synchronize subscriptions of an installed up."""
     from pysmartthings import (
-        CAPABILITIES, SmartThings, SourceType, Subscription)
+        CAPABILITIES, SmartThings, SourceType, Subscription,
+        SubscriptionEntity
+    )
 
     api = SmartThings(async_get_clientsession(hass), auth_token)
-    devices = await api.devices(location_ids=[location_id])
+    tasks = []
 
-    # Build set of capabilities and prune unsupported ones
-    capabilities = set()
-    for device in devices:
-        capabilities.update(device.capabilities)
-    capabilities.intersection_update(CAPABILITIES)
-
-    # Remove all (except for installs)
-    if not skip_delete:
-        await api.delete_subscriptions(installed_app_id)
-
-    # Create for each capability
-    async def create_subscription(target):
+    async def create_subscription(target: str):
         sub = Subscription()
         sub.installed_app_id = installed_app_id
         sub.location_id = location_id
@@ -224,52 +217,89 @@ async def smartapp_sync_subscriptions(
             _LOGGER.exception("Failed to create subscription for '%s' under "
                               "app '%s'", target, installed_app_id)
 
-    tasks = [create_subscription(c) for c in capabilities]
-    await asyncio.gather(*tasks)
+    async def delete_subscription(sub: SubscriptionEntity):
+        try:
+            await api.delete_subscription(
+                installed_app_id, sub.subscription_id)
+            _LOGGER.debug("Removed subscription for '%s' under app '%s' "
+                          "because it was no longer needed",
+                          sub.capability, installed_app_id)
+        except Exception:  # pylint:disable=broad-except
+            _LOGGER.exception("Failed to remove subscription for '%s' under "
+                              "app '%s'", sub.capability, installed_app_id)
+
+    # Build set of capabilities and prune unsupported ones
+    capabilities = set()
+    for device in devices:
+        capabilities.update(device.capabilities)
+    capabilities.intersection_update(CAPABILITIES)
+
+    # Get current subscriptions and find differences
+    subscriptions = await api.subscriptions(installed_app_id)
+    for subscription in subscriptions:
+        if subscription.capability in capabilities:
+            capabilities.remove(subscription.capability)
+        else:
+            # Delete the subscription
+            tasks.append(delete_subscription(subscription))
+
+    # Remaining capabilities need subscriptions created
+    tasks.extend([create_subscription(c) for c in capabilities])
+
+    if tasks:
+        await asyncio.gather(*tasks)
+    else:
+        _LOGGER.debug("Subscriptions for app '%s' are up-to-date",
+                      installed_app_id)
 
 
 async def smartapp_install(hass: HomeAssistantType, req, resp, app):
     """
     Handle when a SmartApp is installed by the user into a location.
 
-    Setup subscriptions using the access token SmartThings provided in the
-    event. An explicit subscription is required for each 'capability' in order
-    to receive the related attribute updates.  Finally, create a config entry
-    representing the installation if this is not the first installation under
-    the account.
+    Create a config entry representing the installation if this is not
+    the first installation under the account, otherwise store the data
+    for the config flow.
     """
-    await smartapp_sync_subscriptions(
-        hass, req.auth_token, req.location_id, req.installed_app_id,
-        skip_delete=True)
-
-    # The permanent access token is copied from another config flow with the
-    # same parent app_id.  If one is not found, that means the user is within
-    # the initial config flow and the entry at the conclusion.
-    access_token = next((
-        entry.data.get(CONF_ACCESS_TOKEN) for entry
+    install_data = {
+        CONF_INSTALLED_APP_ID: req.installed_app_id,
+        CONF_LOCATION_ID: req.location_id,
+        CONF_REFRESH_TOKEN: req.refresh_token
+    }
+    # App attributes (client id/secret, etc...) are copied from another entry
+    # with the same parent app_id.  If one is not found, the install data is
+    # stored for the config flow to retrieve during the wait step.
+    entry = next((
+        entry for entry
         in hass.config_entries.async_entries(DOMAIN)
         if entry.data[CONF_APP_ID] == app.app_id), None)
-    if access_token:
+    if entry:
+        data = entry.data.copy()
+        data.update(install_data)
         # Add as job not needed because the current coroutine was invoked
         # from the dispatcher and is not being awaited.
         await hass.config_entries.flow.async_init(
             DOMAIN, context={'source': 'install'},
-            data={
-                CONF_APP_ID: app.app_id,
-                CONF_INSTALLED_APP_ID: req.installed_app_id,
-                CONF_LOCATION_ID: req.location_id,
-                CONF_ACCESS_TOKEN: access_token
-            })
+            data=data)
+    else:
+        # Store the data where the flow can find it
+        hass.data[DOMAIN][CONF_INSTALLED_APPS].append(install_data)
 
 
 async def smartapp_update(hass: HomeAssistantType, req, resp, app):
     """
     Handle when a SmartApp is updated (reconfigured) by the user.
 
-    Synchronize subscriptions to ensure we're up-to-date.
+    Store the refresh token in the config entry.
     """
-    await smartapp_sync_subscriptions(
-        hass, req.auth_token, req.location_id, req.installed_app_id)
+    # Update refresh token in config entry
+    entry = next((entry for entry in hass.config_entries.async_entries(DOMAIN)
+                  if entry.data.get(CONF_INSTALLED_APP_ID) ==
+                  req.installed_app_id),
+                 None)
+    if entry:
+        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",
                   req.installed_app_id, app.app_id)