Improve config flow of devolo Home Network (#131911)

* Improve config flow of devolo Home Network

* Apply feedback

* Use references

homeassistant/components/devolo_home_network/config_flow.py

@@ -7,7 +7,7 @@ import logging
 from typing import Any
 
 from devolo_plc_api.device import Device
-from devolo_plc_api.exceptions.device import DeviceNotFound
+from devolo_plc_api.exceptions.device import DeviceNotFound, DevicePasswordProtected
 import voluptuous as vol
 
 from homeassistant.components import zeroconf
@@ -22,7 +22,9 @@ from .const import DOMAIN, PRODUCT, SERIAL_NUMBER, TITLE
 
 _LOGGER = logging.getLogger(__name__)
 
-STEP_USER_DATA_SCHEMA = vol.Schema({vol.Required(CONF_IP_ADDRESS): str})
+STEP_USER_DATA_SCHEMA = vol.Schema(
+    {vol.Required(CONF_IP_ADDRESS): str, vol.Optional(CONF_PASSWORD): str}
+)
 STEP_REAUTH_DATA_SCHEMA = vol.Schema({vol.Optional(CONF_PASSWORD): str})
 
 
@@ -36,7 +38,16 @@ async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> dict[str,
 
     device = Device(data[CONF_IP_ADDRESS], zeroconf_instance=zeroconf_instance)
 
+    device.password = data[CONF_PASSWORD]
+
     await device.async_connect(session_instance=async_client)
+
+    # Try a password protected, non-writing device API call that raises, if the password is wrong.
+    # If only the plcnet API is available, we can continue without trying a password as the plcnet
+    # API does not require a password.
+    if device.device:
+        await device.device.async_uptime()
+
     await device.async_disconnect()
 
     return {
@@ -59,22 +70,21 @@ class DevoloHomeNetworkConfigFlow(ConfigFlow, domain=DOMAIN):
         """Handle the initial step."""
         errors: dict = {}
 
-        if user_input is None:
+        if user_input is not None:
-            return self.async_show_form(
-                step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors
-            )
-
             try:
                 info = await validate_input(self.hass, user_input)
             except DeviceNotFound:
                 errors["base"] = "cannot_connect"
+            except DevicePasswordProtected:
+                errors["base"] = "invalid_auth"
             except Exception:
                 _LOGGER.exception("Unexpected exception")
                 errors["base"] = "unknown"
             else:
-            await self.async_set_unique_id(info[SERIAL_NUMBER], raise_on_progress=False)
+                await self.async_set_unique_id(
+                    info[SERIAL_NUMBER], raise_on_progress=False
+                )
                 self._abort_if_unique_id_configured()
-            user_input[CONF_PASSWORD] = ""
                 return self.async_create_entry(title=info[TITLE], data=user_input)
 
         return self.async_show_form(
@@ -106,15 +116,27 @@ class DevoloHomeNetworkConfigFlow(ConfigFlow, domain=DOMAIN):
     ) -> ConfigFlowResult:
         """Handle a flow initiated by zeroconf."""
         title = self.context["title_placeholders"][CONF_NAME]
+        errors: dict = {}
+        data_schema: vol.Schema | None = None
+
         if user_input is not None:
             data = {
                 CONF_IP_ADDRESS: self.host,
-                CONF_PASSWORD: "",
+                CONF_PASSWORD: user_input.get(CONF_PASSWORD, ""),
             }
+            try:
+                await validate_input(self.hass, data)
+            except DevicePasswordProtected:
+                errors = {"base": "invalid_auth"}
+                data_schema = STEP_REAUTH_DATA_SCHEMA
+            else:
                 return self.async_create_entry(title=title, data=data)
+
         return self.async_show_form(
             step_id="zeroconf_confirm",
+            data_schema=data_schema,
             description_placeholders={"host_name": title},
+            errors=errors,
         )
 
     async def async_step_reauth(
@@ -134,14 +156,21 @@ class DevoloHomeNetworkConfigFlow(ConfigFlow, domain=DOMAIN):
         self, user_input: dict[str, Any] | None = None
     ) -> ConfigFlowResult:
         """Handle a flow initiated by reauthentication."""
-        if user_input is None:
+        errors: dict = {}
-            return self.async_show_form(
+        if user_input is not None:
-                step_id="reauth_confirm",
-                data_schema=STEP_REAUTH_DATA_SCHEMA,
-            )
-
             data = {
                 CONF_IP_ADDRESS: self.host,
                 CONF_PASSWORD: user_input[CONF_PASSWORD],
             }
+            try:
+                await validate_input(self.hass, data)
+            except DevicePasswordProtected:
+                errors = {"base": "invalid_auth"}
+            else:
                 return self.async_update_reload_and_abort(self._reauth_entry, data=data)
+
+        return self.async_show_form(
+            step_id="reauth_confirm",
+            data_schema=STEP_REAUTH_DATA_SCHEMA,
+            errors=errors,
+        )

homeassistant/components/devolo_home_network/strings.json

@@ -5,10 +5,12 @@
       "user": {
         "description": "[%key:common::config_flow::description::confirm_setup%]",
         "data": {
-          "ip_address": "[%key:common::config_flow::data::ip%]"
+          "ip_address": "[%key:common::config_flow::data::ip%]",
+          "password": "[%key:common::config_flow::data::password%]"
         },
         "data_description": {
-          "ip_address": "IP address of your devolo Home Network device. This can be found in the devolo Home Network App on the device dashboard."
+          "ip_address": "IP address of your devolo Home Network device. This can be found in the devolo Home Network App on the device dashboard.",
+          "password": "Password you protected the device with."
         }
       },
       "reauth_confirm": {
@@ -16,16 +18,23 @@
           "password": "[%key:common::config_flow::data::password%]"
         },
         "data_description": {
-          "password": "Password you protected the device with."
+          "password": "[%key:component::devolo_home_network::config::step::user::data_description::password%]"
         }
       },
       "zeroconf_confirm": {
         "description": "Do you want to add the devolo home network device with the hostname `{host_name}` to Home Assistant?",
-        "title": "Discovered devolo home network device"
+        "title": "Discovered devolo home network device",
+        "data": {
+          "password": "[%key:common::config_flow::data::password%]"
+        },
+        "data_description": {
+          "password": "[%key:component::devolo_home_network::config::step::user::data_description::password%]"
+        }
       }
     },
     "error": {
       "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]",
+      "invalid_auth": "[%key:common::config_flow::error::invalid_auth%]",
       "unknown": "[%key:common::config_flow::error::unknown%]"
     },
     "abort": {

tests/components/devolo_home_network/mock.py

@@ -6,6 +6,7 @@ from unittest.mock import AsyncMock
 
 from devolo_plc_api.device import Device
 from devolo_plc_api.device_api.deviceapi import DeviceApi
+from devolo_plc_api.exceptions.device import DevicePasswordProtected
 from devolo_plc_api.plcnet_api.plcnetapi import PlcNetApi
 import httpx
 from zeroconf import Zeroconf
@@ -81,3 +82,16 @@ class MockDevice(Device):
         self.plcnet.async_get_network_overview = AsyncMock(return_value=PLCNET)
         self.plcnet.async_identify_device_start = AsyncMock(return_value=True)
         self.plcnet.async_pair_device = AsyncMock(return_value=True)
+
+
+class MockDeviceWrongPassword(MockDevice):
+    """Mock of a devolo Home Network device, that always complains about a wrong password."""
+
+    def __init__(
+        self,
+        ip: str,
+        zeroconf_instance: AsyncZeroconf | Zeroconf | None = None,
+    ) -> None:
+        """Bring mock in a well defined state."""
+        super().__init__(ip, zeroconf_instance)
+        self.device.async_uptime = AsyncMock(side_effect=DevicePasswordProtected)

tests/components/devolo_home_network/test_config_flow.py

@@ -5,11 +5,10 @@ from __future__ import annotations
 from typing import Any
 from unittest.mock import patch
 
-from devolo_plc_api.exceptions.device import DeviceNotFound
+from devolo_plc_api.exceptions.device import DeviceNotFound, DevicePasswordProtected
 import pytest
 
 from homeassistant import config_entries
-from homeassistant.components.devolo_home_network import config_flow
 from homeassistant.components.devolo_home_network.const import (
     DOMAIN,
     SERIAL_NUMBER,
@@ -27,7 +26,7 @@ from .const import (
     IP,
     IP_ALT,
 )
-from .mock import MockDevice
+from .mock import MockDevice, MockDeviceWrongPassword
 
 
 async def test_form(hass: HomeAssistant, info: dict[str, Any]) -> None:
@@ -44,15 +43,13 @@ async def test_form(hass: HomeAssistant, info: dict[str, Any]) -> None:
     ) as mock_setup_entry:
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
-            {
+            {CONF_IP_ADDRESS: IP, CONF_PASSWORD: ""},
-                CONF_IP_ADDRESS: IP,
-            },
         )
         await hass.async_block_till_done()
 
     assert result2["type"] is FlowResultType.CREATE_ENTRY
-    assert result2["result"].unique_id == info["serial_number"]
+    assert result2["result"].unique_id == info[SERIAL_NUMBER]
-    assert result2["title"] == info["title"]
+    assert result2["title"] == info[TITLE]
     assert result2["data"] == {
         CONF_IP_ADDRESS: IP,
         CONF_PASSWORD: "",
@@ -62,7 +59,11 @@ async def test_form(hass: HomeAssistant, info: dict[str, Any]) -> None:
 
 @pytest.mark.parametrize(
     ("exception_type", "expected_error"),
-    [(DeviceNotFound(IP), "cannot_connect"), (Exception, "unknown")],
+    [
+        (DeviceNotFound(IP), "cannot_connect"),
+        (DevicePasswordProtected, "invalid_auth"),
+        (Exception, "unknown"),
+    ],
 )
 async def test_form_error(hass: HomeAssistant, exception_type, expected_error) -> None:
     """Test we handle errors."""
@@ -108,9 +109,15 @@ async def test_zeroconf(hass: HomeAssistant) -> None:
         == DISCOVERY_INFO.hostname.split(".", maxsplit=1)[0]
     )
 
-    with patch(
+    with (
+        patch(
             "homeassistant.components.devolo_home_network.async_setup_entry",
             return_value=True,
+        ),
+        patch(
+            "homeassistant.components.devolo_home_network.config_flow.Device",
+            new=MockDevice,
+        ),
     ):
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
@@ -127,6 +134,69 @@ async def test_zeroconf(hass: HomeAssistant) -> None:
     assert result2["result"].unique_id == "1234567890"
 
 
+async def test_zeroconf_wrong_auth(hass: HomeAssistant) -> None:
+    """Test that the zeroconf form asks for password if authorization fails."""
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={"source": config_entries.SOURCE_ZEROCONF},
+        data=DISCOVERY_INFO,
+    )
+
+    assert result["step_id"] == "zeroconf_confirm"
+    assert result["type"] is FlowResultType.FORM
+    assert result["description_placeholders"] == {"host_name": "test"}
+
+    context = next(
+        flow["context"]
+        for flow in hass.config_entries.flow.async_progress()
+        if flow["flow_id"] == result["flow_id"]
+    )
+
+    assert (
+        context["title_placeholders"][CONF_NAME]
+        == DISCOVERY_INFO.hostname.split(".", maxsplit=1)[0]
+    )
+
+    with (
+        patch(
+            "homeassistant.components.devolo_home_network.async_setup_entry",
+            return_value=True,
+        ),
+        patch(
+            "homeassistant.components.devolo_home_network.config_flow.Device",
+            new=MockDeviceWrongPassword,
+        ),
+    ):
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            {},
+        )
+        await hass.async_block_till_done()
+
+    assert result2["type"] is FlowResultType.FORM
+    assert result2["errors"] == {CONF_BASE: "invalid_auth"}
+
+    with (
+        patch(
+            "homeassistant.components.devolo_home_network.async_setup_entry",
+            return_value=True,
+        ),
+        patch(
+            "homeassistant.components.devolo_home_network.config_flow.Device",
+            new=MockDevice,
+        ),
+    ):
+        result3 = await hass.config_entries.flow.async_configure(
+            result2["flow_id"],
+            {
+                CONF_PASSWORD: "new-password",
+            },
+        )
+        await hass.async_block_till_done()
+
+    assert result3["type"] is FlowResultType.CREATE_ENTRY
+
+
 async def test_abort_zeroconf_wrong_device(hass: HomeAssistant) -> None:
     """Test we abort zeroconf for wrong devices."""
     result = await hass.config_entries.flow.async_init(
@@ -179,31 +249,43 @@ async def test_form_reauth(hass: HomeAssistant) -> None:
     assert result["step_id"] == "reauth_confirm"
     assert result["type"] is FlowResultType.FORM
 
-    with patch(
+    with (
+        patch(
             "homeassistant.components.devolo_home_network.async_setup_entry",
             return_value=True,
-    ) as mock_setup_entry:
+        ),
+        patch(
+            "homeassistant.components.devolo_home_network.config_flow.Device",
+            new=MockDeviceWrongPassword,
+        ),
+    ):
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
-            {CONF_PASSWORD: "test-password-new"},
+            {CONF_PASSWORD: "test-wrong-password"},
         )
         await hass.async_block_till_done()
 
-    assert result2["type"] is FlowResultType.ABORT
+    assert result2["type"] is FlowResultType.FORM
-    assert result2["reason"] == "reauth_successful"
+    assert result2["errors"] == {CONF_BASE: "invalid_auth"}
+
+    with (
+        patch(
+            "homeassistant.components.devolo_home_network.async_setup_entry",
+            return_value=True,
+        ) as mock_setup_entry,
+        patch(
+            "homeassistant.components.devolo_home_network.config_flow.Device",
+            new=MockDevice,
+        ),
+    ):
+        result3 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            {CONF_PASSWORD: "test-right-password"},
+        )
+        await hass.async_block_till_done()
+
+    assert result3["type"] is FlowResultType.ABORT
+    assert result3["reason"] == "reauth_successful"
     assert len(mock_setup_entry.mock_calls) == 1
 
     await hass.config_entries.async_unload(entry.entry_id)
-
-
-@pytest.mark.usefixtures("mock_device")
-@pytest.mark.usefixtures("mock_zeroconf")
-async def test_validate_input(hass: HomeAssistant) -> None:
-    """Test input validation."""
-    with patch(
-        "homeassistant.components.devolo_home_network.config_flow.Device",
-        new=MockDevice,
-    ):
-        info = await config_flow.validate_input(hass, {CONF_IP_ADDRESS: IP})
-        assert SERIAL_NUMBER in info
-        assert TITLE in info