From 193b2694a98cb8a8dfa2c414838e156bc79def5a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 29 Apr 2023 20:17:56 -0500 Subject: [PATCH] Handle AttributeError from wrong port in ONVIF config flow (#92272) * Handle AttributeError from wrong port in ONVIF config flow fixes ``` 2023-04-29 19:17:22.289 ERROR (MainThread) [aiohttp.server] Error handling request Traceback (most recent call last): File "/Users/bdraco/home-assistant/venv/lib/python3.10/site-packages/aiohttp/web_protocol.py", line 433, in _handle_request resp = await request_handler(request) File "/Users/bdraco/home-assistant/venv/lib/python3.10/site-packages/aiohttp/web_app.py", line 504, in _handle resp = await handler(request) File "/Users/bdraco/home-assistant/venv/lib/python3.10/site-packages/aiohttp/web_middlewares.py", line 117, in impl return await handler(request) File "/Users/bdraco/home-assistant/homeassistant/components/http/security_filter.py", line 85, in security_filter_middleware return await handler(request) File "/Users/bdraco/home-assistant/homeassistant/components/http/forwarded.py", line 100, in forwarded_middleware return await handler(request) File "/Users/bdraco/home-assistant/homeassistant/components/http/request_context.py", line 28, in request_context_middleware return await handler(request) File "/Users/bdraco/home-assistant/homeassistant/components/http/ban.py", line 80, in ban_middleware return await handler(request) File "/Users/bdraco/home-assistant/homeassistant/components/http/auth.py", line 235, in auth_middleware return await handler(request) File "/Users/bdraco/home-assistant/homeassistant/components/http/view.py", line 146, in handle result = await result File "/Users/bdraco/home-assistant/homeassistant/components/config/config_entries.py", line 180, in post return await super().post(request, flow_id) File "/Users/bdraco/home-assistant/homeassistant/components/http/data_validator.py", line 72, in wrapper result = await method(view, request, data, *args, **kwargs) File "/Users/bdraco/home-assistant/homeassistant/helpers/data_entry_flow.py", line 110, in post result = await self._flow_mgr.async_configure(flow_id, data) File "/Users/bdraco/home-assistant/homeassistant/data_entry_flow.py", line 271, in async_configure result = await self._async_handle_step( File "/Users/bdraco/home-assistant/homeassistant/data_entry_flow.py", line 367, in _async_handle_step result: FlowResult = await getattr(flow, method)(user_input) File "/Users/bdraco/home-assistant/homeassistant/components/onvif/config_flow.py", line 233, in async_step_configure errors, description_placeholders = await self.async_setup_profiles() File "/Users/bdraco/home-assistant/homeassistant/components/onvif/config_flow.py", line 277, in async_setup_profiles await device.update_xaddrs() File "/Users/bdraco/home-assistant/venv/lib/python3.10/site-packages/onvif/client.py", line 433, in update_xaddrs capabilities = await devicemgmt.GetCapabilities({"Category": "All"}) File "/Users/bdraco/home-assistant/venv/lib/python3.10/site-packages/zeep/proxy.py", line 64, in __call__ return await self._proxy._binding.send_async( File "/Users/bdraco/home-assistant/venv/lib/python3.10/site-packages/zeep/wsdl/bindings/soap.py", line 164, in send_async return self.process_reply(client, operation_obj, response) File "/Users/bdraco/home-assistant/venv/lib/python3.10/site-packages/zeep/wsdl/bindings/soap.py", line 204, in process_reply doc = parse_xml(content, self.transport, settings=client.settings) File "/Users/bdraco/home-assistant/venv/lib/python3.10/site-packages/zeep/loader.py", line 51, in parse_xml docinfo = elementtree.getroottree().docinfo AttributeError: NoneType object has no attribute getroottree ``` * port * Revert "port" This reverts commit 4693f3f33af18af66672dbd5ce6774f35ba28316. * misfire --- homeassistant/components/onvif/config_flow.py | 9 +++ homeassistant/components/onvif/strings.json | 1 + tests/components/onvif/__init__.py | 6 +- tests/components/onvif/test_config_flow.py | 79 +++++++++++++++++++ 4 files changed, 94 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/onvif/config_flow.py b/homeassistant/components/onvif/config_flow.py index 181b3321c53..68a4ce52511 100644 --- a/homeassistant/components/onvif/config_flow.py +++ b/homeassistant/components/onvif/config_flow.py @@ -316,6 +316,15 @@ class OnvifFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): # Verify there is an H264 profile media_service = device.create_media_service() profiles = await media_service.GetProfiles() + except AttributeError: # Likely an empty document or 404 from the wrong port + LOGGER.debug( + "%s: No ONVIF service found at %s:%s", + self.onvif_config[CONF_NAME], + self.onvif_config[CONF_HOST], + self.onvif_config[CONF_PORT], + exc_info=True, + ) + return {CONF_PORT: "no_onvif_service"}, {} except Fault as err: stringified_error = stringify_onvif_error(err) description_placeholders = {"error": stringified_error} diff --git a/homeassistant/components/onvif/strings.json b/homeassistant/components/onvif/strings.json index 07f2e6fb7ac..55413e4bf6c 100644 --- a/homeassistant/components/onvif/strings.json +++ b/homeassistant/components/onvif/strings.json @@ -11,6 +11,7 @@ "error": { "onvif_error": "Error setting up ONVIF device: {error}. Check logs for more information.", "auth_failed": "Could not authenticate: {error}", + "no_onvif_service": "No ONVIF service found. Check that the port number is correct.", "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]" }, "step": { diff --git a/tests/components/onvif/__init__.py b/tests/components/onvif/__init__.py index 1dfcc85cd28..18de9839e1b 100644 --- a/tests/components/onvif/__init__.py +++ b/tests/components/onvif/__init__.py @@ -45,12 +45,14 @@ def setup_mock_onvif_camera( update_xaddrs_fail=False, no_profiles=False, auth_failure=False, + wrong_port=False, ): """Prepare mock onvif.ONVIFCamera.""" devicemgmt = MagicMock() device_info = MagicMock() device_info.SerialNumber = SERIAL_NUMBER if with_serial else None + devicemgmt.GetDeviceInformation = AsyncMock(return_value=device_info) interface = MagicMock() @@ -82,7 +84,9 @@ def setup_mock_onvif_camera( else: media_service.GetProfiles = AsyncMock(return_value=[profile1, profile2]) - if auth_failure: + if wrong_port: + mock_onvif_camera.update_xaddrs = AsyncMock(side_effect=AttributeError) + elif auth_failure: mock_onvif_camera.update_xaddrs = AsyncMock( side_effect=Fault( "not authorized", subcodes=[MagicMock(text="NotAuthorized")] diff --git a/tests/components/onvif/test_config_flow.py b/tests/components/onvif/test_config_flow.py index 805b5d85db8..21ef1cf3fc2 100644 --- a/tests/components/onvif/test_config_flow.py +++ b/tests/components/onvif/test_config_flow.py @@ -829,3 +829,82 @@ async def test_flow_manual_entry_updates_existing_user_password( assert entry.data[config_flow.CONF_USERNAME] == USERNAME assert entry.data[config_flow.CONF_PASSWORD] == "new_password" assert len(mock_setup_entry.mock_calls) == 1 + + +async def test_flow_manual_entry_wrong_port(hass: HomeAssistant) -> None: + """Test that we get a useful error with the wrong port.""" + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + + assert result["type"] == data_entry_flow.FlowResultType.FORM + assert result["step_id"] == "user" + + with patch( + "homeassistant.components.onvif.config_flow.get_device" + ) as mock_onvif_camera, patch( + "homeassistant.components.onvif.config_flow.wsdiscovery" + ) as mock_discovery, patch( + "homeassistant.components.onvif.ONVIFDevice" + ) as mock_device: + setup_mock_onvif_camera(mock_onvif_camera, wrong_port=True) + # no discovery + mock_discovery.return_value = [] + setup_mock_device(mock_device) + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={"auto": False}, + ) + + assert result["type"] == data_entry_flow.FlowResultType.FORM + assert result["step_id"] == "configure" + + with patch( + "homeassistant.components.onvif.async_setup_entry", return_value=True + ) as mock_setup_entry: + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={ + config_flow.CONF_NAME: NAME, + config_flow.CONF_HOST: HOST, + config_flow.CONF_PORT: PORT, + config_flow.CONF_USERNAME: USERNAME, + config_flow.CONF_PASSWORD: PASSWORD, + }, + ) + + await hass.async_block_till_done() + assert len(mock_setup_entry.mock_calls) == 0 + + assert result["type"] == data_entry_flow.FlowResultType.FORM + assert result["step_id"] == "configure" + assert result["errors"] == {"port": "no_onvif_service"} + assert result["description_placeholders"] == {} + setup_mock_onvif_camera(mock_onvif_camera, two_profiles=True) + + with patch( + "homeassistant.components.onvif.async_setup_entry", return_value=True + ) as mock_setup_entry: + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={ + config_flow.CONF_NAME: NAME, + config_flow.CONF_HOST: HOST, + config_flow.CONF_PORT: PORT, + config_flow.CONF_USERNAME: USERNAME, + config_flow.CONF_PASSWORD: PASSWORD, + }, + ) + + await hass.async_block_till_done() + assert len(mock_setup_entry.mock_calls) == 1 + + assert result["title"] == f"{NAME} - {MAC}" + assert result["data"] == { + config_flow.CONF_NAME: NAME, + config_flow.CONF_HOST: HOST, + config_flow.CONF_PORT: PORT, + config_flow.CONF_USERNAME: USERNAME, + config_flow.CONF_PASSWORD: PASSWORD, + }