diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2687c792..15d6eddf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -277,8 +277,15 @@ Report: `app/build/reports/tests/testDebugUnitTest/index.html` ### Chaquopy bridge tests -Cover `app/src/main/python/main.py`, the module Chaquopy packages into the APK. It imports -no Android or Java types, so it runs on plain CPython. +Cover everything under `app/src/main/python/`, which Chaquopy packages into the APK: `main.py`, +`identity.py` and `icloud_bridge.py`. None of them import Android or Java types, so they run on +plain CPython — that constraint is what makes them testable at all, and it is worth keeping. + +They also reach the shared `python/` tree, because `conftest.py` puts it on the path exactly as +the build packages it. What that cannot tell you is whether a module survives *being* packaged — +`icloud_bridge` reaches `findmy.cloudkit` and therefore protobuf, which resolves on a laptop and +is the shape of thing that goes missing on a phone. `PythonPackagingTest`, on the managed device, +is what answers that half. ```bash python -m venv .venv diff --git a/app/build.gradle.kts b/app/build.gradle.kts index f610a85e..8f4f542f 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -290,6 +290,12 @@ chaquopy { install("git+https://github.com/parawanderer/FindMy.py@23a9b8d7109b405f8362ea1e69ebe51f9ca82fca") install("NSKeyedUnArchiver==1.5") + + // OPENTAGVIEWER.yml, read and written by opentagviewer_export.bundle - which the app + // reaches as soon as it touches AccessoryExport, and will need in earnest once it + // writes bundles of its own. Pinned to the exporter's version in python/pyproject.toml + // so one repository ships one PyYAML, for the same reason it ships one FindMy.py. + install("PyYAML==6.0.2") } } productFlavors {} @@ -315,7 +321,22 @@ chaquopy { // has no top-level .py files for it to catch by accident, and the test is what // notices if that stops being true. srcDir("../python") - include("*.py", "opentagviewer_export/**") + // **Named files from `exporter/`, not the package.** That directory also holds the + // tkinter wizard, the questionary CLI and prompt_toolkit prompts, none of which + // exist on Android - importing the package wholesale would drag them in. These four + // are stdlib plus FindMy.py, and `exporter/__init__.py` is empty, so importing + // `exporter.icloud` reaches none of the rest. + // + // A list rather than a glob for the same reason: a module added to `exporter/` later + // should have to be considered before it ships in the APK, not swept in. + include( + "*.py", + "opentagviewer_export/**", + "exporter/__init__.py", + "exporter/icloud.py", + "exporter/device.py", + "exporter/identity.py", + ) // The package's own test suite is not part of the app. It imports pytest, which is // not in the APK, so it is dead weight that would fail if anything ever touched it. exclude("opentagviewer_export/tests/**") diff --git a/app/src/androidTest/java/dev/wander/android/opentagviewer/python/PythonPackagingTest.java b/app/src/androidTest/java/dev/wander/android/opentagviewer/python/PythonPackagingTest.java index 3f146fc8..0f0a500f 100644 --- a/app/src/androidTest/java/dev/wander/android/opentagviewer/python/PythonPackagingTest.java +++ b/app/src/androidTest/java/dev/wander/android/opentagviewer/python/PythonPackagingTest.java @@ -125,12 +125,83 @@ public void theExportersTestsAreNotOnThePath() { * *

It imports tkinter, which does not exist here - so a build that shipped it would fail * at import time on a phone rather than at build time on a desktop. + * + *

Note this is now a statement about named modules, not about the package. Four + * files from {@code exporter/} are packaged deliberately - see below - so "the directory is + * absent" stopped being the guarantee and "the interactive parts are absent" took over. */ @Test public void theDesktopWizardIsNotPackaged() { - assertThrows("python/exporter/ is the tkinter wizard and must not reach the APK", + assertThrows("exporter/asyncui.py drives tkinter and must not reach the APK", Exception.class, () -> Python.getInstance().getModule("exporter.asyncui")); + assertThrows("exporter/wizard.py is the tkinter window itself", + Exception.class, + () -> Python.getInstance().getModule("exporter.wizard")); + assertThrows("exporter/prompts.py needs prompt_toolkit and a terminal", + Exception.class, + () -> Python.getInstance().getModule("exporter.prompts")); + assertThrows("exporter/cli.py needs questionary", + Exception.class, + () -> Python.getInstance().getModule("exporter.cli")); + } + + /** + * The iCloud pipeline imports, which is what lets the app read an account without a zip. + * + *

Packaged and importable are different claims, and only the second one matters. + * These four modules are named individually in the Chaquopy {@code include} precisely because + * their neighbours cannot be imported here - so the thing worth asserting is that pulling + * {@code exporter.icloud} in does not transitively reach tkinter, questionary or + * prompt_toolkit. A build where it did would fail on a phone, at the moment somebody tried + * to sign in, having built cleanly on a desktop. + */ + @Test + public void theicloudPipelineIsPackagedAndImports() { + for (final String module : new String[]{ + "exporter", "exporter.icloud", "exporter.device", "exporter.identity"}) { + assertNotNull(module + " must be importable in the APK", + Python.getInstance().getModule(module)); + } + } + + /** + * And it still knows who the exporter is, so the desktop side is unchanged. + * + *

The identity became a parameter so the app can present its own serial rather than the + * exporter's - two programs sharing one are one device to Apple, and removing either from + * the device list would break the other. This asserts the default did not move while that + * was done. + */ + @Test + public void theexporterKeepsItsOwnIdentityByDefault() { + final PyObject icloud = Python.getInstance().getModule("exporter.icloud"); + final PyObject identity = icloud.get("EXPORTER_IDENTITY"); + + assertNotNull("exporter.icloud must still expose its default identity", identity); + assertEquals("0PENTAGXPORT", identity.get("serial").toString()); + } + + /** + * And the app's own side of it imports, presenting the app's identity rather than the + * exporter's. + * + *

The one thing a desktop test suite cannot say. {@code test_icloud_bridge.py} runs the + * whole flow against fakes on CPython, which proves the logic and proves nothing about + * whether the module survives being packaged - it reaches {@code findmy.cloudkit}, and + * therefore protobuf, which is a dependency with a native half and the exact shape of thing + * that resolves on a laptop and is missing on a phone. + * + *

The serial is asserted here as well as in the Python tests because this is where it is + * real. Rule 11: a phone presenting {@code 0PENTAGXPORT} would share a device-list entry + * with the desktop exporter, and removing either would break the other. + */ + @Test + public void theappsIcloudBridgeIsPackagedAndKnowsWhoItIs() { + final PyObject bridge = Python.getInstance().getModule("icloud_bridge"); + + assertNotNull("icloud_bridge must be importable in the APK", bridge); + assertEquals("0PENTAGVIEWR", bridge.get("APP_IDENTITY").get("serial").toString()); } /** diff --git a/app/src/androidTest/java/dev/wander/android/opentagviewer/util/parse/CustomAccessoryImportTest.java b/app/src/androidTest/java/dev/wander/android/opentagviewer/util/parse/CustomAccessoryImportTest.java index 85d77b18..dc47c8b8 100644 --- a/app/src/androidTest/java/dev/wander/android/opentagviewer/util/parse/CustomAccessoryImportTest.java +++ b/app/src/androidTest/java/dev/wander/android/opentagviewer/util/parse/CustomAccessoryImportTest.java @@ -17,7 +17,9 @@ import dev.wander.android.opentagviewer.ui.BeaconIcon; import dev.wander.android.opentagviewer.db.repo.model.BeaconData; import dev.wander.android.opentagviewer.db.repo.model.ImportData; +import dev.wander.android.opentagviewer.db.room.entity.BeaconNamingRecord; import dev.wander.android.opentagviewer.db.room.entity.OwnedBeacon; +import dev.wander.android.opentagviewer.db.util.BeaconCombinerUtil; import dev.wander.android.opentagviewer.db.room.entity.UserBeaconOptions; import org.json.JSONObject; @@ -200,4 +202,85 @@ public void anapplePairedTagStillTakesTheOldPath() { !CustomAccessoryParser.isCustomAccessory( new BeaconData(paired.id, paired, null, null))); } + + /** + * The one that was missing, and the reason all of the above passed while the feature did not + * work. + * + *

Every test before this hands {@link BeaconDataParser} a {@code BeaconData} it built + * itself, which skips the step that actually decides what reaches a screen. + * {@link BeaconCombinerUtil#combine} used to iterate the naming records - so a tag + * with none was dropped on the floor before the parser was ever asked about it. The import + * reported "1 device", the fetch path collected reports for it happily, and it appeared + * nowhere. + * + *

So this goes through the join, and the two that follow go through it for the two + * screens that call it. + */ + @Test + public void itsurvivesTheJoinThatFeedsEveryScreen() { + final List joined = BeaconCombinerUtil.combine(imported()); + + assertEquals("a tag with no naming record must not be dropped by the join", + 1, joined.size()); + assertEquals(IDENTIFIER, joined.get(0).getBeaconId()); + assertNotNull("and it must keep the row that has its keys in it", + joined.get(0).getOwnedBeaconInfo()); + assertNull("nothing ever named it, so there is nothing to join to", + joined.get(0).getBeaconNamingRecord()); + } + + /** + * The device list's own call, which passes user options as a third list. + * + *

A separate case because it is a different overload, and because the options have to + * survive the change of what the join iterates - they are keyed by beacon id either way, + * but that is worth an assertion rather than an assumption. + */ + @Test + public void thedeviceListSeesItToo() { + final OwnedBeacon row = imported().getOwnedBeacons().get(0); + final UserBeaconOptions chosen = new UserBeaconOptions( + row.id, System.currentTimeMillis(), "My hidden bike", "🚲"); + + final List joined = BeaconCombinerUtil.combine( + List.of(row), List.of(), List.of(chosen)); + + assertEquals(1, joined.size()); + assertNotNull("the user's own name and emoji must still find it", + joined.get(0).getUserBeaconOptions()); + } + + /** End to end, as the screen does it: import, join, parse, and read the name off it. */ + @Test + public void thewholeChainProducesSomethingToShow() { + final BeaconInformation info = + BeaconDataParser.parse(BeaconCombinerUtil.combine(imported())).get(0); + + assertEquals(NAME, info.getName()); + assertTrue(info.isCustomAccessory()); + } + + /** + * And an Apple tag still comes out of the join whole. + * + *

The risk in turning the join around is the mirror of the bug it fixes: driving from the + * owned beacons could just as easily leave the naming record behind, which would cost every + * real tag its name. + */ + @Test + public void anapplePairedTagKeepsItsNamingRecord() { + final String id = "2C2A1B0C-9D8E-4F6A-8B4C-3D2E1F0A9B8C"; + final OwnedBeacon paired = OwnedBeacon.builder().id(id).content("").build(); + final BeaconNamingRecord named = + BeaconNamingRecord.builder().id(id).content("").build(); + + final List joined = + BeaconCombinerUtil.combine(List.of(paired), List.of(named), List.of()); + + assertEquals(1, joined.size()); + assertNotNull(joined.get(0).getOwnedBeaconInfo()); + assertNotNull("without this an Apple tag loses its name and emoji", + joined.get(0).getBeaconNamingRecord()); + } } diff --git a/app/src/main/java/dev/wander/android/opentagviewer/db/repo/model/BeaconData.java b/app/src/main/java/dev/wander/android/opentagviewer/db/repo/model/BeaconData.java index 8840aa88..d32eded8 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/db/repo/model/BeaconData.java +++ b/app/src/main/java/dev/wander/android/opentagviewer/db/repo/model/BeaconData.java @@ -11,6 +11,13 @@ public class BeaconData { private final String beaconId; private final OwnedBeacon ownedBeaconInfo; + /** + * What an Apple device wrote about this tag - its name and emoji. + * + *

Null for a tag that was never in an Apple account, because nothing ever named it. The + * name for one of those comes out of the accessory JSON instead; see + * {@link dev.wander.android.opentagviewer.util.parse.CustomAccessoryParser}. + */ private final BeaconNamingRecord beaconNamingRecord; /** * Optional, only if configured diff --git a/app/src/main/java/dev/wander/android/opentagviewer/db/util/BeaconCombinerUtil.java b/app/src/main/java/dev/wander/android/opentagviewer/db/util/BeaconCombinerUtil.java index ff1dd458..c58e9481 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/db/util/BeaconCombinerUtil.java +++ b/app/src/main/java/dev/wander/android/opentagviewer/db/util/BeaconCombinerUtil.java @@ -18,24 +18,42 @@ @NoArgsConstructor(access = AccessLevel.PRIVATE) public final class BeaconCombinerUtil { + /** + * Join a tag's rows into the shape the UI reads. + * + *

Driven by the owned beacons, because that is what a tag is. A + * {@link BeaconNamingRecord} is something an Apple device wrote about a tag - a name, + * an emoji - and a tag that was never in an Apple account has none, because no iPad ever + * named it. Iterating the naming records instead made those tags invisible to every screen + * built on this, while the fetch path - which reads owned beacons directly - went on + * collecting reports for them perfectly happily. An imported self-generated tag reported + * "1 device imported" and then appeared nowhere. + * + *

For an Apple tag nothing changes: the importer inner-joins the two sets, so every owned + * beacon has exactly one naming record and vice versa. Driving from this side is in fact the + * safer of the two - the old direction could hand out a {@code BeaconData} whose + * {@code ownedBeaconInfo} was null, which is the half nothing downstream can work without. + * + * @return one entry per owned beacon, with a null naming record where there is none. + */ public static List combine( final List ownedBeacons, final List beaconNamingRecords, final List userBeaconOptions) { - Map idToBeaconMap = ownedBeacons.stream() - .collect(Collectors.toMap((beacon) -> beacon.id, beacon -> beacon)); + Map idToNamingRecordMap = beaconNamingRecords.stream() + .collect(Collectors.toMap((namingRec) -> namingRec.id, namingRec -> namingRec)); Map idToOptionsMap = userBeaconOptions.stream() .collect(Collectors.toMap((options) -> options.beaconId, options -> options)); - return beaconNamingRecords.stream() - .map(namingRec -> new BeaconData( - namingRec.id, - idToBeaconMap.get(namingRec.id), - namingRec, - idToOptionsMap.getOrDefault(namingRec.id, null) + return ownedBeacons.stream() + .map(beacon -> new BeaconData( + beacon.id, + beacon, + idToNamingRecordMap.getOrDefault(beacon.id, null), + idToOptionsMap.getOrDefault(beacon.id, null) )) .collect(Collectors.toList()); } diff --git a/app/src/main/java/dev/wander/android/opentagviewer/util/parse/BeaconDataParser.java b/app/src/main/java/dev/wander/android/opentagviewer/util/parse/BeaconDataParser.java index 2f1eb500..e345a183 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/util/parse/BeaconDataParser.java +++ b/app/src/main/java/dev/wander/android/opentagviewer/util/parse/BeaconDataParser.java @@ -7,6 +7,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Optional; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; @@ -75,7 +76,6 @@ public static List parse(final List rawBeaconData continue; } - final String beaconNamingRecordPList = beaconData.getBeaconNamingRecord().content; final String ownedBeaconPList = beaconData.getOwnedBeaconInfo().content; // Extract the most relevant fields from decrypted plist files @@ -87,13 +87,34 @@ public static List parse(final List rawBeaconData // multiple `.plist` files per `beacon-identifier`. // In my testing thus far the directory for a single `beacon-identifier` // only contained a single `.plist` file. - final Document beaconNamingData = XmlParser.parse(beaconNamingRecordPList); - final String beaconNamingRecordIdentifier = getString(beaconNamingData, identifierQuery); - final String associatedBeacon = getString(beaconNamingData, associatedBeaconQuery); - final String emoji = getString(beaconNamingData, emojiQuery); - final String name = getString(beaconNamingData, nameQuery); - // nested PLIST evaluation (this contains some useful info) - var metaData = BeaconNamingRecordInnerParser.extractBeaconNamingRecordMetadata(xPath, beaconNamingData); + // + // **All of it is optional.** A zip always carries the pair, because its importer + // inner-joins them, but an account read directly does not have to: CloudKit + // holds no naming record for an accessory nobody ever named. That is not a + // broken tag and must not be treated as one - it is a tag with no name yet, + // which the user can give it like any other. Skipping it here made it look like + // the import had lost it. + String beaconId = beaconData.getBeaconId(); + String beaconNamingRecordIdentifier = null; + String emoji = null; + String name = null; + Optional metaData = Optional.empty(); + + if (beaconData.getBeaconNamingRecord() != null) { + final Document beaconNamingData = + XmlParser.parse(beaconData.getBeaconNamingRecord().content); + beaconNamingRecordIdentifier = getString(beaconNamingData, identifierQuery); + final String associatedBeacon = + getString(beaconNamingData, associatedBeaconQuery); + if (associatedBeacon != null && !associatedBeacon.isEmpty()) { + beaconId = associatedBeacon; + } + emoji = getString(beaconNamingData, emojiQuery); + name = getString(beaconNamingData, nameQuery); + // nested PLIST evaluation (this contains some useful info) + metaData = BeaconNamingRecordInnerParser.extractBeaconNamingRecordMetadata( + xPath, beaconNamingData); + } // Extract the most relevant fields from decrypted plist files @@ -120,10 +141,14 @@ public static List parse(final List rawBeaconData var extractedData = BeaconInformation.builder() // BeaconNamingRecord - .beaconId(associatedBeacon) + .beaconId(beaconId) .namingRecordId(beaconNamingRecordIdentifier) .originalEmoji(emoji) - .originalName(name) + // What the tag's own record says it is, when nobody has ever named it - + // "AirTag" rather than a blank row. Not a name pretending to be the + // user's choice: it is what Find My shows for an unnamed accessory, + // and renaming it works exactly as it does for any other tag. + .originalName(name == null || name.isEmpty() ? model : name) // BeaconNamingRecord->cloudKitMetadata .namingRecordModifiedTime( metaData.map(BeaconNamingRecordCloudKitMetadata::getModifiedTime).orElse(null)) diff --git a/app/src/main/python/icloud_bridge.py b/app/src/main/python/icloud_bridge.py new file mode 100644 index 00000000..a88d162c --- /dev/null +++ b/app/src/main/python/icloud_bridge.py @@ -0,0 +1,408 @@ +""" +Reading an Apple account directly, instead of importing a zip somebody made on a Mac. + +The desktop exporter is a UI over :mod:`exporter.icloud`, and that module is pure Python with no +desktop in it - so the app can run the same pipeline. This is the layer between it and Java: +one flow, four steps, each a separate call because a person has to answer something between +them. + + open -> recoveryOptions -> unlock(serial, passcode) -> fetch + +**Why a session object rather than four functions.** Two of the steps need what the previous one +opened - a keychain session and a Find My client, both of which hold sockets - and the passcode +step waits on a dialog. Java holds the session for as long as that takes and closes it when the +screen goes away. + +**Everything here returns a JSON string, including failures.** A raised Python exception arrives +in Java as a `PyException` whose message is whatever `str()` produced, which for several of the +errors worth reporting here is the empty string. The app has already shipped one dialog reading +`Login failed:` with nothing after it. So a failure is a value: `{"ok": false, "reason": ..., +"message": ...}`, and the `reason` is what Java branches on so the wording stays in +`strings.xml` where it can be translated. + +**Nothing here writes to the account.** Recovery unwraps shares to a key; it does not enrol this +device as a peer, sign anything, or create a record. That is a deliberate line, and the moment +something crosses it this docstring should stop saying so. +""" + +from __future__ import annotations + +import json +import plistlib +import traceback +from typing import Any + +import identity as app_identity +from exporter import icloud +from findmy.keychain.recovery import RecoveryError + +REASON_NOT_SIGNED_IN = "not_signed_in" +"""The account handed over is not in a state that can talk to iCloud.""" + +REASON_NOTHING_TO_RECOVER_FROM = "nothing_to_recover_from" +""" +The account has no escrow record this can unlock the keychain with. + +**The expected answer for a real class of user**, not an error: somebody who has an Apple ID but +has never owned an iPhone, iPad or Mac has nothing that ever escrowed a keychain. There is no +way for them to reach the tags in an account, and no amount of retrying changes that. +""" + +REASON_SERVICE_UNSURE = "service_unsure" +""" +Nothing was reported usable *at all*, which reads as a service having a bad day. + +Distinguished from :data:`REASON_NOTHING_TO_RECOVER_FROM` because the advice is opposite: this +one is worth trying again later, and that one never will be. FindMy.py draws the same +distinction through `viability_is_trustworthy`, for the same reason. +""" + +REASON_PASSCODE_REJECTED = "passcode_rejected" +"""The escrow service did not accept the passcode. **Not proof it was wrong** - see below.""" + +REASON_NO_SUCH_RECORD = "no_such_record" +"""The serial Java asked to unlock with is not in the list it was given.""" + +REASON_NO_SUCH_ACCESSORY = "no_such_accessory" +"""An id was asked for that this session never fetched, or has not fetched since reopening.""" + + +REASON_UNKNOWN = "unknown" +"""Anything else, with the exception text carried through so a report can be answered.""" + + +APP_IDENTITY = icloud.ClientIdentity( + serial=app_identity.APP_SERIAL, + device_name=app_identity.APP_CLOUDKIT_DEVICE_NAME, +) +""" +Who this app says it is to CloudKit. + +Rule 11: the same identity every path already sends. Defaulting this would present Apple with +`0PENTAGXPORT` - the *desktop exporter* - from a phone, and the two would share one device-list +entry that neither could be removed from safely. +""" + + +def _failure(reason: str, message: str) -> str: + return json.dumps({"ok": False, "reason": reason, "message": message}) + + +def _unexpected(what: str) -> str: + """ + Report a failure nothing anticipated, with the traceback in the log and the text in the UI. + + Both halves matter. The log is where a maintainer looks and the dialog is where the user is, + and an empty dialog is what sent somebody to the issue tracker last time. + """ + detail = traceback.format_exc() + print(f"iCloud bridge: {what} failed:\n{detail}") + + # `str(e)` is empty for several of these - TimeoutError most of all - so the last line of + # the traceback stands in. It names the exception type, which is not a good message but is + # infinitely better than a colon with nothing after it. + lastLine = detail.strip().splitlines()[-1] if detail.strip() else "" + + return _failure(REASON_UNKNOWN, lastLine or f"{what} failed for an unrecorded reason") + + +def _asyncAccount(account: Any) -> Any: + """ + The async account inside the app's synchronous one. + + `AppleAccount` is a thin wrapper that owns an `AsyncAppleAccount` and an event loop, and + FindMy.py exposes neither. Reaching in rather than restoring a second account from the same + stored JSON, because a second account is a second `aiohttp` session on a second loop + presenting the same identity - two clients, one device, and sockets nobody closes. + + :mod:`main` already reaches for the same attribute when it swaps the Anisette provider, with + the same guard: if a rename upstream makes this None, the caller reports a clean failure + rather than raising something unreadable. + """ + return getattr(account, "_asyncacc", None) + + +class ICloudSession: + """ + One conversation with iCloud, held open across the calls a person has to answer. + + Java constructs this through :func:`openSession`, calls the steps in order, and calls + :meth:`close` when the screen is finished with - in a `finally`, because two of these steps + hold sockets and an abandoned session leaks them for the life of the process. + + **Not thread-safe, and it cannot be made so.** Every step runs a coroutine on the account's + own event loop, and a loop cannot be driven from two threads at once. Java calls these from + a single Rx chain. + """ + + def __init__(self, account: Any, asyncAccount: Any, loop: Any) -> None: + self._account = account + self._async = asyncAccount + self._loop = loop + self._client: Any = None + self._records: list[Any] = [] + self._candidates: dict[str, Any] = {} + + def open(self) -> str: + """ + Open the Find My client, which is a keychain session and a CloudKit client. + + Nothing is decrypted yet - that needs keys, and keys need :meth:`unlock`. + """ + if self._client is not None: + return json.dumps({"ok": True}) + + try: + client = self._loop.run_until_complete( + icloud.open_client(self._async, APP_IDENTITY)) + self._loop.run_until_complete(client.__aenter__()) + self._client = client + + return json.dumps({"ok": True}) + except Exception: + return _unexpected("opening the Find My client") + + def recoveryOptions(self) -> str: + """ + What this account could unlock its keychain from, as a list to choose between. + + **The two empty answers are different and are reported differently.** An account with no + recoverable record has nothing this app can ever do for it; one where the service + reported nothing usable at all is very likely a bad afternoon at Apple. Telling a user + the first when it is the second sends them away for good. + + Serials are held here rather than sent to Java and back, so the unlock step names a + record this session actually saw. A record is a live object with key material in it, and + it is not something to reconstruct from a string. + """ + if self._client is None: + return _failure(REASON_NOT_SIGNED_IN, "The Find My client is not open.") + + try: + options = self._loop.run_until_complete(self._client.recovery_options()) + except Exception: + return _unexpected("asking what this account can be recovered from") + + self._records = list(options.recoverable) + + if not self._records: + if not options.viability_is_trustworthy: + return _failure( + REASON_SERVICE_UNSURE, + "Nothing was reported usable at all, which reads as a service having a bad" + " day rather than an account with nothing to recover from.") + + return _failure( + REASON_NOTHING_TO_RECOVER_FROM, + "No record on this account can currently be recovered from.") + + return json.dumps({ + "ok": True, + "devices": [ + { + "serial": record.serial, + "description": record.describe(), + } + for record in self._records + ], + }) + + def unlock(self, serial: str, passcode: str) -> str: + """ + Recover the keychain keys with one device's screen-lock passcode. + + **One attempt per call, and the retry belongs to Java.** :func:`exporter.icloud.unlock` + loops with a callback because the CLI has a terminal to ask at; here the question is a + dialog, and a Python function blocking a Java thread while it waits for one is a worse + shape than returning and being called again. + + That leaves the cap - `MAX_UNLOCK_ATTEMPTS`, three - on Java's side, and it has to be + respected there: attempts are probably a limited resource on Apple's end, and what this + particular service allows is not established. + + **A rejection is not proof the passcode was wrong.** FindMy.py's own first advice is to + try again with the same passcode, because the exchange has been seen to fail + intermittently and then succeed. The message says so; do not reword it into "incorrect + passcode". + """ + if self._client is None: + return _failure(REASON_NOT_SIGNED_IN, "The Find My client is not open.") + + record = next((r for r in self._records if r.serial == serial), None) + if record is None: + return _failure( + REASON_NO_SUCH_RECORD, + f"No recoverable device in this session has the serial {serial!r}.") + + try: + self._loop.run_until_complete(self._client.unlock(record, passcode)) + + return json.dumps({"ok": True}) + except RecoveryError as e: + # The library's own text, whole. It says what was rejected and then what is worth + # doing about it, in the order worth doing it. + print(f"iCloud bridge: escrow recovery rejected a passcode for {serial}") + + return _failure(REASON_PASSCODE_REJECTED, str(e) or "The passcode was not accepted.") + except Exception: + return _unexpected("unlocking the keychain") + finally: + # Not this module's to hold a moment longer than the call needs it. + del passcode + + def fetch(self) -> str: + """ + Read and decrypt the account's accessories, and describe what is there. + + **Descriptions only - no key material.** The records themselves come from + :meth:`records`, for the ones the user actually picks. Rendering every accessory's + private key into a JSON string that crosses into Java, so that a screen can show a list + of names, is more of the secret in more places than the screen needs. + + `hasName` is false for an accessory with no naming record in CloudKit. Not a problem to + solve before importing - the app can show and rename a nameless tag perfectly well - but + worth knowing in a picker, where `label` would otherwise read "unnamed" and `details` is + the only thing telling one from another. + """ + if self._client is None: + return _failure(REASON_NOT_SIGNED_IN, "The Find My client is not open.") + + try: + fetched = self._loop.run_until_complete(icloud.fetch(self._client)) + except Exception: + return _unexpected("reading the account's accessories") + + self._candidates = {c.beacon_id: c for c in fetched.candidates} + + return json.dumps({ + "ok": True, + "accessories": [ + { + "beaconId": candidate.beacon_id, + "name": candidate.name, + "emoji": candidate.emoji, + # What to show when it has no name of its own: what kind of thing it is, the + # serial Find My shows for it, and when it was paired - which is often the + # one a person recognises, because they remember buying it. + "label": candidate.label, + "details": candidate.details, + "hasAlignment": candidate.has_alignment, + "hasName": candidate.name is not None, + } + for candidate in fetched.candidates + ], + # Named rather than dropped quietly: "fewer tags than expected" and "some of those + # were never tags" look identical from outside, and the second is the common one - + # an account's own iPhones and Macs come back in the same records. + "skipped": [ + {"beaconId": skipped.beacon_id, "reason": skipped.reason} + for skipped in fetched.skipped + ], + }) + + def records(self, selectionJson: str) -> str: + """ + The chosen accessories, as the plists the importer already reads. + + **The same documents a bundle carries**, so Java feeds them to the path it has rather + than growing a second one that means the same thing. An accessory read from an account + and one read from a zip become the same rows in the same tables. That is most of why + `opentagviewer_export` is a shared package. + + **Not through `to_export`, and the difference matters.** That function refuses an + accessory with no naming record unless it is given a name to synthesise one from, which + is right for what it is for: a *bundle* is inner-joined by its importer, so an accessory + exported without a name is one silently missing after import. None of that applies here. + Nothing is being written to a zip, the app left-joins the two, and a tag with no naming + record is a thing it already knows how to show - it is what a self-generated tag is. + + So there is nothing to ask the user, and nothing for this module to invent. A name they + have already given a tag lives in `UserBeaconOptions` and wins at display time anyway, + which is the same mechanism that renames any other tag. + + :param selectionJson: `[{"beaconId": ...}]`, in the order to return them. + """ + if self._client is None: + return _failure(REASON_NOT_SIGNED_IN, "The Find My client is not open.") + + try: + selection = json.loads(selectionJson) + except ValueError: + return _unexpected("reading the selection Java sent") + + accessories = [] + for chosen in selection: + beaconId = chosen.get("beaconId") + candidate = self._candidates.get(beaconId) + + if candidate is None: + return _failure( + REASON_NO_SUCH_ACCESSORY, + f"No accessory in this session has the id {beaconId!r}. It was either never" + " fetched, or the session has been reopened since.") + + accessories.append({ + "beaconId": candidate.beacon_id, + "ownedBeaconPlist": _plist(candidate.owned_beacon), + # Null where CloudKit holds none. The app shows such a tag by what its own + # record says it is, and the user can name it like any other. + "namingRecordPlist": _plist(candidate.naming_record), + "keyAlignmentPlist": _plist(candidate.key_alignment_record), + }) + + return json.dumps({"ok": True, "accessories": accessories}) + + def close(self) -> None: + """Close the client, and say so rather than raising if it will not go quietly.""" + if self._client is None: + return + + try: + self._loop.run_until_complete(self._client.__aexit__(None, None, None)) + except Exception: + print(f"iCloud bridge: closing the Find My client failed:\n{traceback.format_exc()}") + finally: + self._client = None + self._records = [] + # Dropped with the client, because they hold decrypted key material and the session + # is over. A later `records` call then fails with a reason rather than handing back + # secrets from a conversation that has ended. + self._candidates = {} + + +def _plist(mapping: Any) -> str | None: + """ + One plist mapping as the XML the importer reads, or None where there is no record. + + XML rather than binary because that is what a bundle carries and what Java's XPath parses. + `plistlib` handles the two types that matter and are easy to forget: key material arrives as + `bytes` and becomes ``, and dates become ``. + """ + if mapping is None: + return None + + return plistlib.dumps(dict(mapping), fmt=plistlib.FMT_XML).decode("utf-8") + + +def openSession(account: Any) -> ICloudSession | None: + """ + Start an iCloud conversation on the account the app is already signed in with. + + **The account the app already has, not a second one restored from the same JSON.** One + install is one device to Apple (rule 11), and the identity, the ADI state and the session + all live on the object Java is holding. Restoring a parallel copy would put a second client + on the wire under the same name. + + Returns None when there is nothing usable to work with, which Java reports as needing a + sign-in - the same recovery as a session that has expired. + """ + asyncAccount = _asyncAccount(account) + loop = getattr(account, "_evt_loop", None) + + if asyncAccount is None or loop is None: + print( + "FindMy.py's account internals have changed: no _asyncacc or _evt_loop, so the" + " iCloud flow cannot be driven from the account the app is signed in with.") + return None + + return ICloudSession(account, asyncAccount, loop) diff --git a/app/src/main/python/identity.py b/app/src/main/python/identity.py index cdcec217..d04cf0a3 100644 --- a/app/src/main/python/identity.py +++ b/app/src/main/python/identity.py @@ -51,7 +51,21 @@ rather than the program, in the one place the user ever looks. """ -# **There is deliberately no device name here, and no announce_device() call.** +APP_CLOUDKIT_DEVICE_NAME = "OpenTagViewer" +""" +What this app calls itself *to CloudKit*, in the `AsyncCloudKitClient` device name. + +**Not the device-list name**, despite reading like one - see the note below on why that entry +cannot be named. This is a separate field on a separate service, sent when the app reads the +account's beacon records, and it is not optional: `AsyncCloudKitClient` takes a string, so a +client that set nothing here would be named by the library instead. That is the same +second-identity problem as the serial, one layer down. + +Distinct from the exporter's `OpenTagViewer Exporter` for the same reason `APP_SERIAL` is +distinct from `0PENTAGXPORT`: two programs, two devices, deliberately. +""" + +# **There is deliberately no device-list name here, and no announce_device() call.** # # Naming the device-list entry needs `announce_device()`, which authenticates with the # `com.apple.gs.idms.hb` heartbeat token. That token arrives once, in the same set as the PET, diff --git a/app/src/test/python/conftest.py b/app/src/test/python/conftest.py index 2af80f8b..c5737097 100644 --- a/app/src/test/python/conftest.py +++ b/app/src/test/python/conftest.py @@ -11,14 +11,14 @@ APP_PYTHON = Path(__file__).resolve().parents[2] / "main" / "python" RESOURCES = Path(__file__).resolve().parents[1] / "resources" -# The shared export package. Put on the path here so these tests can exercise it, and **the app -# cannot yet**: Chaquopy is not given it as a second source directory, so `import -# opentagviewer_export` fails inside the APK and main.py's two bridge functions return None there. +# The shared export package, and the four modules of `exporter/` that are stdlib plus FindMy.py. +# Chaquopy is given `../python` as a second source directory, so the same imports work inside the +# APK - `PythonPackagingTest` is what says so, and it names the wizard, the CLI and the prompts as +# things that must *not* be in there. They import tkinter, questionary and prompt_toolkit, none of +# which exist on a phone. # -# That is a known gap rather than an oversight - `../python` also holds the tkinter wizard and a -# top-level package called `test`, which would shadow the standard library's on a phone. See -# "Wire the shared package into Chaquopy" in docs/android-import-handover.md for the two ways out. -# Whichever is taken changes the layout, and this line with it. +# So this line is not a test-only convenience: it mirrors what the build packages, and the two +# have to move together. SHARED_PYTHON = Path(__file__).resolve().parents[4] / "python" sys.path.insert(0, str(APP_PYTHON)) diff --git a/app/src/test/python/requirements.txt b/app/src/test/python/requirements.txt index cb6d1553..cf2f3833 100644 --- a/app/src/test/python/requirements.txt +++ b/app/src/test/python/requirements.txt @@ -11,5 +11,6 @@ # the fork's Anisette providers take `serial=` and PyPI's do not. git+https://github.com/parawanderer/FindMy.py@23a9b8d7109b405f8362ea1e69ebe51f9ca82fca NSKeyedUnArchiver==1.5 +PyYAML==6.0.2 pytest>=8.0 diff --git a/app/src/test/python/test_icloud_bridge.py b/app/src/test/python/test_icloud_bridge.py new file mode 100644 index 00000000..cd6e1feb --- /dev/null +++ b/app/src/test/python/test_icloud_bridge.py @@ -0,0 +1,547 @@ +""" +Driving an Apple account from the app, without an Apple account. + +Everything here runs against fakes standing in for the Find My client. That is not a compromise +for the sake of a test suite - the alternative needs a real Apple ID, a real device with a +passcode and a real escrow record, so the paths that would never be covered are exactly the ones +users meet: the account with nothing to recover from, the service having a bad day, the rejected +passcode. + +**What the fakes cannot check is checked elsewhere.** That the module imports at all inside the +APK is `PythonPackagingTest`; that the identity it presents is this app's and not the exporter's +is asserted here *and* pinned on the Java side by `IdentityBridgeTest`. +""" + +from __future__ import annotations + +import asyncio +import json +import plistlib +from datetime import datetime, timezone + +import pytest + +import icloud_bridge +from exporter import icloud +from findmy.keychain.recovery import RecoveryError + + +class FakeRecord: + """An escrow record, as far as this module is concerned: a serial and a description.""" + + def __init__(self, serial: str) -> None: + self.serial = serial + + def describe(self) -> str: + return f"A device, serial {self.serial}" + + +class FakeOptions: + def __init__(self, recoverable, trustworthy: bool = True) -> None: + self.recoverable = recoverable + self.viability_is_trustworthy = trustworthy + + +class FakeClient: + """ + The Find My client, with the two calls the bridge makes on it and the sockets left out. + + `unlock` is the interesting one: it records what it was given, so a test can assert that a + passcode reached it, and raises whatever it was told to. + """ + + def __init__(self, options=None, unlockError=None) -> None: + self._options = options or FakeOptions([FakeRecord("F2LX9Q")]) + self._unlockError = unlockError + self.unlockedWith: list[tuple[str, str]] = [] + self.entered = False + self.exited = False + + async def __aenter__(self): + self.entered = True + return self + + async def __aexit__(self, *_): + self.exited = True + return False + + async def recovery_options(self, *, refresh: bool = False): + return self._options + + async def unlock(self, record, passcode): + self.unlockedWith.append((record.serial, passcode)) + if self._unlockError is not None: + raise self._unlockError + + +class FakeAccount: + """ + The app's `AppleAccount`, in the two attributes the bridge reaches for. + + Named after the real private attributes on purpose. If FindMy.py renames either, this fake + keeps the old names and the tests keep passing while the app breaks - so `openSession` + guards on both and `testItRefusesAnAccountWhoseInternalsMoved` is what actually covers the + rename. A fake cannot notice one. + """ + + def __init__(self, loop) -> None: + self._asyncacc = object() + self._evt_loop = loop + + +@pytest.fixture +def loop(): + made = asyncio.new_event_loop() + yield made + made.close() + + +@pytest.fixture +def session(loop, monkeypatch): + """An opened session over a `FakeClient`, with the client reachable as `session.client`.""" + + def open_with(client): + async def fake_open_client(account, identity=None): + return client + + monkeypatch.setattr(icloud, "open_client", fake_open_client) + + made = icloud_bridge.openSession(FakeAccount(loop)) + assert json.loads(made.open())["ok"] + made.client = client + + return made + + return open_with + + +class TestStartingASession: + def test_it_uses_the_account_the_app_is_already_signed_in_with(self, loop): + account = FakeAccount(loop) + + made = icloud_bridge.openSession(account) + + assert made is not None + assert made._async is account._asyncacc + assert made._loop is loop + + def testItRefusesAnAccountWhoseInternalsMoved(self, loop): + """ + FindMy.py renaming `_asyncacc` must degrade, not explode. + + The app reaches into a private attribute here and in `main._preferLocalAnisette`, and + both guard the same way. Returning None sends the user to sign in again, which is + wrong but survivable; an AttributeError crossing the Chaquopy boundary is a crash. + """ + class Moved: + _evt_loop = None + + assert icloud_bridge.openSession(Moved()) is None + + def test_nothing_can_be_done_before_the_client_is_open(self, loop): + made = icloud_bridge.openSession(FakeAccount(loop)) + + for call in (made.recoveryOptions, lambda: made.unlock("F2LX9Q", "1234"), made.fetch, + lambda: made.records("[]")): + assert json.loads(call())["reason"] == icloud_bridge.REASON_NOT_SIGNED_IN + + +class TestTheIdentityItPresents: + """ + Rule 11, at the one place this app talks to CloudKit. + + A default here would present `0PENTAGXPORT` from a phone - the desktop exporter's serial - + and the two programs would share one device-list entry that neither could be removed from + without breaking the other. + """ + + def test_it_is_this_app_and_not_the_exporter(self): + assert icloud_bridge.APP_IDENTITY.serial == "0PENTAGVIEWR" + assert icloud_bridge.APP_IDENTITY != icloud.EXPORTER_IDENTITY + + def test_the_cloudkit_name_is_set_rather_than_left_to_the_library(self): + assert icloud_bridge.APP_IDENTITY.device_name + assert icloud_bridge.APP_IDENTITY.device_name != icloud.EXPORTER_IDENTITY.device_name + + def test_it_is_what_reaches_open_client(self, loop, monkeypatch): + seen = {} + + async def fake_open_client(account, identity=None): + seen["identity"] = identity + return FakeClient() + + monkeypatch.setattr(icloud, "open_client", fake_open_client) + icloud_bridge.openSession(FakeAccount(loop)).open() + + assert seen["identity"] is icloud_bridge.APP_IDENTITY + + +class TestWhatCanBeRecoveredFrom: + def test_the_devices_come_back_with_something_to_choose_between(self, session): + made = session(FakeClient(FakeOptions([FakeRecord("F2LX9Q"), FakeRecord("C02XK")]))) + + answer = json.loads(made.recoveryOptions()) + + assert answer["ok"] + assert [d["serial"] for d in answer["devices"]] == ["F2LX9Q", "C02XK"] + assert all(d["description"] for d in answer["devices"]) + + def test_an_account_with_nothing_to_recover_from_says_so(self, session): + """ + The real case this whole flow has to answer for. + + Somebody with an Apple ID and no Apple hardware has never escrowed a keychain, so there + is nothing here for them and no amount of retrying will change it. Telling them to try + again later would be a lie that costs them an evening. + """ + made = session(FakeClient(FakeOptions([], trustworthy=True))) + + answer = json.loads(made.recoveryOptions()) + + assert not answer["ok"] + assert answer["reason"] == icloud_bridge.REASON_NOTHING_TO_RECOVER_FROM + + def test_a_service_having_a_bad_day_is_a_different_answer(self, session): + """ + And the advice is the opposite one, which is why they are two reasons and not one. + + Nothing reported usable *at all* is far more likely an outage than an account where + every record went bad at once. + """ + made = session(FakeClient(FakeOptions([], trustworthy=False))) + + answer = json.loads(made.recoveryOptions()) + + assert answer["reason"] == icloud_bridge.REASON_SERVICE_UNSURE + + def test_the_two_empty_answers_are_not_the_same_reason(self, session): + """Belt and braces, because collapsing them is a one-character edit.""" + assert (icloud_bridge.REASON_NOTHING_TO_RECOVER_FROM + != icloud_bridge.REASON_SERVICE_UNSURE) + + +class TestUnlocking: + def test_the_passcode_reaches_the_chosen_record(self, session): + made = session(FakeClient(FakeOptions([FakeRecord("AAAA"), FakeRecord("BBBB")]))) + made.recoveryOptions() + + assert json.loads(made.unlock("BBBB", "1234"))["ok"] + assert made.client.unlockedWith == [("BBBB", "1234")] + + def test_a_serial_this_session_never_saw_is_refused_without_asking_apple(self, session): + """ + Attempts are probably a limited resource, so a mistake here must not spend one. + """ + made = session(FakeClient()) + made.recoveryOptions() + + answer = json.loads(made.unlock("NOPE", "1234")) + + assert answer["reason"] == icloud_bridge.REASON_NO_SUCH_RECORD + assert made.client.unlockedWith == [] + + def test_a_rejection_carries_the_librarys_own_words(self, session): + """ + **Not reworded into "incorrect passcode".** + + A rejection is not proof the passcode was wrong - FindMy.py's first advice is to try the + same one again, because the exchange fails intermittently. Its text says that; ours + would not. + """ + rejection = RecoveryError("Rejected. Worth trying the same passcode again first.") + made = session(FakeClient(unlockError=rejection)) + made.recoveryOptions() + + answer = json.loads(made.unlock("F2LX9Q", "0000")) + + assert answer["reason"] == icloud_bridge.REASON_PASSCODE_REJECTED + assert "trying the same passcode again" in answer["message"] + + def test_one_attempt_per_call(self, session): + """ + The retry is Java's, so the cap can live beside the dialog that spends it. + + A loop in here would burn all three attempts behind one press of a button. + """ + made = session(FakeClient(unlockError=RecoveryError("no"))) + made.recoveryOptions() + made.unlock("F2LX9Q", "0000") + + assert len(made.client.unlockedWith) == 1 + + +A_KEY = b"\x01" * 28 + + +def _candidate(name=None, alignment=None): + return icloud.Candidate( + beacon_id="F1C4A0E2-1111-4222-8333-444455556666", + name=name, + emoji="🚲" if name else None, + has_alignment=alignment is not None, + owned_beacon={"identifier": "F1C4A0E2", "privateKey": A_KEY, "batteryLevel": 100}, + naming_record=None if name is None else {"name": name, "emoji": "🚲"}, + key_alignment_record=alignment, + hardware="AirTag", + serial_number="HXXXXXXXXXXX", + paired_at=datetime(2024, 3, 1, tzinfo=timezone.utc), + ) + + +class TestListingWhatIsThere: + """ + The first half: what the account holds, described well enough to choose from. + + Deliberately without key material - see `testTheListingCarriesNoSecrets`, which is the + assertion that keeps it that way. + """ + + def test_each_accessory_is_described(self, session, monkeypatch): + made = session(FakeClient()) + monkeypatch.setattr(icloud, "fetch", _fetching([_candidate(name="Bike")], [])) + + answer = json.loads(made.fetch()) + first = answer["accessories"][0] + + assert answer["ok"] + assert first["beaconId"] == "F1C4A0E2-1111-4222-8333-444455556666" + assert first["name"] == "Bike" + assert first["hasName"] is True + + def testTheListingCarriesNoSecrets(self, session, monkeypatch): + """ + A screen that shows a list of names does not need anybody's private keys. + + Rendering every accessory's key material into a JSON string so that a picker can be + drawn puts more of the secret in more places than the picker needs, and the user has + not chosen anything yet. The records come later, for what they picked. + """ + made = session(FakeClient()) + monkeypatch.setattr(icloud, "fetch", _fetching([_candidate(name="Bike")], [])) + + listing = made.fetch() + + assert "privateKey" not in listing + assert A_KEY.hex() not in listing.lower() + assert "ownedBeaconPlist" not in listing + + def test_an_accessory_nobody_named_is_flagged_but_not_a_problem(self, session, monkeypatch): + """ + Nothing here invents a name, and nothing here demands one. + + `to_export` refuses a nameless accessory because a *bundle* is inner-joined by its + importer, so one exported without a name goes silently missing. That does not apply to + an account read straight into the app, which left-joins and shows a nameless tag + perfectly well. The flag is for the picker, where `details` is all a person has to tell + one unnamed accessory from another. + """ + made = session(FakeClient()) + monkeypatch.setattr(icloud, "fetch", _fetching([_candidate(name=None)], [])) + + first = json.loads(made.fetch())["accessories"][0] + + assert first["hasName"] is False + assert first["details"], "with no name, this is all the user has to recognise it by" + + def test_what_was_set_aside_is_named_rather_than_dropped(self, session, monkeypatch): + """ + "Fewer tags than expected" and "some of those were never tags" look identical from + outside, and the second is the common one - an account's own iPhones come back in the + same records. + """ + made = session(FakeClient()) + monkeypatch.setattr(icloud, "fetch", _fetching( + [], [icloud.Skipped("My MacBook", "no private key, so it is a device not a tag")])) + + answer = json.loads(made.fetch()) + + assert answer["accessories"] == [] + assert answer["skipped"][0]["beaconId"] == "My MacBook" + assert "not a tag" in answer["skipped"][0]["reason"] + + +def _fetching(candidates, skipped): + async def fake_fetch(client): + return icloud.Fetched(candidates=candidates, skipped=skipped) + + return fake_fetch + + +ANID = "F1C4A0E2-1111-4222-8333-444455556666" + + +class TestTakingTheRecords: + """ + The second half: the chosen accessories, as the documents the importer already reads. + """ + + def test_they_arrive_as_the_plists_the_importer_already_reads(self, session, monkeypatch): + """ + The point of the whole exercise: no second format, and no zip in the middle. + + An accessory read from an account and one read from a bundle become the same rows in + the same tables, because they arrive as the same documents. + """ + made = session(FakeClient()) + monkeypatch.setattr(icloud, "fetch", _fetching([_candidate(name="Bike")], [])) + made.fetch() + + first = json.loads(made.records(json.dumps([{"beaconId": ANID}])))["accessories"][0] + + parsed = plistlib.loads(first["ownedBeaconPlist"].encode("utf-8")) + assert parsed["privateKey"] == A_KEY, "key material must survive as , not repr()" + assert parsed["batteryLevel"] == 100 + assert plistlib.loads(first["namingRecordPlist"].encode("utf-8"))["name"] == "Bike" + + def test_only_what_was_asked_for(self, session, monkeypatch): + """The keys of a tag the user did not pick have no business leaving Python.""" + made = session(FakeClient()) + other = icloud.Candidate( + beacon_id="OTHER", name="Keys", emoji=None, has_alignment=False, + owned_beacon={"privateKey": b"\x02" * 28}, naming_record={"name": "Keys"}, + key_alignment_record=None) + monkeypatch.setattr(icloud, "fetch", _fetching([_candidate(name="Bike"), other], [])) + made.fetch() + + answer = made.records(json.dumps([{"beaconId": ANID}])) + + assert len(json.loads(answer)["accessories"]) == 1 + assert (b"\x02" * 28).hex() not in answer.lower() + + def testANamelessAccessoryComesThroughWithNoNamingRecord(self, session, monkeypatch): + """ + Rather than failing, and rather than being given a name it never had. + + Both alternatives were wrong. Refusing it makes an importable tag unimportable over a + label; inventing one puts a tag the user did not name into their list as though they + had. Null is the true answer, and the app already knows what to do with it - it is what + a self-generated tag looks like, and `UserBeaconOptions` is how anything gets renamed. + """ + made = session(FakeClient()) + monkeypatch.setattr(icloud, "fetch", _fetching([_candidate(name=None)], [])) + made.fetch() + + answer = json.loads(made.records(json.dumps([{"beaconId": ANID}]))) + first = answer["accessories"][0] + + assert answer["ok"] + assert first["namingRecordPlist"] is None + assert first["ownedBeaconPlist"], "the tag itself is entirely fine" + + def test_no_alignment_record_is_null_rather_than_an_empty_document( + self, session, monkeypatch): + made = session(FakeClient()) + monkeypatch.setattr(icloud, "fetch", _fetching([_candidate(name="Bike")], [])) + made.fetch() + + first = json.loads(made.records(json.dumps([{"beaconId": ANID}])))["accessories"][0] + + assert first["keyAlignmentPlist"] is None + + def test_an_id_this_session_never_saw_is_refused(self, session, monkeypatch): + made = session(FakeClient()) + monkeypatch.setattr(icloud, "fetch", _fetching([_candidate(name="Bike")], [])) + made.fetch() + + answer = json.loads(made.records(json.dumps([{"beaconId": "NOPE"}]))) + + assert answer["reason"] == icloud_bridge.REASON_NO_SUCH_ACCESSORY + + def testClosingDropsTheDecryptedRecords(self, session, monkeypatch): + """ + They are decrypted key material, and the session is over. + + Held on the session only so the two halves can be separate calls; keeping them past + `close` would mean a screen that has gone away can still hand out secrets. + """ + made = session(FakeClient()) + monkeypatch.setattr(icloud, "fetch", _fetching([_candidate(name="Bike")], [])) + made.fetch() + + made.close() + + assert made._candidates == {} + + +class TestFailingWithSomethingToShow: + """ + Every failure has to arrive with words in it. + + The app has already shipped a dialog reading `Login failed:` and then nothing, because + `str(TimeoutError())` is the empty string. A bare exception crossing the Chaquopy boundary + does the same thing again, one screen along. + """ + + def test_an_exception_with_no_message_still_produces_one(self, session, monkeypatch): + made = session(FakeClient()) + + async def timing_out(client): + raise TimeoutError + + monkeypatch.setattr(icloud, "fetch", timing_out) + + answer = json.loads(made.fetch()) + + assert not answer["ok"] + assert answer["reason"] == icloud_bridge.REASON_UNKNOWN + assert answer["message"].strip(), "an empty message is the bug this exists to prevent" + assert "TimeoutError" in answer["message"] + + def test_a_failure_to_open_is_a_value_rather_than_a_raise(self, loop, monkeypatch): + async def refusing(account, identity=None): + msg = "the keychain service said no" + raise RuntimeError(msg) + + monkeypatch.setattr(icloud, "open_client", refusing) + + answer = json.loads(icloud_bridge.openSession(FakeAccount(loop)).open()) + + assert not answer["ok"] + assert "the keychain service said no" in answer["message"] + + def test_every_step_answers_in_the_same_shape(self, session, monkeypatch): + """ + So Java has one parser and not four. + + A step that returned a bare string, or None, on one of its paths would be the one Java + forgot to handle - and it would be a failure path, which is where nobody looks. + """ + made = session(FakeClient(FakeOptions([]))) + monkeypatch.setattr(icloud, "fetch", _fetching([], [])) + + for call in (made.recoveryOptions, lambda: made.unlock("X", "1"), made.fetch, + lambda: made.records(json.dumps([{"beaconId": "X"}])), + lambda: made.records("not json at all")): + answer = json.loads(call()) + assert isinstance(answer["ok"], bool) + if not answer["ok"]: + assert answer["reason"] and answer["message"] + + +class TestClosing: + def test_it_closes_the_client(self, session): + made = session(FakeClient()) + + made.close() + + assert made.client.exited + + def test_closing_twice_is_not_an_error(self, session): + made = session(FakeClient()) + + made.close() + made.close() + + def test_a_client_that_will_not_close_does_not_raise(self, session): + """ + Java calls this from a `finally`. An exception here would replace whatever real failure + sent it there with a confusing one about closing. + """ + made = session(FakeClient()) + + async def refusing(*_): + msg = "still busy" + raise RuntimeError(msg) + + made.client.__aexit__ = refusing + made.close() diff --git a/docs/android-import-handover.md b/docs/android-import-handover.md index ad712433..24b33fd3 100644 --- a/docs/android-import-handover.md +++ b/docs/android-import-handover.md @@ -240,6 +240,24 @@ Catch it where it happens rather than asking users to classify themselves up fro Then drop them into the import path. The sold-or-wiped-device case reaches the same screen and needs no branch of its own — that user still has Apple's own routes to restore keychain access. +**It is reached earlier than this section implies, and one thing that looks like it must not +reach it at all.** Written above as though the app fetches and finds nothing. It does not get +that far: an account with no Apple device has no escrow record either, so the flow stops one +step sooner, at unlocking the keychain — before the user is ever asked for a passcode they do +not have. Better, in fact. The screen is right; the trigger is `recoveryOptions` returning +nothing, not `fetch` returning nothing. + +Which makes the distinction the bridge already draws load-bearing: + +| `icloud_bridge` reason | What to show | +| --- | --- | +| `nothing_to_recover_from` | the screen above — final, and the import path is the answer | +| `service_unsure` | **not that screen.** Nothing was reported usable *at all*, which reads as a service having a bad day rather than every record on an account going bad at once. Say so, and offer to try again later | + +Collapsing the two would tell somebody with a perfectly good account that they own no tags, +permanently, because Apple had a bad afternoon — and send them off to find a friend with a Mac. +FindMy.py draws the same line for the same reason, in `viability_is_trustworthy`. + ### Settings: one switch **Read my tags from my Apple account — on or off.** On joins. There is no separate "stay @@ -250,6 +268,24 @@ Turning it off should say what it does not undo: it stops reading the tag list, remove the entry from the user's Apple device list. That removal is theirs to do, in Apple's interface. +### A tag should say where it came from + +Once both routes exist, a tag in the list can have arrived three ways, and the device details +screen should say which — it already carries this kind of line for a self-generated tag: + +| | What it means to the user | +| --- | --- | +| **From your Apple account** | live; it updates because the app can read the account | +| **Imported** | from a zip; it updates only as far as the keys in that zip reach | +| **Self-generated** | never in an Apple account at all | + +Worth having because the three behave differently and nothing else on the screen distinguishes +them. A recipient wondering why their shared tag stopped updating is looking at an *imported* +one, and the answer is on the screen the moment the screen says so. + +Not a schema question: `OwnedBeacon` already records enough to tell them apart — a self-generated +tag has no `content`, and an account-read one can be marked when it is written. + ### The one prompt that interrupts a connected app Once joined, key rotation is handled — a member is given the new keys. So the only thing that diff --git a/python/exporter/icloud.py b/python/exporter/icloud.py index 4fa7b396..2eea938d 100644 --- a/python/exporter/icloud.py +++ b/python/exporter/icloud.py @@ -21,6 +21,7 @@ import logging import uuid from io import BytesIO +from pathlib import Path from dataclasses import dataclass from datetime import datetime, timedelta, timezone from typing import Any, Awaitable, Callable, Sequence @@ -46,6 +47,7 @@ to_owned_beacon_plist, ) from findmy.cloudkit.client import AsyncCloudKitClient +from findmy.reports.anisette import BaseAnisetteProvider from findmy.icloud import AsyncFindMyClient from findmy.keychain.recovery import RecoveryError from findmy.keychain.session import AsyncKeychainSession @@ -131,7 +133,44 @@ class Fetched: skipped: list[Skipped] -def make_account(anisette_url: str | None = None, libs_path: str | None = None) -> AsyncAppleAccount: +@dataclass(frozen=True) +class ClientIdentity: + """ + Who a client says it is, in the two fields Apple shows the user. + + **There is more than one client now.** This module was written for the desktop exporter and + read :mod:`exporter.identity` directly, which was right while it was the only caller. The + Android app runs the same flow through Chaquopy and must present its own serial - two + programs sharing one are one device to Apple, so removing either from the device list breaks + the other. Rule 11 in AGENTS.md. + + Defaulted everywhere to the exporter's, so nothing on the desktop side changes. + + :param serial: `X-Apple-I-SRL-NO`, and the one field in the device-list row a person can + actually read. + :param device_name: What CloudKit is told this client is called. **Not the device-list name**, + which needs the `postdata` announce Apple refuses without a push token - see + :func:`remember`. Required rather than optional because `AsyncCloudKitClient` takes a + string; a client that set nothing here would be named by the library instead, which is + the same second-identity problem one layer down. + """ + + serial: str + device_name: str + + +EXPORTER_IDENTITY = ClientIdentity(serial=EXPORTER_SERIAL, device_name=DEVICE_NAME) +"""The desktop exporter's, and the default for every entry point here.""" + + +def make_account( + anisette_url: str | None = None, + libs_path: str | None = None, + *, + provider: BaseAnisetteProvider | None = None, + identity: ClientIdentity = EXPORTER_IDENTITY, + identity_path: Path | None = None, +) -> AsyncAppleAccount: """ Build an account that presents this exporter's identity - the same one as last time. @@ -146,13 +185,22 @@ def make_account(anisette_url: str | None = None, libs_path: str | None = None) headers, and a session is bound to whichever established it. :param libs_path: Where to cache Apple's ADI libraries, so a later run does not fetch them again. They are downloaded on first use when this is None. + :param provider: An Anisette provider to use instead of building one. **This is what makes + the flow runnable on Android at all**: the local provider built below is FindMy.py's, + which reaches the `anisette` package and therefore `unicorn`, a CPU emulator that cannot + be built for Android. The app produces Anisette from Apple's own ADI libraries in-process + and passes the result here. See AGENTS.md rule 4. + :param identity: Who this client says it is. Defaults to the exporter's. + :param identity_path: Where the device identity is kept between runs. Defaults to the + desktop's per-platform location; the app passes its own storage. """ - stored = device.load() + stored = device.load(identity_path) - provider = _make_provider(anisette_url, libs_path, stored) + if provider is None: + provider = _make_provider(anisette_url, libs_path, stored, identity) if stored is None: - return AsyncAppleAccount(provider, device_name=DEVICE_NAME) + return AsyncAppleAccount(provider, device_name=identity.device_name) # 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 @@ -167,17 +215,18 @@ def make_account(anisette_url: str | None = None, libs_path: str | None = None) logger.info("Reusing the stored device identity, so this is not a new device to Apple") - return AsyncAppleAccount(provider, state_info=state, device_name=DEVICE_NAME) + return AsyncAppleAccount(provider, state_info=state, device_name=identity.device_name) def _make_provider( anisette_url: str | None, libs_path: str | None, stored: dict[str, Any] | None, + identity: ClientIdentity = EXPORTER_IDENTITY, ) -> 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=EXPORTER_SERIAL) + return RemoteAnisetteProvider(anisette_url, serial=identity.serial) saved = (stored or {}).get("anisette") state_blob = None @@ -190,10 +239,11 @@ def _make_provider( except (ValueError, TypeError): logger.warning("The stored Anisette provisioning could not be read; starting fresh") - return LocalAnisetteProvider(libs_path=libs_path, serial=EXPORTER_SERIAL, state_blob=state_blob) + return LocalAnisetteProvider( + libs_path=libs_path, serial=identity.serial, state_blob=state_blob) -def remember(account: AsyncAppleAccount) -> None: +def remember(account: AsyncAppleAccount, identity_path: Path | None = None) -> None: """ Store this account's device identity, so the next export is the same device. @@ -221,7 +271,8 @@ def remember(account: AsyncAppleAccount) -> None: # fields rather than handing the whole mapping to `device.save`. state = account.to_json() - device.save(state["ids"]["uid"], state["ids"]["devid"], state["anisette"]) + device.save( + state["ids"]["uid"], state["ids"]["devid"], state["anisette"], identity_path) MAX_LOGIN_ATTEMPTS = 3 @@ -360,7 +411,9 @@ def _describe_factor(method: object) -> str: return type(method).__name__ -async def open_client(account: AsyncAppleAccount) -> AsyncFindMyClient: +async def open_client( + account: AsyncAppleAccount, identity: ClientIdentity = EXPORTER_IDENTITY, +) -> AsyncFindMyClient: """ Open a Find My client that reports this exporter's identity to CloudKit as well. @@ -374,8 +427,8 @@ async def open_client(account: AsyncAppleAccount) -> AsyncFindMyClient: account, client=AsyncCloudKitClient( account, - device_name=DEVICE_NAME, - device_serial=EXPORTER_SERIAL, + device_name=identity.device_name, + device_serial=identity.serial, ), ) except Exception: