From 1bcf3cfbb217cdfff8ea09c4690dcac7920924da Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 11 Sep 2025 09:58:59 -0500 Subject: [PATCH] Fix DoorBird being updated with wrong IP addresses during discovery (#152088) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../components/doorbird/config_flow.py | 56 ++++++++++++- .../components/doorbird/strings.json | 2 + tests/components/doorbird/test_config_flow.py | 84 ++++++++++++++++++- 3 files changed, 140 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/doorbird/config_flow.py b/homeassistant/components/doorbird/config_flow.py index 6a954f5310f..ac08ad0e1f6 100644 --- a/homeassistant/components/doorbird/config_flow.py +++ b/homeassistant/components/doorbird/config_flow.py @@ -19,8 +19,10 @@ from homeassistant.config_entries import ( ) from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PASSWORD, CONF_USERNAME from homeassistant.core import HomeAssistant, callback +from homeassistant.data_entry_flow import AbortFlow from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.aiohttp_client import async_get_clientsession +from homeassistant.helpers.device_registry import format_mac from homeassistant.helpers.service_info.zeroconf import ZeroconfServiceInfo from homeassistant.helpers.typing import VolDictType @@ -103,6 +105,43 @@ class DoorBirdConfigFlow(ConfigFlow, domain=DOMAIN): """Initialize the DoorBird config flow.""" self.discovery_schema: vol.Schema | None = None + async def _async_verify_existing_device_for_discovery( + self, + existing_entry: ConfigEntry, + host: str, + macaddress: str, + ) -> None: + """Verify discovered device matches existing entry before updating IP. + + This method performs the following verification steps: + 1. Ensures that the stored credentials work before updating the entry. + 2. Verifies that the device at the discovered IP address has the expected MAC address. + """ + info, errors = await self._async_validate_or_error( + { + **existing_entry.data, + CONF_HOST: host, + } + ) + + if errors: + _LOGGER.debug( + "Cannot validate DoorBird at %s with existing credentials: %s", + host, + errors, + ) + raise AbortFlow("cannot_connect") + + # Verify the MAC address matches what was advertised + if format_mac(info["mac_addr"]) != format_mac(macaddress): + _LOGGER.debug( + "DoorBird at %s reports MAC %s but zeroconf advertised %s, ignoring", + host, + info["mac_addr"], + macaddress, + ) + raise AbortFlow("wrong_device") + async def async_step_reauth( self, entry_data: Mapping[str, Any] ) -> ConfigFlowResult: @@ -172,7 +211,22 @@ class DoorBirdConfigFlow(ConfigFlow, domain=DOMAIN): await self.async_set_unique_id(macaddress) host = discovery_info.host - self._abort_if_unique_id_configured(updates={CONF_HOST: host}) + + # Check if we have an existing entry for this MAC + existing_entry = self.hass.config_entries.async_entry_for_domain_unique_id( + DOMAIN, macaddress + ) + + if existing_entry: + # Check if the host is actually changing + if existing_entry.data.get(CONF_HOST) != host: + await self._async_verify_existing_device_for_discovery( + existing_entry, host, macaddress + ) + + # All checks passed or no change needed, abort + # if already configured with potential IP update + self._abort_if_unique_id_configured(updates={CONF_HOST: host}) self._async_abort_entries_match({CONF_HOST: host}) diff --git a/homeassistant/components/doorbird/strings.json b/homeassistant/components/doorbird/strings.json index 285b544e465..341976e8a8f 100644 --- a/homeassistant/components/doorbird/strings.json +++ b/homeassistant/components/doorbird/strings.json @@ -49,6 +49,8 @@ "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", "link_local_address": "Link local addresses are not supported", "not_doorbird_device": "This device is not a DoorBird", + "not_ipv4_address": "Only IPv4 addresses are supported", + "wrong_device": "Device MAC address does not match", "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]" }, "flow_title": "{name} ({host})", diff --git a/tests/components/doorbird/test_config_flow.py b/tests/components/doorbird/test_config_flow.py index 98b2189dfd9..493762df5ef 100644 --- a/tests/components/doorbird/test_config_flow.py +++ b/tests/components/doorbird/test_config_flow.py @@ -108,7 +108,9 @@ async def test_form_zeroconf_link_local_ignored(hass: HomeAssistant) -> None: assert result["reason"] == "link_local_address" -async def test_form_zeroconf_ipv4_address(hass: HomeAssistant) -> None: +async def test_form_zeroconf_ipv4_address( + hass: HomeAssistant, doorbird_api: DoorBird +) -> None: """Test we abort and update the ip address from zeroconf with an ipv4 address.""" config_entry = MockConfigEntry( @@ -118,6 +120,13 @@ async def test_form_zeroconf_ipv4_address(hass: HomeAssistant) -> None: options={CONF_EVENTS: ["event1", "event2", "event3"]}, ) config_entry.add_to_hass(hass) + + # Mock the API to return the correct MAC when validating + doorbird_api.info.return_value = { + "PRIMARY_MAC_ADDR": "1CCAE3AAAAAA", + "WIFI_MAC_ADDR": "1CCAE3BBBBBB", + } + result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_ZEROCONF}, @@ -136,6 +145,79 @@ async def test_form_zeroconf_ipv4_address(hass: HomeAssistant) -> None: assert config_entry.data[CONF_HOST] == "4.4.4.4" +async def test_form_zeroconf_ipv4_address_wrong_device( + hass: HomeAssistant, doorbird_api: DoorBird +) -> None: + """Test we abort when the device MAC doesn't match during zeroconf update.""" + + config_entry = MockConfigEntry( + domain=DOMAIN, + unique_id="1CCAE3AAAAAA", + data=VALID_CONFIG, + options={CONF_EVENTS: ["event1", "event2", "event3"]}, + ) + config_entry.add_to_hass(hass) + + # Mock the API to return a different MAC (wrong device) + doorbird_api.info.return_value = { + "PRIMARY_MAC_ADDR": "1CCAE3DIFFERENT", # Different MAC! + "WIFI_MAC_ADDR": "1CCAE3BBBBBB", + } + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_ZEROCONF}, + data=ZeroconfServiceInfo( + ip_address=ip_address("4.4.4.4"), + ip_addresses=[ip_address("4.4.4.4")], + hostname="mock_hostname", + name="Doorstation - abc123._axis-video._tcp.local.", + port=None, + properties={"macaddress": "1CCAE3AAAAAA"}, + type="mock_type", + ), + ) + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "wrong_device" + # Host should not be updated since it's the wrong device + assert config_entry.data[CONF_HOST] == "1.2.3.4" + + +async def test_form_zeroconf_ipv4_address_cannot_connect( + hass: HomeAssistant, doorbird_api: DoorBird +) -> None: + """Test we abort when we cannot connect to validate during zeroconf update.""" + + config_entry = MockConfigEntry( + domain=DOMAIN, + unique_id="1CCAE3AAAAAA", + data=VALID_CONFIG, + options={CONF_EVENTS: ["event1", "event2", "event3"]}, + ) + config_entry.add_to_hass(hass) + + # Mock the API to fail connection (e.g., wrong credentials or network error) + doorbird_api.info.side_effect = mock_unauthorized_exception() + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_ZEROCONF}, + data=ZeroconfServiceInfo( + ip_address=ip_address("4.4.4.4"), + ip_addresses=[ip_address("4.4.4.4")], + hostname="mock_hostname", + name="Doorstation - abc123._axis-video._tcp.local.", + port=None, + properties={"macaddress": "1CCAE3AAAAAA"}, + type="mock_type", + ), + ) + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "cannot_connect" + # Host should not be updated since we couldn't validate + assert config_entry.data[CONF_HOST] == "1.2.3.4" + + async def test_form_zeroconf_non_ipv4_ignored(hass: HomeAssistant) -> None: """Test we abort when we get a non ipv4 address via zeroconf."""