Move roborock unique id to be based on roborock userid instead of email (#141337)

* Move roborock unique id to be based on roborock userid instead of email

* Remove unnecessary data update

* Update tests

* Add tests coverage for removal of config entry

* Use config entry migration

* Remove unused fixtues

* Remove unnecessary logging

homeassistant/components/roborock/__init__.py

@@ -164,6 +164,31 @@ async def async_setup_entry(hass: HomeAssistant, entry: RoborockConfigEntry) ->
     return True
 
 
+async def async_migrate_entry(hass: HomeAssistant, entry: RoborockConfigEntry) -> bool:
+    """Migrate old configuration entries to the new format."""
+    _LOGGER.debug(
+        "Migrating configuration from version %s.%s",
+        entry.version,
+        entry.minor_version,
+    )
+    if entry.version > 1:
+        # Downgrade from future version
+        return False
+
+    # 1->2: Migrate from unique id as email address to unique id as rruid
+    if entry.minor_version == 1:
+        user_data = UserData.from_dict(entry.data[CONF_USER_DATA])
+        _LOGGER.debug("Updating unique id to %s", user_data.rruid)
+        hass.config_entries.async_update_entry(
+            entry,
+            unique_id=user_data.rruid,
+            version=1,
+            minor_version=2,
+        )
+
+    return True
+
+
 def build_setup_functions(
     hass: HomeAssistant,
     entry: RoborockConfigEntry,

homeassistant/components/roborock/config_flow.py

@@ -48,6 +48,7 @@ class RoborockFlowHandler(ConfigFlow, domain=DOMAIN):
     """Handle a config flow for Roborock."""
 
     VERSION = 1
+    MINOR_VERSION = 2
 
     def __init__(self) -> None:
         """Initialize the config flow."""
@@ -62,8 +63,6 @@ class RoborockFlowHandler(ConfigFlow, domain=DOMAIN):
 
         if user_input is not None:
             username = user_input[CONF_USERNAME]
-            await self.async_set_unique_id(username.lower())
-            self._abort_if_unique_id_configured(error="already_configured_account")
             self._username = username
             _LOGGER.debug("Requesting code for Roborock account")
             self._client = RoborockApiClient(
@@ -111,7 +110,7 @@ class RoborockFlowHandler(ConfigFlow, domain=DOMAIN):
             code = user_input[CONF_ENTRY_CODE]
             _LOGGER.debug("Logging into Roborock account using email provided code")
             try:
-                login_data = await self._client.code_login(code)
+                user_data = await self._client.code_login(code)
             except RoborockInvalidCode:
                 errors["base"] = "invalid_code"
             except RoborockException:
@@ -121,17 +120,20 @@ class RoborockFlowHandler(ConfigFlow, domain=DOMAIN):
                 _LOGGER.exception("Unexpected exception")
                 errors["base"] = "unknown"
             else:
+                await self.async_set_unique_id(user_data.rruid)
                 if self.source == SOURCE_REAUTH:
+                    self._abort_if_unique_id_mismatch(reason="wrong_account")
                     reauth_entry = self._get_reauth_entry()
                     self.hass.config_entries.async_update_entry(
                         reauth_entry,
                         data={
                             **reauth_entry.data,
-                            CONF_USER_DATA: login_data.as_dict(),
+                            CONF_USER_DATA: user_data.as_dict(),
                         },
                     )
                     return self.async_abort(reason="reauth_successful")
-                return self._create_entry(self._client, self._username, login_data)
+                self._abort_if_unique_id_configured(error="already_configured_account")
+                return self._create_entry(self._client, self._username, user_data)
 
         return self.async_show_form(
             step_id="code",

homeassistant/components/roborock/strings.json

@@ -35,7 +35,8 @@
     },
     "abort": {
       "already_configured_account": "[%key:common::config_flow::abort::already_configured_account%]",
-      "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]"
+      "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]",
+      "wrong_account": "Wrong account: Please authenticate with the right account."
     }
   },
   "options": {

tests/components/roborock/conftest.py

@@ -28,6 +28,7 @@ from .mock_data import (
     MULTI_MAP_LIST,
     NETWORK_INFO,
     PROP,
+    ROBOROCK_RRUID,
     SCENES,
     USER_DATA,
     USER_EMAIL,
@@ -188,18 +189,28 @@ def bypass_api_fixture_v1_only(bypass_api_fixture) -> None:
         yield
 
 
+@pytest.fixture(name="config_entry_data")
+def config_entry_data_fixture() -> dict[str, Any]:
+    """Fixture that returns the unique id for the config entry."""
+    return {
+        CONF_USERNAME: USER_EMAIL,
+        CONF_USER_DATA: USER_DATA.as_dict(),
+        CONF_BASE_URL: BASE_URL,
+    }
+
+
 @pytest.fixture
-def mock_roborock_entry(hass: HomeAssistant) -> MockConfigEntry:
+def mock_roborock_entry(
+    hass: HomeAssistant, config_entry_data: dict[str, Any]
+) -> MockConfigEntry:
     """Create a Roborock Entry that has not been setup."""
     mock_entry = MockConfigEntry(
         domain=DOMAIN,
         title=USER_EMAIL,
-        data={
+        data=config_entry_data,
-            CONF_USERNAME: USER_EMAIL,
+        unique_id=ROBOROCK_RRUID,
-            CONF_USER_DATA: USER_DATA.as_dict(),
+        version=1,
-            CONF_BASE_URL: BASE_URL,
+        minor_version=2,
-        },
-        unique_id=USER_EMAIL,
     )
     mock_entry.add_to_hass(hass)
     return mock_entry
@@ -211,18 +222,26 @@ def mock_platforms() -> list[Platform]:
     return []
 
 
+@pytest.fixture(autouse=True)
+async def mock_patforms_fixture(
+    hass: HomeAssistant,
+    platforms: list[Platform],
+) -> Generator[None]:
+    """Set up the Roborock platform."""
+    with patch("homeassistant.components.roborock.PLATFORMS", platforms):
+        yield
+
+
 @pytest.fixture
 async def setup_entry(
     hass: HomeAssistant,
     bypass_api_fixture,
     mock_roborock_entry: MockConfigEntry,
-    platforms: list[Platform],
 ) -> Generator[MockConfigEntry]:
     """Set up the Roborock platform."""
-    with patch("homeassistant.components.roborock.PLATFORMS", platforms):
     await hass.config_entries.async_setup(mock_roborock_entry.entry_id)
     await hass.async_block_till_done()
-        yield mock_roborock_entry
+    return mock_roborock_entry
 
 
 @pytest.fixture(autouse=True, name="storage_path")

tests/components/roborock/mock_data.py

@@ -28,6 +28,7 @@ USER_EMAIL = "user@domain.com"
 
 BASE_URL = "https://usiot.roborock.com"
 
+ROBOROCK_RRUID = "roboborock-userid-abc-123"
 USER_DATA = UserData.from_dict(
     {
         "tuyaname": "abc123",
@@ -35,7 +36,7 @@ USER_DATA = UserData.from_dict(
         "uid": 123456,
         "tokentype": "",
         "token": "abc123",
-        "rruid": "abc123",
+        "rruid": ROBOROCK_RRUID,
         "region": "us",
         "countrycode": "1",
         "country": "US",

tests/components/roborock/test_config_flow.py

@@ -16,12 +16,12 @@ from vacuum_map_parser_base.config.drawable import Drawable
 
 from homeassistant import config_entries
 from homeassistant.components.roborock.const import CONF_ENTRY_CODE, DOMAIN, DRAWABLES
-from homeassistant.const import CONF_USERNAME
+from homeassistant.const import CONF_USERNAME, Platform
 from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResultType
 from homeassistant.helpers.service_info.dhcp import DhcpServiceInfo
 
-from .mock_data import MOCK_CONFIG, NETWORK_INFO, USER_DATA, USER_EMAIL
+from .mock_data import MOCK_CONFIG, NETWORK_INFO, ROBOROCK_RRUID, USER_DATA, USER_EMAIL
 
 from tests.common import MockConfigEntry
 
@@ -64,6 +64,7 @@ async def test_config_flow_success(
             )
 
     assert result["type"] is FlowResultType.CREATE_ENTRY
+    assert result["context"]["unique_id"] == ROBOROCK_RRUID
     assert result["title"] == USER_EMAIL
     assert result["data"] == MOCK_CONFIG
     assert result["result"]
@@ -128,6 +129,7 @@ async def test_config_flow_failures_request_code(
             )
 
     assert result["type"] is FlowResultType.CREATE_ENTRY
+    assert result["context"]["unique_id"] == ROBOROCK_RRUID
     assert result["title"] == USER_EMAIL
     assert result["data"] == MOCK_CONFIG
     assert result["result"]
@@ -189,6 +191,7 @@ async def test_config_flow_failures_code_login(
             )
 
     assert result["type"] is FlowResultType.CREATE_ENTRY
+    assert result["context"]["unique_id"] == ROBOROCK_RRUID
     assert result["title"] == USER_EMAIL
     assert result["data"] == MOCK_CONFIG
     assert result["result"]
@@ -256,6 +259,7 @@ async def test_reauth_flow(
         )
     assert result["type"] is FlowResultType.ABORT
     assert result["reason"] == "reauth_successful"
+    assert mock_roborock_entry.unique_id == ROBOROCK_RRUID
     assert mock_roborock_entry.data["user_data"]["rriot"]["s"] == "new_password_hash"
 
 
@@ -264,7 +268,8 @@ async def test_account_already_configured(
     bypass_api_fixture,
     mock_roborock_entry: MockConfigEntry,
 ) -> None:
-    """Handle the config flow and make sure it succeeds."""
+    """Ensure the same account cannot be setup twice."""
+    assert mock_roborock_entry.unique_id == ROBOROCK_RRUID
     with patch(
         "homeassistant.components.roborock.async_setup_entry", return_value=True
     ):
@@ -280,10 +285,59 @@ async def test_account_already_configured(
                 result["flow_id"], {CONF_USERNAME: USER_EMAIL}
             )
 
+        assert result["step_id"] == "code"
+        assert result["type"] is FlowResultType.FORM
+        with patch(
+            "homeassistant.components.roborock.config_flow.RoborockApiClient.code_login",
+            return_value=USER_DATA,
+        ):
+            result = await hass.config_entries.flow.async_configure(
+                result["flow_id"], user_input={CONF_ENTRY_CODE: "123456"}
+            )
             assert result["type"] is FlowResultType.ABORT
             assert result["reason"] == "already_configured_account"
 
 
+async def test_reauth_wrong_account(
+    hass: HomeAssistant,
+    bypass_api_fixture,
+    mock_roborock_entry: MockConfigEntry,
+) -> None:
+    """Ensure that reauthentication must use the same account."""
+
+    # Start reauth
+    result = mock_roborock_entry.async_start_reauth(hass)
+    await hass.async_block_till_done()
+    flows = hass.config_entries.flow.async_progress()
+    assert len(flows) == 1
+    [result] = flows
+    assert result["step_id"] == "reauth_confirm"
+
+    with patch(
+        "homeassistant.components.roborock.async_setup_entry", return_value=True
+    ):
+        with patch(
+            "homeassistant.components.roborock.config_flow.RoborockApiClient.request_code"
+        ):
+            result = await hass.config_entries.flow.async_configure(
+                result["flow_id"], {CONF_USERNAME: USER_EMAIL}
+            )
+
+        assert result["step_id"] == "code"
+        assert result["type"] is FlowResultType.FORM
+        new_user_data = deepcopy(USER_DATA)
+        new_user_data.rruid = "new_rruid"
+        with patch(
+            "homeassistant.components.roborock.config_flow.RoborockApiClient.code_login",
+            return_value=new_user_data,
+        ):
+            result = await hass.config_entries.flow.async_configure(
+                result["flow_id"], user_input={CONF_ENTRY_CODE: "123456"}
+            )
+            assert result["type"] is FlowResultType.ABORT
+            assert result["reason"] == "wrong_account"
+
+
 async def test_discovery_not_setup(
     hass: HomeAssistant,
     bypass_api_fixture,
@@ -322,11 +376,13 @@ async def test_discovery_not_setup(
             )
 
     assert result["type"] is FlowResultType.CREATE_ENTRY
+    assert result["context"]["unique_id"] == ROBOROCK_RRUID
     assert result["title"] == USER_EMAIL
     assert result["data"] == MOCK_CONFIG
     assert result["result"]
 
 
+@pytest.mark.parametrize("platforms", [[Platform.SENSOR]])
 async def test_discovery_already_setup(
     hass: HomeAssistant,
     bypass_api_fixture,
@@ -346,3 +402,4 @@ async def test_discovery_already_setup(
     )
 
     assert result["type"] is FlowResultType.ABORT
+    assert result["reason"] == "already_configured"

tests/components/roborock/test_coordinator.py

@@ -13,6 +13,7 @@ from homeassistant.components.roborock.const import (
     V1_LOCAL_IN_CLEANING_INTERVAL,
     V1_LOCAL_NOT_CLEANING_INTERVAL,
 )
+from homeassistant.const import Platform
 from homeassistant.core import HomeAssistant
 from homeassistant.util import dt as dt_util
 
@@ -21,6 +22,12 @@ from .mock_data import PROP
 from tests.common import MockConfigEntry, async_fire_time_changed
 
 
+@pytest.fixture
+def platforms() -> list[Platform]:
+    """Fixture to set platforms used in the test."""
+    return [Platform.SENSOR]
+
+
 @pytest.mark.parametrize(
     ("interval", "in_cleaning"),
     [

tests/components/roborock/test_init.py

@@ -3,6 +3,7 @@
 from copy import deepcopy
 from http import HTTPStatus
 import pathlib
+from typing import Any
 from unittest.mock import patch
 
 import pytest
@@ -20,7 +21,13 @@ from homeassistant.core import HomeAssistant
 from homeassistant.helpers.device_registry import DeviceRegistry
 from homeassistant.setup import async_setup_component
 
-from .mock_data import HOME_DATA, NETWORK_INFO, NETWORK_INFO_2
+from .mock_data import (
+    HOME_DATA,
+    NETWORK_INFO,
+    NETWORK_INFO_2,
+    ROBOROCK_RRUID,
+    USER_EMAIL,
+)
 
 from tests.common import MockConfigEntry
 from tests.typing import ClientSessionGenerator
@@ -300,6 +307,7 @@ async def test_no_user_agreement(
         assert mock_roborock_entry.error_reason_translation_key == "no_user_agreement"
 
 
+@pytest.mark.parametrize("platforms", [[Platform.SENSOR]])
 async def test_stale_device(
     hass: HomeAssistant,
     bypass_api_fixture,
@@ -341,6 +349,7 @@ async def test_stale_device(
     # therefore not deleted.
 
 
+@pytest.mark.parametrize("platforms", [[Platform.SENSOR]])
 async def test_no_stale_device(
     hass: HomeAssistant,
     bypass_api_fixture,
@@ -369,3 +378,25 @@ async def test_no_stale_device(
         mock_roborock_entry.entry_id
     )
     assert len(new_devices) == 6  # 2 for each robot, 1 for A01, 1 for Zeo
+
+
+async def test_migrate_config_entry_unique_id(
+    hass: HomeAssistant,
+    bypass_api_fixture,
+    config_entry_data: dict[str, Any],
+) -> None:
+    """Test migrating the config entry unique id."""
+    config_entry = MockConfigEntry(
+        domain=DOMAIN,
+        unique_id=USER_EMAIL,
+        data=config_entry_data,
+        version=1,
+        minor_version=1,
+    )
+    config_entry.add_to_hass(hass)
+    await hass.config_entries.async_setup(config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert len(hass.config_entries.async_entries(DOMAIN)) == 1
+    assert config_entry.state is ConfigEntryState.LOADED
+    assert config_entry.unique_id == ROBOROCK_RRUID