From 97be9d17b382959e253f06a55bf230bddad3aa0d Mon Sep 17 00:00:00 2001 From: Bradley Florence <11214309+bradleyseanf@users.noreply.github.com> Date: Sun, 20 Sep 2026 11:10:24 -0400 Subject: [PATCH] Add reauthentication to Monarch Money (#182773) --- .../components/monarch_money/config_flow.py | 51 ++++- .../components/monarch_money/coordinator.py | 40 ++-- .../monarch_money/quality_scale.yaml | 15 +- .../components/monarch_money/strings.json | 13 +- .../monarch_money/test_config_flow.py | 191 +++++++++++++++++- .../monarch_money/test_coordinator.py | 115 +++++++++++ 6 files changed, 385 insertions(+), 40 deletions(-) diff --git a/homeassistant/components/monarch_money/config_flow.py b/homeassistant/components/monarch_money/config_flow.py index 8236571540a8..e2d8196ad001 100644 --- a/homeassistant/components/monarch_money/config_flow.py +++ b/homeassistant/components/monarch_money/config_flow.py @@ -1,15 +1,18 @@ """Config flow for Monarch Money integration.""" +from collections.abc import Mapping import logging from typing import Any, override +from aiohttp import ClientError, ClientResponseError +from gql.transport.exceptions import TransportError, TransportServerError from monarchmoney import LoginFailedException, RequireMFAException from monarchmoney.monarchmoney import SESSION_FILE import probatio from typedmonarchmoney import TypedMonarchMoney from typedmonarchmoney.models import MonarchSubscription -from homeassistant.config_entries import ConfigFlow, ConfigFlowResult +from homeassistant.config_entries import SOURCE_REAUTH, ConfigFlow, ConfigFlowResult from homeassistant.const import CONF_EMAIL, CONF_ID, CONF_PASSWORD, CONF_TOKEN from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError @@ -112,6 +115,24 @@ class MonarchMoneyConfigFlow(ConfigFlow, domain=DOMAIN): self, user_input: dict[str, Any] | None = None ) -> ConfigFlowResult: """Handle the initial step.""" + return await self._async_step_login("user", user_input) + + async def async_step_reauth( + self, entry_data: Mapping[str, Any] + ) -> ConfigFlowResult: + """Start reauthentication after credentials expire.""" + return await self.async_step_reauth_confirm() + + async def async_step_reauth_confirm( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Ask for credentials to renew the existing subscription token.""" + return await self._async_step_login("reauth_confirm", user_input) + + async def _async_step_login( + self, step_id: str, user_input: dict[str, Any] | None + ) -> ConfigFlowResult: + """Authenticate a new or existing entry, including MFA.""" errors: dict[str, str] = {} if user_input is not None: @@ -124,20 +145,38 @@ class MonarchMoneyConfigFlow(ConfigFlow, domain=DOMAIN): self.password = user_input[CONF_PASSWORD] return self.async_show_form( - step_id="user", + step_id=step_id, data_schema=STEP_MFA_DATA_SCHEMA, errors={"base": "mfa_required"}, ) except BadMFA: return self.async_show_form( - step_id="user", + step_id=step_id, data_schema=STEP_MFA_DATA_SCHEMA, errors={"base": "bad_mfa"}, ) - except InvalidAuth: + except InvalidAuth, LoginFailedException: + self.email = self.password = None errors["base"] = "invalid_auth" + except (ClientResponseError, TransportServerError) as err: + status = ( + err.status if isinstance(err, ClientResponseError) else err.code + ) + if status in (401, 403): + self.email = self.password = None + errors["base"] = "invalid_auth" + else: + errors["base"] = "cannot_connect" + except ClientError, TransportError, TimeoutError: + errors["base"] = "cannot_connect" else: await self.async_set_unique_id(info[CONF_ID]) + if self.source == SOURCE_REAUTH: + self._abort_if_unique_id_mismatch() + return self.async_update_reload_and_abort( + self._get_reauth_entry(), + data_updates={CONF_TOKEN: info[CONF_TOKEN]}, + ) self._abort_if_unique_id_configured() return self.async_create_entry( @@ -145,7 +184,9 @@ class MonarchMoneyConfigFlow(ConfigFlow, domain=DOMAIN): data={CONF_TOKEN: info[CONF_TOKEN]}, ) return self.async_show_form( - step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors + step_id=step_id, + data_schema=STEP_MFA_DATA_SCHEMA if self.email else STEP_USER_DATA_SCHEMA, + errors=errors, ) diff --git a/homeassistant/components/monarch_money/coordinator.py b/homeassistant/components/monarch_money/coordinator.py index 54f0b7cb6df6..6011fc8fcd21 100644 --- a/homeassistant/components/monarch_money/coordinator.py +++ b/homeassistant/components/monarch_money/coordinator.py @@ -3,10 +3,10 @@ import asyncio from dataclasses import dataclass from datetime import timedelta -from typing import override +from typing import Never, override -from aiohttp import ClientResponseError -from gql.transport.exceptions import TransportServerError +from aiohttp import ClientError, ClientResponseError +from gql.transport.exceptions import TransportError, TransportServerError from monarchmoney import LoginFailedException from typedmonarchmoney import TypedMonarchMoney from typedmonarchmoney.models import ( @@ -17,8 +17,8 @@ from typedmonarchmoney.models import ( from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant -from homeassistant.exceptions import ConfigEntryError -from homeassistant.helpers.update_coordinator import DataUpdateCoordinator +from homeassistant.exceptions import ConfigEntryAuthFailed +from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed from homeassistant.util import dt as dt_util from .const import LOGGER @@ -35,6 +35,17 @@ class MonarchData: type MonarchMoneyConfigEntry = ConfigEntry[MonarchMoneyDataUpdateCoordinator] +def _raise_update_error(err: Exception) -> Never: + """Translate Monarch Money API errors to coordinator errors.""" + if isinstance(err, LoginFailedException) or ( + isinstance(err, (TransportServerError, ClientResponseError)) + and (err.status if isinstance(err, ClientResponseError) else err.code) + in (401, 403) + ): + raise ConfigEntryAuthFailed("Authentication failed") from err + raise UpdateFailed("Error communicating with Monarch Money") from err + + class MonarchMoneyDataUpdateCoordinator(DataUpdateCoordinator[MonarchData]): """Data update coordinator for Monarch Money.""" @@ -64,8 +75,8 @@ class MonarchMoneyDataUpdateCoordinator(DataUpdateCoordinator[MonarchData]): sub_details: MonarchSubscription = ( await self.client.get_subscription_details() ) - except (TransportServerError, LoginFailedException, ClientResponseError) as err: - raise ConfigEntryError("Authentication failed") from err + except (LoginFailedException, TransportError, ClientError, TimeoutError) as err: + _raise_update_error(err) self.subscription_id = sub_details.id @override @@ -74,12 +85,15 @@ class MonarchMoneyDataUpdateCoordinator(DataUpdateCoordinator[MonarchData]): now = dt_util.now() - account_data, cashflow_summary = await asyncio.gather( - self.client.get_accounts_as_dict_with_id_key(), - self.client.get_cashflow_summary( - start_date=f"{now.year}-01-01", end_date=f"{now.year}-12-31" - ), - ) + try: + account_data, cashflow_summary = await asyncio.gather( + self.client.get_accounts_as_dict_with_id_key(), + self.client.get_cashflow_summary( + start_date=f"{now.year}-01-01", end_date=f"{now.year}-12-31" + ), + ) + except (LoginFailedException, TransportError, ClientError, TimeoutError) as err: + _raise_update_error(err) return MonarchData(account_data=account_data, cashflow_summary=cashflow_summary) diff --git a/homeassistant/components/monarch_money/quality_scale.yaml b/homeassistant/components/monarch_money/quality_scale.yaml index 3ff4fb77d51a..cba15f4aab8d 100644 --- a/homeassistant/components/monarch_money/quality_scale.yaml +++ b/homeassistant/components/monarch_money/quality_scale.yaml @@ -65,16 +65,8 @@ rules: status: todo comment: | The sensor platform does not define a PARALLEL_UPDATES constant. - reauthentication-flow: - status: todo - comment: | - The config flow does not implement async_step_reauth to handle expired or - revoked tokens without requiring the user to delete and re-add the entry. - test-coverage: - status: todo - comment: | - Only a single snapshot test exists for the sensor platform. Overall test - coverage has not been verified to exceed 95%. + reauthentication-flow: done + test-coverage: done # Gold devices: done @@ -155,8 +147,7 @@ rules: repair-issues: status: todo comment: | - No repair issues are raised. With no reauthentication flow, token expiry - has no user-facing repair path. + No repair issues are raised. stale-devices: status: todo comment: | diff --git a/homeassistant/components/monarch_money/strings.json b/homeassistant/components/monarch_money/strings.json index 983feef8ad2e..a3443c4bbba9 100644 --- a/homeassistant/components/monarch_money/strings.json +++ b/homeassistant/components/monarch_money/strings.json @@ -1,14 +1,25 @@ { "config": { "abort": { - "already_configured": "[%key:common::config_flow::abort::already_configured_device%]" + "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", + "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]", + "unique_id_mismatch": "The authenticated Monarch Money account does not match the configured account." }, "error": { "bad_mfa": "Your code was invalid, please try again or use a recovery token.", + "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", "invalid_auth": "[%key:common::config_flow::error::invalid_auth%]", "mfa_required": "Multi-factor authentication required." }, "step": { + "reauth_confirm": { + "data": { + "email": "[%key:common::config_flow::data::email%]", + "mfa_code": "[%key:component::monarch_money::config::step::user::data::mfa_code%]", + "password": "[%key:common::config_flow::data::password%]" + }, + "description": "Your Monarch Money credentials have expired. Enter your email and password to reconnect your existing account. If required, you will also be prompted for your MFA code." + }, "user": { "data": { "email": "[%key:common::config_flow::data::email%]", diff --git a/tests/components/monarch_money/test_config_flow.py b/tests/components/monarch_money/test_config_flow.py index b33b5f0ff676..e7050ba04946 100644 --- a/tests/components/monarch_money/test_config_flow.py +++ b/tests/components/monarch_money/test_config_flow.py @@ -1,7 +1,9 @@ """Test the Monarch Money config flow.""" -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, Mock, call, patch +from aiohttp import ClientResponseError +from gql.transport.exceptions import TransportError, TransportServerError from monarchmoney import LoginFailedException, RequireMFAException import pytest @@ -11,6 +13,8 @@ from homeassistant.const import CONF_EMAIL, CONF_PASSWORD, CONF_TOKEN from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType +from tests.common import MockConfigEntry + async def test_form_simple( hass: HomeAssistant, mock_setup_entry: AsyncMock, mock_config_api: AsyncMock @@ -64,20 +68,51 @@ async def test_add_duplicate_entry( assert result["reason"] == "already_configured" -async def test_form_invalid_auth( - hass: HomeAssistant, mock_setup_entry: AsyncMock, mock_config_api: AsyncMock +@pytest.mark.parametrize( + ("api_error", "expected_error"), + [ + pytest.param( + LoginFailedException("invalid credentials"), + "invalid_auth", + id="login_failed", + ), + pytest.param( + ClientResponseError(None, (), status=401), + "invalid_auth", + id="client_unauthorized", + ), + pytest.param( + TransportServerError("forbidden", code=403), + "invalid_auth", + id="transport_forbidden", + ), + pytest.param( + ClientResponseError(None, (), status=500), + "cannot_connect", + id="client_server_error", + ), + pytest.param( + TransportServerError("server error", code=500), + "cannot_connect", + id="transport_server_error", + ), + ], +) +async def test_form_login_error( + hass: HomeAssistant, + mock_setup_entry: AsyncMock, + mock_config_api: AsyncMock, + api_error: Exception, + expected_error: str, ) -> None: - """Test config flow with a login error.""" + """Test config flow login errors and recovery.""" result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_USER} ) assert result["type"] is FlowResultType.FORM assert result["errors"] == {} - # Change the login mock to raise an MFA required error - mock_config_api.return_value.login.side_effect = LoginFailedException( - "Invalid Auth" - ) + mock_config_api.return_value.login.side_effect = api_error result = await hass.config_entries.flow.async_configure( result["flow_id"], @@ -88,7 +123,7 @@ async def test_form_invalid_auth( ) assert result["type"] is FlowResultType.FORM - assert result["errors"] == {"base": "invalid_auth"} + assert result["errors"] == {"base": expected_error} mock_config_api.return_value.login.side_effect = None result = await hass.config_entries.flow.async_configure( @@ -108,6 +143,144 @@ async def test_form_invalid_auth( assert len(mock_setup_entry.mock_calls) == 1 +@pytest.mark.parametrize( + ("subscription_id", "expected_reason", "expected_token", "reload_count"), + [ + pytest.param( + "222260252323873333", + "reauth_successful", + "mocked_token", + 1, + id="same_account", + ), + pytest.param( + "different-subscription", + "unique_id_mismatch", + "fake_token_of_doom", + 0, + id="different_account", + ), + ], +) +async def test_reauth_account_validation( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_config_api: AsyncMock, + subscription_id: str, + expected_reason: str, + expected_token: str, + reload_count: int, +) -> None: + """Test reauthentication only updates the matching existing entry.""" + mock_config_entry.add_to_hass(hass) + entry_id = mock_config_entry.entry_id + mock_config_api.return_value.get_subscription_details.return_value = Mock( + id=subscription_id + ) + + result = await mock_config_entry.start_reauth_flow(hass) + + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "reauth_confirm" + + with patch.object( + hass.config_entries, "async_reload", new=AsyncMock(return_value=True) + ) as mock_reload: + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_EMAIL: "test-username", + CONF_PASSWORD: "test-password", + }, + ) + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == expected_reason + assert mock_config_entry.entry_id == entry_id + assert mock_config_entry.unique_id == "222260252323873333" + assert mock_config_entry.data == {CONF_TOKEN: expected_token} + assert mock_reload.await_args_list == [call(entry_id)] * reload_count + + +async def test_reauth_mfa_retry( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_config_api: AsyncMock, +) -> None: + """Test an MFA reauthentication can recover from code and connection errors.""" + mock_config_entry.add_to_hass(hass) + client = mock_config_api.return_value + client.login.side_effect = RequireMFAException("mfa_required") + + result = await mock_config_entry.start_reauth_flow(hass) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_EMAIL: "test-username", + CONF_PASSWORD: "test-password", + }, + ) + + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "reauth_confirm" + assert result["errors"] == {"base": "mfa_required"} + assert CONF_MFA_CODE in result["data_schema"].schema + + client.multi_factor_authenticate.side_effect = LoginFailedException("bad code") + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_MFA_CODE: "123456"} + ) + + assert result["type"] is FlowResultType.FORM + assert result["errors"] == {"base": "bad_mfa"} + assert CONF_MFA_CODE in result["data_schema"].schema + + client.multi_factor_authenticate.side_effect = TransportError("offline") + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_MFA_CODE: "654321"} + ) + + assert result["type"] is FlowResultType.FORM + assert result["errors"] == {"base": "cannot_connect"} + assert CONF_MFA_CODE in result["data_schema"].schema + + client.multi_factor_authenticate.side_effect = None + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_MFA_CODE: "654321"} + ) + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "reauth_successful" + client.multi_factor_authenticate.assert_awaited_with( + "test-username", "test-password", "654321" + ) + + +async def test_reauth_subscription_auth_failure( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_config_api: AsyncMock, +) -> None: + """Test an auth failure after login returns to the credential form.""" + mock_config_entry.add_to_hass(hass) + client = mock_config_api.return_value + client.get_subscription_details.side_effect = LoginFailedException("expired") + + result = await mock_config_entry.start_reauth_flow(hass) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_EMAIL: "test-username", + CONF_PASSWORD: "test-password", + }, + ) + + assert result["type"] is FlowResultType.FORM + assert result["errors"] == {"base": "invalid_auth"} + assert CONF_EMAIL in result["data_schema"].schema + assert CONF_PASSWORD in result["data_schema"].schema + + async def test_form_mfa( hass: HomeAssistant, mock_setup_entry: AsyncMock, mock_config_api: AsyncMock ) -> None: diff --git a/tests/components/monarch_money/test_coordinator.py b/tests/components/monarch_money/test_coordinator.py index 3f7664964922..af4cb2eae592 100644 --- a/tests/components/monarch_money/test_coordinator.py +++ b/tests/components/monarch_money/test_coordinator.py @@ -2,8 +2,17 @@ from unittest.mock import AsyncMock +from aiohttp import ClientError from freezegun.api import FrozenDateTimeFactory +from gql.transport.exceptions import TransportError, TransportServerError +from monarchmoney import LoginFailedException +import pytest +from homeassistant.components.monarch_money.coordinator import ( + MonarchMoneyDataUpdateCoordinator, +) +from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState +from homeassistant.const import STATE_UNAVAILABLE from homeassistant.core import HomeAssistant from . import setup_integration @@ -31,3 +40,109 @@ async def test_cashflow_year_follows_configured_time_zone( mock_config_api.return_value.get_cashflow_summary.assert_called_with( start_date="2026-01-01", end_date="2026-12-31" ) + + +@pytest.mark.parametrize( + "api_error", + [ + pytest.param(LoginFailedException("expired"), id="login_failed"), + pytest.param( + TransportServerError("unauthorized", code=401), id="http_unauthorized" + ), + ], +) +async def test_setup_auth_error_starts_reauthentication( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_config_api: AsyncMock, + api_error: Exception, +) -> None: + """Test setup authentication errors start reauthentication.""" + mock_config_api.return_value.get_subscription_details.side_effect = api_error + + await setup_integration(hass, mock_config_entry) + + assert mock_config_entry.state is ConfigEntryState.SETUP_ERROR + flows = hass.config_entries.flow.async_progress() + assert len(flows) == 1 + assert flows[0]["context"]["source"] == SOURCE_REAUTH + assert flows[0]["step_id"] == "reauth_confirm" + + +async def test_update_auth_error_starts_reauthentication( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_config_api: AsyncMock, +) -> None: + """Test refresh authentication errors start reauthentication.""" + await setup_integration(hass, mock_config_entry) + coordinator: MonarchMoneyDataUpdateCoordinator = mock_config_entry.runtime_data + mock_config_api.return_value.get_accounts_as_dict_with_id_key.side_effect = ( + TransportServerError("forbidden", code=403) + ) + + await coordinator.async_refresh() + await hass.async_block_till_done() + + assert mock_config_entry.state is ConfigEntryState.LOADED + assert not coordinator.last_update_success + state = hass.states.get("sensor.cashflow_expense_year_to_date") + assert state is not None + assert state.state == STATE_UNAVAILABLE + flows = hass.config_entries.flow.async_progress() + assert len(flows) == 1 + assert flows[0]["context"]["source"] == SOURCE_REAUTH + assert flows[0]["step_id"] == "reauth_confirm" + + +@pytest.mark.parametrize( + "api_error", + [ + pytest.param(TransportServerError("server error", code=500), id="http_error"), + pytest.param(TimeoutError(), id="timeout"), + ], +) +async def test_setup_connection_error_is_retryable( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_config_api: AsyncMock, + api_error: Exception, +) -> None: + """Test setup connection errors schedule a retry.""" + mock_config_api.return_value.get_subscription_details.side_effect = api_error + + await setup_integration(hass, mock_config_entry) + + assert mock_config_entry.state is ConfigEntryState.SETUP_RETRY + assert hass.config_entries.flow.async_progress() == [] + + +@pytest.mark.parametrize( + "api_error", + [ + pytest.param(TransportError("transport error"), id="transport_error"), + pytest.param(ClientError("client error"), id="client_error"), + ], +) +async def test_update_connection_error_is_retryable( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_config_api: AsyncMock, + api_error: Exception, +) -> None: + """Test refresh connection errors mark data unavailable without reauth.""" + await setup_integration(hass, mock_config_entry) + coordinator: MonarchMoneyDataUpdateCoordinator = mock_config_entry.runtime_data + mock_config_api.return_value.get_accounts_as_dict_with_id_key.side_effect = ( + api_error + ) + + await coordinator.async_refresh() + await hass.async_block_till_done() + + assert mock_config_entry.state is ConfigEntryState.LOADED + assert not coordinator.last_update_success + state = hass.states.get("sensor.cashflow_expense_year_to_date") + assert state is not None + assert state.state == STATE_UNAVAILABLE + assert hass.config_entries.flow.async_progress() == []