diff --git a/python/exporter/icloud.py b/python/exporter/icloud.py index 2eea938d..a1c15fe1 100644 --- a/python/exporter/icloud.py +++ b/python/exporter/icloud.py @@ -163,6 +163,24 @@ class ClientIdentity: """The desktop exporter's, and the default for every entry point here.""" +LOGIN_TIMEOUT_SECONDS = 30 +""" +Seconds any one request may take, in place of FindMy.py's default of five. + +**Five is a per-request cap, and this makes far more than one request.** Signing in is several +round trips, and what follows it - escrow recovery, then the CloudKit fetches that carry the +actual keys - is heavier than the login. A link slow enough to spend five seconds on any one of +them fails partway through with a bare timeout that names no cause. + +Local ADI is the default here and generates its Anisette on this machine, so that part is not +waiting on anybody. It is Apple's own hosts that this is about - and `--anisette-url`, for anyone +who does point it at a server. + +Thirty rather than a larger number because it is what the Android app settled on for the same +sign-in, and the two should not disagree about how patient this protocol needs somebody to be. +""" + + def make_account( anisette_url: str | None = None, libs_path: str | None = None, @@ -200,7 +218,8 @@ def make_account( provider = _make_provider(anisette_url, libs_path, stored, identity) if stored is None: - return AsyncAppleAccount(provider, device_name=identity.device_name) + return AsyncAppleAccount( + provider, device_name=identity.device_name, timeout=LOGIN_TIMEOUT_SECONDS) # Only the ids are restored. The rest of the shape has to be there because the library reads # it, and every field of it is empty on purpose - this is a logged-out account that happens to @@ -215,7 +234,12 @@ def make_account( logger.info("Reusing the stored device identity, so this is not a new device to Apple") - return AsyncAppleAccount(provider, state_info=state, device_name=identity.device_name) + return AsyncAppleAccount( + provider, + state_info=state, + device_name=identity.device_name, + timeout=LOGIN_TIMEOUT_SECONDS, + ) def _make_provider( @@ -226,7 +250,11 @@ def _make_provider( ) -> LocalAnisetteProvider | RemoteAnisetteProvider: """Build the Anisette provider, restoring its provisioning data if there is any.""" if anisette_url is not None: - return RemoteAnisetteProvider(anisette_url, serial=identity.serial) + # Its own session, so raising the account's does nothing for this one: the Anisette fetch + # happens inside the login but from the provider's own client. The local provider below + # takes no timeout, because there is no HTTP in it to spend one on. + return RemoteAnisetteProvider( + anisette_url, serial=identity.serial, timeout=LOGIN_TIMEOUT_SECONDS) saved = (stored or {}).get("anisette") state_blob = None diff --git a/python/test/test_login_timeout.py b/python/test/test_login_timeout.py new file mode 100644 index 00000000..853a8330 --- /dev/null +++ b/python/test/test_login_timeout.py @@ -0,0 +1,103 @@ +""" +How long the exporter is prepared to wait for one request. + +FindMy.py's default is five seconds per request, which is a desktop assumption about a single +call. This makes many: a sign-in is several round trips, and escrow recovery and the CloudKit +fetches after it are heavier than the sign-in. A link slow enough to spend five seconds on any one +of them fails partway through with a bare timeout that names no cause. + +**Asserted on what the call passes, not on FindMy.py's internals.** The timeout ends up on a +private session attribute with no accessor, and a test reaching in there breaks on a rename that +changed nothing real. What can actually regress here is this module forgetting to pass it - which +is the whole bug, since the default is silent. +""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from exporter import device, icloud + + +@pytest.fixture +def built(monkeypatch): + """Record what `make_account` hands to the library, instead of building a real account.""" + seen: dict[str, Any] = {} + + class RecordingAccount: + def __init__(self, provider, **kwargs): + seen["provider"] = provider + seen["kwargs"] = kwargs + + monkeypatch.setattr(icloud, "AsyncAppleAccount", RecordingAccount) + + return seen + + +@pytest.fixture +def no_stored_identity(monkeypatch): + monkeypatch.setattr(device, "load", lambda *_args, **_kwargs: None) + monkeypatch.setattr(icloud.device, "load", lambda *_args, **_kwargs: None) + + +@pytest.fixture +def a_stored_identity(monkeypatch): + """An install that has run before, which is the path that is easy to miss.""" + stored = {"uid": "a-uid", "devid": "a-devid", "anisette": None} + monkeypatch.setattr(icloud.device, "load", lambda *_args, **_kwargs: stored) + + return stored + + +class TestTheAccount: + def test_a_first_run_gets_the_raised_timeout(self, built, no_stored_identity): + icloud.make_account(provider=object()) + + assert built["kwargs"]["timeout"] == icloud.LOGIN_TIMEOUT_SECONDS + + def test_a_later_run_gets_it_too(self, built, a_stored_identity): + # Two constructions of the same object, and only one of them was reached by the fix that + # first added a keyword here. The second is the one nearly every export takes. + icloud.make_account(provider=_JsonableProvider()) + + assert built["kwargs"]["timeout"] == icloud.LOGIN_TIMEOUT_SECONDS + + def test_it_is_longer_than_the_library_default(self): + from findmy.util.http import DEFAULT_TIMEOUT + + # If upstream ever raises its own default past this, passing ours would be a downgrade + # rather than a fix, and nothing else here would notice. + assert icloud.LOGIN_TIMEOUT_SECONDS > DEFAULT_TIMEOUT + + +class TestTheAnisetteProvider: + """ + Its own session, so raising the account's does nothing for this one. + + The Anisette fetch happens inside the login but from the provider's own client. Asserted + through `to_json`, which is the only place the value is visible without reaching into a + private attribute - it is carried there precisely so a restored session keeps it. + """ + + def test_a_remote_server_gets_the_raised_timeout(self): + provider = icloud._make_provider("https://ani.example.invalid", None, None) + + assert provider.to_json()["timeout"] == icloud.LOGIN_TIMEOUT_SECONDS + + def test_a_restored_session_keeps_it(self): + # `to_json` omits the timeout when it equals the default, so a session restored from a + # blob written without one silently goes back to five seconds. + original = icloud._make_provider("https://ani.example.invalid", None, None) + + restored = type(original).from_json(original.to_json()) + + assert restored.to_json()["timeout"] == icloud.LOGIN_TIMEOUT_SECONDS + + +class _JsonableProvider: + """Stands in for an Anisette provider, which `make_account` serialises into the state blob.""" + + def to_json(self): + return {"type": "aniLocal"}