diff --git a/homeassistant/components/doorbird/__init__.py b/homeassistant/components/doorbird/__init__.py index 8376d75ccbb..3e8e59df203 100644 --- a/homeassistant/components/doorbird/__init__.py +++ b/homeassistant/components/doorbird/__init__.py @@ -1,11 +1,10 @@ """Support for DoorBird devices.""" import asyncio import logging -import urllib -from urllib.error import HTTPError from aiohttp import web from doorbirdpy import DoorBird +import requests import voluptuous as vol from homeassistant.components.http import HomeAssistantView @@ -130,8 +129,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry): device = DoorBird(device_ip, username, password) try: status, info = await hass.async_add_executor_job(_init_doorbird_device, device) - except urllib.error.HTTPError as err: - if err.code == HTTP_UNAUTHORIZED: + except requests.exceptions.HTTPError as err: + if err.response.status_code == HTTP_UNAUTHORIZED: _LOGGER.error( "Authorization rejected by DoorBird for %s@%s", username, device_ip ) @@ -202,7 +201,7 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry): async def _async_register_events(hass, doorstation): try: await hass.async_add_executor_job(doorstation.register_events, hass) - except HTTPError: + except requests.exceptions.HTTPError: hass.components.persistent_notification.async_create( "Doorbird configuration failed. Please verify that API " "Operator permission is enabled for the Doorbird user. " diff --git a/homeassistant/components/doorbird/config_flow.py b/homeassistant/components/doorbird/config_flow.py index 1b39bb4a8c3..f69b38c7a7a 100644 --- a/homeassistant/components/doorbird/config_flow.py +++ b/homeassistant/components/doorbird/config_flow.py @@ -1,9 +1,9 @@ """Config flow for DoorBird integration.""" from ipaddress import ip_address import logging -import urllib from doorbirdpy import DoorBird +import requests import voluptuous as vol from homeassistant import config_entries, core, exceptions @@ -34,17 +34,18 @@ def _schema_with_defaults(host=None, name=None): ) -async def validate_input(hass: core.HomeAssistant, data): - """Validate the user input allows us to connect. +def _check_device(device): + """Verify we can connect to the device and return the status.""" + return device.ready(), device.info() - Data has the keys from DATA_SCHEMA with values provided by the user. - """ + +async def validate_input(hass: core.HomeAssistant, data): + """Validate the user input allows us to connect.""" device = DoorBird(data[CONF_HOST], data[CONF_USERNAME], data[CONF_PASSWORD]) try: - status = await hass.async_add_executor_job(device.ready) - info = await hass.async_add_executor_job(device.info) - except urllib.error.HTTPError as err: - if err.code == HTTP_UNAUTHORIZED: + status, info = await hass.async_add_executor_job(_check_device, device) + except requests.exceptions.HTTPError as err: + if err.response.status_code == HTTP_UNAUTHORIZED: raise InvalidAuth from err raise CannotConnect from err except OSError as err: @@ -59,6 +60,19 @@ async def validate_input(hass: core.HomeAssistant, data): return {"title": data[CONF_HOST], "mac_addr": mac_addr} +async def async_verify_supported_device(hass, host): + """Verify the doorbell state endpoint returns a 401.""" + device = DoorBird(host, "", "") + try: + await hass.async_add_executor_job(device.doorbell_state) + except requests.exceptions.HTTPError as err: + if err.response.status_code == HTTP_UNAUTHORIZED: + return True + except OSError: + return False + return False + + class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle a config flow for DoorBird.""" @@ -85,17 +99,18 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): async def async_step_zeroconf(self, discovery_info): """Prepare configuration for a discovered doorbird device.""" macaddress = discovery_info["properties"]["macaddress"] + host = discovery_info[CONF_HOST] if macaddress[:6] != DOORBIRD_OUI: return self.async_abort(reason="not_doorbird_device") - if is_link_local(ip_address(discovery_info[CONF_HOST])): + if is_link_local(ip_address(host)): return self.async_abort(reason="link_local_address") + if not await async_verify_supported_device(self.hass, host): + return self.async_abort(reason="not_doorbird_device") await self.async_set_unique_id(macaddress) - self._abort_if_unique_id_configured( - updates={CONF_HOST: discovery_info[CONF_HOST]} - ) + self._abort_if_unique_id_configured(updates={CONF_HOST: host}) chop_ending = "._axis-video._tcp.local." friendly_hostname = discovery_info["name"] @@ -104,11 +119,9 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): self.context["title_placeholders"] = { CONF_NAME: friendly_hostname, - CONF_HOST: discovery_info[CONF_HOST], + CONF_HOST: host, } - self.discovery_schema = _schema_with_defaults( - host=discovery_info[CONF_HOST], name=friendly_hostname - ) + self.discovery_schema = _schema_with_defaults(host=host, name=friendly_hostname) return await self.async_step_user() diff --git a/tests/components/doorbird/test_config_flow.py b/tests/components/doorbird/test_config_flow.py index e94f73239f1..d6bbb7412e6 100644 --- a/tests/components/doorbird/test_config_flow.py +++ b/tests/components/doorbird/test_config_flow.py @@ -1,6 +1,8 @@ """Test the DoorBird config flow.""" -from unittest.mock import MagicMock, patch -import urllib +from unittest.mock import MagicMock, Mock, patch + +import pytest +import requests from homeassistant import config_entries, data_entry_flow, setup from homeassistant.components.doorbird import CONF_CUSTOM_URL, CONF_TOKEN @@ -21,7 +23,9 @@ def _get_mock_doorbirdapi_return_values(ready=None, info=None): doorbirdapi_mock = MagicMock() type(doorbirdapi_mock).ready = MagicMock(return_value=ready) type(doorbirdapi_mock).info = MagicMock(return_value=info) - + type(doorbirdapi_mock).doorbell_state = MagicMock( + side_effect=requests.exceptions.HTTPError(response=Mock(status_code=401)) + ) return doorbirdapi_mock @@ -137,17 +141,25 @@ async def test_form_import_with_zeroconf_already_discovered(hass): await setup.async_setup_component(hass, "persistent_notification", {}) + doorbirdapi = _get_mock_doorbirdapi_return_values( + ready=[True], info={"WIFI_MAC_ADDR": "1CCAE3DOORBIRD"} + ) # Running the zeroconf init will make the unique id # in progress - zero_conf = await hass.config_entries.flow.async_init( - DOMAIN, - context={"source": config_entries.SOURCE_ZEROCONF}, - data={ - "properties": {"macaddress": "1CCAE3DOORBIRD"}, - "name": "Doorstation - abc123._axis-video._tcp.local.", - "host": "192.168.1.5", - }, - ) + with patch( + "homeassistant.components.doorbird.config_flow.DoorBird", + return_value=doorbirdapi, + ): + zero_conf = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_ZEROCONF}, + data={ + "properties": {"macaddress": "1CCAE3DOORBIRD"}, + "name": "Doorstation - abc123._axis-video._tcp.local.", + "host": "192.168.1.5", + }, + ) + await hass.async_block_till_done() assert zero_conf["type"] == data_entry_flow.RESULT_TYPE_FORM assert zero_conf["step_id"] == "user" assert zero_conf["errors"] == {} @@ -159,9 +171,6 @@ async def test_form_import_with_zeroconf_already_discovered(hass): CONF_CUSTOM_URL ] = "http://legacy.custom.url/should/only/come/in/from/yaml" - doorbirdapi = _get_mock_doorbirdapi_return_values( - ready=[True], info={"WIFI_MAC_ADDR": "1CCAE3DOORBIRD"} - ) with patch( "homeassistant.components.doorbird.config_flow.DoorBird", return_value=doorbirdapi, @@ -244,24 +253,29 @@ async def test_form_zeroconf_correct_oui(hass): await hass.async_add_executor_job( init_recorder_component, hass ) # force in memory db - - await setup.async_setup_component(hass, "persistent_notification", {}) - - result = await hass.config_entries.flow.async_init( - DOMAIN, - context={"source": config_entries.SOURCE_ZEROCONF}, - data={ - "properties": {"macaddress": "1CCAE3DOORBIRD"}, - "name": "Doorstation - abc123._axis-video._tcp.local.", - "host": "192.168.1.5", - }, - ) - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "user" - assert result["errors"] == {} doorbirdapi = _get_mock_doorbirdapi_return_values( ready=[True], info={"WIFI_MAC_ADDR": "macaddr"} ) + await setup.async_setup_component(hass, "persistent_notification", {}) + + with patch( + "homeassistant.components.doorbird.config_flow.DoorBird", + return_value=doorbirdapi, + ): + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_ZEROCONF}, + data={ + "properties": {"macaddress": "1CCAE3DOORBIRD"}, + "name": "Doorstation - abc123._axis-video._tcp.local.", + "host": "192.168.1.5", + }, + ) + await hass.async_block_till_done() + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["errors"] == {} + with patch( "homeassistant.components.doorbird.config_flow.DoorBird", return_value=doorbirdapi, @@ -288,6 +302,43 @@ async def test_form_zeroconf_correct_oui(hass): assert len(mock_setup_entry.mock_calls) == 1 +@pytest.mark.parametrize( + "doorbell_state_side_effect", + [ + requests.exceptions.HTTPError(response=Mock(status_code=404)), + OSError, + None, + ], +) +async def test_form_zeroconf_correct_oui_wrong_device(hass, doorbell_state_side_effect): + """Test we can setup from zeroconf with the correct OUI source but not a doorstation.""" + await hass.async_add_executor_job( + init_recorder_component, hass + ) # force in memory db + doorbirdapi = _get_mock_doorbirdapi_return_values( + ready=[True], info={"WIFI_MAC_ADDR": "macaddr"} + ) + type(doorbirdapi).doorbell_state = MagicMock(side_effect=doorbell_state_side_effect) + await setup.async_setup_component(hass, "persistent_notification", {}) + + with patch( + "homeassistant.components.doorbird.config_flow.DoorBird", + return_value=doorbirdapi, + ): + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_ZEROCONF}, + data={ + "properties": {"macaddress": "1CCAE3DOORBIRD"}, + "name": "Doorstation - abc123._axis-video._tcp.local.", + "host": "192.168.1.5", + }, + ) + await hass.async_block_till_done() + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "not_doorbird_device" + + async def test_form_user_cannot_connect(hass): """Test we handle cannot connect error.""" await hass.async_add_executor_job( @@ -322,10 +373,8 @@ async def test_form_user_invalid_auth(hass): DOMAIN, context={"source": config_entries.SOURCE_USER} ) - mock_urllib_error = urllib.error.HTTPError( - "http://xyz.tld", 401, "login failed", {}, None - ) - doorbirdapi = _get_mock_doorbirdapi_side_effects(ready=mock_urllib_error) + mock_error = requests.exceptions.HTTPError(response=Mock(status_code=401)) + doorbirdapi = _get_mock_doorbirdapi_side_effects(ready=mock_error) with patch( "homeassistant.components.doorbird.config_flow.DoorBird", return_value=doorbirdapi,