diff --git a/pyicloud/base.py b/pyicloud/base.py index 8e3ca2b6..9e2714ee 100644 --- a/pyicloud/base.py +++ b/pyicloud/base.py @@ -31,6 +31,7 @@ PyiCloud2FARequiredException, PyiCloudAcceptTermsException, PyiCloudAPIResponseException, + PyiCloudConnectionException, PyiCloudFailedLoginException, PyiCloudNoTrustedNumberAvailable, PyiCloudPasswordException, @@ -636,6 +637,9 @@ def _srp_authentication(self, pause_2fa: bool = False) -> bool: headers=self._get_auth_headers(), ) response.raise_for_status() + except PyiCloudConnectionException: + # Not a credentials problem; let it through as itself. + raise except ( PyiCloudAPIResponseException, HTTPError, @@ -697,6 +701,9 @@ def _srp_authentication(self, pause_2fa: bool = False) -> bool: PyiCloudNoTrustedNumberAvailable, ) as error: LOGGER.debug("Automatic 2FA code delivery failed: %s", error) + except PyiCloudConnectionException: + # Not a credentials problem; let it through as itself. + raise except PyiCloudAPIResponseException as error: msg = "Invalid email/password combination." raise PyiCloudFailedLoginException(msg) from error @@ -755,6 +762,9 @@ def _authenticate_with_token(self, require_trust: bool = True) -> None: self._hsa2_boot_context = None self._clear_trusted_device_bridge_state() self._set_two_factor_delivery_state("unknown") + except PyiCloudConnectionException: + # Not a credentials problem; let it through as itself. + raise except (PyiCloudAPIResponseException, HTTPError) as error: msg = "Invalid authentication token." raise PyiCloudFailedLoginException(msg, error) from error @@ -773,6 +783,9 @@ def _authenticate_with_credentials_service(self, service: str | None) -> None: self._handle_accept_terms(login_data) self.data = self._validate_token() + except PyiCloudConnectionException: + # Not a credentials problem; let it through as itself. + raise except PyiCloudAPIResponseException as error: msg = "Invalid email/password combination." raise PyiCloudFailedLoginException(msg, error) from error @@ -1261,6 +1274,9 @@ def validate_2fa_code(self, code: str) -> bool: self._validate_trusted_device_code(code) except PyiCloudTrustedDeviceVerificationException: raise + except PyiCloudConnectionException: + # Never seen by Apple, so it says nothing about the code. + raise except PyiCloudAPIResponseException: # Wrong verification code LOGGER.error("Code verification failed.") @@ -1320,6 +1336,9 @@ def trust_session(self) -> bool: self._authenticate_with_token() LOGGER.debug("Session trust successful.") return True + except PyiCloudConnectionException: + # Apple never answered, so the session was not refused. + raise except (PyiCloudAPIResponseException, PyiCloud2FARequiredException): LOGGER.error("Session trust failed.") return False diff --git a/pyicloud/exceptions.py b/pyicloud/exceptions.py index 2a498fc7..7714d495 100644 --- a/pyicloud/exceptions.py +++ b/pyicloud/exceptions.py @@ -57,6 +57,26 @@ class PyiCloudPCSTimeoutException(PyiCloudAPIResponseException): """Raised when PCS access could not be granted after all retries.""" +class PyiCloudConnectionException(PyiCloudAPIResponseException): + """Raised when iCloud could not be reached at all. + + A connection error, DNS failure or timeout says nothing about whether the + caller's credentials or session are still good, only that no answer came + back. Callers need to tell the two apart because the remedies are opposite: + a rejected password is fixed by asking the user to log in again, while an + unreachable service is fixed by waiting and retrying. + + Distinguishing it matters most during authentication, where every transport + failure used to be reported as a failed login. A consumer acting on that + asks the user to re-enter working credentials in the middle of an outage, + and, if it stops polling until they do, stays down long after iCloud has + come back. + + Subclasses the generic response error so existing handlers keep catching + it, and carries no ``code`` because no HTTP response was received. + """ + + class PyiCloudEndpointGoneException(PyiCloudAPIResponseException): """Raised when Apple reports an endpoint as permanently gone (HTTP 410). diff --git a/pyicloud/session.py b/pyicloud/session.py index eb6e6132..8cbdddea 100644 --- a/pyicloud/session.py +++ b/pyicloud/session.py @@ -28,6 +28,7 @@ PyiCloud2SARequiredException, PyiCloudAPIResponseException, PyiCloudAuthRequiredException, + PyiCloudConnectionException, PyiCloudEndpointGoneException, PyiCloudServiceNotActivatedException, ) @@ -376,7 +377,9 @@ def _raise_request_exception(err: requests.exceptions.RequestException) -> NoRet reason=err.response.text, code=err.response.status_code, ) from err - raise PyiCloudAPIResponseException("Request failed to iCloud") from err + # No response came back at all, so this is a connection error rather + # than anything iCloud said about the request. + raise PyiCloudConnectionException("Request failed to iCloud") from err def _handle_request_error( self, diff --git a/tests/test_base.py b/tests/test_base.py index 0c2eb4ad..0eed647f 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -25,6 +25,7 @@ PyiCloud2SARequiredException, PyiCloudAcceptTermsException, PyiCloudAPIResponseException, + PyiCloudConnectionException, PyiCloudEndpointGoneException, PyiCloudFailedLoginException, PyiCloudPCSTimeoutException, @@ -3283,6 +3284,92 @@ def test_authenticate_with_token_require_trust_true_raises_for_paused_session( pyicloud_service._authenticate_with_token(require_trust=True) +def test_authenticate_with_token_keeps_connection_failure_distinct( + pyicloud_service: PyiCloudService, monkeypatch: pytest.MonkeyPatch +) -> None: + """An unreachable iCloud must not be reported as a failed login. + + Callers act on the difference: a rejected password is fixed by asking the + user to log in again, an outage by retrying later. + """ + monkeypatch.setattr( + pyicloud_service.session, + "post", + MagicMock(side_effect=PyiCloudConnectionException("Request failed to iCloud")), + ) + pyicloud_service.session._data = { + "session_token": "a-token", + "account_country": "USA", + "trust_token": "", + } + + with pytest.raises(PyiCloudConnectionException): + pyicloud_service._authenticate_with_token() + + +def test_authenticate_with_token_still_reports_rejected_token( + pyicloud_service: PyiCloudService, monkeypatch: pytest.MonkeyPatch +) -> None: + """An answer from iCloud rejecting the token is still a failed login.""" + monkeypatch.setattr( + pyicloud_service.session, + "post", + MagicMock( + side_effect=PyiCloudAPIResponseException("Authentication required", 421) + ), + ) + pyicloud_service.session._data = { + "session_token": "a-token", + "account_country": "USA", + "trust_token": "", + } + + with pytest.raises(PyiCloudFailedLoginException): + pyicloud_service._authenticate_with_token() + + +def test_validate_2fa_code_does_not_blame_the_code_for_an_outage( + pyicloud_service: PyiCloudService, monkeypatch: pytest.MonkeyPatch +) -> None: + """An unreachable iCloud must not come back as a wrong verification code.""" + monkeypatch.setattr( + pyicloud_service, + "_validate_trusted_device_code", + MagicMock(side_effect=PyiCloudConnectionException("Request failed to iCloud")), + ) + monkeypatch.setattr( + type(pyicloud_service), + "two_factor_delivery_method", + property(lambda _self: "trusted_device"), + ) + + with pytest.raises(PyiCloudConnectionException): + pyicloud_service.validate_2fa_code("123456") + + +def test_trust_session_does_not_report_refusal_for_an_outage( + pyicloud_service: PyiCloudService, monkeypatch: pytest.MonkeyPatch +) -> None: + """An unreachable iCloud must not come back as a refused session trust.""" + monkeypatch.setattr( + pyicloud_service.session, + "get", + MagicMock(side_effect=PyiCloudConnectionException("Request failed to iCloud")), + ) + + with pytest.raises(PyiCloudConnectionException): + pyicloud_service.trust_session() + + +def test_connection_exception_is_still_an_api_response_exception() -> None: + """Existing handlers must keep catching it.""" + error = PyiCloudConnectionException("Request failed to iCloud") + + assert isinstance(error, PyiCloudAPIResponseException) + # No response came back, so there is no status to report. + assert error.code is None + + def test_srp_authentication_pause_2fa_includes_pause2fa_flag( pyicloud_service: PyiCloudService, ) -> None: