From efa339ca543c62a191e8c93ee27d283656be3b9e Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Sun, 21 Feb 2021 03:26:17 +0100 Subject: [PATCH] Allow upnp ignore SSDP-discoveries (#46592) --- homeassistant/components/upnp/__init__.py | 16 ++++- homeassistant/components/upnp/config_flow.py | 13 ++++ homeassistant/components/upnp/const.py | 2 + homeassistant/components/upnp/device.py | 15 ++++- homeassistant/components/upnp/sensor.py | 6 +- tests/components/upnp/mock_device.py | 9 ++- tests/components/upnp/test_config_flow.py | 67 +++++++++++++++++++- tests/components/upnp/test_init.py | 4 ++ 8 files changed, 120 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/upnp/__init__.py b/homeassistant/components/upnp/__init__.py index 7b46037d99d..5d251ce7dd8 100644 --- a/homeassistant/components/upnp/__init__.py +++ b/homeassistant/components/upnp/__init__.py @@ -13,6 +13,7 @@ from homeassistant.util import get_local_ip from .const import ( CONF_LOCAL_IP, + CONFIG_ENTRY_HOSTNAME, CONFIG_ENTRY_ST, CONFIG_ENTRY_UDN, DISCOVERY_LOCATION, @@ -31,7 +32,13 @@ NOTIFICATION_ID = "upnp_notification" NOTIFICATION_TITLE = "UPnP/IGD Setup" CONFIG_SCHEMA = vol.Schema( - {DOMAIN: vol.Schema({vol.Optional(CONF_LOCAL_IP): vol.All(ip_address, cv.string)})}, + { + DOMAIN: vol.Schema( + { + vol.Optional(CONF_LOCAL_IP): vol.All(ip_address, cv.string), + }, + ) + }, extra=vol.ALLOW_EXTRA, ) @@ -115,6 +122,13 @@ async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry) unique_id=device.unique_id, ) + # Ensure entry has a hostname, for older entries. + if CONFIG_ENTRY_HOSTNAME not in config_entry.data: + hass.config_entries.async_update_entry( + entry=config_entry, + data={CONFIG_ENTRY_HOSTNAME: device.hostname, **config_entry.data}, + ) + # Create device registry entry. device_registry = await dr.async_get_registry(hass) device_registry.async_get_or_create( diff --git a/homeassistant/components/upnp/config_flow.py b/homeassistant/components/upnp/config_flow.py index d3811b7e18b..1d212441bfa 100644 --- a/homeassistant/components/upnp/config_flow.py +++ b/homeassistant/components/upnp/config_flow.py @@ -10,10 +10,12 @@ from homeassistant.const import CONF_SCAN_INTERVAL from homeassistant.core import callback from .const import ( + CONFIG_ENTRY_HOSTNAME, CONFIG_ENTRY_SCAN_INTERVAL, CONFIG_ENTRY_ST, CONFIG_ENTRY_UDN, DEFAULT_SCAN_INTERVAL, + DISCOVERY_HOSTNAME, DISCOVERY_LOCATION, DISCOVERY_NAME, DISCOVERY_ST, @@ -179,6 +181,16 @@ class UpnpFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): await self.async_set_unique_id(unique_id) self._abort_if_unique_id_configured() + # Handle devices changing their UDN, only allow a single + existing_entries = self.hass.config_entries.async_entries(DOMAIN) + for config_entry in existing_entries: + entry_hostname = config_entry.data.get(CONFIG_ENTRY_HOSTNAME) + if entry_hostname == discovery[DISCOVERY_HOSTNAME]: + _LOGGER.debug( + "Found existing config_entry with same hostname, discovery ignored" + ) + return self.async_abort(reason="discovery_ignored") + # Store discovery. self._discoveries = [discovery] @@ -222,6 +234,7 @@ class UpnpFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): data = { CONFIG_ENTRY_UDN: discovery[DISCOVERY_UDN], CONFIG_ENTRY_ST: discovery[DISCOVERY_ST], + CONFIG_ENTRY_HOSTNAME: discovery[DISCOVERY_HOSTNAME], } return self.async_create_entry(title=title, data=data) diff --git a/homeassistant/components/upnp/const.py b/homeassistant/components/upnp/const.py index 4ccf6d3d7ea..6575139c4a4 100644 --- a/homeassistant/components/upnp/const.py +++ b/homeassistant/components/upnp/const.py @@ -21,6 +21,7 @@ DATA_PACKETS = "packets" DATA_RATE_PACKETS_PER_SECOND = f"{DATA_PACKETS}/{TIME_SECONDS}" KIBIBYTE = 1024 UPDATE_INTERVAL = timedelta(seconds=30) +DISCOVERY_HOSTNAME = "hostname" DISCOVERY_LOCATION = "location" DISCOVERY_NAME = "name" DISCOVERY_ST = "st" @@ -30,4 +31,5 @@ DISCOVERY_USN = "usn" CONFIG_ENTRY_SCAN_INTERVAL = "scan_interval" CONFIG_ENTRY_ST = "st" CONFIG_ENTRY_UDN = "udn" +CONFIG_ENTRY_HOSTNAME = "hostname" DEFAULT_SCAN_INTERVAL = timedelta(seconds=30).seconds diff --git a/homeassistant/components/upnp/device.py b/homeassistant/components/upnp/device.py index 39fd09089b4..a06ca254c87 100644 --- a/homeassistant/components/upnp/device.py +++ b/homeassistant/components/upnp/device.py @@ -4,6 +4,7 @@ from __future__ import annotations import asyncio from ipaddress import IPv4Address from typing import List, Mapping +from urllib.parse import urlparse from async_upnp_client import UpnpFactory from async_upnp_client.aiohttp import AiohttpSessionRequester @@ -17,6 +18,7 @@ from .const import ( BYTES_RECEIVED, BYTES_SENT, CONF_LOCAL_IP, + DISCOVERY_HOSTNAME, DISCOVERY_LOCATION, DISCOVERY_NAME, DISCOVERY_ST, @@ -66,10 +68,10 @@ class Device: cls, hass: HomeAssistantType, discovery: Mapping ) -> Mapping: """Get additional data from device and supplement discovery.""" - device = await Device.async_create_device(hass, discovery[DISCOVERY_LOCATION]) + location = discovery[DISCOVERY_LOCATION] + device = await Device.async_create_device(hass, location) discovery[DISCOVERY_NAME] = device.name - - # Set unique_id. + discovery[DISCOVERY_HOSTNAME] = device.hostname discovery[DISCOVERY_UNIQUE_ID] = discovery[DISCOVERY_USN] return discovery @@ -126,6 +128,13 @@ class Device: """Get the unique id.""" return self.usn + @property + def hostname(self) -> str: + """Get the hostname.""" + url = self._igd_device.device.device_url + parsed = urlparse(url) + return parsed.hostname + def __str__(self) -> str: """Get string representation.""" return f"IGD Device: {self.name}/{self.udn}::{self.device_type}" diff --git a/homeassistant/components/upnp/sensor.py b/homeassistant/components/upnp/sensor.py index 59205f49667..97d3c1a702c 100644 --- a/homeassistant/components/upnp/sensor.py +++ b/homeassistant/components/upnp/sensor.py @@ -1,6 +1,6 @@ """Support for UPnP/IGD Sensors.""" from datetime import timedelta -from typing import Any, Mapping +from typing import Any, Mapping, Optional from homeassistant.config_entries import ConfigEntry from homeassistant.const import DATA_BYTES, DATA_RATE_KIBIBYTES_PER_SECOND @@ -176,7 +176,7 @@ class RawUpnpSensor(UpnpSensor): """Representation of a UPnP/IGD sensor.""" @property - def state(self) -> str: + def state(self) -> Optional[str]: """Return the state of the device.""" device_value_key = self._sensor_type["device_value_key"] value = self.coordinator.data[device_value_key] @@ -214,7 +214,7 @@ class DerivedUpnpSensor(UpnpSensor): return current_value < self._last_value @property - def state(self) -> str: + def state(self) -> Optional[str]: """Return the state of the device.""" # Can't calculate any derivative if we have only one value. device_value_key = self._sensor_type["device_value_key"] diff --git a/tests/components/upnp/mock_device.py b/tests/components/upnp/mock_device.py index a70b3fa0237..d6027608137 100644 --- a/tests/components/upnp/mock_device.py +++ b/tests/components/upnp/mock_device.py @@ -16,14 +16,14 @@ import homeassistant.util.dt as dt_util class MockDevice(Device): """Mock device for Device.""" - def __init__(self, udn): + def __init__(self, udn: str) -> None: """Initialize mock device.""" igd_device = object() super().__init__(igd_device) self._udn = udn @classmethod - async def async_create_device(cls, hass, ssdp_location): + async def async_create_device(cls, hass, ssdp_location) -> "MockDevice": """Return self.""" return cls("UDN") @@ -52,6 +52,11 @@ class MockDevice(Device): """Get the device type.""" return "urn:schemas-upnp-org:device:InternetGatewayDevice:1" + @property + def hostname(self) -> str: + """Get the hostname.""" + return "mock-hostname" + async def async_get_traffic_data(self) -> Mapping[str, any]: """Get traffic data.""" return { diff --git a/tests/components/upnp/test_config_flow.py b/tests/components/upnp/test_config_flow.py index f702d770ee6..77d04381a12 100644 --- a/tests/components/upnp/test_config_flow.py +++ b/tests/components/upnp/test_config_flow.py @@ -6,10 +6,12 @@ from unittest.mock import AsyncMock, patch from homeassistant import config_entries, data_entry_flow from homeassistant.components import ssdp from homeassistant.components.upnp.const import ( + CONFIG_ENTRY_HOSTNAME, CONFIG_ENTRY_SCAN_INTERVAL, CONFIG_ENTRY_ST, CONFIG_ENTRY_UDN, DEFAULT_SCAN_INTERVAL, + DISCOVERY_HOSTNAME, DISCOVERY_LOCATION, DISCOVERY_NAME, DISCOVERY_ST, @@ -41,6 +43,7 @@ async def test_flow_ssdp_discovery(hass: HomeAssistantType): DISCOVERY_UDN: mock_device.udn, DISCOVERY_UNIQUE_ID: mock_device.unique_id, DISCOVERY_USN: mock_device.usn, + DISCOVERY_HOSTNAME: mock_device.hostname, } ] with patch.object( @@ -75,10 +78,11 @@ async def test_flow_ssdp_discovery(hass: HomeAssistantType): assert result["data"] == { CONFIG_ENTRY_ST: mock_device.device_type, CONFIG_ENTRY_UDN: mock_device.udn, + CONFIG_ENTRY_HOSTNAME: mock_device.hostname, } -async def test_flow_ssdp_discovery_incomplete(hass: HomeAssistantType): +async def test_flow_ssdp_incomplete_discovery(hass: HomeAssistantType): """Test config flow: incomplete discovery through ssdp.""" udn = "uuid:device_1" location = "dummy" @@ -89,15 +93,64 @@ async def test_flow_ssdp_discovery_incomplete(hass: HomeAssistantType): DOMAIN, context={"source": config_entries.SOURCE_SSDP}, data={ - ssdp.ATTR_SSDP_ST: mock_device.device_type, - # ssdp.ATTR_UPNP_UDN: mock_device.udn, # Not provided. ssdp.ATTR_SSDP_LOCATION: location, + ssdp.ATTR_SSDP_ST: mock_device.device_type, + ssdp.ATTR_SSDP_USN: mock_device.usn, + # ssdp.ATTR_UPNP_UDN: mock_device.udn, # Not provided. }, ) assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["reason"] == "incomplete_discovery" +async def test_flow_ssdp_discovery_ignored(hass: HomeAssistantType): + """Test config flow: discovery through ssdp, but ignored.""" + udn = "uuid:device_random_1" + location = "dummy" + mock_device = MockDevice(udn) + + # Existing entry. + config_entry = MockConfigEntry( + domain=DOMAIN, + data={ + CONFIG_ENTRY_UDN: "uuid:device_random_2", + CONFIG_ENTRY_ST: mock_device.device_type, + CONFIG_ENTRY_HOSTNAME: mock_device.hostname, + }, + options={CONFIG_ENTRY_SCAN_INTERVAL: DEFAULT_SCAN_INTERVAL}, + ) + config_entry.add_to_hass(hass) + + discoveries = [ + { + DISCOVERY_LOCATION: location, + DISCOVERY_NAME: mock_device.name, + DISCOVERY_ST: mock_device.device_type, + DISCOVERY_UDN: mock_device.udn, + DISCOVERY_UNIQUE_ID: mock_device.unique_id, + DISCOVERY_USN: mock_device.usn, + DISCOVERY_HOSTNAME: mock_device.hostname, + } + ] + + with patch.object( + Device, "async_supplement_discovery", AsyncMock(return_value=discoveries[0]) + ): + # Discovered via step ssdp, but ignored. + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_SSDP}, + data={ + ssdp.ATTR_SSDP_LOCATION: location, + ssdp.ATTR_SSDP_ST: mock_device.device_type, + ssdp.ATTR_SSDP_USN: mock_device.usn, + ssdp.ATTR_UPNP_UDN: mock_device.udn, + }, + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "discovery_ignored" + + async def test_flow_user(hass: HomeAssistantType): """Test config flow: discovered + configured through user.""" udn = "uuid:device_1" @@ -111,6 +164,7 @@ async def test_flow_user(hass: HomeAssistantType): DISCOVERY_UDN: mock_device.udn, DISCOVERY_UNIQUE_ID: mock_device.unique_id, DISCOVERY_USN: mock_device.usn, + DISCOVERY_HOSTNAME: mock_device.hostname, } ] @@ -139,6 +193,7 @@ async def test_flow_user(hass: HomeAssistantType): assert result["data"] == { CONFIG_ENTRY_ST: mock_device.device_type, CONFIG_ENTRY_UDN: mock_device.udn, + CONFIG_ENTRY_HOSTNAME: mock_device.hostname, } @@ -155,6 +210,7 @@ async def test_flow_import(hass: HomeAssistantType): DISCOVERY_UDN: mock_device.udn, DISCOVERY_UNIQUE_ID: mock_device.unique_id, DISCOVERY_USN: mock_device.usn, + DISCOVERY_HOSTNAME: mock_device.hostname, } ] @@ -175,6 +231,7 @@ async def test_flow_import(hass: HomeAssistantType): assert result["data"] == { CONFIG_ENTRY_ST: mock_device.device_type, CONFIG_ENTRY_UDN: mock_device.udn, + CONFIG_ENTRY_HOSTNAME: mock_device.hostname, } @@ -189,6 +246,7 @@ async def test_flow_import_already_configured(hass: HomeAssistantType): data={ CONFIG_ENTRY_UDN: mock_device.udn, CONFIG_ENTRY_ST: mock_device.device_type, + CONFIG_ENTRY_HOSTNAME: mock_device.hostname, }, options={CONFIG_ENTRY_SCAN_INTERVAL: DEFAULT_SCAN_INTERVAL}, ) @@ -216,6 +274,7 @@ async def test_flow_import_incomplete(hass: HomeAssistantType): DISCOVERY_UDN: mock_device.udn, DISCOVERY_UNIQUE_ID: mock_device.unique_id, DISCOVERY_USN: mock_device.usn, + DISCOVERY_HOSTNAME: mock_device.hostname, } ] @@ -243,6 +302,7 @@ async def test_options_flow(hass: HomeAssistantType): DISCOVERY_UDN: mock_device.udn, DISCOVERY_UNIQUE_ID: mock_device.unique_id, DISCOVERY_USN: mock_device.usn, + DISCOVERY_HOSTNAME: mock_device.hostname, } ] config_entry = MockConfigEntry( @@ -250,6 +310,7 @@ async def test_options_flow(hass: HomeAssistantType): data={ CONFIG_ENTRY_UDN: mock_device.udn, CONFIG_ENTRY_ST: mock_device.device_type, + CONFIG_ENTRY_HOSTNAME: mock_device.hostname, }, options={CONFIG_ENTRY_SCAN_INTERVAL: DEFAULT_SCAN_INTERVAL}, ) diff --git a/tests/components/upnp/test_init.py b/tests/components/upnp/test_init.py index 26b2f970fed..086fbd677ab 100644 --- a/tests/components/upnp/test_init.py +++ b/tests/components/upnp/test_init.py @@ -5,6 +5,7 @@ from unittest.mock import AsyncMock, patch from homeassistant.components.upnp.const import ( CONFIG_ENTRY_ST, CONFIG_ENTRY_UDN, + DISCOVERY_HOSTNAME, DISCOVERY_LOCATION, DISCOVERY_NAME, DISCOVERY_ST, @@ -35,6 +36,7 @@ async def test_async_setup_entry_default(hass: HomeAssistantType): DISCOVERY_UDN: mock_device.udn, DISCOVERY_UNIQUE_ID: mock_device.unique_id, DISCOVERY_USN: mock_device.usn, + DISCOVERY_HOSTNAME: mock_device.hostname, } ] entry = MockConfigEntry( @@ -83,6 +85,7 @@ async def test_sync_setup_entry_multiple_discoveries(hass: HomeAssistantType): DISCOVERY_UDN: mock_device_0.udn, DISCOVERY_UNIQUE_ID: mock_device_0.unique_id, DISCOVERY_USN: mock_device_0.usn, + DISCOVERY_HOSTNAME: mock_device_0.hostname, }, { DISCOVERY_LOCATION: location_1, @@ -91,6 +94,7 @@ async def test_sync_setup_entry_multiple_discoveries(hass: HomeAssistantType): DISCOVERY_UDN: mock_device_1.udn, DISCOVERY_UNIQUE_ID: mock_device_1.unique_id, DISCOVERY_USN: mock_device_1.usn, + DISCOVERY_HOSTNAME: mock_device_1.hostname, }, ] entry = MockConfigEntry(