From ca20663c31f1ddcbb3f0fb654360e93df2aa253a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 2 Jul 2023 09:29:45 -0500 Subject: [PATCH] Handle missing or incorrect device name and unique id for ESPHome during manual add (#95678) * Handle incorrect or missing device name for ESPHome noise encryption If we did not have the device name during setup we could never get the key from the dashboard. The device will send us its name if we try encryption which allows us to find the right key from the dashboard. This should help get users unstuck when they change the key and cannot get the device back online after deleting and trying to set it up again manually * bump lib to get name * tweak * reduce number of connections * less connections when we know we will fail * coverage shows it works but it does not * add more coverage * fix test * bump again --- .../components/esphome/config_flow.py | 42 ++- .../components/esphome/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/esphome/test_config_flow.py | 315 +++++++++++++++++- 5 files changed, 342 insertions(+), 21 deletions(-) diff --git a/homeassistant/components/esphome/config_flow.py b/homeassistant/components/esphome/config_flow.py index 11deb5bb486b07..53c8577be44c72 100644 --- a/homeassistant/components/esphome/config_flow.py +++ b/homeassistant/components/esphome/config_flow.py @@ -40,6 +40,8 @@ ESPHOME_URL = "https://esphome.io/" _LOGGER = logging.getLogger(__name__) +ZERO_NOISE_PSK = "MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDA=" + class EsphomeFlowHandler(ConfigFlow, domain=DOMAIN): """Handle a esphome config flow.""" @@ -149,11 +151,22 @@ def _name(self, value: str) -> None: async def _async_try_fetch_device_info(self) -> FlowResult: error = await self.fetch_device_info() - if ( - error == ERROR_REQUIRES_ENCRYPTION_KEY - and await self._retrieve_encryption_key_from_dashboard() - ): - error = await self.fetch_device_info() + if error == ERROR_REQUIRES_ENCRYPTION_KEY: + if not self._device_name and not self._noise_psk: + # If device name is not set we can send a zero noise psk + # to get the device name which will allow us to populate + # the device name and hopefully get the encryption key + # from the dashboard. + self._noise_psk = ZERO_NOISE_PSK + error = await self.fetch_device_info() + self._noise_psk = None + + if ( + self._device_name + and await self._retrieve_encryption_key_from_dashboard() + ): + error = await self.fetch_device_info() + # If the fetched key is invalid, unset it again. if error == ERROR_INVALID_ENCRYPTION_KEY: self._noise_psk = None @@ -323,7 +336,10 @@ async def fetch_device_info(self) -> str | None: self._device_info = await cli.device_info() except RequiresEncryptionAPIError: return ERROR_REQUIRES_ENCRYPTION_KEY - except InvalidEncryptionKeyAPIError: + except InvalidEncryptionKeyAPIError as ex: + if ex.received_name: + self._device_name = ex.received_name + self._name = ex.received_name return ERROR_INVALID_ENCRYPTION_KEY except ResolveAPIError: return "resolve_error" @@ -334,9 +350,8 @@ async def fetch_device_info(self) -> str | None: self._name = self._device_info.friendly_name or self._device_info.name self._device_name = self._device_info.name - await self.async_set_unique_id( - self._device_info.mac_address, raise_on_progress=False - ) + mac_address = format_mac(self._device_info.mac_address) + await self.async_set_unique_id(mac_address, raise_on_progress=False) if not self._reauth_entry: self._abort_if_unique_id_configured( updates={CONF_HOST: self._host, CONF_PORT: self._port} @@ -373,14 +388,13 @@ async def _retrieve_encryption_key_from_dashboard(self) -> bool: Return boolean if a key was retrieved. """ - if self._device_name is None: - return False - - if (dashboard := async_get_dashboard(self.hass)) is None: + if ( + self._device_name is None + or (dashboard := async_get_dashboard(self.hass)) is None + ): return False await dashboard.async_request_refresh() - if not dashboard.last_update_success: return False diff --git a/homeassistant/components/esphome/manifest.json b/homeassistant/components/esphome/manifest.json index 085437fb02e24b..8f5e6b95c39097 100644 --- a/homeassistant/components/esphome/manifest.json +++ b/homeassistant/components/esphome/manifest.json @@ -15,7 +15,7 @@ "iot_class": "local_push", "loggers": ["aioesphomeapi", "noiseprotocol"], "requirements": [ - "aioesphomeapi==15.0.1", + "aioesphomeapi==15.1.1", "bluetooth-data-tools==1.3.0", "esphome-dashboard-api==1.2.3" ], diff --git a/requirements_all.txt b/requirements_all.txt index 619a87215528f4..7241667fb82f07 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -234,7 +234,7 @@ aioecowitt==2023.5.0 aioemonitor==1.0.5 # homeassistant.components.esphome -aioesphomeapi==15.0.1 +aioesphomeapi==15.1.1 # homeassistant.components.flo aioflo==2021.11.0 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index b1b5b9f2cbe006..ab6fd014adc080 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -212,7 +212,7 @@ aioecowitt==2023.5.0 aioemonitor==1.0.5 # homeassistant.components.esphome -aioesphomeapi==15.0.1 +aioesphomeapi==15.1.1 # homeassistant.components.flo aioflo==2021.11.0 diff --git a/tests/components/esphome/test_config_flow.py b/tests/components/esphome/test_config_flow.py index affe65949b2e92..4a99de77c1aab3 100644 --- a/tests/components/esphome/test_config_flow.py +++ b/tests/components/esphome/test_config_flow.py @@ -1,4 +1,5 @@ """Test config flow.""" +import asyncio from unittest.mock import AsyncMock, MagicMock, patch from aioesphomeapi import ( @@ -10,6 +11,7 @@ RequiresEncryptionAPIError, ResolveAPIError, ) +import aiohttp import pytest from homeassistant import config_entries, data_entry_flow @@ -35,6 +37,7 @@ from tests.common import MockConfigEntry INVALID_NOISE_PSK = "lSYBYEjQI1bVL8s2Vask4YytGMj1f1epNtmoim2yuTM=" +WRONG_NOISE_PSK = "GP+ciK+nVfTQ/gcz6uOdS+oKEdJgesU+jeu8Ssj2how=" @pytest.fixture(autouse=False) @@ -115,6 +118,58 @@ async def test_user_connection_updates_host( assert entry.data[CONF_HOST] == "127.0.0.1" +async def test_user_sets_unique_id( + hass: HomeAssistant, mock_client, mock_zeroconf: None, mock_setup_entry: None +) -> None: + """Test that the user flow sets the unique id.""" + service_info = zeroconf.ZeroconfServiceInfo( + host="192.168.43.183", + addresses=["192.168.43.183"], + hostname="test8266.local.", + name="mock_name", + port=6053, + properties={ + "mac": "1122334455aa", + }, + type="mock_type", + ) + discovery_result = await hass.config_entries.flow.async_init( + "esphome", context={"source": config_entries.SOURCE_ZEROCONF}, data=service_info + ) + + assert discovery_result["type"] == FlowResultType.FORM + assert discovery_result["step_id"] == "discovery_confirm" + + discovery_result = await hass.config_entries.flow.async_configure( + discovery_result["flow_id"], + {}, + ) + assert discovery_result["type"] == FlowResultType.CREATE_ENTRY + assert discovery_result["data"] == { + CONF_HOST: "192.168.43.183", + CONF_PORT: 6053, + CONF_PASSWORD: "", + CONF_NOISE_PSK: "", + CONF_DEVICE_NAME: "test", + } + + result = await hass.config_entries.flow.async_init( + "esphome", + context={"source": config_entries.SOURCE_USER}, + data=None, + ) + + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "user" + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_HOST: "127.0.0.1", CONF_PORT: 6053}, + ) + assert result["type"] == FlowResultType.ABORT + assert result["reason"] == "already_configured" + + async def test_user_resolve_error( hass: HomeAssistant, mock_client, mock_zeroconf: None, mock_setup_entry: None ) -> None: @@ -140,6 +195,53 @@ async def test_user_resolve_error( assert len(mock_client.disconnect.mock_calls) == 1 +async def test_user_causes_zeroconf_to_abort( + hass: HomeAssistant, mock_client, mock_zeroconf: None, mock_setup_entry: None +) -> None: + """Test that the user flow sets the unique id and aborts the zeroconf flow.""" + service_info = zeroconf.ZeroconfServiceInfo( + host="192.168.43.183", + addresses=["192.168.43.183"], + hostname="test8266.local.", + name="mock_name", + port=6053, + properties={ + "mac": "1122334455aa", + }, + type="mock_type", + ) + discovery_result = await hass.config_entries.flow.async_init( + "esphome", context={"source": config_entries.SOURCE_ZEROCONF}, data=service_info + ) + + assert discovery_result["type"] == FlowResultType.FORM + assert discovery_result["step_id"] == "discovery_confirm" + + result = await hass.config_entries.flow.async_init( + "esphome", + context={"source": config_entries.SOURCE_USER}, + data=None, + ) + + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "user" + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_HOST: "127.0.0.1", CONF_PORT: 6053}, + ) + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"] == { + CONF_HOST: "127.0.0.1", + CONF_PORT: 6053, + CONF_PASSWORD: "", + CONF_NOISE_PSK: "", + CONF_DEVICE_NAME: "test", + } + + assert not hass.config_entries.flow.async_progress_by_handler(DOMAIN) + + async def test_user_connection_error( hass: HomeAssistant, mock_client, mock_zeroconf: None, mock_setup_entry: None ) -> None: @@ -217,6 +319,211 @@ async def test_user_invalid_password( assert result["errors"] == {"base": "invalid_auth"} +async def test_user_dashboard_has_wrong_key( + hass: HomeAssistant, + mock_client, + mock_dashboard, + mock_zeroconf: None, + mock_setup_entry: None, +) -> None: + """Test user step with key from dashboard that is incorrect.""" + mock_client.device_info.side_effect = [ + RequiresEncryptionAPIError, + InvalidEncryptionKeyAPIError, + DeviceInfo( + uses_password=False, + name="test", + mac_address="11:22:33:44:55:AA", + ), + ] + + with patch( + "homeassistant.components.esphome.dashboard.ESPHomeDashboardAPI.get_encryption_key", + return_value=WRONG_NOISE_PSK, + ): + result = await hass.config_entries.flow.async_init( + "esphome", + context={"source": config_entries.SOURCE_USER}, + data={CONF_HOST: "127.0.0.1", CONF_PORT: 6053}, + ) + await hass.async_block_till_done() + + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "encryption_key" + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={CONF_NOISE_PSK: VALID_NOISE_PSK} + ) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"] == { + CONF_HOST: "127.0.0.1", + CONF_PORT: 6053, + CONF_PASSWORD: "", + CONF_NOISE_PSK: VALID_NOISE_PSK, + CONF_DEVICE_NAME: "test", + } + assert mock_client.noise_psk == VALID_NOISE_PSK + + +async def test_user_discovers_name_and_gets_key_from_dashboard( + hass: HomeAssistant, + mock_client, + mock_dashboard, + mock_zeroconf: None, + mock_setup_entry: None, +) -> None: + """Test user step can discover the name and get the key from the dashboard.""" + mock_client.device_info.side_effect = [ + RequiresEncryptionAPIError, + InvalidEncryptionKeyAPIError("Wrong key", "test"), + DeviceInfo( + uses_password=False, + name="test", + mac_address="11:22:33:44:55:AA", + ), + ] + + mock_dashboard["configured"].append( + { + "name": "test", + "configuration": "test.yaml", + } + ) + await dashboard.async_get_dashboard(hass).async_refresh() + + with patch( + "homeassistant.components.esphome.dashboard.ESPHomeDashboardAPI.get_encryption_key", + return_value=VALID_NOISE_PSK, + ): + result = await hass.config_entries.flow.async_init( + "esphome", + context={"source": config_entries.SOURCE_USER}, + data={CONF_HOST: "127.0.0.1", CONF_PORT: 6053}, + ) + await hass.async_block_till_done() + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"] == { + CONF_HOST: "127.0.0.1", + CONF_PORT: 6053, + CONF_PASSWORD: "", + CONF_NOISE_PSK: VALID_NOISE_PSK, + CONF_DEVICE_NAME: "test", + } + assert mock_client.noise_psk == VALID_NOISE_PSK + + +async def test_user_discovers_name_and_gets_key_from_dashboard_fails( + hass: HomeAssistant, + mock_client, + mock_dashboard, + mock_zeroconf: None, + mock_setup_entry: None, +) -> None: + """Test user step can discover the name and get the key from the dashboard.""" + mock_client.device_info.side_effect = [ + RequiresEncryptionAPIError, + InvalidEncryptionKeyAPIError("Wrong key", "test"), + DeviceInfo( + uses_password=False, + name="test", + mac_address="11:22:33:44:55:aa", + ), + ] + + mock_dashboard["configured"].append( + { + "name": "test", + "configuration": "test.yaml", + } + ) + await dashboard.async_get_dashboard(hass).async_refresh() + + with patch( + "homeassistant.components.esphome.dashboard.ESPHomeDashboardAPI.get_encryption_key", + side_effect=aiohttp.ClientError, + ): + result = await hass.config_entries.flow.async_init( + "esphome", + context={"source": config_entries.SOURCE_USER}, + data={CONF_HOST: "127.0.0.1", CONF_PORT: 6053}, + ) + await hass.async_block_till_done() + + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "encryption_key" + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={CONF_NOISE_PSK: VALID_NOISE_PSK} + ) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"] == { + CONF_HOST: "127.0.0.1", + CONF_PORT: 6053, + CONF_PASSWORD: "", + CONF_NOISE_PSK: VALID_NOISE_PSK, + CONF_DEVICE_NAME: "test", + } + assert mock_client.noise_psk == VALID_NOISE_PSK + + +async def test_user_discovers_name_and_dashboard_is_unavailable( + hass: HomeAssistant, + mock_client, + mock_dashboard, + mock_zeroconf: None, + mock_setup_entry: None, +) -> None: + """Test user step can discover the name but the dashboard is unavailable.""" + mock_client.device_info.side_effect = [ + RequiresEncryptionAPIError, + InvalidEncryptionKeyAPIError("Wrong key", "test"), + DeviceInfo( + uses_password=False, + name="test", + mac_address="11:22:33:44:55:AA", + ), + ] + + mock_dashboard["configured"].append( + { + "name": "test", + "configuration": "test.yaml", + } + ) + + with patch( + "esphome_dashboard_api.ESPHomeDashboardAPI.get_devices", + side_effect=asyncio.TimeoutError, + ): + await dashboard.async_get_dashboard(hass).async_refresh() + result = await hass.config_entries.flow.async_init( + "esphome", + context={"source": config_entries.SOURCE_USER}, + data={CONF_HOST: "127.0.0.1", CONF_PORT: 6053}, + ) + await hass.async_block_till_done() + + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "encryption_key" + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={CONF_NOISE_PSK: VALID_NOISE_PSK} + ) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"] == { + CONF_HOST: "127.0.0.1", + CONF_PORT: 6053, + CONF_PASSWORD: "", + CONF_NOISE_PSK: VALID_NOISE_PSK, + CONF_DEVICE_NAME: "test", + } + assert mock_client.noise_psk == VALID_NOISE_PSK + + async def test_login_connection_error( hass: HomeAssistant, mock_client, mock_zeroconf: None, mock_setup_entry: None ) -> None: @@ -398,9 +705,9 @@ async def test_user_requires_psk( assert result["step_id"] == "encryption_key" assert result["errors"] == {} - assert len(mock_client.connect.mock_calls) == 1 - assert len(mock_client.device_info.mock_calls) == 1 - assert len(mock_client.disconnect.mock_calls) == 1 + assert len(mock_client.connect.mock_calls) == 2 + assert len(mock_client.device_info.mock_calls) == 2 + assert len(mock_client.disconnect.mock_calls) == 2 async def test_encryption_key_valid_psk( @@ -894,7 +1201,7 @@ async def test_zeroconf_encryption_key_via_dashboard( DeviceInfo( uses_password=False, name="test8266", - mac_address="11:22:33:44:55:aa", + mac_address="11:22:33:44:55:AA", ), ]