From dfcb977a1de57117b8ad570d03557d67677dca70 Mon Sep 17 00:00:00 2001 From: Andrew Sayre <6730289+andrewsayre@users.noreply.github.com> Date: Fri, 3 Jan 2025 12:11:10 -0600 Subject: [PATCH] Add HEOS Reauth Flow (#134465) --- homeassistant/components/heos/__init__.py | 14 +- homeassistant/components/heos/config_flow.py | 142 +++++++++++------- .../components/heos/quality_scale.yaml | 5 +- homeassistant/components/heos/strings.json | 19 ++- tests/components/heos/test_config_flow.py | 138 +++++++++++++++++ tests/components/heos/test_init.py | 35 ++++- 6 files changed, 290 insertions(+), 63 deletions(-) diff --git a/homeassistant/components/heos/__init__.py b/homeassistant/components/heos/__init__.py index acec35a4be7e1..862fc9092c275 100644 --- a/homeassistant/components/heos/__init__.py +++ b/homeassistant/components/heos/__init__.py @@ -85,9 +85,21 @@ async def async_setup_entry(hass: HomeAssistant, entry: HeosConfigEntry) -> bool credentials=credentials, ) ) + + # Auth failure handler must be added before connecting to the host, otherwise + # the event will be missed when login fails during connection. + async def auth_failure(event: str) -> None: + """Handle authentication failure.""" + if event == heos_const.EVENT_USER_CREDENTIALS_INVALID: + entry.async_start_reauth(hass) + + entry.async_on_unload( + controller.dispatcher.connect(heos_const.SIGNAL_HEOS_EVENT, auth_failure) + ) + try: + # Auto reconnect only operates if initial connection was successful. await controller.connect() - # Auto reconnect only operates if initial connection was successful. except HeosError as error: await controller.disconnect() _LOGGER.debug("Unable to connect to controller %s: %s", host, error) diff --git a/homeassistant/components/heos/config_flow.py b/homeassistant/components/heos/config_flow.py index 8f985f0823f8f..c47d83d3475e1 100644 --- a/homeassistant/components/heos/config_flow.py +++ b/homeassistant/components/heos/config_flow.py @@ -1,5 +1,6 @@ """Config flow to configure Heos.""" +from collections.abc import Mapping import logging from typing import TYPE_CHECKING, Any, cast from urllib.parse import urlparse @@ -22,6 +23,15 @@ _LOGGER = logging.getLogger(__name__) +AUTH_SCHEMA = vol.Schema( + { + vol.Optional(CONF_USERNAME): selector.TextSelector(), + vol.Optional(CONF_PASSWORD): selector.TextSelector( + selector.TextSelectorConfig(type=selector.TextSelectorType.PASSWORD) + ), + } +) + def format_title(host: str) -> str: """Format the title for config entries.""" @@ -41,6 +51,54 @@ async def _validate_host(host: str, errors: dict[str, str]) -> bool: return True +async def _validate_auth( + user_input: dict[str, str], heos: Heos, errors: dict[str, str] +) -> bool: + """Validate authentication by signing in or out, otherwise populate errors if needed.""" + if not user_input: + # Log out (neither username nor password provided) + try: + await heos.sign_out() + except HeosError: + errors["base"] = "unknown" + _LOGGER.exception("Unexpected error occurred during sign-out") + return False + else: + _LOGGER.debug("Successfully signed-out of HEOS Account") + return True + + # Ensure both username and password are provided + authentication = CONF_USERNAME in user_input or CONF_PASSWORD in user_input + if authentication and CONF_USERNAME not in user_input: + errors[CONF_USERNAME] = "username_missing" + return False + if authentication and CONF_PASSWORD not in user_input: + errors[CONF_PASSWORD] = "password_missing" + return False + + # Attempt to login (both username and password provided) + try: + await heos.sign_in(user_input[CONF_USERNAME], user_input[CONF_PASSWORD]) + except CommandFailedError as err: + if err.error_id in (6, 8, 10): # Auth-specific errors + errors["base"] = "invalid_auth" + _LOGGER.warning("Failed to sign-in to HEOS Account: %s", err) + else: + errors["base"] = "unknown" + _LOGGER.exception("Unexpected error occurred during sign-in") + return False + except HeosError: + errors["base"] = "unknown" + _LOGGER.exception("Unexpected error occurred during sign-in") + return False + else: + _LOGGER.debug( + "Successfully signed-in to HEOS Account: %s", + heos.signed_in_username, + ) + return True + + class HeosFlowHandler(ConfigFlow, domain=DOMAIN): """Define a flow for HEOS.""" @@ -117,15 +175,30 @@ async def async_step_reconfigure( errors=errors, ) + async def async_step_reauth( + self, entry_data: Mapping[str, Any] + ) -> ConfigFlowResult: + """Perform reauthentication after auth failure event.""" + return await self.async_step_reauth_confirm() + + async def async_step_reauth_confirm( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Validate account credentials and update options.""" + errors: dict[str, str] = {} + entry = self._get_reauth_entry() + if user_input is not None: + heos = cast(Heos, entry.runtime_data.controller_manager.controller) + if await _validate_auth(user_input, heos, errors): + return self.async_update_reload_and_abort(entry, options=user_input) -OPTIONS_SCHEMA = vol.Schema( - { - vol.Optional(CONF_USERNAME): selector.TextSelector(), - vol.Optional(CONF_PASSWORD): selector.TextSelector( - selector.TextSelectorConfig(type=selector.TextSelectorType.PASSWORD) - ), - } -) + return self.async_show_form( + step_id="reauth_confirm", + errors=errors, + data_schema=self.add_suggested_values_to_schema( + AUTH_SCHEMA, user_input or entry.options + ), + ) class HeosOptionsFlowHandler(OptionsFlow): @@ -137,59 +210,16 @@ async def async_step_init( """Manage the options.""" errors: dict[str, str] = {} if user_input is not None: - authentication = CONF_USERNAME in user_input or CONF_PASSWORD in user_input - if authentication and CONF_USERNAME not in user_input: - errors[CONF_USERNAME] = "username_missing" - if authentication and CONF_PASSWORD not in user_input: - errors[CONF_PASSWORD] = "password_missing" - - if not errors: - heos = cast( - Heos, self.config_entry.runtime_data.controller_manager.controller - ) - - if user_input: - # Attempt to login - try: - await heos.sign_in( - user_input[CONF_USERNAME], user_input[CONF_PASSWORD] - ) - except CommandFailedError as err: - if err.error_id in (6, 8, 10): # Auth-specific errors - errors["base"] = "invalid_auth" - _LOGGER.warning( - "Failed to sign-in to HEOS Account: %s", err - ) - else: - errors["base"] = "unknown" - _LOGGER.exception( - "Unexpected error occurred during sign-in" - ) - except HeosError: - errors["base"] = "unknown" - _LOGGER.exception("Unexpected error occurred during sign-in") - else: - _LOGGER.debug( - "Successfully signed-in to HEOS Account: %s", - heos.signed_in_username, - ) - else: - # Log out - try: - await heos.sign_out() - except HeosError: - errors["base"] = "unknown" - _LOGGER.exception("Unexpected error occurred during sign-out") - else: - _LOGGER.debug("Successfully signed-out of HEOS Account") - - if not errors: + heos = cast( + Heos, self.config_entry.runtime_data.controller_manager.controller + ) + if await _validate_auth(user_input, heos, errors): return self.async_create_entry(data=user_input) return self.async_show_form( errors=errors, step_id="init", data_schema=self.add_suggested_values_to_schema( - OPTIONS_SCHEMA, user_input or self.config_entry.options + AUTH_SCHEMA, user_input or self.config_entry.options ), ) diff --git a/homeassistant/components/heos/quality_scale.yaml b/homeassistant/components/heos/quality_scale.yaml index 7ffac95218ec2..1fdc244490757 100644 --- a/homeassistant/components/heos/quality_scale.yaml +++ b/homeassistant/components/heos/quality_scale.yaml @@ -44,10 +44,7 @@ rules: parallel-updates: status: todo comment: Needs to be set to 0. The underlying library handles parallel updates. - reauthentication-flow: - status: exempt - comment: | - This integration doesn't require re-authentication. + reauthentication-flow: done test-coverage: status: todo comment: | diff --git a/homeassistant/components/heos/strings.json b/homeassistant/components/heos/strings.json index f048e2c4c36af..bc06390fe73ea 100644 --- a/homeassistant/components/heos/strings.json +++ b/homeassistant/components/heos/strings.json @@ -20,13 +20,30 @@ "data_description": { "host": "[%key:component::heos::config::step::user::data_description::host%]" } + }, + "reauth_confirm": { + "title": "Reauthenticate HEOS", + "description": "Please update your HEOS Account credentials. Alternatively, you can clear the credentials if you do not want the integration to access favorites, playlists, and streaming services.", + "data": { + "username": "[%key:common::config_flow::data::username%]", + "password": "[%key:common::config_flow::data::password%]" + }, + "data_description": { + "username": "[%key:component::heos::options::step::init::data_description::username%]", + "password": "[%key:component::heos::options::step::init::data_description::password%]" + } } }, "error": { - "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]" + "username_missing": "[%key:component::heos::options::error::username_missing%]", + "password_missing": "[%key:component::heos::options::error::password_missing%]", + "invalid_auth": "[%key:common::config_flow::error::invalid_auth%]", + "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", + "unknown": "[%key:common::config_flow::error::unknown%]" }, "abort": { "already_in_progress": "[%key:common::config_flow::abort::already_in_progress%]", + "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]", "reconfigure_successful": "[%key:common::config_flow::abort::reconfigure_successful%]", "single_instance_allowed": "[%key:common::config_flow::abort::single_instance_allowed%]" } diff --git a/tests/components/heos/test_config_flow.py b/tests/components/heos/test_config_flow.py index ea4cb5a580c9b..45c2fbf4eb149 100644 --- a/tests/components/heos/test_config_flow.py +++ b/tests/components/heos/test_config_flow.py @@ -10,6 +10,8 @@ from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType +from tests.common import MockConfigEntry + async def test_flow_aborts_already_setup(hass: HomeAssistant, config_entry) -> None: """Test flow aborts when entry already setup.""" @@ -329,3 +331,139 @@ async def test_options_flow_missing_one_param_recovers( assert controller.sign_out.call_count == 0 assert result["data"] == user_input assert result["type"] is FlowResultType.CREATE_ENTRY + + +@pytest.mark.parametrize( + ("error", "expected_error_key"), + [ + ( + CommandFailedError("sign_in", "Invalid credentials", 6), + "invalid_auth", + ), + ( + CommandFailedError("sign_in", "User not logged in", 8), + "invalid_auth", + ), + (CommandFailedError("sign_in", "user not found", 10), "invalid_auth"), + (CommandFailedError("sign_in", "System error", 12), "unknown"), + (HeosError(), "unknown"), + ], +) +async def test_reauth_signs_in_aborts( + hass: HomeAssistant, + config_entry: MockConfigEntry, + controller, + error: HeosError, + expected_error_key: str, +) -> None: + """Test reauth flow signs-in with entered credentials and aborts.""" + config_entry.add_to_hass(hass) + result = await config_entry.start_reauth_flow(hass) + + assert result["step_id"] == "reauth_confirm" + assert result["errors"] == {} + assert result["type"] is FlowResultType.FORM + + # Invalid credentials, system error, or unexpected error. + user_input = {CONF_USERNAME: "user", CONF_PASSWORD: "pass"} + controller.sign_in.side_effect = error + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input + ) + assert controller.sign_in.call_count == 1 + assert controller.sign_out.call_count == 0 + assert result["step_id"] == "reauth_confirm" + assert result["errors"] == {"base": expected_error_key} + assert result["type"] is FlowResultType.FORM + + # Valid credentials signs-in, updates options, and aborts + controller.sign_in.reset_mock() + controller.sign_in.side_effect = None + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input + ) + assert controller.sign_in.call_count == 1 + assert controller.sign_out.call_count == 0 + assert config_entry.options[CONF_USERNAME] == user_input[CONF_USERNAME] + assert config_entry.options[CONF_PASSWORD] == user_input[CONF_PASSWORD] + assert result["reason"] == "reauth_successful" + assert result["type"] is FlowResultType.ABORT + + +async def test_reauth_signs_out(hass: HomeAssistant, config_entry, controller) -> None: + """Test reauth flow signs-out when credentials cleared and aborts.""" + config_entry.add_to_hass(hass) + result = await config_entry.start_reauth_flow(hass) + + assert result["step_id"] == "reauth_confirm" + assert result["errors"] == {} + assert result["type"] is FlowResultType.FORM + + # Fail to sign-out, show error + user_input = {} + controller.sign_out.side_effect = HeosError() + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input + ) + assert controller.sign_in.call_count == 0 + assert controller.sign_out.call_count == 1 + assert result["step_id"] == "reauth_confirm" + assert result["errors"] == {"base": "unknown"} + assert result["type"] is FlowResultType.FORM + + # Cleared credentials signs-out, updates options, and aborts + controller.sign_out.reset_mock() + controller.sign_out.side_effect = None + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input + ) + assert controller.sign_in.call_count == 0 + assert controller.sign_out.call_count == 1 + assert CONF_USERNAME not in config_entry.options + assert CONF_PASSWORD not in config_entry.options + assert result["reason"] == "reauth_successful" + assert result["type"] is FlowResultType.ABORT + + +@pytest.mark.parametrize( + ("user_input", "expected_errors"), + [ + ({CONF_USERNAME: "user"}, {CONF_PASSWORD: "password_missing"}), + ({CONF_PASSWORD: "pass"}, {CONF_USERNAME: "username_missing"}), + ], +) +async def test_reauth_flow_missing_one_param_recovers( + hass: HomeAssistant, + config_entry, + controller, + user_input: dict[str, str], + expected_errors: dict[str, str], +) -> None: + """Test reauth flow signs-in after recovering from only username or password being entered.""" + config_entry.add_to_hass(hass) + + # Start the options flow. Entry has not current options. + result = await config_entry.start_reauth_flow(hass) + assert result["step_id"] == "reauth_confirm" + assert result["errors"] == {} + assert result["type"] is FlowResultType.FORM + + # Enter only username or password + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input + ) + assert result["step_id"] == "reauth_confirm" + assert result["errors"] == expected_errors + assert result["type"] is FlowResultType.FORM + + # Enter valid credentials + user_input = {CONF_USERNAME: "user", CONF_PASSWORD: "pass"} + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input + ) + assert controller.sign_in.call_count == 1 + assert controller.sign_out.call_count == 0 + assert config_entry.options[CONF_USERNAME] == user_input[CONF_USERNAME] + assert config_entry.options[CONF_PASSWORD] == user_input[CONF_PASSWORD] + assert result["reason"] == "reauth_successful" + assert result["type"] is FlowResultType.ABORT diff --git a/tests/components/heos/test_init.py b/tests/components/heos/test_init.py index 31ddb80edb75e..905346b8b4a56 100644 --- a/tests/components/heos/test_init.py +++ b/tests/components/heos/test_init.py @@ -15,12 +15,14 @@ async_unload_entry, ) from homeassistant.components.heos.const import DOMAIN -from homeassistant.config_entries import ConfigEntryState +from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_USERNAME from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.setup import async_setup_component +from tests.common import MockConfigEntry + async def test_async_setup_returns_true( hass: HomeAssistant, config_entry, config @@ -91,6 +93,37 @@ async def test_async_setup_entry_with_options_loads_platforms( controller.disconnect.assert_not_called() +async def test_async_setup_entry_auth_failure_starts_reauth( + hass: HomeAssistant, + config_entry_options: MockConfigEntry, + controller: Mock, +) -> None: + """Test load with auth failure starts reauth, loads platforms.""" + config_entry_options.add_to_hass(hass) + + # Simulates what happens when the controller can't sign-in during connection + async def connect_send_auth_failure() -> None: + controller.is_signed_in = False + controller.signed_in_username = None + controller.dispatcher.send( + const.SIGNAL_HEOS_EVENT, const.EVENT_USER_CREDENTIALS_INVALID + ) + + controller.connect.side_effect = connect_send_auth_failure + + assert await async_setup_component(hass, DOMAIN, {}) + await hass.async_block_till_done() + + # Assert entry loaded and reauth flow started + assert controller.connect.call_count == 1 + assert controller.get_favorites.call_count == 0 + controller.disconnect.assert_not_called() + assert config_entry_options.state is ConfigEntryState.LOADED + assert any( + config_entry_options.async_get_active_flows(hass, sources=[SOURCE_REAUTH]) + ) + + async def test_async_setup_entry_not_signed_in_loads_platforms( hass: HomeAssistant, config_entry,