From 399e9053fcf791f4da4221d680f7f8642618c45a Mon Sep 17 00:00:00 2001 From: Tero Pihlaja Date: Fri, 4 Sep 2026 21:33:07 +0300 Subject: [PATCH 1/5] Fix iCloud polling loop dying permanently on transient errors keep_alive() runs from a timer callback and is the only thing that schedules the next fetch. Anything raised inside it - a transient network error from authenticate(), or a device payload missing the fields update_devices() indexes into - escaped into the event loop, so no further fetch was ever scheduled and every device stayed frozen at its last known state until Home Assistant was restarted. Schedule the next fetch on every failure path, including the early return when a 2FA challenge is pending, so the account resumes polling by itself once it can talk to iCloud again. Rejected credentials are handled separately, since those do not recover on their own: the user is asked to log in again, and the login is not retried while that is outstanding. Retrying underneath them would either add failed attempts against the account or quietly succeed and strand the repair they were shown. The session is kept when the account only needs a 2FA code, because async_step_reauth reuses it to validate the code and sends a None api back to the password form instead. A malformed device is now skipped rather than abandoning the cycle, which would leave the devices already collected in place without ever dispatching signal_device_new for them, so their entities were never created by any later poll. The timer was also only ever armed at the end of update_devices(), so an account whose first login failed was left loaded with no timer at all. setup() now arms one before returning either way. --- homeassistant/components/icloud/account.py | 115 ++++++++- tests/components/icloud/test_account.py | 284 ++++++++++++++++++++- 2 files changed, 391 insertions(+), 8 deletions(-) diff --git a/homeassistant/components/icloud/account.py b/homeassistant/components/icloud/account.py index a760e6c32e3c5..63480f73b8faf 100644 --- a/homeassistant/components/icloud/account.py +++ b/homeassistant/components/icloud/account.py @@ -7,6 +7,7 @@ from pyicloud import PyiCloudService from pyicloud.exceptions import ( + PyiCloudAuthRequiredException, PyiCloudFailedLoginException, PyiCloudNoDevicesException, PyiCloudServiceNotActivatedException, @@ -15,7 +16,7 @@ from pyicloud.services.findmyiphone import AppleDevice from homeassistant.components.zone import async_active_zone -from homeassistant.config_entries import ConfigEntry +from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntry from homeassistant.const import CONF_USERNAME, EntityStateAttribute from homeassistant.core import CALLBACK_TYPE, HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady @@ -105,7 +106,18 @@ def __init__( self.photo_cache: PhotoCache | None = None def setup(self) -> None: - """Set up an iCloud account.""" + """Set up an iCloud account, leaving it with a fetch scheduled. + + _setup() only arms the timer if it reaches update_devices(), so the + paths that return before then are given one here. + """ + self._setup() + if self._unsub_fetch is None: + self._fetch_interval = self._max_interval + self._schedule_next_fetch() + + def _setup(self) -> None: + """Log in and read the account's devices.""" try: self.api = PyiCloudService( self._username, @@ -169,6 +181,10 @@ def update_devices(self) -> None: if self.api.requires_2fa: self._require_reauth() + # Keep the timer running so polling resumes by itself once the + # user has entered their verification code. + self._fetch_interval = self._max_interval + self._schedule_next_fetch() return api_devices = {} @@ -188,6 +204,16 @@ def update_devices(self) -> None: device_id = status[DEVICE_ID] device_name = status[DEVICE_NAME] + if device_id is None or device_name is None: + # status() reports every requested field, using None for the + # ones iCloud left out, so an unusable device has to be + # rejected rather than left to raise. Skipping it keeps the + # devices already collected, which would otherwise stay in + # _devices without signal_device_new ever being dispatched + # for them, so their entities were never created. + _LOGGER.warning("Skipping iCloud device with no id or name") + continue + if ( status[DEVICE_BATTERY_STATUS] == "Unknown" or status.get(DEVICE_BATTERY_LEVEL) is None @@ -318,16 +344,91 @@ def _schedule_next_fetch(self) -> None: utcnow() + timedelta(minutes=self._fetch_interval), ) + def _reauth_pending(self) -> bool: + """Return whether the user is already being asked to log in again. + + This runs in the executor, so the flow list has to be read on the + event loop like the rest of the shared state here. + """ + return run_callback_threadsafe( + self.hass.loop, + lambda: bool( + any( + self._config_entry.async_get_active_flows( + self.hass, {SOURCE_REAUTH} + ) + ) + ), + ).result() + def keep_alive(self, now=None) -> None: - """Keep the API alive.""" - if self.api is None: - self.setup() + """Keep the API alive. + + This runs from a timer callback and is what schedules the next one, so + every path out of here has to schedule that fetch: anything raised + escapes into the event loop and stops the account polling entirely. + """ + if self.api is None and not self._reauth_pending(): + # Only retry the login when the user is not already being asked to + # do it. Retrying underneath them would either add failed attempts + # against the account or quietly succeed and leave the repair they + # were shown standing with nothing to complete it. Finishing that + # flow reloads the entry, which is what recovers the account. + try: + self.setup() + except Exception: + # setup() reports its own failures; it must not stop the loop. + # Drop whatever session it did establish: it may have failed + # after logging in but before reading the account's owner and + # family names, and those are only filled in by a full setup. + _LOGGER.exception("Error setting up iCloud account, will retry") + self.api = None if self.api is None: + # Still no session. Try again at the longest interval rather than + # giving up, so the account recovers on its own once iCloud is + # reachable again or the user has finished logging in. + self._fetch_interval = self._max_interval + self._schedule_next_fetch() return - self.api.authenticate() - self.update_devices() + try: + self.api.authenticate() + self.update_devices() + except PyiCloudFailedLoginException, PyiCloudAuthRequiredException: + # Neither of these comes back on its own, so ask the user instead + # of retrying every couple of minutes forever. + if self.api is not None and self.api.requires_2fa: + # Keep the session: the reauth flow reuses it to validate the + # code, and async_step_reauth sends a None api back to the + # password form instead of straight to code entry. + _LOGGER.warning( + ( + "2FA authentication required for '%s'; Go to the Integrations " + "menu and click on Configure on the discovered Apple iCloud " + "card to enter your verification code" + ), + self._config_entry.data[CONF_USERNAME], + ) + else: + self.api = None + _LOGGER.error( + ( + "Your iCloud account for '%s' is no longer working; Go to the " + "Integrations menu and click on Configure on the discovered " + "Apple iCloud card to login again" + ), + self._config_entry.data[CONF_USERNAME], + ) + self._require_reauth() + self._fetch_interval = self._max_interval + self._schedule_next_fetch() + except Exception: + # update_devices() reschedules itself on the errors it handles; + # this covers the rest, such as a device missing fields. + _LOGGER.exception("Error updating iCloud devices, will retry") + self._fetch_interval = 2 + self._schedule_next_fetch() def get_devices_with_name(self, name: str) -> list[Any]: """Get devices by name.""" diff --git a/tests/components/icloud/test_account.py b/tests/components/icloud/test_account.py index 3ddc13b5d6992..b40ae9bfbbe86 100644 --- a/tests/components/icloud/test_account.py +++ b/tests/components/icloud/test_account.py @@ -1,7 +1,11 @@ """Tests for the iCloud account.""" +from datetime import timedelta from unittest.mock import MagicMock, Mock, patch +from freezegun.api import FrozenDateTimeFactory +from pyicloud.exceptions import PyiCloudFailedLoginException +from pyicloud.services.findmyiphone import AppleDevice import pytest from homeassistant.components.icloud.account import IcloudAccount @@ -9,6 +13,7 @@ CONF_GPS_ACCURACY_THRESHOLD, CONF_MAX_INTERVAL, CONF_WITH_FAMILY, + DEFAULT_MAX_INTERVAL, DOMAIN, ) from homeassistant.const import CONF_PASSWORD, CONF_USERNAME @@ -18,7 +23,7 @@ from .const import DEVICE, MOCK_CONFIG, USER_INFO, USERNAME -from tests.common import MockConfigEntry +from tests.common import MockConfigEntry, async_fire_time_changed @pytest.fixture(name="mock_store") @@ -165,3 +170,280 @@ async def test_setup_success_with_devices( assert account.owner_fullname == "user name" assert "johntravolta" in account.family_members_fullname assert account.family_members_fullname["johntravolta"] == "John TRAVOLTA" + + +class FakeAppleDevice: + """A device that reports status through pyicloud's own implementation. + + Reusing AppleDevice.status keeps these tests honest about how iCloud data + actually reaches the integration: it reports every requested field, using + None for the ones the payload omits, so a stand-in that raised KeyError + would be testing a case that cannot happen. + """ + + # Bound to pyicloud's implementation, which only reads _content. + status = AppleDevice.status + + def __init__(self, content: dict) -> None: + """Store the payload iCloud would have returned.""" + self._content = content + + def __getitem__(self, key): + """Proxy into the raw payload, as AppleDevice does.""" + return self._content[key] + + +class MockDevicesWithLocation(MockDevicesContainer): + """Devices container whose single device reports a location.""" + + def refresh(self, locate: bool = True) -> None: + """Match the FindMyiPhone service interface.""" + + +def _located_device_status(battery_level: float) -> dict: + """Return a device status carrying a usable location.""" + return { + **DEVICE, + "batteryLevel": battery_level, + "location": { + "latitude": 1.0, + "longitude": 2.0, + "horizontalAccuracy": 10, + }, + } + + +@pytest.fixture(name="polling_service") +def mock_polling_service_fixture(): + """Mock a service whose device can change between fetches.""" + status = _located_device_status(0.8) + with patch( + "homeassistant.components.icloud.account.PyiCloudService" + ) as service_mock: + service_instance = MagicMock() + service_instance.requires_2fa = False + service_instance.devices = MockDevicesWithLocation( + USER_INFO, [FakeAppleDevice(status)] + ) + service_mock.return_value = service_instance + yield service_instance, status + + +async def test_polling_survives_authentication_error( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that a failing fetch does not stop the account from polling.""" + service, status = polling_service + + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert hass.states.get("sensor.iphone_battery").state == "80" + + # A transient failure used to escape the timer callback, after which no + # further fetch was ever scheduled. + service.authenticate.side_effect = ConnectionError("boom") + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + # The account recovers and the next fetch still happens. + service.authenticate.side_effect = None + status["batteryLevel"] = 0.5 + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert hass.states.get("sensor.iphone_battery").state == "50" + + +async def test_failed_login_is_not_retried_under_the_user( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that a rejected login is left for the user to fix. + + The account still has to keep its fetch timer, which is only armed once + update_devices() completes, so it used to sit loaded with no timer at all. + But retrying the login while the user is already being asked to log in + either adds failed attempts against their account or quietly succeeds and + strands the repair they were shown. Finishing that flow reloads the entry, + which is what recovers the account. + """ + del polling_service # only the patched service class is needed here + + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.icloud.account.PyiCloudService", + side_effect=PyiCloudFailedLoginException("nope"), + ) as service: + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert config_entry.runtime_data.api is None + assert [ + flow + for flow in hass.config_entries.flow.async_progress() + if flow["context"]["source"] == "reauth" + ] + attempts = service.call_count + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + # The timer fired, but it did not try to log in again. + assert service.call_count == attempts + + assert hass.states.get("sensor.iphone_battery") is None + + +async def test_device_without_identity_is_skipped( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that a device with no id is skipped rather than stored. + + pyicloud reports every requested field, using None for the ones iCloud + left out, so an unusable device arrives looking like any other rather + than raising on access. + """ + _, status = polling_service + status["id"] = None + + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + # Nothing is stored under a None identity, so no entity is built from one. + assert config_entry.runtime_data.devices == {} + assert hass.states.get("sensor.iphone_battery") is None + + # A later poll picks the device up once iCloud reports it properly. + status["id"] = "device1" + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert hass.states.get("sensor.iphone_battery").state == "80" + + +async def test_polling_resumes_after_2fa_challenge( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that polling continues across a 2FA challenge. + + update_devices() returns early while a code is outstanding, which used to + leave nothing to schedule the fetch that would notice it had been entered. + """ + service, status = polling_service + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + assert hass.states.get("sensor.iphone_battery").state == "80" + + # iCloud starts asking for a verification code. + service.requires_2fa = True + status["batteryLevel"] = 0.6 + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert hass.states.get("sensor.iphone_battery").state == "80" + + # The user enters the code; polling has to pick up again on its own. + service.requires_2fa = False + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert hass.states.get("sensor.iphone_battery").state == "60" + + +async def test_rejected_credentials_ask_the_user( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that a rejected login starts reauth instead of retrying forever.""" + service, _ = polling_service + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + service.authenticate.side_effect = PyiCloudFailedLoginException("rejected") + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + # The user is asked to log in again rather than the session being retried + # every couple of minutes indefinitely. + assert [ + flow + for flow in hass.config_entries.flow.async_progress() + if flow["context"]["source"] == "reauth" + ] + assert config_entry.runtime_data.api is None + + +async def test_2fa_challenge_keeps_session_for_reauth( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that a 2FA challenge while polling keeps the session. + + async_step_reauth reuses this session to send and validate the code and + sends a None api back to the password form instead, so a challenge must + not clear it. + """ + service, _ = polling_service + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + service.requires_2fa = True + service.authenticate.side_effect = PyiCloudFailedLoginException("2FA required") + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert config_entry.runtime_data.api is not None From 8b6938439fb7075faf226222083f294f61027a66 Mon Sep 17 00:00:00 2001 From: Tero Pihlaja Date: Fri, 11 Sep 2026 22:36:38 +0300 Subject: [PATCH 2/5] Ask the user to log in when iCloud rejects the polling session keep_alive() only treated PyiCloudFailedLoginException and PyiCloudAuthRequiredException as needing the user. A 2FA challenge can also arrive as PyiCloud2FARequiredException, raised out of _get_mfa_auth_options() before requires_2fa is ever set, and every rejection other than a 409 carrying an hsa2 body arrives as a plain PyiCloudAPIResponseException with the status in .code. Both derive straight from PyiCloudException, so neither was recognised and both fell through to the transient handling: the account was retried every couple of minutes indefinitely and the user was never asked for the code or password that was actually needed, which is exactly the behaviour this handler exists to avoid. Treat 409, 421 and 450 as needing authentication. GENERAL_AUTH_ERROR (500) is left out on purpose: pyicloud groups it with the authentication statuses, but a 500 is just as likely to be a transient iCloud failure, and those have to keep being retried instead of parking the account. A non-authentication API error is re-raised into the surrounding handler so it keeps being retried, which is why the authentication handling is nested rather than sitting alongside it: nothing may escape keep_alive(), since it is what schedules the next fetch. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XjrRexCGPR5ymCuJaSUiuN --- homeassistant/components/icloud/account.py | 96 +++++++++++---- tests/components/icloud/test_account.py | 135 ++++++++++++++++++++- 2 files changed, 209 insertions(+), 22 deletions(-) diff --git a/homeassistant/components/icloud/account.py b/homeassistant/components/icloud/account.py index 63480f73b8faf..19ececcc887db 100644 --- a/homeassistant/components/icloud/account.py +++ b/homeassistant/components/icloud/account.py @@ -6,7 +6,10 @@ from typing import TYPE_CHECKING, Any from pyicloud import PyiCloudService +from pyicloud.const import AppleAuthError from pyicloud.exceptions import ( + PyiCloud2FARequiredException, + PyiCloudAPIResponseException, PyiCloudAuthRequiredException, PyiCloudFailedLoginException, PyiCloudNoDevicesException, @@ -62,6 +65,28 @@ _LOGGER = logging.getLogger(__name__) +# pyicloud only raises PyiCloud2FARequiredException for a 409 whose body is JSON +# with authType == "hsa2". Every other authentication rejection falls through to +# Session._raise_error(), which raises a plain PyiCloudAPIResponseException +# carrying the HTTP status in .code, so the status is what has to be inspected. +# +# GENERAL_AUTH_ERROR (500) is excluded on purpose: pyicloud groups it with the +# authentication statuses, but a 500 is just as likely to be a transient iCloud +# failure, and those have to keep being retried rather than parked for the user. +_AUTH_REQUIRED_STATUSES = frozenset( + { + AppleAuthError.TWO_FACTOR_REQUIRED, + AppleAuthError.LOGIN_TOKEN_EXPIRED, + AppleAuthError.FIND_MY_REAUTH_REQUIRED, + } +) + + +def _is_auth_error(err: PyiCloudAPIResponseException) -> bool: + """Return True if the account has to authenticate again to recover.""" + return isinstance(err.code, int) and err.code in _AUTH_REQUIRED_STATUSES + + type IcloudConfigEntry = ConfigEntry[IcloudAccount] @@ -393,24 +418,18 @@ def keep_alive(self, now=None) -> None: return try: - self.api.authenticate() - self.update_devices() - except PyiCloudFailedLoginException, PyiCloudAuthRequiredException: - # Neither of these comes back on its own, so ask the user instead - # of retrying every couple of minutes forever. - if self.api is not None and self.api.requires_2fa: - # Keep the session: the reauth flow reuses it to validate the - # code, and async_step_reauth sends a None api back to the - # password form instead of straight to code entry. - _LOGGER.warning( - ( - "2FA authentication required for '%s'; Go to the Integrations " - "menu and click on Configure on the discovered Apple iCloud " - "card to enter your verification code" - ), - self._config_entry.data[CONF_USERNAME], - ) - else: + try: + self.api.authenticate() + self.update_devices() + except PyiCloudAPIResponseException as err: + if not _is_auth_error(err): + # Not an authentication failure. Hand it to the transient + # handling below so it keeps being retried, rather than + # parking the account for credentials that are not at fault. + raise + # Reported as a login failure: the status says the session has + # to be established again, and there is no code to send through + # one that has just been rejected. self.api = None _LOGGER.error( ( @@ -420,9 +439,44 @@ def keep_alive(self, now=None) -> None: ), self._config_entry.data[CONF_USERNAME], ) - self._require_reauth() - self._fetch_interval = self._max_interval - self._schedule_next_fetch() + self._require_reauth() + self._fetch_interval = self._max_interval + self._schedule_next_fetch() + except ( + PyiCloud2FARequiredException, + PyiCloudFailedLoginException, + PyiCloudAuthRequiredException, + ) as err: + # None of these comes back on its own, so ask the user instead + # of retrying every couple of minutes forever. + if isinstance(err, PyiCloud2FARequiredException) or ( + self.api is not None and self.api.requires_2fa + ): + # Keep the session: the reauth flow reuses it to validate + # the code, and async_step_reauth sends a None api back to + # the password form instead of straight to code entry. + _LOGGER.warning( + ( + "2FA authentication required for '%s'; Go to the " + "Integrations menu and click on Configure on the " + "discovered Apple iCloud card to enter your " + "verification code" + ), + self._config_entry.data[CONF_USERNAME], + ) + else: + self.api = None + _LOGGER.error( + ( + "Your iCloud account for '%s' is no longer working; Go " + "to the Integrations menu and click on Configure on the " + "discovered Apple iCloud card to login again" + ), + self._config_entry.data[CONF_USERNAME], + ) + self._require_reauth() + self._fetch_interval = self._max_interval + self._schedule_next_fetch() except Exception: # update_devices() reschedules itself on the errors it handles; # this covers the rest, such as a device missing fields. diff --git a/tests/components/icloud/test_account.py b/tests/components/icloud/test_account.py index b40ae9bfbbe86..d3540424d712f 100644 --- a/tests/components/icloud/test_account.py +++ b/tests/components/icloud/test_account.py @@ -4,9 +4,15 @@ from unittest.mock import MagicMock, Mock, patch from freezegun.api import FrozenDateTimeFactory -from pyicloud.exceptions import PyiCloudFailedLoginException +from pyicloud.const import AppleAuthError +from pyicloud.exceptions import ( + PyiCloud2FARequiredException, + PyiCloudAPIResponseException, + PyiCloudFailedLoginException, +) from pyicloud.services.findmyiphone import AppleDevice import pytest +from requests import Response from homeassistant.components.icloud.account import IcloudAccount from homeassistant.components.icloud.const import ( @@ -447,3 +453,130 @@ async def test_2fa_challenge_keeps_session_for_reauth( await hass.async_block_till_done() assert config_entry.runtime_data.api is not None + + +async def test_2fa_exception_while_polling_asks_for_a_code( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that a 2FA challenge raised while polling asks for a code. + + authenticate() raises out of the MFA options request before requires_2fa is + set, so the exception is the only signal that a code is what is missing. + Without it the challenge fell through to the transient handling and was + retried every couple of minutes without ever asking the user. + """ + service, _ = polling_service + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + service.requires_2fa = False + service.authenticate.side_effect = PyiCloud2FARequiredException( + USERNAME, Mock(spec=Response) + ) + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert [ + flow + for flow in hass.config_entries.flow.async_progress() + if flow["context"]["source"] == "reauth" + ] + # The session is kept so the reauth flow can send the code through it. + assert config_entry.runtime_data.api is not None + + +@pytest.mark.parametrize( + "status", + [ + AppleAuthError.TWO_FACTOR_REQUIRED, + AppleAuthError.LOGIN_TOKEN_EXPIRED, + AppleAuthError.FIND_MY_REAUTH_REQUIRED, + ], +) +async def test_auth_status_while_polling_asks_the_user( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, + status: AppleAuthError, +) -> None: + """Test that an authentication status while polling starts reauth. + + pyicloud only raises a dedicated exception for a 409 carrying an hsa2 body, + so every other rejection arrives as a plain PyiCloudAPIResponseException and + the status has to be inspected rather than the exception type. + """ + service, _ = polling_service + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + service.authenticate.side_effect = PyiCloudAPIResponseException( + "Authentication required for Account.", status + ) + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert [ + flow + for flow in hass.config_entries.flow.async_progress() + if flow["context"]["source"] == "reauth" + ] + assert config_entry.runtime_data.api is None + + +async def test_other_api_error_while_polling_keeps_retrying( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that a non-authentication API error keeps the loop retrying. + + Reauthenticating cannot fix a server-side failure, so it has to be treated + as transient and the next fetch still scheduled. + """ + service, _ = polling_service + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + service.authenticate.side_effect = PyiCloudAPIResponseException( + "Service temporarily unavailable", 503 + ) + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert not [ + flow + for flow in hass.config_entries.flow.async_progress() + if flow["context"]["source"] == "reauth" + ] + # The session is kept and the loop keeps going rather than parking. + assert config_entry.runtime_data.api is not None + + service.authenticate.side_effect = None + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert service.authenticate.call_count > 1 From a25e3ef6aee00416bff55893af3105528c6fe263 Mon Sep 17 00:00:00 2001 From: Tero Pihlaja Date: Fri, 11 Sep 2026 23:03:10 +0300 Subject: [PATCH 3/5] Fill in the account names on the poll after a 2FA challenge setup() returns before reading the account owner and family member names when iCloud asks for a verification code, and keep_alive() only runs a full setup again when there is no session at all. The names therefore stayed empty for the rest of the entry's life, and building a family device raised KeyError on every poll once the code had been entered. Read them in update_devices() instead when setup did not get that far. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014pPuNx7ySC3snNVfGc2eNs --- homeassistant/components/icloud/account.py | 38 ++++++++++------ tests/components/icloud/test_account.py | 50 +++++++++++++++++++++- 2 files changed, 75 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/icloud/account.py b/homeassistant/components/icloud/account.py index 19ececcc887db..066cd5400fa64 100644 --- a/homeassistant/components/icloud/account.py +++ b/homeassistant/components/icloud/account.py @@ -1,5 +1,6 @@ """iCloud account.""" +from collections.abc import Mapping from datetime import timedelta import logging import operator @@ -133,8 +134,8 @@ def __init__( def setup(self) -> None: """Set up an iCloud account, leaving it with a fetch scheduled. - _setup() only arms the timer if it reaches update_devices(), so the - paths that return before then are given one here. + Every path out of _setup() has to end with a timer, including the ones + that return before update_devices() arms it. """ self._setup() if self._unsub_fetch is None: @@ -184,20 +185,25 @@ def _setup(self) -> None: if user_info is None: raise ConfigEntryNotReady("No user info found in iCloud devices response") + self._read_account_names(user_info) + + self._devices = {} + self.update_devices() + + def _read_account_names(self, user_info: Mapping[str, Any]) -> None: + """Store the account owner and family member names.""" self._owner_fullname = ( f"{user_info.get('firstName')} {user_info.get('lastName')}" ) self._family_members_fullname = {} - if user_info.get("membersInfo") is not None: - for prs_id, member in user_info.get("membersInfo").items(): + members_info = user_info.get("membersInfo") + if members_info is not None: + for prs_id, member in members_info.items(): self._family_members_fullname[prs_id] = ( f"{member['firstName']} {member['lastName']}" ) - self._devices = {} - self.update_devices() - def update_devices(self) -> None: """Update iCloud devices.""" if self.api is None: @@ -222,6 +228,14 @@ def update_devices(self) -> None: self._schedule_next_fetch() return + if self._owner_fullname is None and api_devices.user_info is not None: + # setup() returns before reading these when iCloud asks for a + # verification code, and keep_alive() only runs a full setup again + # when there is no session at all, so the first poll after the + # challenge is what fills them in. A family device cannot be built + # without them. + self._read_account_names(api_devices.user_info) + # Gets devices infos new_device = False for device in api_devices: @@ -231,11 +245,11 @@ def update_devices(self) -> None: if device_id is None or device_name is None: # status() reports every requested field, using None for the - # ones iCloud left out, so an unusable device has to be - # rejected rather than left to raise. Skipping it keeps the - # devices already collected, which would otherwise stay in - # _devices without signal_device_new ever being dispatched - # for them, so their entities were never created. + # ones iCloud left out, so an unusable device arrives looking + # like any other rather than raising. It has to be rejected + # here to keep a None identity out of the entity and device + # registries; a later poll picks the device up if iCloud + # starts reporting it properly. _LOGGER.warning("Skipping iCloud device with no id or name") continue diff --git a/tests/components/icloud/test_account.py b/tests/components/icloud/test_account.py index d3540424d712f..f6622cce32b5d 100644 --- a/tests/components/icloud/test_account.py +++ b/tests/components/icloud/test_account.py @@ -16,6 +16,7 @@ from homeassistant.components.icloud.account import IcloudAccount from homeassistant.components.icloud.const import ( + ATTR_OWNER_NAME, CONF_GPS_ACCURACY_THRESHOLD, CONF_MAX_INTERVAL, CONF_WITH_FAMILY, @@ -27,7 +28,14 @@ from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers.storage import Store -from .const import DEVICE, MOCK_CONFIG, USER_INFO, USERNAME +from .const import ( + DEVICE, + MEMBER_1_FULL_NAME, + MEMBER_1_PERSON_ID, + MOCK_CONFIG, + USER_INFO, + USERNAME, +) from tests.common import MockConfigEntry, async_fire_time_changed @@ -580,3 +588,43 @@ async def test_other_api_error_while_polling_keeps_retrying( await hass.async_block_till_done() assert service.authenticate.call_count > 1 + + +async def test_family_device_after_setup_time_2fa( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that an account challenged during setup still builds its devices. + + setup() returns before reading the account's owner and family names when + iCloud asks for a verification code, and keep_alive() only runs a full + setup again when there is no session at all. The names therefore have to + be filled in by the poll that follows the challenge, or a family device + can never be built. + """ + service, status = polling_service + status["prsId"] = MEMBER_1_PERSON_ID + service.requires_2fa = True + + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert hass.states.get("sensor.iphone_battery") is None + + # The user enters the code. + service.requires_2fa = False + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + state = hass.states.get("sensor.iphone_battery") + assert state is not None + assert state.state == "80" + assert state.attributes[ATTR_OWNER_NAME] == MEMBER_1_FULL_NAME From 21f81afc87e5b362759df27d9002ae998d8b448e Mon Sep 17 00:00:00 2001 From: Tero Pihlaja Date: Fri, 11 Sep 2026 23:23:09 +0300 Subject: [PATCH 4/5] Let iCloud rejections reach the user instead of being retried forever Two ways an account could still end up stuck: An empty device response raised IndexError out of update_devices(), which during initial setup escaped async_setup_entry() and left the entry in SETUP_ERROR with no retry and no timer. Refreshing the devices is where a stored session is usually turned down, but that request was handled as an unknown error, so a rejected session was retried every couple of minutes and the user was never asked to log in. Those rejections now reach the handling that asks them, from the first fetch as well as from the polling loop. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014pPuNx7ySC3snNVfGc2eNs --- homeassistant/components/icloud/account.py | 105 ++++++++++++++------- tests/components/icloud/test_account.py | 74 ++++++++++++++- 2 files changed, 142 insertions(+), 37 deletions(-) diff --git a/homeassistant/components/icloud/account.py b/homeassistant/components/icloud/account.py index 066cd5400fa64..a6bf921eef387 100644 --- a/homeassistant/components/icloud/account.py +++ b/homeassistant/components/icloud/account.py @@ -88,6 +88,20 @@ def _is_auth_error(err: PyiCloudAPIResponseException) -> bool: return isinstance(err.code, int) and err.code in _AUTH_REQUIRED_STATUSES +def _is_auth_failure(err: BaseException) -> bool: + """Return True if iCloud rejected the session rather than the request.""" + if isinstance( + err, + ( + PyiCloud2FARequiredException, + PyiCloudFailedLoginException, + PyiCloudAuthRequiredException, + ), + ): + return True + return isinstance(err, PyiCloudAPIResponseException) and _is_auth_error(err) + + type IcloudConfigEntry = ConfigEntry[IcloudAccount] @@ -137,7 +151,18 @@ def setup(self) -> None: Every path out of _setup() has to end with a timer, including the ones that return before update_devices() arms it. """ - self._setup() + try: + self._setup() + except ( + PyiCloud2FARequiredException, + PyiCloudFailedLoginException, + PyiCloudAuthRequiredException, + ) as err: + # Refreshing the devices can reject a session that logging in + # accepted. The entry still loads: the user is asked to act on it, + # and the timer below keeps polling so it recovers once they have. + self._ask_to_authenticate(err) + if self._unsub_fetch is None: self._fetch_interval = self._max_interval self._schedule_next_fetch() @@ -218,10 +243,14 @@ def update_devices(self) -> None: self._schedule_next_fetch() return - api_devices = {} try: api_devices = self.api.devices - except Exception as err: # noqa: BLE001 + except Exception as err: + if _is_auth_failure(err): + # Refreshing the devices is where a stored session is usually + # turned down. Retrying that here every couple of minutes gets + # nowhere, so hand it to the caller, which asks the user. + raise _LOGGER.error("Unknown iCloud error: %s", err) self._fetch_interval = 2 dispatcher_send(self.hass, self.signal_device_update) @@ -237,8 +266,9 @@ def update_devices(self) -> None: self._read_account_names(api_devices.user_info) # Gets devices infos + devices = list(api_devices) new_device = False - for device in api_devices: + for device in devices: status = device.status(DEVICE_STATUS_SET) device_id = status[DEVICE_ID] device_name = status[DEVICE_NAME] @@ -275,7 +305,8 @@ def update_devices(self) -> None: new_device = True if ( - DEVICE_STATUS_CODES.get(list(api_devices)[0][DEVICE_STATUS]) == "pending" + devices + and DEVICE_STATUS_CODES.get(devices[0][DEVICE_STATUS]) == "pending" and not self._retried_fetch ): _LOGGER.debug("Pending devices, trying again in 15s") @@ -291,6 +322,34 @@ def update_devices(self) -> None: self._schedule_next_fetch() + def _ask_to_authenticate(self, err: Exception) -> None: + """Ask the user to log in again after iCloud rejected the session.""" + if isinstance(err, PyiCloud2FARequiredException) or ( + self.api is not None and self.api.requires_2fa + ): + # Keep the session: the reauth flow reuses it to validate the code, + # and async_step_reauth sends a None api back to the password form + # instead of straight to code entry. + _LOGGER.warning( + ( + "2FA authentication required for '%s'; Go to the Integrations " + "menu and click on Configure on the discovered Apple iCloud " + "card to enter your verification code" + ), + self._config_entry.data[CONF_USERNAME], + ) + else: + self.api = None + _LOGGER.error( + ( + "Your iCloud account for '%s' is no longer working; Go to the " + "Integrations menu and click on Configure on the discovered " + "Apple iCloud card to login again" + ), + self._config_entry.data[CONF_USERNAME], + ) + self._require_reauth() + def _require_reauth(self): """Require the user to log in again.""" self.hass.add_job(self._config_entry.async_start_reauth, self.hass) @@ -408,11 +467,10 @@ def keep_alive(self, now=None) -> None: escapes into the event loop and stops the account polling entirely. """ if self.api is None and not self._reauth_pending(): - # Only retry the login when the user is not already being asked to - # do it. Retrying underneath them would either add failed attempts - # against the account or quietly succeed and leave the repair they - # were shown standing with nothing to complete it. Finishing that - # flow reloads the entry, which is what recovers the account. + # Not while the user is already being asked: retrying underneath + # them adds failed attempts against the account, or succeeds and + # strands the repair they were shown. Completing that flow reloads + # the entry, which is what recovers the account. try: self.setup() except Exception: @@ -463,32 +521,7 @@ def keep_alive(self, now=None) -> None: ) as err: # None of these comes back on its own, so ask the user instead # of retrying every couple of minutes forever. - if isinstance(err, PyiCloud2FARequiredException) or ( - self.api is not None and self.api.requires_2fa - ): - # Keep the session: the reauth flow reuses it to validate - # the code, and async_step_reauth sends a None api back to - # the password form instead of straight to code entry. - _LOGGER.warning( - ( - "2FA authentication required for '%s'; Go to the " - "Integrations menu and click on Configure on the " - "discovered Apple iCloud card to enter your " - "verification code" - ), - self._config_entry.data[CONF_USERNAME], - ) - else: - self.api = None - _LOGGER.error( - ( - "Your iCloud account for '%s' is no longer working; Go " - "to the Integrations menu and click on Configure on the " - "discovered Apple iCloud card to login again" - ), - self._config_entry.data[CONF_USERNAME], - ) - self._require_reauth() + self._ask_to_authenticate(err) self._fetch_interval = self._max_interval self._schedule_next_fetch() except Exception: diff --git a/tests/components/icloud/test_account.py b/tests/components/icloud/test_account.py index f6622cce32b5d..a5021e2f96223 100644 --- a/tests/components/icloud/test_account.py +++ b/tests/components/icloud/test_account.py @@ -1,7 +1,7 @@ """Tests for the iCloud account.""" from datetime import timedelta -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import MagicMock, Mock, PropertyMock, patch from freezegun.api import FrozenDateTimeFactory from pyicloud.const import AppleAuthError @@ -23,6 +23,7 @@ DEFAULT_MAX_INTERVAL, DOMAIN, ) +from homeassistant.config_entries import ConfigEntryState from homeassistant.const import CONF_PASSWORD, CONF_USERNAME from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady @@ -628,3 +629,74 @@ async def test_family_device_after_setup_time_2fa( assert state is not None assert state.state == "80" assert state.attributes[ATTR_OWNER_NAME] == MEMBER_1_FULL_NAME + + +async def test_account_with_no_devices_keeps_polling( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that an empty device response does not kill the account. + + iCloud answers with no devices while an account is still settling, and + the status of the first device was read without checking there was one. + """ + service, status = polling_service + service.devices = MockDevicesWithLocation(USER_INFO, []) + + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert config_entry.state is ConfigEntryState.LOADED + assert hass.states.get("sensor.iphone_battery") is None + + # The devices show up on a later poll. + service.devices = MockDevicesWithLocation(USER_INFO, [FakeAppleDevice(status)]) + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert hass.states.get("sensor.iphone_battery").state == "80" + + +async def test_rejected_session_on_device_refresh_asks_the_user( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that a session rejected while reading the devices reaches the user. + + Refreshing the devices is where a stored session is usually turned down, + and that request was handled as an unknown error: retried every couple of + minutes with nothing ever shown to the user. + """ + service, _ = polling_service + + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + type(service).devices = PropertyMock( + side_effect=PyiCloudFailedLoginException("rejected") + ) + + freezer.tick(timedelta(minutes=DEFAULT_MAX_INTERVAL + 1)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert [ + flow + for flow in hass.config_entries.flow.async_progress() + if flow["context"]["source"] == "reauth" + ] + assert config_entry.runtime_data.api is None From 9e2dfc9191946c8e77751a53d9860cee56d42c13 Mon Sep 17 00:00:00 2001 From: Tero Pihlaja Date: Fri, 11 Sep 2026 23:37:09 +0300 Subject: [PATCH 5/5] Ask the user when an authentication status ends the first fetch iCloud reports most rejections as an API response carrying the status rather than as a dedicated exception, and update_devices() now hands those to its caller as well. setup() did not expect them, so one raised by the first device refresh escaped async_setup_entry() and left the entry in SETUP_ERROR with no reauth started. The keep-or-drop decision moves into _ask_to_authenticate(), which both callers now share: a session is only kept when there is a code to send through it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014pPuNx7ySC3snNVfGc2eNs --- homeassistant/components/icloud/account.py | 31 ++++++++++--------- tests/components/icloud/test_account.py | 35 ++++++++++++++++++++++ 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/homeassistant/components/icloud/account.py b/homeassistant/components/icloud/account.py index a6bf921eef387..8754ef62697d8 100644 --- a/homeassistant/components/icloud/account.py +++ b/homeassistant/components/icloud/account.py @@ -153,6 +153,10 @@ def setup(self) -> None: """ try: self._setup() + except PyiCloudAPIResponseException as err: + if not _is_auth_error(err): + raise + self._ask_to_authenticate(err) except ( PyiCloud2FARequiredException, PyiCloudFailedLoginException, @@ -323,9 +327,16 @@ def update_devices(self) -> None: self._schedule_next_fetch() def _ask_to_authenticate(self, err: Exception) -> None: - """Ask the user to log in again after iCloud rejected the session.""" - if isinstance(err, PyiCloud2FARequiredException) or ( - self.api is not None and self.api.requires_2fa + """Ask the user to log in again after iCloud rejected the session. + + The session is only kept when there is a code to send through it. An + authentication status on an API response is not that case: it says the + session itself has to be established again, and one that has just been + rejected cannot carry a code. + """ + if not isinstance(err, PyiCloudAPIResponseException) and ( + isinstance(err, PyiCloud2FARequiredException) + or (self.api is not None and self.api.requires_2fa) ): # Keep the session: the reauth flow reuses it to validate the code, # and async_step_reauth sends a None api back to the password form @@ -499,19 +510,7 @@ def keep_alive(self, now=None) -> None: # handling below so it keeps being retried, rather than # parking the account for credentials that are not at fault. raise - # Reported as a login failure: the status says the session has - # to be established again, and there is no code to send through - # one that has just been rejected. - self.api = None - _LOGGER.error( - ( - "Your iCloud account for '%s' is no longer working; Go to the " - "Integrations menu and click on Configure on the discovered " - "Apple iCloud card to login again" - ), - self._config_entry.data[CONF_USERNAME], - ) - self._require_reauth() + self._ask_to_authenticate(err) self._fetch_interval = self._max_interval self._schedule_next_fetch() except ( diff --git a/tests/components/icloud/test_account.py b/tests/components/icloud/test_account.py index a5021e2f96223..6b7d85c136e0f 100644 --- a/tests/components/icloud/test_account.py +++ b/tests/components/icloud/test_account.py @@ -700,3 +700,38 @@ async def test_rejected_session_on_device_refresh_asks_the_user( if flow["context"]["source"] == "reauth" ] assert config_entry.runtime_data.api is None + + +async def test_auth_status_on_first_device_refresh_asks_the_user( + hass: HomeAssistant, + polling_service: tuple[MagicMock, dict], + freezer: FrozenDateTimeFactory, +) -> None: + """Test that an authentication status on the first fetch reaches the user. + + iCloud reports most rejections as an API response carrying the status + rather than as a dedicated exception, and the first device refresh runs + inside setup(), where an unhandled one would leave a dead entry. + """ + service, _ = polling_service + type(service).devices = PropertyMock( + side_effect=PyiCloudAPIResponseException( + "Authentication required for Account.", AppleAuthError.LOGIN_TOKEN_EXPIRED + ) + ) + + config_entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_CONFIG, entry_id="test", unique_id=USERNAME + ) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert config_entry.state is ConfigEntryState.LOADED + assert [ + flow + for flow in hass.config_entries.flow.async_progress() + if flow["context"]["source"] == "reauth" + ] + assert config_entry.runtime_data.api is None