From aaf515ef674f363529ebb27237d69ba74bb175d8 Mon Sep 17 00:00:00 2001 From: Chris Talkington Date: Sat, 16 May 2020 03:48:36 -0500 Subject: [PATCH] Prevent discovery of IPP printers lacking identifier (#35630) --- homeassistant/components/ipp/config_flow.py | 1 + homeassistant/components/ipp/strings.json | 3 +- tests/components/ipp/__init__.py | 98 ++++++++++++++---- tests/components/ipp/test_config_flow.py | 86 ++++++--------- tests/components/ipp/test_init.py | 6 +- tests/components/ipp/test_sensor.py | 4 +- .../get-printer-attributes-success-nodata.bin | Bin 0 -> 72 bytes 7 files changed, 117 insertions(+), 81 deletions(-) create mode 100644 tests/fixtures/ipp/get-printer-attributes-success-nodata.bin diff --git a/homeassistant/components/ipp/config_flow.py b/homeassistant/components/ipp/config_flow.py index 7d1c3d1b1b8..3128583f218 100644 --- a/homeassistant/components/ipp/config_flow.py +++ b/homeassistant/components/ipp/config_flow.py @@ -152,6 +152,7 @@ class IPPFlowHandler(ConfigFlow, domain=DOMAIN): _LOGGER.debug( "Unable to determine unique id from discovery info and IPP response" ) + return self.async_abort(reason="unique_id_required") await self.async_set_unique_id(unique_id) self._abort_if_unique_id_configured( diff --git a/homeassistant/components/ipp/strings.json b/homeassistant/components/ipp/strings.json index 999c868f080..09c2424151f 100644 --- a/homeassistant/components/ipp/strings.json +++ b/homeassistant/components/ipp/strings.json @@ -28,7 +28,8 @@ "connection_upgrade": "Failed to connect to printer due to connection upgrade being required.", "ipp_error": "Encountered IPP error.", "ipp_version_error": "IPP version not supported by printer.", - "parse_error": "Failed to parse response from printer." + "parse_error": "Failed to parse response from printer.", + "unique_id_required": "Device missing unique identification required for discovery." } } } diff --git a/tests/components/ipp/__init__.py b/tests/components/ipp/__init__.py index f0dc45417e1..515543f3cf5 100644 --- a/tests/components/ipp/__init__.py +++ b/tests/components/ipp/__init__.py @@ -1,6 +1,9 @@ """Tests for the IPP integration.""" import os +import aiohttp +from pyipp import IPPConnectionUpgradeRequired, IPPError + from homeassistant.components.ipp.const import CONF_BASE_PATH, CONF_UUID, DOMAIN from homeassistant.const import ( CONF_HOST, @@ -18,21 +21,25 @@ from tests.test_util.aiohttp import AiohttpClientMocker ATTR_HOSTNAME = "hostname" ATTR_PROPERTIES = "properties" +HOST = "192.168.1.31" +PORT = 631 +BASE_PATH = "/ipp/print" + IPP_ZEROCONF_SERVICE_TYPE = "_ipp._tcp.local." IPPS_ZEROCONF_SERVICE_TYPE = "_ipps._tcp.local." ZEROCONF_NAME = "EPSON XP-6000 Series" -ZEROCONF_HOST = "192.168.1.31" +ZEROCONF_HOST = HOST ZEROCONF_HOSTNAME = "EPSON123456.local." -ZEROCONF_PORT = 631 - +ZEROCONF_PORT = PORT +ZEROCONF_RP = "ipp/print" MOCK_USER_INPUT = { - CONF_HOST: "192.168.1.31", - CONF_PORT: 361, + CONF_HOST: HOST, + CONF_PORT: PORT, CONF_SSL: False, CONF_VERIFY_SSL: False, - CONF_BASE_PATH: "/ipp/print", + CONF_BASE_PATH: BASE_PATH, } MOCK_ZEROCONF_IPP_SERVICE_INFO = { @@ -41,7 +48,7 @@ MOCK_ZEROCONF_IPP_SERVICE_INFO = { CONF_HOST: ZEROCONF_HOST, ATTR_HOSTNAME: ZEROCONF_HOSTNAME, CONF_PORT: ZEROCONF_PORT, - ATTR_PROPERTIES: {"rp": "ipp/print"}, + ATTR_PROPERTIES: {"rp": ZEROCONF_RP}, } MOCK_ZEROCONF_IPPS_SERVICE_INFO = { @@ -50,7 +57,7 @@ MOCK_ZEROCONF_IPPS_SERVICE_INFO = { CONF_HOST: ZEROCONF_HOST, ATTR_HOSTNAME: ZEROCONF_HOSTNAME, CONF_PORT: ZEROCONF_PORT, - ATTR_PROPERTIES: {"rp": "ipp/print"}, + ATTR_PROPERTIES: {"rp": ZEROCONF_RP}, } @@ -61,30 +68,75 @@ def load_fixture_binary(filename): return fptr.read() +def mock_connection( + aioclient_mock: AiohttpClientMocker, + host: str = HOST, + port: int = PORT, + ssl: bool = False, + base_path: str = BASE_PATH, + conn_error: bool = False, + conn_upgrade_error: bool = False, + ipp_error: bool = False, + no_unique_id: bool = False, + parse_error: bool = False, + version_not_supported: bool = False, +): + """Mock the IPP connection.""" + scheme = "https" if ssl else "http" + ipp_url = f"{scheme}://{host}:{port}" + + if ipp_error: + aioclient_mock.post(f"{ipp_url}{base_path}", exc=IPPError) + return + + if conn_error: + aioclient_mock.post(f"{ipp_url}{base_path}", exc=aiohttp.ClientError) + return + + if conn_upgrade_error: + aioclient_mock.post(f"{ipp_url}{base_path}", exc=IPPConnectionUpgradeRequired) + return + + fixture = "ipp/get-printer-attributes.bin" + if no_unique_id: + fixture = "ipp/get-printer-attributes-success-nodata.bin" + elif version_not_supported: + fixture = "ipp/get-printer-attributes-error-0x0503.bin" + + if parse_error: + content = "BAD" + else: + content = load_fixture_binary(fixture) + + aioclient_mock.post( + f"{ipp_url}{base_path}", + content=content, + headers={"Content-Type": "application/ipp"}, + ) + + async def init_integration( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker, skip_setup: bool = False, + host: str = HOST, + port: int = PORT, + ssl: bool = False, + base_path: str = BASE_PATH, uuid: str = "cfe92100-67c4-11d4-a45f-f8d027761251", unique_id: str = "cfe92100-67c4-11d4-a45f-f8d027761251", + conn_error: bool = False, ) -> MockConfigEntry: """Set up the IPP integration in Home Assistant.""" - fixture = "ipp/get-printer-attributes.bin" - aioclient_mock.post( - "http://192.168.1.31:631/ipp/print", - content=load_fixture_binary(fixture), - headers={"Content-Type": "application/ipp"}, - ) - entry = MockConfigEntry( domain=DOMAIN, unique_id=unique_id, data={ - CONF_HOST: "192.168.1.31", - CONF_PORT: 631, - CONF_SSL: False, + CONF_HOST: host, + CONF_PORT: port, + CONF_SSL: ssl, CONF_VERIFY_SSL: True, - CONF_BASE_PATH: "/ipp/print", + CONF_BASE_PATH: base_path, CONF_UUID: uuid, }, ) @@ -92,6 +144,14 @@ async def init_integration( entry.add_to_hass(hass) if not skip_setup: + mock_connection( + aioclient_mock, + host=host, + port=port, + ssl=ssl, + base_path=base_path, + conn_error=conn_error, + ) await hass.config_entries.async_setup(entry.entry_id) await hass.async_block_till_done() diff --git a/tests/components/ipp/test_config_flow.py b/tests/components/ipp/test_config_flow.py index fb75ba9caef..0093ba57e5b 100644 --- a/tests/components/ipp/test_config_flow.py +++ b/tests/components/ipp/test_config_flow.py @@ -1,7 +1,4 @@ """Tests for the IPP config flow.""" -import aiohttp -from pyipp import IPPConnectionUpgradeRequired, IPPError - from homeassistant.components.ipp.const import CONF_BASE_PATH, CONF_UUID, DOMAIN from homeassistant.config_entries import SOURCE_USER, SOURCE_ZEROCONF from homeassistant.const import CONF_HOST, CONF_NAME, CONF_SSL @@ -17,7 +14,7 @@ from . import ( MOCK_ZEROCONF_IPP_SERVICE_INFO, MOCK_ZEROCONF_IPPS_SERVICE_INFO, init_integration, - load_fixture_binary, + mock_connection, ) from tests.test_util.aiohttp import AiohttpClientMocker @@ -37,11 +34,7 @@ async def test_show_zeroconf_form( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test that the zeroconf confirmation form is served.""" - aioclient_mock.post( - "http://192.168.1.31:631/ipp/print", - content=load_fixture_binary("ipp/get-printer-attributes.bin"), - headers={"Content-Type": "application/ipp"}, - ) + mock_connection(aioclient_mock) discovery_info = MOCK_ZEROCONF_IPP_SERVICE_INFO.copy() result = await hass.config_entries.flow.async_init( @@ -57,7 +50,7 @@ async def test_connection_error( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test we show user form on IPP connection error.""" - aioclient_mock.post("http://192.168.1.31:631/ipp/print", exc=aiohttp.ClientError) + mock_connection(aioclient_mock, conn_error=True) user_input = MOCK_USER_INPUT.copy() result = await hass.config_entries.flow.async_init( @@ -73,7 +66,7 @@ async def test_zeroconf_connection_error( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test we abort zeroconf flow on IPP connection error.""" - aioclient_mock.post("http://192.168.1.31:631/ipp/print", exc=aiohttp.ClientError) + mock_connection(aioclient_mock, conn_error=True) discovery_info = MOCK_ZEROCONF_IPP_SERVICE_INFO.copy() result = await hass.config_entries.flow.async_init( @@ -88,7 +81,7 @@ async def test_zeroconf_confirm_connection_error( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test we abort zeroconf flow on IPP connection error.""" - aioclient_mock.post("http://192.168.1.31:631/ipp/print", exc=aiohttp.ClientError) + mock_connection(aioclient_mock, conn_error=True) discovery_info = MOCK_ZEROCONF_IPP_SERVICE_INFO.copy() result = await hass.config_entries.flow.async_init( @@ -103,9 +96,7 @@ async def test_user_connection_upgrade_required( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test we show the user form if connection upgrade required by server.""" - aioclient_mock.post( - "http://192.168.1.31:631/ipp/print", exc=IPPConnectionUpgradeRequired - ) + mock_connection(aioclient_mock, conn_upgrade_error=True) user_input = MOCK_USER_INPUT.copy() result = await hass.config_entries.flow.async_init( @@ -121,9 +112,7 @@ async def test_zeroconf_connection_upgrade_required( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test we abort zeroconf flow on IPP connection error.""" - aioclient_mock.post( - "http://192.168.1.31:631/ipp/print", exc=IPPConnectionUpgradeRequired - ) + mock_connection(aioclient_mock, conn_upgrade_error=True) discovery_info = MOCK_ZEROCONF_IPP_SERVICE_INFO.copy() result = await hass.config_entries.flow.async_init( @@ -138,11 +127,7 @@ async def test_user_parse_error( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test we abort user flow on IPP parse error.""" - aioclient_mock.post( - "http://192.168.1.31:631/ipp/print", - content="BAD", - headers={"Content-Type": "application/ipp"}, - ) + mock_connection(aioclient_mock, parse_error=True) user_input = MOCK_USER_INPUT.copy() result = await hass.config_entries.flow.async_init( @@ -157,11 +142,7 @@ async def test_zeroconf_parse_error( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test we abort zeroconf flow on IPP parse error.""" - aioclient_mock.post( - "http://192.168.1.31:631/ipp/print", - content="BAD", - headers={"Content-Type": "application/ipp"}, - ) + mock_connection(aioclient_mock, parse_error=True) discovery_info = MOCK_ZEROCONF_IPP_SERVICE_INFO.copy() result = await hass.config_entries.flow.async_init( @@ -176,7 +157,7 @@ async def test_user_ipp_error( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test we abort the user flow on IPP error.""" - aioclient_mock.post("http://192.168.1.31:631/ipp/print", exc=IPPError) + mock_connection(aioclient_mock, ipp_error=True) user_input = MOCK_USER_INPUT.copy() result = await hass.config_entries.flow.async_init( @@ -191,7 +172,7 @@ async def test_zeroconf_ipp_error( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test we abort zeroconf flow on IPP error.""" - aioclient_mock.post("http://192.168.1.31:631/ipp/print", exc=IPPError) + mock_connection(aioclient_mock, ipp_error=True) discovery_info = MOCK_ZEROCONF_IPP_SERVICE_INFO.copy() result = await hass.config_entries.flow.async_init( @@ -206,11 +187,7 @@ async def test_user_ipp_version_error( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test we abort user flow on IPP version not supported error.""" - aioclient_mock.post( - "http://192.168.1.31:631/ipp/print", - content=load_fixture_binary("ipp/get-printer-attributes-error-0x0503.bin"), - headers={"Content-Type": "application/ipp"}, - ) + mock_connection(aioclient_mock, version_not_supported=True) user_input = {**MOCK_USER_INPUT} result = await hass.config_entries.flow.async_init( @@ -225,11 +202,7 @@ async def test_zeroconf_ipp_version_error( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test we abort zeroconf flow on IPP version not supported error.""" - aioclient_mock.post( - "http://192.168.1.31:631/ipp/print", - content=load_fixture_binary("ipp/get-printer-attributes-error-0x0503.bin"), - headers={"Content-Type": "application/ipp"}, - ) + mock_connection(aioclient_mock, version_not_supported=True) discovery_info = {**MOCK_ZEROCONF_IPP_SERVICE_INFO} result = await hass.config_entries.flow.async_init( @@ -291,15 +264,26 @@ async def test_zeroconf_with_uuid_device_exists_abort( assert result["reason"] == "already_configured" +async def test_zeroconf_unique_id_required_abort( + hass: HomeAssistant, aioclient_mock: AiohttpClientMocker +) -> None: + """Test we abort zeroconf flow if printer lacks unique identification.""" + mock_connection(aioclient_mock, no_unique_id=True) + + discovery_info = MOCK_ZEROCONF_IPP_SERVICE_INFO.copy() + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_ZEROCONF}, data=discovery_info, + ) + + assert result["type"] == RESULT_TYPE_ABORT + assert result["reason"] == "unique_id_required" + + async def test_full_user_flow_implementation( hass: HomeAssistant, aioclient_mock ) -> None: """Test the full manual user flow from start to finish.""" - aioclient_mock.post( - "http://192.168.1.31:631/ipp/print", - content=load_fixture_binary("ipp/get-printer-attributes.bin"), - headers={"Content-Type": "application/ipp"}, - ) + mock_connection(aioclient_mock) result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_USER}, @@ -328,11 +312,7 @@ async def test_full_zeroconf_flow_implementation( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test the full manual user flow from start to finish.""" - aioclient_mock.post( - "http://192.168.1.31:631/ipp/print", - content=load_fixture_binary("ipp/get-printer-attributes.bin"), - headers={"Content-Type": "application/ipp"}, - ) + mock_connection(aioclient_mock) discovery_info = MOCK_ZEROCONF_IPP_SERVICE_INFO.copy() result = await hass.config_entries.flow.async_init( @@ -363,11 +343,7 @@ async def test_full_zeroconf_tls_flow_implementation( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test the full manual user flow from start to finish.""" - aioclient_mock.post( - "https://192.168.1.31:631/ipp/print", - content=load_fixture_binary("ipp/get-printer-attributes.bin"), - headers={"Content-Type": "application/ipp"}, - ) + mock_connection(aioclient_mock, ssl=True) discovery_info = MOCK_ZEROCONF_IPPS_SERVICE_INFO.copy() result = await hass.config_entries.flow.async_init( diff --git a/tests/components/ipp/test_init.py b/tests/components/ipp/test_init.py index 2ec11a1e937..f06be4fc8b5 100644 --- a/tests/components/ipp/test_init.py +++ b/tests/components/ipp/test_init.py @@ -1,6 +1,4 @@ """Tests for the IPP integration.""" -import aiohttp - from homeassistant.components.ipp.const import DOMAIN from homeassistant.config_entries import ( ENTRY_STATE_LOADED, @@ -17,9 +15,7 @@ async def test_config_entry_not_ready( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test the IPP configuration entry not ready.""" - aioclient_mock.post("http://192.168.1.31:631/ipp/print", exc=aiohttp.ClientError) - - entry = await init_integration(hass, aioclient_mock) + entry = await init_integration(hass, aioclient_mock, conn_error=True) assert entry.state == ENTRY_STATE_SETUP_RETRY diff --git a/tests/components/ipp/test_sensor.py b/tests/components/ipp/test_sensor.py index 51caadfceb3..1abf557c93b 100644 --- a/tests/components/ipp/test_sensor.py +++ b/tests/components/ipp/test_sensor.py @@ -8,7 +8,7 @@ from homeassistant.core import HomeAssistant from homeassistant.util import dt as dt_util from tests.async_mock import patch -from tests.components.ipp import init_integration +from tests.components.ipp import init_integration, mock_connection from tests.test_util.aiohttp import AiohttpClientMocker @@ -16,6 +16,8 @@ async def test_sensors( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker ) -> None: """Test the creation and values of the IPP sensors.""" + mock_connection(aioclient_mock) + entry = await init_integration(hass, aioclient_mock, skip_setup=True) registry = await hass.helpers.entity_registry.async_get_registry() diff --git a/tests/fixtures/ipp/get-printer-attributes-success-nodata.bin b/tests/fixtures/ipp/get-printer-attributes-success-nodata.bin new file mode 100644 index 0000000000000000000000000000000000000000..e6061adaccdef6f4705bba31c5166ee7b9cefe5f GIT binary patch literal 72 zcmZQ#00PFkS&Z%sLWw0MMVU#ZC8@=_$r*`7#i=C>tfeJsx)vS`(nxZ7i6x~)i8;DC QiFxUziRq~fOsRRy0Q|TXqyPW_ literal 0 HcmV?d00001