Fix race during homekit controller pairing (#69948)

homeassistant/components/homekit_controller/config_flow.py

@@ -293,7 +293,10 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
         self._abort_if_unique_id_configured(updates=updated_ip_port)
 
         for progress in self._async_in_progress(include_uninitialized=True):
-            if progress["context"].get("unique_id") == normalized_hkid:
+            context = progress["context"]
+            if context.get("unique_id") == normalized_hkid and not context.get(
+                "pairing"
+            ):
                 if paired:
                     # If the device gets paired, we want to dismiss
                     # an existing discovery since we can no longer
@@ -350,6 +353,7 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
             await self._async_setup_controller()
 
         if pair_info and self.finish_pairing:
+            self.context["pairing"] = True
             code = pair_info["pairing_code"]
             try:
                 code = ensure_pin_format(

tests/components/homekit_controller/test_config_flow.py

@@ -1,4 +1,5 @@
 """Tests for homekit_controller config flow."""
+import asyncio
 from unittest import mock
 import unittest.mock
 from unittest.mock import AsyncMock, patch
@@ -14,6 +15,7 @@ from homeassistant import config_entries
 from homeassistant.components import zeroconf
 from homeassistant.components.homekit_controller import config_flow
 from homeassistant.components.homekit_controller.const import KNOWN_DEVICES
+from homeassistant.data_entry_flow import RESULT_TYPE_ABORT, RESULT_TYPE_CREATE_ENTRY
 from homeassistant.helpers import device_registry
 
 from tests.common import MockConfigEntry, mock_device_registry
@@ -133,7 +135,7 @@ def get_flow_context(hass, result):
 
 
 def get_device_discovery_info(
-    device, upper_case_props=False, missing_csharp=False
+    device, upper_case_props=False, missing_csharp=False, paired=False
 ) -> zeroconf.ZeroconfServiceInfo:
     """Turn a aiohomekit format zeroconf entry into a homeassistant one."""
     result = zeroconf.ZeroconfServiceInfo(
@@ -150,7 +152,7 @@ def get_device_discovery_info(
             "s#": device.description.state_num,
             "ff": "0",
             "ci": "0",
-            "sf": "1",
+            "sf": "0" if paired else "1",
             "sh": "",
         },
         type="_hap._tcp.local.",
@@ -250,10 +252,8 @@ async def test_abort_duplicate_flow(hass, controller):
 async def test_pair_already_paired_1(hass, controller):
     """Already paired."""
     device = setup_mock_accessory(controller)
-    discovery_info = get_device_discovery_info(device)
-
     # Flag device as already paired
-    discovery_info.properties["sf"] = 0x0
+    discovery_info = get_device_discovery_info(device, paired=True)
 
     # Device is discovered
     result = await hass.config_entries.flow.async_init(
@@ -692,6 +692,7 @@ async def test_pair_form_errors_on_finish(hass, controller, exception, expected)
         "title_placeholders": {"name": "TestDevice"},
         "unique_id": "00:00:00:00:00:00",
         "source": config_entries.SOURCE_ZEROCONF,
+        "pairing": True,
     }
 
 
@@ -883,3 +884,69 @@ async def test_discovery_dismiss_existing_flow_on_paired(hass, controller):
         len(hass.config_entries.flow.async_progress_by_handler("homekit_controller"))
         == 0
     )
+
+
+async def test_mdns_update_to_paired_during_pairing(hass, controller):
+    """Test we do not abort pairing if mdns is updated to reflect paired during pairing."""
+    device = setup_mock_accessory(controller)
+    discovery_info = get_device_discovery_info(device)
+    discovery_info_paired = get_device_discovery_info(device, paired=True)
+
+    # Device is discovered
+    result = await hass.config_entries.flow.async_init(
+        "homekit_controller",
+        context={"source": config_entries.SOURCE_ZEROCONF},
+        data=discovery_info,
+    )
+
+    assert get_flow_context(hass, result) == {
+        "title_placeholders": {"name": "TestDevice"},
+        "unique_id": "00:00:00:00:00:00",
+        "source": config_entries.SOURCE_ZEROCONF,
+    }
+
+    mdns_update_to_paired = asyncio.Event()
+
+    original_async_start_pairing = device.async_start_pairing
+
+    async def _async_start_pairing(*args, **kwargs):
+        finish_pairing = await original_async_start_pairing(*args, **kwargs)
+
+        async def _finish_pairing(*args, **kwargs):
+            # Insert an event wait to make sure
+            # we trigger the mdns update in the middle of the pairing
+            await mdns_update_to_paired.wait()
+            return await finish_pairing(*args, **kwargs)
+
+        return _finish_pairing
+
+    with patch.object(device, "async_start_pairing", _async_start_pairing):
+        result = await hass.config_entries.flow.async_configure(result["flow_id"])
+
+    assert result["type"] == "form"
+    assert get_flow_context(hass, result) == {
+        "title_placeholders": {"name": "TestDevice"},
+        "unique_id": "00:00:00:00:00:00",
+        "source": config_entries.SOURCE_ZEROCONF,
+    }
+
+    # User enters pairing code
+    task = asyncio.create_task(
+        hass.config_entries.flow.async_configure(
+            result["flow_id"], user_input={"pairing_code": "111-22-333"}
+        )
+    )
+    # Make sure when the device is discovered as paired via mdns
+    # it does not abort pairing if it happens before pairing is finished
+    result2 = await hass.config_entries.flow.async_init(
+        "homekit_controller",
+        context={"source": config_entries.SOURCE_ZEROCONF},
+        data=discovery_info_paired,
+    )
+    assert result2["type"] == RESULT_TYPE_ABORT
+    assert result2["reason"] == "already_paired"
+    mdns_update_to_paired.set()
+    result = await task
+    assert result["type"] == RESULT_TYPE_CREATE_ENTRY
+    assert result["title"] == "Koogeek-LS1-20833F"
+    assert result["data"] == {}