Fix tellduslive discovery and auth issues (#20023)

* fix for #19954, discovered tellsticks shows up to be configured

* fix for #19954, authentication issues

* updated tests

* move I/O to executer thread pool

* Apply suggestions from code review

Co-Authored-By: fredrike <fredrik.e@gmail.com>

homeassistant/components/tellduslive/.translations/en.json

@@ -1,11 +1,14 @@
 {
     "config": {
         "abort": {
-            "all_configured": "TelldusLive is already configured",
+            "already_setup": "TelldusLive is already configured",
             "authorize_url_fail": "Unknown error generating an authorize url.",
             "authorize_url_timeout": "Timeout generating authorize url.",
             "unknown": "Unknown error occurred"
         },
+        "error": {
+            "auth_error": "Authentication error, please try again"
+        },
         "step": {
             "auth": {
                 "description": "To link your TelldusLive account:\n 1. Click the link below\n 2. Login to Telldus Live\n 3. Authorize **{app_name}** (click **Yes**).\n 4. Come back here and click **SUBMIT**.\n\n [Link TelldusLive account]({auth_url})",

homeassistant/components/tellduslive/config_flow.py

@@ -35,6 +35,13 @@ class FlowHandler(config_entries.ConfigFlow):
         self._scan_interval = SCAN_INTERVAL
 
     def _get_auth_url(self):
+        from tellduslive import Session
+        self._session = Session(
+            public_key=PUBLIC_KEY,
+            private_key=NOT_SO_PRIVATE_KEY,
+            host=self._host,
+            application=APPLICATION_NAME,
+        )
         return self._session.authorize_url
 
     async def async_step_user(self, user_input=None):
@@ -56,38 +63,36 @@ class FlowHandler(config_entries.ConfigFlow):
 
     async def async_step_auth(self, user_input=None):
         """Handle the submitted configuration."""
-        if not self._session:
+        errors = {}
-            from tellduslive import Session
+        if user_input is not None:
-            self._session = Session(
+            if await self.hass.async_add_executor_job(
-                public_key=PUBLIC_KEY,
+                    self._session.authorize):
-                private_key=NOT_SO_PRIVATE_KEY,
+                host = self._host or CLOUD_NAME
-                host=self._host,
+                if self._host:
-                application=APPLICATION_NAME,
+                    session = {
-            )
+                        KEY_HOST: host,
-
+                        KEY_TOKEN: self._session.access_token
-        if user_input is not None and self._session.authorize():
+                    }
-            host = self._host or CLOUD_NAME
+                else:
-            if self._host:
+                    session = {
-                session = {
+                        KEY_TOKEN: self._session.access_token,
-                    KEY_HOST: host,
+                        KEY_TOKEN_SECRET: self._session.access_token_secret
-                    KEY_TOKEN: self._session.access_token
+                    }
-                }
+                return self.async_create_entry(
+                    title=host, data={
+                        KEY_HOST: host,
+                        KEY_SCAN_INTERVAL: self._scan_interval.seconds,
+                        KEY_SESSION: session,
+                    })
             else:
-                session = {
+                errors['base'] = 'auth_error'
-                    KEY_TOKEN: self._session.access_token,
-                    KEY_TOKEN_SECRET: self._session.access_token_secret
-                }
-            return self.async_create_entry(
-                title=host, data={
-                    KEY_HOST: host,
-                    KEY_SCAN_INTERVAL: self._scan_interval.seconds,
-                    KEY_SESSION: session,
-                })
 
         try:
             with async_timeout.timeout(10):
                 auth_url = await self.hass.async_add_executor_job(
                     self._get_auth_url)
+            if not auth_url:
+                return self.async_abort(reason='authorize_url_fail')
         except asyncio.TimeoutError:
             return self.async_abort(reason='authorize_url_timeout')
         except Exception:  # pylint: disable=broad-except
@@ -97,6 +102,7 @@ class FlowHandler(config_entries.ConfigFlow):
         _LOGGER.debug('Got authorization URL %s', auth_url)
         return self.async_show_form(
             step_id='auth',
+            errors=errors,
             description_placeholders={
                 'app_name': APPLICATION_NAME,
                 'auth_url': auth_url,
@@ -107,17 +113,10 @@ class FlowHandler(config_entries.ConfigFlow):
         """Run when a Tellstick is discovered."""
         from tellduslive import supports_local_api
         _LOGGER.info('Discovered tellstick device: %s', user_input)
-        # Ignore any known devices
+        if supports_local_api(user_input[1]):
-        for entry in self._async_current_entries():
+            _LOGGER.info('%s support local API', user_input[1])
-            if entry.data[KEY_HOST] == user_input[0]:
+            self._hosts.append(user_input[0])
-                return self.async_abort(reason='already_configured')
 
-        if not supports_local_api(user_input[1]):
-            _LOGGER.debug('Tellstick does not support local API')
-            # Configure the cloud service
-            return await self.async_step_auth()
-
-        self._hosts.append(user_input[0])
         return await self.async_step_user()
 
     async def async_step_import(self, user_input):

homeassistant/components/tellduslive/strings.json

@@ -1,24 +1,26 @@
 {
   "config": {
-    "title": "Telldus Live",
+      "abort": {
-    "step": {
+          "already_setup": "TelldusLive is already configured",
-      "user": {
+          "authorize_url_fail": "Unknown error generating an authorize url.",
-        "title": "Pick endpoint.",
+          "authorize_url_timeout": "Timeout generating authorize url.",
-        "description": "",
+          "unknown": "Unknown error occurred"
-        "data": {
-          "host": "Host"
-        }
       },
-      "auth": {
+      "error": {
-          "title": "Authenticate against TelldusLive",
+          "auth_error": "Authentication error, please try again"
-          "description": "To link your TelldusLive account:\n 1. Click the link below\n 2. Login to Telldus Live\n 3. Authorize **{app_name}** (click **Yes**).\n 4. Come back here and click **SUBMIT**.\n\n [Link TelldusLive account]({auth_url})"
+      },
-      }
+      "step": {
-    },
+          "auth": {
-    "abort": {
+              "description": "To link your TelldusLive account:\n 1. Click the link below\n 2. Login to Telldus Live\n 3. Authorize **{app_name}** (click **Yes**).\n 4. Come back here and click **SUBMIT**.\n\n [Link TelldusLive account]({auth_url})",
-      "authorize_url_timeout": "Timeout generating authorize url.",
+              "title": "Authenticate against TelldusLive"
-      "authorize_url_fail": "Unknown error generating an authorize url.",
+          },
-      "all_configured": "TelldusLive is already configured",
+          "user": {
-      "unknown": "Unknown error occurred"
+              "data": {
-    }
+                  "host": "Host"
+              },
+              "title": "Pick endpoint."
+          }
+      },
+      "title": "Telldus Live"
   }
-}
+}

tests/components/tellduslive/test_config_flow.py

@@ -67,6 +67,7 @@ async def test_full_flow_implementation(hass, mock_tellduslive):
     result = await flow.async_step_discovery(['localhost', 'tellstick'])
     assert result['type'] == data_entry_flow.RESULT_TYPE_FORM
     assert result['step_id'] == 'user'
+    assert len(flow._hosts) == 2
 
     result = await flow.async_step_user()
     assert result['type'] == data_entry_flow.RESULT_TYPE_FORM
@@ -156,12 +157,14 @@ async def test_step_disco_no_local_api(hass, mock_tellduslive):
     result = await flow.async_step_discovery(['localhost', 'tellstick'])
     assert result['type'] == data_entry_flow.RESULT_TYPE_FORM
     assert result['step_id'] == 'auth'
+    assert len(flow._hosts) == 1
 
 
 async def test_step_auth(hass, mock_tellduslive):
     """Test that create cloud entity from auth."""
     flow = init_config_flow(hass)
 
+    await flow.async_step_auth()
     result = await flow.async_step_auth(['localhost', 'tellstick'])
     assert result['type'] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
     assert result['title'] == 'Cloud API'
@@ -178,10 +181,11 @@ async def test_wrong_auth_flow_implementation(hass, mock_tellduslive):
     """Test wrong auth."""
     flow = init_config_flow(hass)
 
-    await flow.async_step_user()
+    await flow.async_step_auth()
     result = await flow.async_step_auth('')
     assert result['type'] == data_entry_flow.RESULT_TYPE_FORM
     assert result['step_id'] == 'auth'
+    assert result['errors']['base'] == 'auth_error'
 
 
 async def test_not_pick_host_if_only_one(hass, mock_tellduslive):
@@ -201,6 +205,14 @@ async def test_abort_if_timeout_generating_auth_url(hass, mock_tellduslive):
     assert result['type'] == data_entry_flow.RESULT_TYPE_ABORT
     assert result['reason'] == 'authorize_url_timeout'
 
+async def test_abort_no_auth_url(hass, mock_tellduslive):
+    """Test abort if generating authorize url returns none."""
+    flow = init_config_flow(hass)
+    flow._get_auth_url = Mock(return_value=False)
+
+    result = await flow.async_step_user()
+    assert result['type'] == data_entry_flow.RESULT_TYPE_ABORT
+    assert result['reason'] == 'authorize_url_fail'
 
 async def test_abort_if_exception_generating_auth_url(hass, mock_tellduslive):
     """Test we abort if generating authorize url blows up."""
@@ -220,4 +232,4 @@ async def test_discovery_already_configured(hass, mock_tellduslive):
 
     result = await flow.async_step_discovery(['some-host', ''])
     assert result['type'] == data_entry_flow.RESULT_TYPE_ABORT
-    assert result['reason'] == 'already_configured'
+    assert result['reason'] == 'already_setup'