From 8c22858ae3ad3a1fab530d40a803d919803a9bc7 Mon Sep 17 00:00:00 2001 From: Quentame Date: Mon, 20 Jan 2020 18:59:29 +0100 Subject: [PATCH] Use config_entry.unique_id in iCloud (#30984) * Use unique_id in iCloud * Remove missed self._configuration_exists() * Avoid breaking change * Almost fix tests * Add missing test * Fix tests --- .../components/icloud/.translations/en.json | 3 +- homeassistant/components/icloud/__init__.py | 4 + .../components/icloud/config_flow.py | 17 +-- homeassistant/components/icloud/strings.json | 3 +- tests/components/icloud/test_config_flow.py | 112 ++++++++++++------ 5 files changed, 85 insertions(+), 54 deletions(-) diff --git a/homeassistant/components/icloud/.translations/en.json b/homeassistant/components/icloud/.translations/en.json index 58101759356..3b7da70bcaf 100644 --- a/homeassistant/components/icloud/.translations/en.json +++ b/homeassistant/components/icloud/.translations/en.json @@ -1,12 +1,11 @@ { "config": { "abort": { - "username_exists": "Account already configured" + "already_configured": "Account already configured" }, "error": { "login": "Login error: please check your email & password", "send_verification_code": "Failed to send verification code", - "username_exists": "Account already configured", "validate_verification_code": "Failed to verify your verification code, choose a trust device and start the verification again" }, "step": { diff --git a/homeassistant/components/icloud/__init__.py b/homeassistant/components/icloud/__init__.py index d1e00d65e10..bc1a7535882 100644 --- a/homeassistant/components/icloud/__init__.py +++ b/homeassistant/components/icloud/__init__.py @@ -141,6 +141,10 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry) -> bool max_interval = entry.data[CONF_MAX_INTERVAL] gps_accuracy_threshold = entry.data[CONF_GPS_ACCURACY_THRESHOLD] + # For backwards compat + if entry.unique_id is None: + hass.config_entries.async_update_entry(entry, unique_id=username) + icloud_dir = hass.helpers.storage.Store(STORAGE_VERSION, STORAGE_KEY) account = IcloudAccount( diff --git a/homeassistant/components/icloud/config_flow.py b/homeassistant/components/icloud/config_flow.py index 553ec1a28b4..9b00ccb2a8d 100644 --- a/homeassistant/components/icloud/config_flow.py +++ b/homeassistant/components/icloud/config_flow.py @@ -43,13 +43,6 @@ class IcloudFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): self._trusted_device = None self._verification_code = None - def _configuration_exists(self, username: str) -> bool: - """Return True if username exists in configuration.""" - for entry in self._async_current_entries(): - if entry.data[CONF_USERNAME] == username: - return True - return False - async def _show_setup_form(self, user_input=None, errors=None): """Show the setup form to the user.""" @@ -90,9 +83,10 @@ class IcloudFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): CONF_GPS_ACCURACY_THRESHOLD, DEFAULT_GPS_ACCURACY_THRESHOLD ) - if self._configuration_exists(self._username): - errors[CONF_USERNAME] = "username_exists" - return await self._show_setup_form(user_input, errors) + # Check if already configured + if self.unique_id is None: + await self.async_set_unique_id(self._username) + self._abort_if_unique_id_configured() try: self.api = await self.hass.async_add_executor_job( @@ -119,9 +113,6 @@ class IcloudFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): async def async_step_import(self, user_input): """Import a config entry.""" - if self._configuration_exists(user_input[CONF_USERNAME]): - return self.async_abort(reason="username_exists") - return await self.async_step_user(user_input) async def async_step_trusted_device(self, user_input=None, errors=None): diff --git a/homeassistant/components/icloud/strings.json b/homeassistant/components/icloud/strings.json index 117e26c8830..e0a7b7a32ce 100644 --- a/homeassistant/components/icloud/strings.json +++ b/homeassistant/components/icloud/strings.json @@ -26,13 +26,12 @@ } }, "error": { - "username_exists": "Account already configured", "login": "Login error: please check your email & password", "send_verification_code": "Failed to send verification code", "validate_verification_code": "Failed to verify your verification code, choose a trust device and start the verification again" }, "abort": { - "username_exists": "Account already configured" + "already_configured": "Account already configured" } } } \ No newline at end of file diff --git a/tests/components/icloud/test_config_flow.py b/tests/components/icloud/test_config_flow.py index 035266287f0..747af7c940a 100644 --- a/tests/components/icloud/test_config_flow.py +++ b/tests/components/icloud/test_config_flow.py @@ -5,7 +5,6 @@ from pyicloud.exceptions import PyiCloudFailedLoginException import pytest from homeassistant import data_entry_flow -from homeassistant.components.icloud import config_flow from homeassistant.components.icloud.config_flow import ( CONF_TRUSTED_DEVICE, CONF_VERIFICATION_CODE, @@ -41,6 +40,9 @@ def mock_controller_service(): "homeassistant.components.icloud.config_flow.PyiCloudService" ) as service_mock: service_mock.return_value.requires_2fa = True + service_mock.return_value.trusted_devices = TRUSTED_DEVICES + service_mock.return_value.send_verification_code = Mock(return_value=True) + service_mock.return_value.validate_verification_code = Mock(return_value=True) yield service_mock @@ -63,7 +65,7 @@ def mock_controller_service_send_verification_code_failed(): with patch( "homeassistant.components.icloud.config_flow.PyiCloudService" ) as service_mock: - service_mock.return_value.requires_2fa = False + service_mock.return_value.requires_2fa = True service_mock.return_value.trusted_devices = TRUSTED_DEVICES service_mock.return_value.send_verification_code = Mock(return_value=False) yield service_mock @@ -75,20 +77,13 @@ def mock_controller_service_validate_verification_code_failed(): with patch( "homeassistant.components.icloud.config_flow.PyiCloudService" ) as service_mock: - service_mock.return_value.requires_2fa = False + service_mock.return_value.requires_2fa = True service_mock.return_value.trusted_devices = TRUSTED_DEVICES service_mock.return_value.send_verification_code = Mock(return_value=True) service_mock.return_value.validate_verification_code = Mock(return_value=False) yield service_mock -def init_config_flow(hass: HomeAssistantType): - """Init a configuration flow.""" - flow = config_flow.IcloudFlowHandler() - flow.hass = hass - return flow - - async def test_user(hass: HomeAssistantType, service: MagicMock): """Test user config.""" result = await hass.config_entries.flow.async_init( @@ -118,6 +113,7 @@ async def test_user_with_cookie( data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, ) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["result"].unique_id == USERNAME assert result["title"] == USERNAME assert result["data"][CONF_USERNAME] == USERNAME assert result["data"][CONF_PASSWORD] == PASSWORD @@ -162,6 +158,7 @@ async def test_import_with_cookie( data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, ) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["result"].unique_id == USERNAME assert result["title"] == USERNAME assert result["data"][CONF_USERNAME] == USERNAME assert result["data"][CONF_PASSWORD] == PASSWORD @@ -180,6 +177,7 @@ async def test_import_with_cookie( }, ) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["result"].unique_id == USERNAME_2 assert result["title"] == USERNAME_2 assert result["data"][CONF_USERNAME] == USERNAME_2 assert result["data"][CONF_PASSWORD] == PASSWORD @@ -192,7 +190,9 @@ async def test_two_accounts_setup( ): """Test to setup two accounts.""" MockConfigEntry( - domain=DOMAIN, data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD} + domain=DOMAIN, + data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, + unique_id=USERNAME, ).add_to_hass(hass) # import with username and password @@ -202,6 +202,7 @@ async def test_two_accounts_setup( data={CONF_USERNAME: USERNAME_2, CONF_PASSWORD: PASSWORD}, ) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["result"].unique_id == USERNAME_2 assert result["title"] == USERNAME_2 assert result["data"][CONF_USERNAME] == USERNAME_2 assert result["data"][CONF_PASSWORD] == PASSWORD @@ -212,7 +213,9 @@ async def test_two_accounts_setup( async def test_abort_if_already_setup(hass: HomeAssistantType): """Test we abort if the account is already setup.""" MockConfigEntry( - domain=DOMAIN, data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD} + domain=DOMAIN, + data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, + unique_id=USERNAME, ).add_to_hass(hass) # Should fail, same USERNAME (import) @@ -222,7 +225,7 @@ async def test_abort_if_already_setup(hass: HomeAssistantType): data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, ) assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT - assert result["reason"] == "username_exists" + assert result["reason"] == "already_configured" # Should fail, same USERNAME (flow) result = await hass.config_entries.flow.async_init( @@ -230,8 +233,8 @@ async def test_abort_if_already_setup(hass: HomeAssistantType): context={"source": SOURCE_USER}, data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, ) - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["errors"] == {CONF_USERNAME: "username_exists"} + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "already_configured" async def test_login_failed(hass: HomeAssistantType): @@ -251,20 +254,28 @@ async def test_login_failed(hass: HomeAssistantType): async def test_trusted_device(hass: HomeAssistantType, service: MagicMock): """Test trusted_device step.""" - flow = init_config_flow(hass) - await flow.async_step_user({CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_USER}, + data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, + ) - result = await flow.async_step_trusted_device() + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == CONF_TRUSTED_DEVICE async def test_trusted_device_success(hass: HomeAssistantType, service: MagicMock): """Test trusted_device step success.""" - flow = init_config_flow(hass) - await flow.async_step_user({CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_USER}, + data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, + ) - result = await flow.async_step_trusted_device({CONF_TRUSTED_DEVICE: 0}) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_TRUSTED_DEVICE: 0} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == CONF_VERIFICATION_CODE @@ -273,34 +284,53 @@ async def test_send_verification_code_failed( hass: HomeAssistantType, service_send_verification_code_failed: MagicMock ): """Test when we have errors during send_verification_code.""" - flow = init_config_flow(hass) - await flow.async_step_user({CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_USER}, + data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, + ) - result = await flow.async_step_trusted_device({CONF_TRUSTED_DEVICE: 0}) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_TRUSTED_DEVICE: 0} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == CONF_TRUSTED_DEVICE assert result["errors"] == {CONF_TRUSTED_DEVICE: "send_verification_code"} -async def test_verification_code(hass: HomeAssistantType): +async def test_verification_code(hass: HomeAssistantType, service: MagicMock): """Test verification_code step.""" - flow = init_config_flow(hass) - await flow.async_step_user({CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_USER}, + data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, + ) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_TRUSTED_DEVICE: 0} + ) - result = await flow.async_step_verification_code() + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == CONF_VERIFICATION_CODE -async def test_verification_code_success( - hass: HomeAssistantType, service_with_cookie: MagicMock -): +async def test_verification_code_success(hass: HomeAssistantType, service: MagicMock): """Test verification_code step success.""" - flow = init_config_flow(hass) - await flow.async_step_user({CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_USER}, + data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, + ) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_TRUSTED_DEVICE: 0} + ) + service.return_value.requires_2fa = False - result = await flow.async_step_verification_code({CONF_VERIFICATION_CODE: 0}) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_VERIFICATION_CODE: "0"} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["result"].unique_id == USERNAME assert result["title"] == USERNAME assert result["data"][CONF_USERNAME] == USERNAME assert result["data"][CONF_PASSWORD] == PASSWORD @@ -312,10 +342,18 @@ async def test_validate_verification_code_failed( hass: HomeAssistantType, service_validate_verification_code_failed: MagicMock ): """Test when we have errors during validate_verification_code.""" - flow = init_config_flow(hass) - await flow.async_step_user({CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_USER}, + data={CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, + ) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_TRUSTED_DEVICE: 0} + ) - result = await flow.async_step_verification_code({CONF_VERIFICATION_CODE: 0}) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_VERIFICATION_CODE: "0"} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == CONF_TRUSTED_DEVICE assert result["errors"] == {"base": "validate_verification_code"}