Abort discovery for unsupported doorbird accessories (#48710)

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:
+    except requests.exceptions.HTTPError as err:
-        if err.code == HTTP_UNAUTHORIZED:
+        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. "

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):
+def _check_device(device):
-    """Validate the user input allows us to connect.
+    """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)
+        status, info = await hass.async_add_executor_job(_check_device, device)
-        info = await hass.async_add_executor_job(device.info)
+    except requests.exceptions.HTTPError as err:
-    except urllib.error.HTTPError as err:
+        if err.response.status_code == HTTP_UNAUTHORIZED:
-        if err.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(
+        self._abort_if_unique_id_configured(updates={CONF_HOST: host})
-            updates={CONF_HOST: discovery_info[CONF_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(
+        self.discovery_schema = _schema_with_defaults(host=host, name=friendly_hostname)
-            host=discovery_info[CONF_HOST], name=friendly_hostname
-        )
 
         return await self.async_step_user()
 

tests/components/doorbird/test_config_flow.py

@@ -1,6 +1,8 @@
 """Test the DoorBird config flow."""
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, Mock, patch
-import urllib
+
+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,8 +141,15 @@ 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
+    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},
@@ -148,6 +159,7 @@ async def test_form_import_with_zeroconf_already_discovered(hass):
                 "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,9 +253,15 @@ async def test_form_zeroconf_correct_oui(hass):
     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"}
+    )
     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},
@@ -256,12 +271,11 @@ async def test_form_zeroconf_correct_oui(hass):
                 "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"] == {}
-    doorbirdapi = _get_mock_doorbirdapi_return_values(
+
-        ready=[True], info={"WIFI_MAC_ADDR": "macaddr"}
-    )
     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(
+    mock_error = requests.exceptions.HTTPError(response=Mock(status_code=401))
-        "http://xyz.tld", 401, "login failed", {}, None
+    doorbirdapi = _get_mock_doorbirdapi_side_effects(ready=mock_error)
-    )
-    doorbirdapi = _get_mock_doorbirdapi_side_effects(ready=mock_urllib_error)
     with patch(
         "homeassistant.components.doorbird.config_flow.DoorBird",
         return_value=doorbirdapi,