Bump UPC connect / fix auth issue (#26570)

* Bump UPC connect / fix auth issue

* Fix lint

* Fix platform schema

* Fix config value

* Address comment / add session cleanup

* Fix device handling

.coveragerc

@@ -675,6 +675,7 @@ omit =
     homeassistant/components/ue_smart_radio/media_player.py
     homeassistant/components/upcloud/*
     homeassistant/components/upnp/*
+    homeassistant/components/upc_connect/*
     homeassistant/components/ups/sensor.py
     homeassistant/components/uptimerobot/binary_sensor.py
     homeassistant/components/uscis/sensor.py

CODEOWNERS

@@ -290,6 +290,7 @@ homeassistant/components/twentemilieu/* @frenck
 homeassistant/components/twilio_call/* @robbiet480
 homeassistant/components/twilio_sms/* @robbiet480
 homeassistant/components/unifi/* @kane610
+homeassistant/components/upc_connect/* @pvizeli
 homeassistant/components/upcloud/* @scop
 homeassistant/components/updater/* @home-assistant/core
 homeassistant/components/upnp/* @robbiet480

homeassistant/components/upc_connect/device_tracker.py

@@ -1,10 +1,9 @@
 """Support for UPC ConnectBox router."""
-import asyncio
 import logging
+from typing import List, Optional
 
-import aiohttp
+from connect_box import ConnectBox
-from aiohttp.hdrs import REFERER, USER_AGENT
+from connect_box.exceptions import ConnectBoxError, ConnectBoxLoginError
-import async_timeout
 import voluptuous as vol
 
 from homeassistant.components.device_tracker import (
@@ -12,118 +11,66 @@ from homeassistant.components.device_tracker import (
     PLATFORM_SCHEMA,
     DeviceScanner,
 )
-from homeassistant.const import CONF_HOST, HTTP_HEADER_X_REQUESTED_WITH
+from homeassistant.const import CONF_HOST, CONF_PASSWORD, EVENT_HOMEASSISTANT_STOP
-from homeassistant.helpers.aiohttp_client import async_get_clientsession
 import homeassistant.helpers.config_validation as cv
 
 _LOGGER = logging.getLogger(__name__)
 
-CMD_DEVICES = 123
-
 DEFAULT_IP = "192.168.0.1"
 
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend(
-    {vol.Optional(CONF_HOST, default=DEFAULT_IP): cv.string}
+    {
+        vol.Required(CONF_PASSWORD): cv.string,
+        vol.Optional(CONF_HOST, default=DEFAULT_IP): cv.string,
+    }
 )
 
 
 async def async_get_scanner(hass, config):
     """Return the UPC device scanner."""
-    scanner = UPCDeviceScanner(hass, config[DOMAIN])
+    conf = config[DOMAIN]
-    success_init = await scanner.async_initialize_token()
+    session = hass.helpers.aiohttp_client.async_get_clientsession()
+    connect_box = ConnectBox(session, conf[CONF_PASSWORD], host=conf[CONF_HOST])
 
-    return scanner if success_init else None
+    # Check login data
+    try:
+        await connect_box.async_initialize_token()
+    except ConnectBoxLoginError:
+        _LOGGER.error("ConnectBox login data error!")
+        return None
+    except ConnectBoxError:
+        pass
+
+    async def _shutdown(event):
+        """Shutdown event."""
+        await connect_box.async_close_session()
+
+    hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, _shutdown)
+
+    return UPCDeviceScanner(connect_box)
 
 
 class UPCDeviceScanner(DeviceScanner):
     """This class queries a router running UPC ConnectBox firmware."""
 
-    def __init__(self, hass, config):
+    def __init__(self, connect_box: ConnectBox):
         """Initialize the scanner."""
-        self.hass = hass
+        self.connect_box: ConnectBox = connect_box
-        self.host = config[CONF_HOST]
 
-        self.data = {}
+    async def async_scan_devices(self) -> List[str]:
-        self.token = None
-
-        self.headers = {
-            HTTP_HEADER_X_REQUESTED_WITH: "XMLHttpRequest",
-            REFERER: f"http://{self.host}/index.html",
-            USER_AGENT: (
-                "Mozilla/5.0 (Windows NT 10.0; WOW64) "
-                "AppleWebKit/537.36 (KHTML, like Gecko) "
-                "Chrome/47.0.2526.106 Safari/537.36"
-            ),
-        }
-
-        self.websession = async_get_clientsession(hass)
-
-    async def async_scan_devices(self):
         """Scan for new devices and return a list with found device IDs."""
-        import defusedxml.ElementTree as ET
-
-        if self.token is None:
-            token_initialized = await self.async_initialize_token()
-            if not token_initialized:
-                _LOGGER.error("Not connected to %s", self.host)
-                return []
-
-        raw = await self._async_ws_function(CMD_DEVICES)
-
         try:
-            xml_root = ET.fromstring(raw)
+            await self.connect_box.async_get_devices()
-            return [mac.text for mac in xml_root.iter("MACAddr")]
+        except ConnectBoxError:
-        except (ET.ParseError, TypeError):
-            _LOGGER.warning("Can't read device from %s", self.host)
-            self.token = None
             return []
 
-    async def async_get_device_name(self, device):
+        return [device.mac for device in self.connect_box.devices]
+
+    async def async_get_device_name(self, device: str) -> Optional[str]:
         """Get the device name (the name of the wireless device not used)."""
+        for connected_device in self.connect_box.devices:
+            if connected_device != device:
+                continue
+            return connected_device.hostname
+
         return None
-
-    async def async_initialize_token(self):
-        """Get first token."""
-        try:
-            # get first token
-            with async_timeout.timeout(10):
-                response = await self.websession.get(
-                    f"http://{self.host}/common_page/login.html", headers=self.headers
-                )
-
-                await response.text()
-
-            self.token = response.cookies["sessionToken"].value
-
-            return True
-
-        except (asyncio.TimeoutError, aiohttp.ClientError):
-            _LOGGER.error("Can not load login page from %s", self.host)
-            return False
-
-    async def _async_ws_function(self, function):
-        """Execute a command on UPC firmware webservice."""
-        try:
-            with async_timeout.timeout(10):
-                # The 'token' parameter has to be first, and 'fun' second
-                # or the UPC firmware will return an error
-                response = await self.websession.post(
-                    f"http://{self.host}/xml/getter.xml",
-                    data=f"token={self.token}&fun={function}",
-                    headers=self.headers,
-                    allow_redirects=False,
-                )
-
-                # Error?
-                if response.status != 200:
-                    _LOGGER.warning("Receive http code %d", response.status)
-                    self.token = None
-                    return
-
-                # Load data, store token for next request
-                self.token = response.cookies["sessionToken"].value
-                return await response.text()
-
-        except (asyncio.TimeoutError, aiohttp.ClientError):
-            _LOGGER.error("Error on %s", function)
-            self.token = None

homeassistant/components/upc_connect/manifest.json

@@ -2,9 +2,7 @@
   "domain": "upc_connect",
   "name": "Upc connect",
   "documentation": "https://www.home-assistant.io/components/upc_connect",
-  "requirements": [
+  "requirements": ["connect-box==0.2.3"],
-    "defusedxml==0.6.0"
-  ],
   "dependencies": [],
-  "codeowners": []
+  "codeowners": ["@pvizeli"]
 }

requirements_all.txt

@@ -354,6 +354,9 @@ colorlog==4.0.2
 # homeassistant.components.concord232
 concord232==0.15
 
+# homeassistant.components.upc_connect
+connect-box==0.2.3
+
 # homeassistant.components.eddystone_temperature
 # homeassistant.components.eq3btsmart
 # homeassistant.components.xiaomi_miio
@@ -380,7 +383,6 @@ datapoint==0.4.3
 # homeassistant.components.ihc
 # homeassistant.components.namecheapdns
 # homeassistant.components.ohmconnect
-# homeassistant.components.upc_connect
 defusedxml==0.6.0
 
 # homeassistant.components.deluge

requirements_test_all.txt

@@ -105,7 +105,6 @@ coinmarketcap==5.0.3
 # homeassistant.components.ihc
 # homeassistant.components.namecheapdns
 # homeassistant.components.ohmconnect
-# homeassistant.components.upc_connect
 defusedxml==0.6.0
 
 # homeassistant.components.dsmr

tests/components/upc_connect/__init__.py

@@ -1 +0,0 @@
-"""Tests for the upc_connect component."""

tests/components/upc_connect/test_device_tracker.py

@@ -1,221 +0,0 @@
-"""The tests for the UPC ConnextBox device tracker platform."""
-import asyncio
-
-from asynctest import patch
-import pytest
-
-from homeassistant.components.device_tracker import DOMAIN
-import homeassistant.components.upc_connect.device_tracker as platform
-from homeassistant.const import CONF_HOST, CONF_PLATFORM
-from homeassistant.setup import async_setup_component
-
-from tests.common import assert_setup_component, load_fixture, mock_component
-
-HOST = "127.0.0.1"
-
-
-async def async_scan_devices_mock(scanner):
-    """Mock async_scan_devices."""
-    return []
-
-
-@pytest.fixture(autouse=True)
-def setup_comp_deps(hass, mock_device_tracker_conf):
-    """Set up component dependencies."""
-    mock_component(hass, "zone")
-    mock_component(hass, "group")
-    yield
-
-
-async def test_setup_platform_timeout_loginpage(hass, caplog, aioclient_mock):
-    """Set up a platform with timeout on loginpage."""
-    aioclient_mock.get(
-        "http://{}/common_page/login.html".format(HOST), exc=asyncio.TimeoutError()
-    )
-    aioclient_mock.post("http://{}/xml/getter.xml".format(HOST), content=b"successful")
-
-    assert await async_setup_component(
-        hass, DOMAIN, {DOMAIN: {CONF_PLATFORM: "upc_connect", CONF_HOST: HOST}}
-    )
-
-    assert len(aioclient_mock.mock_calls) == 1
-
-    assert "Error setting up platform" in caplog.text
-
-
-async def test_setup_platform_timeout_webservice(hass, caplog, aioclient_mock):
-    """Set up a platform with api timeout."""
-    aioclient_mock.get(
-        "http://{}/common_page/login.html".format(HOST),
-        cookies={"sessionToken": "654321"},
-        content=b"successful",
-        exc=asyncio.TimeoutError(),
-    )
-
-    assert await async_setup_component(
-        hass, DOMAIN, {DOMAIN: {CONF_PLATFORM: "upc_connect", CONF_HOST: HOST}}
-    )
-
-    assert len(aioclient_mock.mock_calls) == 1
-
-    assert "Error setting up platform" in caplog.text
-
-
-@patch(
-    "homeassistant.components.upc_connect.device_tracker."
-    "UPCDeviceScanner.async_scan_devices",
-    return_value=async_scan_devices_mock,
-)
-async def test_setup_platform(scan_mock, hass, aioclient_mock):
-    """Set up a platform."""
-    aioclient_mock.get(
-        "http://{}/common_page/login.html".format(HOST),
-        cookies={"sessionToken": "654321"},
-    )
-    aioclient_mock.post("http://{}/xml/getter.xml".format(HOST), content=b"successful")
-
-    with assert_setup_component(1, DOMAIN):
-        assert await async_setup_component(
-            hass, DOMAIN, {DOMAIN: {CONF_PLATFORM: "upc_connect", CONF_HOST: HOST}}
-        )
-
-    assert len(aioclient_mock.mock_calls) == 1
-
-
-async def test_scan_devices(hass, aioclient_mock):
-    """Set up a upc platform and scan device."""
-    aioclient_mock.get(
-        "http://{}/common_page/login.html".format(HOST),
-        cookies={"sessionToken": "654321"},
-    )
-    aioclient_mock.post(
-        "http://{}/xml/getter.xml".format(HOST),
-        content=b"successful",
-        cookies={"sessionToken": "654321"},
-    )
-
-    scanner = await platform.async_get_scanner(
-        hass, {DOMAIN: {CONF_PLATFORM: "upc_connect", CONF_HOST: HOST}}
-    )
-
-    assert len(aioclient_mock.mock_calls) == 1
-
-    aioclient_mock.clear_requests()
-    aioclient_mock.post(
-        "http://{}/xml/getter.xml".format(HOST),
-        text=load_fixture("upc_connect.xml"),
-        cookies={"sessionToken": "1235678"},
-    )
-
-    mac_list = await scanner.async_scan_devices()
-
-    assert len(aioclient_mock.mock_calls) == 1
-    assert aioclient_mock.mock_calls[0][2] == "token=654321&fun=123"
-    assert mac_list == ["30:D3:2D:0:69:21", "5C:AA:FD:25:32:02", "70:EE:50:27:A1:38"]
-
-
-async def test_scan_devices_without_session(hass, aioclient_mock):
-    """Set up a upc platform and scan device with no token."""
-    aioclient_mock.get(
-        "http://{}/common_page/login.html".format(HOST),
-        cookies={"sessionToken": "654321"},
-    )
-    aioclient_mock.post(
-        "http://{}/xml/getter.xml".format(HOST),
-        content=b"successful",
-        cookies={"sessionToken": "654321"},
-    )
-
-    scanner = await platform.async_get_scanner(
-        hass, {DOMAIN: {CONF_PLATFORM: "upc_connect", CONF_HOST: HOST}}
-    )
-
-    assert len(aioclient_mock.mock_calls) == 1
-
-    aioclient_mock.clear_requests()
-    aioclient_mock.get(
-        "http://{}/common_page/login.html".format(HOST),
-        cookies={"sessionToken": "654321"},
-    )
-    aioclient_mock.post(
-        "http://{}/xml/getter.xml".format(HOST),
-        text=load_fixture("upc_connect.xml"),
-        cookies={"sessionToken": "1235678"},
-    )
-
-    scanner.token = None
-    mac_list = await scanner.async_scan_devices()
-
-    assert len(aioclient_mock.mock_calls) == 2
-    assert aioclient_mock.mock_calls[1][2] == "token=654321&fun=123"
-    assert mac_list == ["30:D3:2D:0:69:21", "5C:AA:FD:25:32:02", "70:EE:50:27:A1:38"]
-
-
-async def test_scan_devices_without_session_wrong_re(hass, aioclient_mock):
-    """Set up a upc platform and scan device with no token and wrong."""
-    aioclient_mock.get(
-        "http://{}/common_page/login.html".format(HOST),
-        cookies={"sessionToken": "654321"},
-    )
-    aioclient_mock.post(
-        "http://{}/xml/getter.xml".format(HOST),
-        content=b"successful",
-        cookies={"sessionToken": "654321"},
-    )
-
-    scanner = await platform.async_get_scanner(
-        hass, {DOMAIN: {CONF_PLATFORM: "upc_connect", CONF_HOST: HOST}}
-    )
-
-    assert len(aioclient_mock.mock_calls) == 1
-
-    aioclient_mock.clear_requests()
-    aioclient_mock.get(
-        "http://{}/common_page/login.html".format(HOST),
-        cookies={"sessionToken": "654321"},
-    )
-    aioclient_mock.post(
-        "http://{}/xml/getter.xml".format(HOST),
-        status=400,
-        cookies={"sessionToken": "1235678"},
-    )
-
-    scanner.token = None
-    mac_list = await scanner.async_scan_devices()
-
-    assert len(aioclient_mock.mock_calls) == 2
-    assert aioclient_mock.mock_calls[1][2] == "token=654321&fun=123"
-    assert mac_list == []
-
-
-async def test_scan_devices_parse_error(hass, aioclient_mock):
-    """Set up a upc platform and scan device with parse error."""
-    aioclient_mock.get(
-        "http://{}/common_page/login.html".format(HOST),
-        cookies={"sessionToken": "654321"},
-    )
-    aioclient_mock.post(
-        "http://{}/xml/getter.xml".format(HOST),
-        content=b"successful",
-        cookies={"sessionToken": "654321"},
-    )
-
-    scanner = await platform.async_get_scanner(
-        hass, {DOMAIN: {CONF_PLATFORM: "upc_connect", CONF_HOST: HOST}}
-    )
-
-    assert len(aioclient_mock.mock_calls) == 1
-
-    aioclient_mock.clear_requests()
-    aioclient_mock.post(
-        "http://{}/xml/getter.xml".format(HOST),
-        text="Blablebla blabalble",
-        cookies={"sessionToken": "1235678"},
-    )
-
-    mac_list = await scanner.async_scan_devices()
-
-    assert len(aioclient_mock.mock_calls) == 1
-    assert aioclient_mock.mock_calls[0][2] == "token=654321&fun=123"
-    assert scanner.token is None
-    assert mac_list == []