From f2a4566eefb153429fc3c45a78f35a5b4816f1df Mon Sep 17 00:00:00 2001 From: "Mr. Bubbles" Date: Sat, 22 Jun 2024 15:14:53 +0200 Subject: [PATCH] Add re-auth flow to Bring integration (#115327) --- homeassistant/components/bring/__init__.py | 7 +- homeassistant/components/bring/config_flow.py | 87 ++++++++++++---- homeassistant/components/bring/coordinator.py | 16 ++- homeassistant/components/bring/strings.json | 11 ++- tests/components/bring/conftest.py | 4 +- tests/components/bring/test_config_flow.py | 98 ++++++++++++++++++- tests/components/bring/test_init.py | 4 +- 7 files changed, 194 insertions(+), 33 deletions(-) diff --git a/homeassistant/components/bring/__init__.py b/homeassistant/components/bring/__init__.py index 72d3894af3a..30cbbbbbfa0 100644 --- a/homeassistant/components/bring/__init__.py +++ b/homeassistant/components/bring/__init__.py @@ -14,7 +14,7 @@ from bring_api.exceptions import ( from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_EMAIL, CONF_PASSWORD, Platform from homeassistant.core import HomeAssistant -from homeassistant.exceptions import ConfigEntryError, ConfigEntryNotReady +from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady from homeassistant.helpers.aiohttp_client import async_get_clientsession from .const import DOMAIN @@ -38,7 +38,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: BringConfigEntry) -> boo try: await bring.login() - await bring.load_lists() except BringRequestException as e: raise ConfigEntryNotReady( translation_domain=DOMAIN, @@ -47,10 +46,10 @@ async def async_setup_entry(hass: HomeAssistant, entry: BringConfigEntry) -> boo except BringParseException as e: raise ConfigEntryNotReady( translation_domain=DOMAIN, - translation_key="setup_request_exception", + translation_key="setup_parse_exception", ) from e except BringAuthException as e: - raise ConfigEntryError( + raise ConfigEntryAuthFailed( translation_domain=DOMAIN, translation_key="setup_authentication_exception", translation_placeholders={CONF_EMAIL: email}, diff --git a/homeassistant/components/bring/config_flow.py b/homeassistant/components/bring/config_flow.py index 756b2312e88..333837a20f2 100644 --- a/homeassistant/components/bring/config_flow.py +++ b/homeassistant/components/bring/config_flow.py @@ -2,15 +2,17 @@ from __future__ import annotations +from collections.abc import Mapping import logging from typing import Any from bring_api.bring import Bring from bring_api.exceptions import BringAuthException, BringRequestException +from bring_api.types import BringAuthResponse import voluptuous as vol from homeassistant.config_entries import ConfigFlow, ConfigFlowResult -from homeassistant.const import CONF_EMAIL, CONF_PASSWORD +from homeassistant.const import CONF_EMAIL, CONF_NAME, CONF_PASSWORD from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.selector import ( TextSelector, @@ -18,6 +20,7 @@ from homeassistant.helpers.selector import ( TextSelectorType, ) +from . import BringConfigEntry from .const import DOMAIN _LOGGER = logging.getLogger(__name__) @@ -42,33 +45,75 @@ class BringConfigFlow(ConfigFlow, domain=DOMAIN): """Handle a config flow for Bring!.""" VERSION = 1 + reauth_entry: BringConfigEntry | None = None + info: BringAuthResponse async def async_step_user( self, user_input: dict[str, Any] | None = None ) -> ConfigFlowResult: """Handle the initial step.""" errors: dict[str, str] = {} - if user_input is not None: - session = async_get_clientsession(self.hass) - bring = Bring(session, user_input[CONF_EMAIL], user_input[CONF_PASSWORD]) - - try: - info = await bring.login() - await bring.load_lists() - except BringRequestException: - errors["base"] = "cannot_connect" - except BringAuthException: - errors["base"] = "invalid_auth" - except Exception: - _LOGGER.exception("Unexpected exception") - errors["base"] = "unknown" - else: - await self.async_set_unique_id(bring.uuid) - self._abort_if_unique_id_configured() - return self.async_create_entry( - title=info["name"] or user_input[CONF_EMAIL], data=user_input - ) + if user_input is not None and not ( + errors := await self.validate_input(user_input) + ): + self._abort_if_unique_id_configured() + return self.async_create_entry( + title=self.info["name"] or user_input[CONF_EMAIL], data=user_input + ) return self.async_show_form( step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors ) + + async def async_step_reauth( + self, entry_data: 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() + + async def async_step_reauth_confirm( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Dialog that informs the user that reauth is required.""" + errors: dict[str, str] = {} + + assert self.reauth_entry + + if user_input is not None: + if not (errors := await self.validate_input(user_input)): + return self.async_update_reload_and_abort( + self.reauth_entry, data=user_input + ) + + return self.async_show_form( + step_id="reauth_confirm", + data_schema=self.add_suggested_values_to_schema( + data_schema=STEP_USER_DATA_SCHEMA, + suggested_values={CONF_EMAIL: self.reauth_entry.data[CONF_EMAIL]}, + ), + description_placeholders={CONF_NAME: self.reauth_entry.title}, + errors=errors, + ) + + async def validate_input(self, user_input: Mapping[str, Any]) -> dict[str, str]: + """Auth Helper.""" + + errors: dict[str, str] = {} + session = async_get_clientsession(self.hass) + bring = Bring(session, user_input[CONF_EMAIL], user_input[CONF_PASSWORD]) + + try: + self.info = await bring.login() + except BringRequestException: + errors["base"] = "cannot_connect" + except BringAuthException: + errors["base"] = "invalid_auth" + except Exception: + _LOGGER.exception("Unexpected exception") + errors["base"] = "unknown" + else: + await self.async_set_unique_id(bring.uuid) + return errors diff --git a/homeassistant/components/bring/coordinator.py b/homeassistant/components/bring/coordinator.py index 1447338d408..222c650e614 100644 --- a/homeassistant/components/bring/coordinator.py +++ b/homeassistant/components/bring/coordinator.py @@ -14,7 +14,9 @@ from bring_api.exceptions import ( from bring_api.types import BringItemsResponse, BringList from homeassistant.config_entries import ConfigEntry +from homeassistant.const import CONF_EMAIL from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ConfigEntryAuthFailed from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed from .const import DOMAIN @@ -49,8 +51,20 @@ class BringDataUpdateCoordinator(DataUpdateCoordinator[dict[str, BringData]]): except BringParseException as e: raise UpdateFailed("Unable to parse response from bring") from e except BringAuthException as e: + # try to recover by refreshing access token, otherwise + # initiate reauth flow + try: + await self.bring.retrieve_new_access_token() + except (BringRequestException, BringParseException) as exc: + raise UpdateFailed("Refreshing authentication token failed") from exc + except BringAuthException as exc: + raise ConfigEntryAuthFailed( + translation_domain=DOMAIN, + translation_key="setup_authentication_exception", + translation_placeholders={CONF_EMAIL: self.bring.mail}, + ) from exc raise UpdateFailed( - "Unable to retrieve data from bring, authentication failed" + "Authentication failed but re-authentication was successful, trying again later" ) from e list_dict: dict[str, BringData] = {} diff --git a/homeassistant/components/bring/strings.json b/homeassistant/components/bring/strings.json index 5deb0759c17..652958a1b1f 100644 --- a/homeassistant/components/bring/strings.json +++ b/homeassistant/components/bring/strings.json @@ -6,6 +6,14 @@ "email": "[%key:common::config_flow::data::email%]", "password": "[%key:common::config_flow::data::password%]" } + }, + "reauth_confirm": { + "title": "[%key:common::config_flow::title::reauth%]", + "description": "The Bring! integration needs to re-authenticate your account", + "data": { + "email": "[%key:common::config_flow::data::email%]", + "password": "[%key:common::config_flow::data::password%]" + } } }, "error": { @@ -14,7 +22,8 @@ "unknown": "[%key:common::config_flow::error::unknown%]" }, "abort": { - "already_configured": "[%key:common::config_flow::abort::already_configured_service%]" + "already_configured": "[%key:common::config_flow::abort::already_configured_service%]", + "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]" } }, "exceptions": { diff --git a/tests/components/bring/conftest.py b/tests/components/bring/conftest.py index 0760bdd296a..25330c10ba4 100644 --- a/tests/components/bring/conftest.py +++ b/tests/components/bring/conftest.py @@ -1,7 +1,9 @@ """Common fixtures for the Bring! tests.""" +from typing import cast from unittest.mock import AsyncMock, patch +from bring_api.types import BringAuthResponse import pytest from typing_extensions import Generator @@ -40,7 +42,7 @@ def mock_bring_client() -> Generator[AsyncMock]: ): client = mock_client.return_value client.uuid = UUID - client.login.return_value = {"name": "Bring"} + client.login.return_value = cast(BringAuthResponse, {"name": "Bring"}) client.load_lists.return_value = {"lists": []} yield client diff --git a/tests/components/bring/test_config_flow.py b/tests/components/bring/test_config_flow.py index 86fdbc1853b..d307e0ccbbe 100644 --- a/tests/components/bring/test_config_flow.py +++ b/tests/components/bring/test_config_flow.py @@ -9,8 +9,8 @@ from bring_api.exceptions import ( ) import pytest -from homeassistant import config_entries from homeassistant.components.bring.const import DOMAIN +from homeassistant.config_entries import SOURCE_REAUTH, SOURCE_USER from homeassistant.const import CONF_EMAIL, CONF_PASSWORD from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType @@ -30,7 +30,7 @@ async def test_form( ) -> None: """Test we get the form.""" result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} + DOMAIN, context={"source": SOURCE_USER} ) assert result["type"] is FlowResultType.FORM assert result["errors"] == {} @@ -66,7 +66,7 @@ async def test_flow_user_init_data_unknown_error_and_recover( mock_bring_client.login.side_effect = raise_error result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": "user"} + DOMAIN, context={"source": SOURCE_USER} ) result = await hass.config_entries.flow.async_configure( result["flow_id"], @@ -112,3 +112,95 @@ async def test_flow_user_init_data_already_configured( assert result["type"] is FlowResultType.ABORT assert result["reason"] == "already_configured" + + +async def test_flow_reauth( + hass: HomeAssistant, + mock_bring_client: AsyncMock, + bring_config_entry: MockConfigEntry, +) -> None: + """Test reauth flow.""" + + bring_config_entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={ + "source": SOURCE_REAUTH, + "entry_id": bring_config_entry.entry_id, + "unique_id": bring_config_entry.unique_id, + }, + ) + + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "reauth_confirm" + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_EMAIL: "new-email", CONF_PASSWORD: "new-password"}, + ) + + await hass.async_block_till_done() + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "reauth_successful" + assert bring_config_entry.data == { + CONF_EMAIL: "new-email", + CONF_PASSWORD: "new-password", + } + assert len(hass.config_entries.async_entries()) == 1 + + +@pytest.mark.parametrize( + ("raise_error", "text_error"), + [ + (BringRequestException(), "cannot_connect"), + (BringAuthException(), "invalid_auth"), + (BringParseException(), "unknown"), + (IndexError(), "unknown"), + ], +) +async def test_flow_reauth_error_and_recover( + hass: HomeAssistant, + mock_bring_client: AsyncMock, + bring_config_entry: MockConfigEntry, + raise_error, + text_error, +) -> None: + """Test reauth flow.""" + + bring_config_entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={ + "source": SOURCE_REAUTH, + "entry_id": bring_config_entry.entry_id, + "unique_id": bring_config_entry.unique_id, + }, + ) + + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "reauth_confirm" + + mock_bring_client.login.side_effect = raise_error + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_EMAIL: "new-email", CONF_PASSWORD: "new-password"}, + ) + + assert result["type"] is FlowResultType.FORM + assert result["errors"] == {"base": text_error} + + mock_bring_client.login.side_effect = None + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_EMAIL: "new-email", CONF_PASSWORD: "new-password"}, + ) + + await hass.async_block_till_done() + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "reauth_successful" + + assert len(hass.config_entries.async_entries()) == 1 diff --git a/tests/components/bring/test_init.py b/tests/components/bring/test_init.py index db402bdd6d1..f1b1f78e775 100644 --- a/tests/components/bring/test_init.py +++ b/tests/components/bring/test_init.py @@ -13,7 +13,7 @@ from homeassistant.components.bring import ( from homeassistant.components.bring.const import DOMAIN from homeassistant.config_entries import ConfigEntryState from homeassistant.core import HomeAssistant -from homeassistant.exceptions import ConfigEntryError, ConfigEntryNotReady +from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady from tests.common import MockConfigEntry @@ -70,7 +70,7 @@ async def test_init_failure( ("exception", "expected"), [ (BringRequestException, ConfigEntryNotReady), - (BringAuthException, ConfigEntryError), + (BringAuthException, ConfigEntryAuthFailed), (BringParseException, ConfigEntryNotReady), ], )