Accept homekit_controller pairing codes both with and without dashes (#30273)

* Handle MalformedPinError from homekit_python
* Handle both formats of pin codes

homeassistant/components/homekit_controller/config_flow.py

@@ -2,6 +2,7 @@
 import json
 import logging
 import os
+import re
 
 import homekit
 from homekit.controller.ip_implementation import IpPairing
@@ -17,6 +18,8 @@ HOMEKIT_IGNORE = ["Home Assistant Bridge"]
 HOMEKIT_DIR = ".homekit"
 PAIRING_FILE = "pairing.json"
 
+PIN_FORMAT = re.compile(r"^(\d{3})-{0,1}(\d{2})-{0,1}(\d{3})$")
+
 _LOGGER = logging.getLogger(__name__)
 
 
@@ -59,6 +62,20 @@ def find_existing_host(hass, serial):
             return entry
 
 
+def ensure_pin_format(pin):
+    """
+    Ensure a pin code is correctly formatted.
+
+    Ensures a pin code is in the format 111-11-111. Handles codes with and without dashes.
+
+    If incorrect code is entered, an exception is raised.
+    """
+    match = PIN_FORMAT.search(pin)
+    if not match:
+        raise homekit.exceptions.MalformedPinError(f"Invalid PIN code f{pin}")
+    return "{}-{}-{}".format(*match.groups())
+
+
 @config_entries.HANDLERS.register(DOMAIN)
 class HomekitControllerFlowHandler(config_entries.ConfigFlow):
     """Handle a HomeKit config flow."""
@@ -277,6 +294,8 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow):
         if pair_info:
             code = pair_info["pairing_code"]
             try:
+                code = ensure_pin_format(code)
+
                 await self.hass.async_add_executor_job(self.finish_pairing, code)
 
                 pairing = self.controller.pairings.get(self.hkid)
@@ -284,6 +303,9 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow):
                     return await self._entry_from_accessory(pairing)
 
                 errors["pairing_code"] = "unable_to_pair"
+            except homekit.exceptions.MalformedPinError:
+                # Library claimed pin was invalid before even making an API call
+                errors["pairing_code"] = "authentication_error"
             except homekit.AuthenticationError:
                 # PairSetup M4 - SRP proof failed
                 # PairSetup M6 - Ed25519 signature verification failed

tests/components/homekit_controller/test_config_flow.py

@@ -27,6 +27,7 @@ PAIRING_START_ABORT_ERRORS = [
 ]
 
 PAIRING_FINISH_FORM_ERRORS = [
+    (homekit.exceptions.MalformedPinError, "authentication_error"),
     (homekit.MaxPeersError, "max_peers_error"),
     (homekit.AuthenticationError, "authentication_error"),
     (homekit.UnknownError, "unknown_error"),
@@ -37,6 +38,27 @@ PAIRING_FINISH_ABORT_ERRORS = [
     (homekit.AccessoryNotFoundError, "accessory_not_found_error")
 ]
 
+INVALID_PAIRING_CODES = [
+    "aaa-aa-aaa",
+    "aaa-11-aaa",
+    "111-aa-aaa",
+    "aaa-aa-111",
+    "1111-1-111",
+    "a111-11-111",
+    " 111-11-111",
+    "111-11-111 ",
+    "111-11-111a",
+    "1111111",
+]
+
+
+VALID_PAIRING_CODES = [
+    "111-11-111",
+    "123-45-678",
+    "11111111",
+    "98765432",
+]
+
 
 def _setup_flow_handler(hass):
     flow = config_flow.HomekitControllerFlowHandler()
@@ -56,6 +78,23 @@ async def _setup_flow_zeroconf(hass, discovery_info):
     return result
 
 
+@pytest.mark.parametrize("pairing_code", INVALID_PAIRING_CODES)
+def test_invalid_pairing_codes(pairing_code):
+    """Test ensure_pin_format raises for an invalid pin code."""
+    with pytest.raises(homekit.exceptions.MalformedPinError):
+        config_flow.ensure_pin_format(pairing_code)
+
+
+@pytest.mark.parametrize("pairing_code", VALID_PAIRING_CODES)
+def test_valid_pairing_codes(pairing_code):
+    """Test ensure_pin_format corrects format for a valid pin in an alternative format."""
+    valid_pin = config_flow.ensure_pin_format(pairing_code).split("-")
+    assert len(valid_pin) == 3
+    assert len(valid_pin[0]) == 3
+    assert len(valid_pin[1]) == 2
+    assert len(valid_pin[2]) == 3
+
+
 async def test_discovery_works(hass):
     """Test a device being discovered."""
     discovery_info = {
@@ -99,7 +138,7 @@ async def test_discovery_works(hass):
 
     # Pairing doesn't error error and pairing results
     flow.controller.pairings = {"00:00:00:00:00:00": pairing}
-    result = await flow.async_step_pair({"pairing_code": "111-22-33"})
+    result = await flow.async_step_pair({"pairing_code": "111-22-333"})
     assert result["type"] == "create_entry"
     assert result["title"] == "Koogeek-LS1-20833F"
     assert result["data"] == pairing.pairing_data
@@ -147,7 +186,7 @@ async def test_discovery_works_upper_case(hass):
     ]
 
     flow.controller.pairings = {"00:00:00:00:00:00": pairing}
-    result = await flow.async_step_pair({"pairing_code": "111-22-33"})
+    result = await flow.async_step_pair({"pairing_code": "111-22-333"})
     assert result["type"] == "create_entry"
     assert result["title"] == "Koogeek-LS1-20833F"
     assert result["data"] == pairing.pairing_data
@@ -196,7 +235,7 @@ async def test_discovery_works_missing_csharp(hass):
 
     flow.controller.pairings = {"00:00:00:00:00:00": pairing}
 
-    result = await flow.async_step_pair({"pairing_code": "111-22-33"})
+    result = await flow.async_step_pair({"pairing_code": "111-22-333"})
     assert result["type"] == "create_entry"
     assert result["title"] == "Koogeek-LS1-20833F"
     assert result["data"] == pairing.pairing_data
@@ -379,7 +418,7 @@ async def test_pair_unable_to_pair(hass):
     assert flow.controller.start_pairing.call_count == 1
 
     # Pairing doesn't error but no pairing object is generated
-    result = await flow.async_step_pair({"pairing_code": "111-22-33"})
+    result = await flow.async_step_pair({"pairing_code": "111-22-333"})
     assert result["type"] == "form"
     assert result["errors"]["pairing_code"] == "unable_to_pair"
 
@@ -486,7 +525,7 @@ async def test_pair_abort_errors_on_finish(hass, exception, expected):
 
     # User submits code - pairing fails but can be retried
     flow.finish_pairing.side_effect = exception("error")
-    result = await flow.async_step_pair({"pairing_code": "111-22-33"})
+    result = await flow.async_step_pair({"pairing_code": "111-22-333"})
     assert result["type"] == "abort"
     assert result["reason"] == expected
     assert flow.context == {
@@ -526,7 +565,7 @@ async def test_pair_form_errors_on_finish(hass, exception, expected):
 
     # User submits code - pairing fails but can be retried
     flow.finish_pairing.side_effect = exception("error")
-    result = await flow.async_step_pair({"pairing_code": "111-22-33"})
+    result = await flow.async_step_pair({"pairing_code": "111-22-333"})
     assert result["type"] == "form"
     assert result["errors"]["pairing_code"] == expected
     assert flow.context == {
@@ -639,7 +678,7 @@ async def test_user_works(hass):
     assert result["type"] == "form"
     assert result["step_id"] == "pair"
 
-    result = await flow.async_step_pair({"pairing_code": "111-22-33"})
+    result = await flow.async_step_pair({"pairing_code": "111-22-333"})
     assert result["type"] == "create_entry"
     assert result["title"] == "Koogeek-LS1-20833F"
     assert result["data"] == pairing.pairing_data
@@ -888,7 +927,7 @@ async def test_unignore_works(hass):
     assert flow.controller.start_pairing.call_count == 1
 
     # Pairing finalized
-    result = await flow.async_step_pair({"pairing_code": "111-22-33"})
+    result = await flow.async_step_pair({"pairing_code": "111-22-333"})
     assert result["type"] == "create_entry"
     assert result["title"] == "Koogeek-LS1-20833F"
     assert result["data"] == pairing.pairing_data