From 3d1fbe444e62f2b14aa8f82ea8fd17f655345ebb Mon Sep 17 00:00:00 2001 From: hopkins-tk Date: Mon, 4 Mar 2024 17:20:46 +0100 Subject: [PATCH] Fix authentication issues for asekopool (#99495) * fix: handle authentication issues for asekopool * fix: handle authentication issues for asekopool * feat: add config migration * feat: add re-authentication step * fix: add reauth message * fix: add tests for config flow * fix: tests clean up * Update homeassistant/components/aseko_pool_live/__init__.py Co-authored-by: Erik Montnemery * Update homeassistant/components/aseko_pool_live/__init__.py Co-authored-by: Erik Montnemery * fix: Reformat code * Fix bad merge * Really fix bad merge * Update config_flow.py --------- Co-authored-by: Erik Montnemery --- .../components/aseko_pool_live/__init__.py | 31 +++- .../components/aseko_pool_live/config_flow.py | 116 +++++++++---- .../components/aseko_pool_live/manifest.json | 2 +- .../components/aseko_pool_live/strings.json | 9 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- .../aseko_pool_live/test_config_flow.py | 158 +++++++++++++++--- 7 files changed, 261 insertions(+), 59 deletions(-) diff --git a/homeassistant/components/aseko_pool_live/__init__.py b/homeassistant/components/aseko_pool_live/__init__.py index b09682fcaf9..8f973af7cf7 100644 --- a/homeassistant/components/aseko_pool_live/__init__.py +++ b/homeassistant/components/aseko_pool_live/__init__.py @@ -3,12 +3,12 @@ from __future__ import annotations import logging -from aioaseko import APIUnavailable, MobileAccount +from aioaseko import APIUnavailable, InvalidAuthCredentials, MobileAccount from homeassistant.config_entries import ConfigEntry -from homeassistant.const import CONF_ACCESS_TOKEN, Platform +from homeassistant.const import CONF_EMAIL, CONF_PASSWORD, Platform from homeassistant.core import HomeAssistant -from homeassistant.exceptions import ConfigEntryNotReady +from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady from homeassistant.helpers.aiohttp_client import async_get_clientsession from .const import DOMAIN @@ -22,11 +22,15 @@ PLATFORMS: list[str] = [Platform.BINARY_SENSOR, Platform.SENSOR] async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Aseko Pool Live from a config entry.""" account = MobileAccount( - async_get_clientsession(hass), access_token=entry.data[CONF_ACCESS_TOKEN] + async_get_clientsession(hass), + username=entry.data[CONF_EMAIL], + password=entry.data[CONF_PASSWORD], ) try: units = await account.get_units() + except InvalidAuthCredentials as err: + raise ConfigEntryAuthFailed from err except APIUnavailable as err: raise ConfigEntryNotReady from err @@ -48,3 +52,22 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: hass.data[DOMAIN].pop(entry.entry_id) return unload_ok + + +async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: + """Migrate old entry.""" + _LOGGER.debug("Migrating from version %s", config_entry.version) + + if config_entry.version == 1: + new = { + CONF_EMAIL: config_entry.title, + CONF_PASSWORD: "", + } + + hass.config_entries.async_update_entry(config_entry, data=new, version=2) + + _LOGGER.debug("Migration to version %s successful", config_entry.version) + return True + + _LOGGER.error("Attempt to migrate from unknown version %s", config_entry.version) + return False diff --git a/homeassistant/components/aseko_pool_live/config_flow.py b/homeassistant/components/aseko_pool_live/config_flow.py index ef9ea9ef29e..49143652fa9 100644 --- a/homeassistant/components/aseko_pool_live/config_flow.py +++ b/homeassistant/components/aseko_pool_live/config_flow.py @@ -1,19 +1,15 @@ """Config flow for Aseko Pool Live integration.""" from __future__ import annotations +from collections.abc import Mapping import logging from typing import Any -from aioaseko import APIUnavailable, InvalidAuthCredentials, MobileAccount, WebAccount +from aioaseko import APIUnavailable, InvalidAuthCredentials, WebAccount import voluptuous as vol -from homeassistant.config_entries import ConfigFlow, ConfigFlowResult -from homeassistant.const import ( - CONF_ACCESS_TOKEN, - CONF_EMAIL, - CONF_PASSWORD, - CONF_UNIQUE_ID, -) +from homeassistant.config_entries import ConfigEntry, ConfigFlow, ConfigFlowResult +from homeassistant.const import CONF_EMAIL, CONF_PASSWORD, CONF_UNIQUE_ID from homeassistant.helpers.aiohttp_client import async_get_clientsession from .const import DOMAIN @@ -24,7 +20,16 @@ _LOGGER = logging.getLogger(__name__) class AsekoConfigFlow(ConfigFlow, domain=DOMAIN): """Handle a config flow for Aseko Pool Live.""" - VERSION = 1 + VERSION = 2 + + data_schema = vol.Schema( + { + vol.Required(CONF_EMAIL): str, + vol.Required(CONF_PASSWORD): str, + } + ) + + reauth_entry: ConfigEntry | None = None async def get_account_info(self, email: str, password: str) -> dict: """Get account info from the mobile API and the web API.""" @@ -33,19 +38,83 @@ class AsekoConfigFlow(ConfigFlow, domain=DOMAIN): web_account = WebAccount(session, email, password) web_account_info = await web_account.login() - mobile_account = MobileAccount(session, email, password) - await mobile_account.login() - return { - CONF_ACCESS_TOKEN: mobile_account.access_token, - CONF_EMAIL: web_account_info.email, + CONF_EMAIL: email, + CONF_PASSWORD: password, CONF_UNIQUE_ID: web_account_info.user_id, } async def async_step_user( - self, user_input: dict[str, Any] | None = None + self, user_input: Mapping[str, Any] | None = None ) -> ConfigFlowResult: """Handle the initial step.""" + + self.reauth_entry = None + errors = {} + + if user_input is not None: + try: + info = await self.get_account_info( + user_input[CONF_EMAIL], user_input[CONF_PASSWORD] + ) + except APIUnavailable: + errors["base"] = "cannot_connect" + except InvalidAuthCredentials: + errors["base"] = "invalid_auth" + except Exception: # pylint: disable=broad-except + _LOGGER.exception("Unexpected exception") + errors["base"] = "unknown" + else: + return await self.async_store_credentials(info) + + return self.async_show_form( + step_id="user", + data_schema=self.data_schema, + errors=errors, + ) + + async def async_store_credentials(self, info: dict[str, Any]) -> ConfigFlowResult: + """Store validated credentials.""" + + if self.reauth_entry: + self.hass.config_entries.async_update_entry( + self.reauth_entry, + title=info[CONF_EMAIL], + data={ + CONF_EMAIL: info[CONF_EMAIL], + CONF_PASSWORD: info[CONF_PASSWORD], + }, + ) + await self.hass.config_entries.async_reload(self.reauth_entry.entry_id) + return self.async_abort(reason="reauth_successful") + + await self.async_set_unique_id(info[CONF_UNIQUE_ID]) + self._abort_if_unique_id_configured() + + return self.async_create_entry( + title=info[CONF_EMAIL], + data={ + CONF_EMAIL: info[CONF_EMAIL], + CONF_PASSWORD: info[CONF_PASSWORD], + }, + ) + + async def async_step_reauth( + self, user_input: Mapping[str, Any] + ) -> ConfigFlowResult: + """Perform reauth upon an API authentication error.""" + + self.reauth_entry = self.hass.config_entries.async_get_entry( + self.context["entry_id"] + ) + + return await self.async_step_reauth_confirm(user_input) + + async def async_step_reauth_confirm( + self, user_input: Mapping | None = None + ) -> ConfigFlowResult: + """Dialog that informs the user that reauth is required.""" + errors = {} if user_input is not None: try: @@ -60,21 +129,10 @@ class AsekoConfigFlow(ConfigFlow, domain=DOMAIN): _LOGGER.exception("Unexpected exception") errors["base"] = "unknown" else: - await self.async_set_unique_id(info[CONF_UNIQUE_ID]) - self._abort_if_unique_id_configured() - - return self.async_create_entry( - title=info[CONF_EMAIL], - data={CONF_ACCESS_TOKEN: info[CONF_ACCESS_TOKEN]}, - ) + return await self.async_store_credentials(info) return self.async_show_form( - step_id="user", - data_schema=vol.Schema( - { - vol.Required(CONF_EMAIL): str, - vol.Required(CONF_PASSWORD): str, - } - ), + step_id="reauth_confirm", + data_schema=self.data_schema, errors=errors, ) diff --git a/homeassistant/components/aseko_pool_live/manifest.json b/homeassistant/components/aseko_pool_live/manifest.json index 487032bb09d..f7c29277977 100644 --- a/homeassistant/components/aseko_pool_live/manifest.json +++ b/homeassistant/components/aseko_pool_live/manifest.json @@ -6,5 +6,5 @@ "documentation": "https://www.home-assistant.io/integrations/aseko_pool_live", "iot_class": "cloud_polling", "loggers": ["aioaseko"], - "requirements": ["aioaseko==0.0.2"] + "requirements": ["aioaseko==0.1.1"] } diff --git a/homeassistant/components/aseko_pool_live/strings.json b/homeassistant/components/aseko_pool_live/strings.json index 2a6df30b148..7f77b9ec69b 100644 --- a/homeassistant/components/aseko_pool_live/strings.json +++ b/homeassistant/components/aseko_pool_live/strings.json @@ -6,6 +6,12 @@ "email": "[%key:common::config_flow::data::email%]", "password": "[%key:common::config_flow::data::password%]" } + }, + "reauth_confirm": { + "data": { + "email": "[%key:common::config_flow::data::email%]", + "password": "[%key:common::config_flow::data::password%]" + } } }, "error": { @@ -14,7 +20,8 @@ "unknown": "[%key:common::config_flow::error::unknown%]" }, "abort": { - "already_configured": "[%key:common::config_flow::abort::already_configured_account%]" + "already_configured": "[%key:common::config_flow::abort::already_configured_account%]", + "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]" } }, "entity": { diff --git a/requirements_all.txt b/requirements_all.txt index 47e4dcf5971..6bf84d90c45 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -200,7 +200,7 @@ aioambient==2024.01.0 aioapcaccess==0.4.2 # homeassistant.components.aseko_pool_live -aioaseko==0.0.2 +aioaseko==0.1.1 # homeassistant.components.asuswrt aioasuswrt==1.4.0 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 18f8208ae26..16284a1f096 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -179,7 +179,7 @@ aioambient==2024.01.0 aioapcaccess==0.4.2 # homeassistant.components.aseko_pool_live -aioaseko==0.0.2 +aioaseko==0.1.1 # homeassistant.components.asuswrt aioasuswrt==1.4.0 diff --git a/tests/components/aseko_pool_live/test_config_flow.py b/tests/components/aseko_pool_live/test_config_flow.py index 9d7a5b83d78..fc109f8ac4e 100644 --- a/tests/components/aseko_pool_live/test_config_flow.py +++ b/tests/components/aseko_pool_live/test_config_flow.py @@ -1,37 +1,40 @@ """Test the Aseko Pool Live config flow.""" -from unittest.mock import AsyncMock, patch +from unittest.mock import patch from aioaseko import AccountInfo, APIUnavailable, InvalidAuthCredentials import pytest -from homeassistant import config_entries, setup +from homeassistant import config_entries from homeassistant.components.aseko_pool_live.const import DOMAIN -from homeassistant.const import CONF_ACCESS_TOKEN, CONF_EMAIL, CONF_PASSWORD +from homeassistant.const import CONF_EMAIL, CONF_PASSWORD from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType +from tests.common import MockConfigEntry -async def test_form(hass: HomeAssistant) -> None: + +async def test_async_step_user_form(hass: HomeAssistant) -> None: """Test we get the form.""" - await setup.async_setup_component(hass, "persistent_notification", {}) result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) assert result["type"] == FlowResultType.FORM assert result["errors"] == {} + +async def test_async_step_user_success(hass: HomeAssistant) -> None: + """Test we get the form.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + with patch( "homeassistant.components.aseko_pool_live.config_flow.WebAccount.login", return_value=AccountInfo("aseko@example.com", "a_user_id", "any_language"), ), patch( - "homeassistant.components.aseko_pool_live.config_flow.MobileAccount", - ) as mock_mobile_account, patch( "homeassistant.components.aseko_pool_live.async_setup_entry", return_value=True, ) as mock_setup_entry: - mobile_account = mock_mobile_account.return_value - mobile_account.login = AsyncMock() - mobile_account.access_token = "any_access_token" result2 = await hass.config_entries.flow.async_configure( result["flow_id"], { @@ -43,23 +46,56 @@ async def test_form(hass: HomeAssistant) -> None: assert result2["type"] == FlowResultType.CREATE_ENTRY assert result2["title"] == "aseko@example.com" - assert result2["data"] == {CONF_ACCESS_TOKEN: "any_access_token"} + assert result2["data"] == { + CONF_EMAIL: "aseko@example.com", + CONF_PASSWORD: "passw0rd", + } assert len(mock_setup_entry.mock_calls) == 1 @pytest.mark.parametrize( - ("error_web", "error_mobile", "reason"), + ("error_web", "reason"), [ - (APIUnavailable, None, "cannot_connect"), - (InvalidAuthCredentials, None, "invalid_auth"), - (Exception, None, "unknown"), - (None, APIUnavailable, "cannot_connect"), - (None, InvalidAuthCredentials, "invalid_auth"), - (None, Exception, "unknown"), + (APIUnavailable, "cannot_connect"), + (InvalidAuthCredentials, "invalid_auth"), + (Exception, "unknown"), + ], +) +async def test_async_step_user_exception( + hass: HomeAssistant, error_web: Exception, reason: str +) -> None: + """Test we get the form.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + + with patch( + "homeassistant.components.aseko_pool_live.config_flow.WebAccount.login", + return_value=AccountInfo("aseko@example.com", "a_user_id", "any_language"), + side_effect=error_web, + ): + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_EMAIL: "aseko@example.com", + CONF_PASSWORD: "passw0rd", + }, + ) + + assert result2["type"] == FlowResultType.FORM + assert result2["errors"] == {"base": reason} + + +@pytest.mark.parametrize( + ("error_web", "reason"), + [ + (APIUnavailable, "cannot_connect"), + (InvalidAuthCredentials, "invalid_auth"), + (Exception, "unknown"), ], ) async def test_get_account_info_exceptions( - hass: HomeAssistant, error_web: Exception, error_mobile: Exception, reason: str + hass: HomeAssistant, error_web: Exception, reason: str ) -> None: """Test we handle config flow exceptions.""" result = await hass.config_entries.flow.async_init( @@ -70,9 +106,6 @@ async def test_get_account_info_exceptions( "homeassistant.components.aseko_pool_live.config_flow.WebAccount.login", return_value=AccountInfo("aseko@example.com", "a_user_id", "any_language"), side_effect=error_web, - ), patch( - "homeassistant.components.aseko_pool_live.config_flow.MobileAccount.login", - side_effect=error_mobile, ): result2 = await hass.config_entries.flow.async_configure( result["flow_id"], @@ -84,3 +117,84 @@ async def test_get_account_info_exceptions( assert result2["type"] == FlowResultType.FORM assert result2["errors"] == {"base": reason} + + +async def test_async_step_reauth_success(hass: HomeAssistant) -> None: + """Test successful reauthentication.""" + + mock_entry = MockConfigEntry( + domain=DOMAIN, + unique_id="UID", + data={CONF_EMAIL: "aseko@example.com"}, + ) + mock_entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={ + "source": config_entries.SOURCE_REAUTH, + "entry_id": mock_entry.entry_id, + }, + ) + + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "reauth_confirm" + assert result["errors"] == {} + + with patch( + "homeassistant.components.aseko_pool_live.config_flow.WebAccount.login", + return_value=AccountInfo("aseko@example.com", "a_user_id", "any_language"), + ) as mock_setup_entry: + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_EMAIL: "aseko@example.com", CONF_PASSWORD: "passw0rd"}, + ) + + assert result["type"] == FlowResultType.ABORT + assert result["reason"] == "reauth_successful" + assert len(mock_setup_entry.mock_calls) == 1 + + +@pytest.mark.parametrize( + ("error_web", "reason"), + [ + (APIUnavailable, "cannot_connect"), + (InvalidAuthCredentials, "invalid_auth"), + (Exception, "unknown"), + ], +) +async def test_async_step_reauth_exception( + hass: HomeAssistant, error_web: Exception, reason: str +) -> None: + """Test we get the form.""" + + mock_entry = MockConfigEntry( + domain=DOMAIN, + unique_id="UID", + data={CONF_EMAIL: "aseko@example.com"}, + ) + mock_entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={ + "source": config_entries.SOURCE_REAUTH, + "entry_id": mock_entry.entry_id, + }, + ) + + with patch( + "homeassistant.components.aseko_pool_live.config_flow.WebAccount.login", + return_value=AccountInfo("aseko@example.com", "a_user_id", "any_language"), + side_effect=error_web, + ): + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_EMAIL: "aseko@example.com", + CONF_PASSWORD: "passw0rd", + }, + ) + + assert result2["type"] == FlowResultType.FORM + assert result2["errors"] == {"base": reason}