diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 88602742..39c6432a 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -407,7 +407,7 @@ chaquopy { // wheel for desktop platforms and a pure-Python `py3-none-any` one as well. // There is no Android wheel, so pip falls back to the pure-Python build - which // is correct but markedly slower. The messages here are small enough not to care. - install("git+https://github.com/parawanderer/FindMy.py@eda897ba995a21ceaeac427efd8edcd8625f3618") + install("git+https://github.com/parawanderer/FindMy.py@d956fc8b2679be56b0f4b0053940c5091fc0f1bb") install("NSKeyedUnArchiver==1.5") diff --git a/app/src/androidTest/java/dev/wander/android/opentagviewer/AppleLoginFlowTest.java b/app/src/androidTest/java/dev/wander/android/opentagviewer/AppleLoginFlowTest.java index 8fb9ed21..9d312f62 100644 --- a/app/src/androidTest/java/dev/wander/android/opentagviewer/AppleLoginFlowTest.java +++ b/app/src/androidTest/java/dev/wander/android/opentagviewer/AppleLoginFlowTest.java @@ -348,6 +348,45 @@ public void appleDecliningIsNotBlamedOnThePasswordOrTheNetwork() { not(withText(containsString("Grand Slam"))))); } + /** + * iCloud being refused for the account is not reported as terms of service. + * + *

Issue #221. Apple took the password and the code, then its {@code mobileme} + * delegate refused the account on its own {@code status} with nothing on the + * {@code localizedError} channel terms arrive on. Every delegate failure was classified as + * terms pending, so the reporter - whose terms were fine, as iCloud on the web confirmed - + * was sent to a document list with nothing in it and told to accept something. + * + *

Asserted on four wrong turns, because each sends somebody somewhere different: not the + * terms sentence, not "Apple declined" (which says to wait, and here waiting does not work), + * not the raw delegate text, and the Anisette-server offer stays hidden - a different machine + * identity changes nothing about an account Apple will not open. + */ + @Test + public void icloudBeingRefusedIsNotReportedAsTermsOfService() { + this.apple = FakeAppleAuthService.icloudIsRefusedForTheAccount(); + AppDependencies.replaceAuthService(this.apple); + + launch(); + signIn(); + + final android.content.Context context = getInstrumentation().getTargetContext(); + + Eventually.check(() -> onView(withId(R.id.login_error_message_text)).check(matches( + withText(context.getString(R.string.login_failed_icloud_refused))))); + + onView(withId(R.id.login_error_message_text)).check(matches( + not(withText(context.getString(R.string.login_failed_terms))))); + onView(withId(R.id.login_error_message_text)).check(matches( + not(withText(context.getString(R.string.login_failed_apple_declined))))); + onView(withId(R.id.login_error_message_text)).check(matches( + not(withText(containsString("com.apple.mobileme"))))); + + // The server route is for a refused sign-in, and this sign-in was not refused. + onView(withId(R.id.login_error_try_remote_anisette)) + .check(matches(not(isDisplayed()))); + } + /** * When Apple names a wait, the screen gives it rather than "try again shortly". * diff --git a/app/src/androidTest/java/dev/wander/android/opentagviewer/python/FakeAppleAuthService.java b/app/src/androidTest/java/dev/wander/android/opentagviewer/python/FakeAppleAuthService.java index 126406ce..1260de74 100644 --- a/app/src/androidTest/java/dev/wander/android/opentagviewer/python/FakeAppleAuthService.java +++ b/app/src/androidTest/java/dev/wander/android/opentagviewer/python/FakeAppleAuthService.java @@ -163,6 +163,29 @@ public static FakeAppleAuthService appleIsDeclining() { return fake; } + /** + * Apple signs the user in and then will not open iCloud for the account. + * + *

Issue #221. The message is the real one, word for word off the reporter's screen: + * the delegate's own {@code status}, with nothing on the {@code localizedError} channel that + * terms arrive on. Every delegate failure used to be reported as terms pending, so this + * account - whose terms were fine - was shown an empty document list and told to accept + * something. + * + *

Carries no account, which is what keeps it out of the terms flow: {@code + * hasTermsToAccept()} needs both the reason and the session. + */ + public static FakeAppleAuthService icloudIsRefusedForTheAccount() { + final FakeAppleAuthService fake = + new FakeAppleAuthService(LOGIN_STATE.LOGGED_OUT, null); + fake.loginFailsWith = new PythonAccountLoginException( + "The com.apple.mobileme delegate request failed, reporting status=1," + + " status-message='A server problem is blocking Apple ID sign in." + + " Try signing in later.'", + PythonAccountLoginException.REASON_ICLOUD_REFUSED); + return fake; + } + /** * Apple declining, and saying how long to leave it - a {@code Retry-After} on the refusal. * diff --git a/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java b/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java index d2196949..b0f4b89a 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java +++ b/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java @@ -824,6 +824,13 @@ private String describeLoginFailure(final Throwable error) { return this.getString(R.string.login_failed_apple_declined); } + // Apple took the password and the code, then would not open iCloud for the account. Kept + // apart from the terms sentence because the remedy is elsewhere entirely, and apart from + // "Apple declined" because that one says to wait - see REASON_ICLOUD_REFUSED. + if (PythonAccountLoginException.REASON_ICLOUD_REFUSED.equals(reason)) { + return this.getString(R.string.login_failed_icloud_refused); + } + // Reached when the terms path was tried and produced nothing to accept, so the sentence // says what Apple said and then that accepting terms will not fix something else - // rather than asserting a cause that has not been established. diff --git a/app/src/main/java/dev/wander/android/opentagviewer/python/PythonAccountLoginException.java b/app/src/main/java/dev/wander/android/opentagviewer/python/PythonAccountLoginException.java index 4c67849a..4a4d289d 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/python/PythonAccountLoginException.java +++ b/app/src/main/java/dev/wander/android/opentagviewer/python/PythonAccountLoginException.java @@ -33,7 +33,25 @@ public class PythonAccountLoginException extends RuntimeException { */ public static final String REASON_TERMS = "terms"; - /** Anything not recognised. The detail is shown as-is rather than guessed at. */ + /** + * Apple signed the user in and then refused to open iCloud. Matches + * {@code REASON_ICLOUD_REFUSED}. + * + *

Split out of {@link #REASON_TERMS}, which used to swallow it. Every delegate + * failure was reported as terms pending, because terms were the only cause with a remedy. + * The response has two error channels and terms arrive on only one; when that channel is + * empty, the delegate refused the account itself and the terms flow has nothing to show. + * Issue #221 is somebody whose terms were fine being shown an empty document list. + * + *

Carries no account, deliberately: unlike terms, there is nothing further to do with + * the session from here. + * + *

Not {@link #REASON_APPLE_DECLINED}, which advises waiting. Apple's own wording + * here says to try later, and across the clients sharing this sign-in path it does not + * clear on its own - so repeating that advice sends somebody to retry forever. + */ + public static final String REASON_ICLOUD_REFUSED = "icloud_refused"; + /** * Apple answered and refused to serve. Matches {@code REASON_APPLE_DECLINED}. * @@ -42,6 +60,7 @@ public class PythonAccountLoginException extends RuntimeException { */ public static final String REASON_APPLE_DECLINED = "apple_declined"; + /** Anything not recognised. The detail is shown as-is rather than guessed at. */ public static final String REASON_UNKNOWN = "unknown"; private final String reason; diff --git a/app/src/main/python/main.py b/app/src/main/python/main.py index 197060b2..2328bf28 100644 --- a/app/src/main/python/main.py +++ b/app/src/main/python/main.py @@ -464,6 +464,34 @@ def assertAnisetteIsSupported(serializedAccountData: str) -> str | None: screen that keeps refusing them for a reason nothing tells them. """ +REASON_ICLOUD_REFUSED = "icloud_refused" +""" +Apple accepted the sign-in and then refused to open iCloud for this account. + +**Authentication worked.** The password was right, the second factor was right, and the +`com.apple.mobileme` delegate exchange that follows them is what failed - so nothing on the +sign-in screen is wrong and re-entering it changes nothing. + +**Split out of :data:`REASON_TERMS`, which used to swallow it.** Every `MobileMeDelegateError` +was reported as terms pending, because terms were the only cause anybody had a remedy for. The +response has two independent error channels and terms arrive on only one of them: when +`localizedError` is absent, the delegate refused the account on its own `status` and the terms +flow has nothing to show. Reported as issue #221, where somebody whose terms were fine was shown +an empty document list and told to accept some. + +**What it does mean is not established, so the screen does not assert one.** It is met across +every client sharing this sign-in path, on Apple IDs that have never been used with an Apple +device - macless-haystack#84, #86 and #87, where the same delegate status arrives both with +"A server problem is blocking Apple ID sign in" and with "Account limit reached". The advice +that circulates there is to fill the account out at appleid.apple.com, and that is offered as +the thing to try rather than as the answer. + +**Deliberately not :data:`REASON_APPLE_DECLINED`.** That one says wait and try again, which is +also what Apple's own wording here says - and across those clients it does not clear on its own. +Sending somebody back to retry an unchanged sign-in indefinitely is worse than saying plainly +that the account looks like the problem. +""" + REASON_APPLE_DECLINED = "apple_declined" """ Apple answered, and refused to serve the request. Not the password, and not the network. @@ -506,8 +534,21 @@ def classifyLoginFailure(error: BaseException) -> str: # "terms pending" is not established**, so this reports the possibility rather than asserting # it - the screen offers to fetch them and says plainly that if the cause is something else, # accepting terms will not fix it. The desktop CLI makes the same judgement the same way. + # + # **But only when the response used that channel at all.** `localizedError` is where a + # response explains itself in words, and where terms arrive; the delegate's own `status` + # fails independently of it. Treating both as terms sent somebody with perfectly good terms + # to an empty document list and told them to accept something - issue #221. `status`-only + # failures are a refusal of the account, and get their own sentence. + # + # **Reads `localized_error` rather than `names_a_localized_error`, on purpose.** The property + # is the nicer spelling and says exactly this, but it landed in the fork after the commit + # pinned in `app/build.gradle.kts`, and the attribute behind it has been there all along. So + # this works on the pinned version rather than requiring the pin to move first - which would + # drag rule 14's four files into a fix that does not otherwise need them. Switching to the + # property is safe whenever the pin next moves; both spellings coexist. if isinstance(error, MobileMeDelegateError): - return REASON_TERMS + return REASON_TERMS if error.localized_error is not None else REASON_ICLOUD_REFUSED # Before the network checks, and not because of ordering hazards - it is a RuntimeError and # collides with none of them. It is here because it reads as the same thing to a user and is diff --git a/app/src/main/res/values-de/strings.xml b/app/src/main/res/values-de/strings.xml index 4605fdb3..8dc98c12 100644 --- a/app/src/main/res/values-de/strings.xml +++ b/app/src/main/res/values-de/strings.xml @@ -389,4 +389,5 @@ Es wurde kein Verlauf importiert und bereits gespeicherte Daten wurden nicht ge Wenn Apple die Anfrage lediglich abgelehnt hat, lohnt es sich, zu warten und es erneut zu versuchen. Schlägt es weiterhin fehl, macht ein Bericht mit diesem Protokoll es behebbar. Apple hat die Anmeldung abgelehnt und bittet, %1$s bis zum nächsten Versuch zu warten. Das ist eine Ablehnung durch Apple und kein Problem mit deiner Apple-ID oder deinem Passwort. Remote-Server versuchen + Dein Passwort und dein Code wurden akzeptiert, danach hat Apple iCloud für dieses Konto nicht freigegeben. Es geht dabei nicht um Nutzungsbedingungen. Andere Apps, die sich auf diesem Weg anmelden, sehen das bei Apple-IDs, die noch nie mit einem Apple-Gerät verwendet wurden, und sich bei appleid.apple.com anzumelden und die Kontodaten zu vervollständigen — etwa eine Zahlungsmethode hinzuzufügen — behebt es meistens. Bloßes Warten hilft in der Regel nicht. \ No newline at end of file diff --git a/app/src/main/res/values-en/strings.xml b/app/src/main/res/values-en/strings.xml index bb9b5049..ca542e72 100644 --- a/app/src/main/res/values-en/strings.xml +++ b/app/src/main/res/values-en/strings.xml @@ -389,4 +389,5 @@ No history was imported and nothing already stored was changed. If Apple simply refused the request, waiting and trying again is worth doing first. If it keeps failing, a report with this log attached is what makes it fixable. Apple refused the sign-in and asked for %1$s before trying again. This is Apple declining, not a problem with your Apple ID or password. Try a remote server + Your password and code were accepted, and then Apple would not open iCloud for this account. This is not about terms of service. Other apps that sign in this way see it on Apple IDs that have never been used with an Apple device, and signing in at appleid.apple.com and completing the account details — adding a payment method, for instance — is what usually clears it. Waiting on its own generally does not. \ No newline at end of file diff --git a/app/src/main/res/values-fr/strings.xml b/app/src/main/res/values-fr/strings.xml index c6a2e031..0f10c714 100644 --- a/app/src/main/res/values-fr/strings.xml +++ b/app/src/main/res/values-fr/strings.xml @@ -389,4 +389,5 @@ Aucun historique n’a été importé et les données déjà enregistrées n’o Si Apple a simplement refusé la requête, il vaut mieux attendre et réessayer. Si l\'échec persiste, un rapport accompagné de ce journal permet de corriger le problème. Apple a refusé la connexion et demande d\'attendre %1$s avant de réessayer. C\'est un refus d\'Apple, pas un problème avec votre identifiant ou votre mot de passe. Essayer un serveur distant + Votre mot de passe et votre code ont été acceptés, puis Apple n\'a pas ouvert iCloud pour ce compte. Il ne s\'agit pas des conditions d\'utilisation. D\'autres applications qui se connectent de cette façon le rencontrent sur des identifiants Apple qui n\'ont jamais été utilisés avec un appareil Apple, et se connecter sur appleid.apple.com puis compléter les informations du compte — ajouter un moyen de paiement, par exemple — suffit généralement à débloquer la situation. Attendre seul n\'y suffit pas en général. \ No newline at end of file diff --git a/app/src/main/res/values-ja/strings.xml b/app/src/main/res/values-ja/strings.xml index 538342ad..555f3b90 100644 --- a/app/src/main/res/values-ja/strings.xml +++ b/app/src/main/res/values-ja/strings.xml @@ -389,4 +389,5 @@ Appleが要求を拒否しただけなら、少し待ってからもう一度お試しください。それでも失敗する場合は、このログを添えて報告すると修正できます。 Apple がサインインを拒否し、再試行まで %1$s 待つよう求めています。Apple ID やパスワードの問題ではなく、Apple 側の拒否です。 リモートサーバーを試す + パスワードと確認コードは受け付けられましたが、その後 Apple がこのアカウントの iCloud を開きませんでした。これは利用規約の問題ではありません。同じ方法でサインインする他のアプリでも、Apple 製デバイスで一度も使われたことのない Apple ID でこれが起きると報告されています。appleid.apple.com にサインインし、支払い方法の追加などアカウント情報を入力すると解消することがほとんどです。待つだけでは通常は解消しません。 \ No newline at end of file diff --git a/app/src/main/res/values-ko/strings.xml b/app/src/main/res/values-ko/strings.xml index a8a77345..85875fd7 100644 --- a/app/src/main/res/values-ko/strings.xml +++ b/app/src/main/res/values-ko/strings.xml @@ -389,4 +389,5 @@ Apple이 요청을 거부한 것뿐이라면 잠시 기다렸다가 다시 시도해 보세요. 계속 실패한다면 이 로그를 첨부해 신고하면 해결할 수 있습니다. Apple이 로그인을 거부했으며, 다시 시도하기 전에 %1$s 기다려 달라고 요청했습니다. Apple ID나 비밀번호의 문제가 아니라 Apple 쪽의 거부입니다. 원격 서버 시도 + 비밀번호와 인증 코드는 통과했지만, 그 뒤에 Apple이 이 계정의 iCloud를 열어 주지 않았습니다. 약관과는 관계가 없습니다. 같은 방식으로 로그인하는 다른 앱에서도 Apple 기기에서 한 번도 사용한 적이 없는 Apple ID에서 이런 일이 보고되며, appleid.apple.com에 로그인해 결제 수단 추가처럼 계정 정보를 채우면 대개 해결됩니다. 기다리는 것만으로는 보통 해결되지 않습니다. \ No newline at end of file diff --git a/app/src/main/res/values-nl/strings.xml b/app/src/main/res/values-nl/strings.xml index 58176ad9..e88e3e4e 100644 --- a/app/src/main/res/values-nl/strings.xml +++ b/app/src/main/res/values-nl/strings.xml @@ -389,4 +389,5 @@ Er is geen geschiedenis geïmporteerd en eerder opgeslagen gegevens zijn niet ge Als Apple het verzoek simpelweg weigerde, is even wachten en opnieuw proberen het eerste om te doen. Blijft het mislukken, dan maakt een melding met dit logboek het oplosbaar. Apple heeft de aanmelding geweigerd en vraagt %1$s te wachten voor je het opnieuw probeert. Dit is een weigering van Apple, geen probleem met je Apple ID of wachtwoord. Externe server proberen + Je wachtwoord en code zijn geaccepteerd, waarna Apple iCloud voor dit account niet heeft geopend. Dit gaat niet over de voorwaarden. Andere apps die op deze manier inloggen zien dit bij Apple ID\'s die nooit met een Apple-apparaat zijn gebruikt, en inloggen op appleid.apple.com en de accountgegevens aanvullen — bijvoorbeeld een betaalmethode toevoegen — lost het meestal op. Alleen wachten helpt doorgaans niet. \ No newline at end of file diff --git a/app/src/main/res/values-ru/strings.xml b/app/src/main/res/values-ru/strings.xml index 32e811be..06ccaf32 100644 --- a/app/src/main/res/values-ru/strings.xml +++ b/app/src/main/res/values-ru/strings.xml @@ -389,4 +389,5 @@ Если Apple просто отклонила запрос, сначала стоит подождать и попробовать снова. Если ошибка повторяется, отчёт с этим журналом позволит её исправить. Apple отклонила вход и просит подождать %1$s, прежде чем повторить попытку. Это отказ со стороны Apple, а не проблема с вашим Apple ID или паролем. Попробовать удалённый сервер + Пароль и код были приняты, но затем Apple не открыла iCloud для этой учётной записи. Дело не в условиях использования. Другие приложения, которые входят таким же образом, сталкиваются с этим на Apple ID, ни разу не использовавшихся с устройством Apple, и вход на appleid.apple.com с заполнением данных учётной записи — например, добавлением способа оплаты — обычно решает проблему. Просто подождать, как правило, не помогает. \ No newline at end of file diff --git a/app/src/main/res/values-zh-rCN/strings.xml b/app/src/main/res/values-zh-rCN/strings.xml index 7d6a15e7..ab0f6902 100644 --- a/app/src/main/res/values-zh-rCN/strings.xml +++ b/app/src/main/res/values-zh-rCN/strings.xml @@ -389,4 +389,5 @@ 如果只是 Apple 拒绝了请求，先等一会儿再试。若持续失败，附上此日志的报告才能让问题得到修复。 Apple 拒绝了此次登录，并要求等待 %1$s 后再试。这是 Apple 的拒绝，不是你的 Apple ID 或密码有问题。 尝试远程服务器 + 密码和验证码都通过了，但随后 Apple 没有为此账户开放 iCloud。这与服务条款无关。以同样方式登录的其他应用也报告，从未在 Apple 设备上使用过的 Apple ID 会出现这种情况；登录 appleid.apple.com 并补全账户信息——例如添加付款方式——通常就能解决。仅仅等待通常没有用。 diff --git a/app/src/main/res/values-zh-rTW/strings.xml b/app/src/main/res/values-zh-rTW/strings.xml index 5183e6e0..b191dc0a 100644 --- a/app/src/main/res/values-zh-rTW/strings.xml +++ b/app/src/main/res/values-zh-rTW/strings.xml @@ -389,4 +389,5 @@ 如果只是 Apple 拒絕了請求，先等一會兒再試。若持續失敗，附上此日誌的回報才能讓問題獲得修正。 Apple 拒絕了此次登入，並要求等待 %1$s 後再試。這是 Apple 的拒絕，不是你的 Apple ID 或密碼有問題。 嘗試遠端伺服器 + 密碼和驗證碼都通過了，但接著 Apple 沒有為此帳戶開放 iCloud。這與服務條款無關。以同樣方式登入的其他應用程式也回報，從未在 Apple 裝置上使用過的 Apple ID 會出現這種情況；登入 appleid.apple.com 並補齊帳戶資訊——例如新增付款方式——通常就能解決。只是等待通常沒有用。 diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index acbdf3fd..a55c7b3c 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -422,4 +422,5 @@ No history was imported and nothing already stored was changed. If Apple simply refused the request, waiting and trying again is worth doing first. If it keeps failing, a report with this log attached is what makes it fixable. Apple refused the sign-in and asked for %1$s before trying again. This is Apple declining, not a problem with your Apple ID or password. Try a remote server + Your password and code were accepted, and then Apple would not open iCloud for this account. This is not about terms of service. Other apps that sign in this way see it on Apple IDs that have never been used with an Apple device, and signing in at appleid.apple.com and completing the account details — adding a payment method, for instance — is what usually clears it. Waiting on its own generally does not. diff --git a/app/src/test/python/requirements.txt b/app/src/test/python/requirements.txt index 8383c90a..68675d1a 100644 --- a/app/src/test/python/requirements.txt +++ b/app/src/test/python/requirements.txt @@ -9,7 +9,7 @@ # `FindMy==0.9.8` here for as long as the app built the fork, so every bridge test # ran against a library the app does not ship - which is not a small difference: # the fork's Anisette providers take `serial=` and PyPI's do not. -git+https://github.com/parawanderer/FindMy.py@eda897ba995a21ceaeac427efd8edcd8625f3618 +git+https://github.com/parawanderer/FindMy.py@d956fc8b2679be56b0f4b0053940c5091fc0f1bb NSKeyedUnArchiver==1.5 PyYAML==6.0.2 diff --git a/app/src/test/python/test_terms_flow.py b/app/src/test/python/test_terms_flow.py index a967eb97..7a78cc5c 100644 --- a/app/src/test/python/test_terms_flow.py +++ b/app/src/test/python/test_terms_flow.py @@ -363,3 +363,58 @@ def test_the_detail_is_never_empty(self): detail = main.describeLoginFailure(MobileMeDelegateError(localized_error="TERMS")) assert detail.strip() + + +class TestADelegateFailureThatIsNotAboutTerms: + """ + Issue #221, and the reason this class exists separately from the one above. + + The delegate response has two error channels. `localizedError` is where terms arrive; + the delegate's own `status` fails independently of it. Every `MobileMeDelegateError` was + reported as terms pending, so an account whose terms were fine was shown an empty document + list and told to accept something. + """ + + # The shape that actually arrived, quoted from the screenshot on #221. + REFUSED = MobileMeDelegateError( + status=1, + status_message="A server problem is blocking Apple ID sign in. Try signing in later.", + ) + + def test_a_status_only_failure_is_not_reported_as_terms(self): + assert main.classifyLoginFailure(self.REFUSED) == main.REASON_ICLOUD_REFUSED + assert main.classifyLoginFailure(self.REFUSED) != main.REASON_TERMS + + def test_a_localized_error_is_still_reported_as_terms(self): + # The other half of the branch. Without this, "fix" the split by always returning the + # new reason and the terms flow becomes unreachable with nothing going red. + assert main.classifyLoginFailure( + MobileMeDelegateError(localized_error="TERMS", status=1)) == main.REASON_TERMS + + def test_it_is_not_reported_as_apple_declining(self): + """ + `apple_declined` tells somebody to wait and try again, and offers the Anisette server + route. Apple's own wording here says to try later too - and across the clients sharing + this sign-in path it does not clear on its own, so that advice is a loop. + """ + assert main.classifyLoginFailure(self.REFUSED) != main.REASON_APPLE_DECLINED + + def test_it_is_not_left_unclassified(self): + # UNKNOWN renders the raw exception text, which here is a paragraph about a delegate. + assert main.classifyLoginFailure(self.REFUSED) != main.REASON_UNKNOWN + + def test_the_account_is_not_handed_back(self, signingIn): + """ + Unlike terms, there is nothing further to do with the session, so holding it is only + an invitation to store it - the bad write behind #43 and #119. + """ + _, answer = signingIn(self.REFUSED) + + assert answer["reason"] == main.REASON_ICLOUD_REFUSED + assert "account" not in answer + + def test_what_apple_said_survives_into_the_detail(self): + # The status message is the only evidence a bug report can carry about this. + detail = main.describeLoginFailure(self.REFUSED) + + assert "A server problem is blocking Apple ID sign in." in detail diff --git a/python/exporter/cli.py b/python/exporter/cli.py index 666354bf..e3ce181b 100644 --- a/python/exporter/cli.py +++ b/python/exporter/cli.py @@ -48,7 +48,7 @@ suggested_identifier, suggested_name, ) -from exporter.icloud import Candidate, ExportSourceError +from exporter.icloud import Candidate, ExportSourceError, not_a_terms_problem from exporter.certs import ensure_ca_bundle from exporter.version import EXPORT_VIA_CLI, GITHUB_ISSUES_LINK, VERSION, describe_build from opentagviewer_export import ( @@ -519,6 +519,14 @@ async def sign_in(arguments: argparse.Namespace): announce=_say, ) except MobileMeDelegateError as e: + # **First, because for this shape the offer below is a dead end.** A delegate failure + # naming no `localizedError` is not about terms, so fetching them can only come back + # empty - and OpenTagViewer#221 is somebody being walked to that empty answer. + refusal = not_a_terms_problem(e) + if refusal is not None: + print(f"\n{refusal}\n", file=sys.stderr) + raise ExportSourceError("Apple would not open iCloud for this account.") from None + # Authentication itself worked; the exchange that follows it did not. Unaccepted terms # are the one cause of that with a remedy here, and which error value means "terms # pending" is not established - so this says what Apple said and then offers, rather diff --git a/python/exporter/icloud.py b/python/exporter/icloud.py index f0b72628..164f08c0 100644 --- a/python/exporter/icloud.py +++ b/python/exporter/icloud.py @@ -37,7 +37,7 @@ SmsSecondFactorMethod, TrustedDeviceSecondFactorMethod, ) -from findmy.errors import AppleServiceUnavailableError, UnhandledProtocolError +from findmy.errors import AppleServiceUnavailableError, MobileMeDelegateError, UnhandledProtocolError from findmy.accessory import _extract_serial_from_stable_id # noqa: PLC2701 - see _candidate from findmy.cloudkit.beacons import ( AsyncBeaconStore, @@ -71,6 +71,49 @@ class ExportSourceError(Exception): """Raised when the account cannot produce what an export needs.""" +def not_a_terms_problem(error: MobileMeDelegateError) -> str | None: + """ + Explain a delegate failure that terms cannot account for, or None when they still might. + + **The response has two independent error channels, and terms arrive on only one of them.** + `localizedError` is where a response explains itself in words; the delegate's own `status` + fails separately from it. So a failure carrying no `localizedError` at all is not a terms + problem, and going on to fetch the terms in order to discover that costs a round trip and + tells the person nothing they can use. + + **What it is instead is not established, so this offers rather than asserts.** Every client + sharing this sign-in path meets it on Apple IDs that have never been used with an Apple + device, and the advice that circulates is to fill the account out at appleid.apple.com -- + dchristl/macless-haystack#84, #86 and #87, where the same delegate status arrives both with + this "server problem" wording and with "Account limit reached". That is somebody else's + finding rather than ours, and it is repeated here because it is the only remedy anybody has. + + **Apple's own text says to try later, and this deliberately contradicts it.** Across those + clients it does not clear on its own, so repeating Apple's advice sends somebody to retry an + unchanged sign-in indefinitely. + + :param error: The delegate failure, as raised by the sign-in. + :return: A message to show instead of the terms flow, or None to let the terms flow run. + """ + if error.localized_error is not None: + return None + + # `status_message` is where this response does its explaining, and it is the only evidence a + # bug report about it can carry - so it is quoted rather than paraphrased. It can be absent, + # in which case the whole error is the best available description. + said = error.status_message or str(error) + + return ( + "Signing in worked - your password and your code were both accepted - and then Apple" + " would not open iCloud for this account.\n\n" + f"This is not about terms of service. Apple said:\n\n {said}\n\n" + "Other programs that sign in this way meet this on Apple IDs that have never been used" + " with an Apple device. Signing in at https://appleid.apple.com and completing the" + " account details - adding a payment method, which is not charged - is what usually" + " clears it. Waiting, despite what Apple's message says, generally does not." + ) + + @dataclass(frozen=True) class Candidate: """ diff --git a/python/exporter/wizard.py b/python/exporter/wizard.py index 6060a2c3..df0c40b1 100644 --- a/python/exporter/wizard.py +++ b/python/exporter/wizard.py @@ -58,7 +58,7 @@ from findmy.errors import AppleServiceUnavailableError from findmy.keychain.recovery import RecoveryError -from exporter.icloud import Candidate, ExportSourceError +from exporter.icloud import Candidate, ExportSourceError, not_a_terms_problem from exporter.version import ( APP_TITLE, EXPORT_VIA_WIZARD, @@ -1148,6 +1148,14 @@ async def _accept_pending_terms(parent: tk.Tk, account, asker: Asker, error) -> :raises TermsDeclined: If any document is rejected. Nothing is sent for it, and no later document is shown. """ + # **Before fetching, because for this shape there is nothing to fetch.** A delegate failure + # that named no `localizedError` is not about terms, so asking Apple which terms are pending + # can only come back empty - and the report that produced this, OpenTagViewer#221, is + # somebody being walked to that empty answer and left to work out what it meant. + refusal = not_a_terms_problem(error) + if refusal is not None: + raise ExportSourceError(refusal) + try: documents = await account.fetch_terms() except TermsError as e: diff --git a/python/pyproject.toml b/python/pyproject.toml index 6a903510..4ca2f1fb 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -69,7 +69,7 @@ constraint-dependencies = [ ] [tool.uv.sources] -FindMy = { git = "https://github.com/parawanderer/FindMy.py", rev = "eda897ba995a21ceaeac427efd8edcd8625f3618" } +FindMy = { git = "https://github.com/parawanderer/FindMy.py", rev = "d956fc8b2679be56b0f4b0053940c5091fc0f1bb" } [dependency-groups] # Only the release build installs this, with `uv sync --no-default-groups --group build`. It is diff --git a/python/test/test_delegate_refusal.py b/python/test/test_delegate_refusal.py new file mode 100644 index 00000000..d28d5d14 --- /dev/null +++ b/python/test/test_delegate_refusal.py @@ -0,0 +1,95 @@ +""" +Telling "Apple will not open iCloud for this account" apart from "terms are pending". + +Both arrive as a `MobileMeDelegateError` from the same call, and for a long time both were +answered with the terms flow - because terms were the only cause anybody had a remedy for. The +response has two independent error channels and terms use only one of them, so that answer was +right about half the time and misleading the rest. + +OpenTagViewer#221 is the misleading half, reported against the app: an account whose terms were +fine, offered terms, shown nothing, and left to work out why. +""" + +from __future__ import annotations + +from findmy.errors import MobileMeDelegateError + +from exporter.icloud import not_a_terms_problem + +# Quoted from the screenshot on #221 rather than invented, because the wording is the evidence. +REFUSED = MobileMeDelegateError( + status=1, + status_message="A server problem is blocking Apple ID sign in. Try signing in later.", +) + + +class TestWhichFailuresTheTermsFlowCanAnswer: + def test_a_failure_with_no_localized_error_is_not_about_terms(self) -> None: + assert not_a_terms_problem(REFUSED) is not None + + def test_a_failure_that_named_one_is_left_to_the_terms_flow(self) -> None: + # The other half of the branch. Without it, "fixing" this by always returning a message + # makes the terms flow unreachable and nothing goes red. + error = MobileMeDelegateError(localized_error="SOME_VALUE", status=1) + + assert not_a_terms_problem(error) is None + + def test_an_unauthorized_response_is_also_left_alone(self) -> None: + # A known value on the same channel: the credential was refused, not the account. It has + # its own remedy in the library's message, and this must not talk over it. + error = MobileMeDelegateError(localized_error="UNAUTHORIZED", status=1) + + assert not_a_terms_problem(error) is None + + +class TestWhatThePersonIsTold: + def test_it_says_signing_in_actually_worked(self) -> None: + # The screen this replaces appears immediately after a verification code was typed, so + # the first thing to settle is that the password and the code were not the problem. + message = not_a_terms_problem(REFUSED) + + assert message is not None + assert "Signing in worked" in message + + def test_it_says_plainly_that_terms_are_not_the_cause(self) -> None: + message = not_a_terms_problem(REFUSED) + + assert message is not None + assert "not about terms of service" in message + + def test_it_quotes_what_apple_said(self) -> None: + # The status message is the only evidence a bug report about this can carry, and the + # thing to search for when the next person meets it. + message = not_a_terms_problem(REFUSED) + + assert message is not None + assert "A server problem is blocking Apple ID sign in." in message + + def test_it_names_the_remedy_other_clients_found(self) -> None: + # Borrowed from macless-haystack#84/#86/#87. Not ours, and not proven - but it is the + # only remedy anybody has, and withholding it helps nobody. + message = not_a_terms_problem(REFUSED) + + assert message is not None + assert "appleid.apple.com" in message + assert "payment method" in message + + def test_it_does_not_repeat_apples_advice_to_wait(self) -> None: + """ + Apple says "try signing in later" and across these clients that does not work. + + Passing it on would send somebody to retry an unchanged sign-in indefinitely, which is + the one outcome worse than saying "this looks like the account". + """ + message = not_a_terms_problem(REFUSED) + + assert message is not None + assert "generally does not" in message + + def test_a_response_that_said_nothing_still_produces_a_description(self) -> None: + # `status_message` can be absent. Falling back to the whole error keeps the message from + # having a blank where its only evidence should be. + message = not_a_terms_problem(MobileMeDelegateError(status=1)) + + assert message is not None + assert "com.apple.mobileme" in message diff --git a/python/uv.lock b/python/uv.lock index c9b6c734..09511c48 100644 --- a/python/uv.lock +++ b/python/uv.lock @@ -659,7 +659,7 @@ wheels = [ [[package]] name = "findmy" version = "0.10.1" -source = { git = "https://github.com/parawanderer/FindMy.py?rev=eda897ba995a21ceaeac427efd8edcd8625f3618#eda897ba995a21ceaeac427efd8edcd8625f3618" } +source = { git = "https://github.com/parawanderer/FindMy.py?rev=d956fc8b2679be56b0f4b0053940c5091fc0f1bb#d956fc8b2679be56b0f4b0053940c5091fc0f1bb" } dependencies = [ { name = "aiohttp" }, { name = "anisette" }, @@ -1032,7 +1032,7 @@ dev = [ [package.metadata] requires-dist = [ - { name = "findmy", git = "https://github.com/parawanderer/FindMy.py?rev=eda897ba995a21ceaeac427efd8edcd8625f3618" }, + { name = "findmy", git = "https://github.com/parawanderer/FindMy.py?rev=d956fc8b2679be56b0f4b0053940c5091fc0f1bb" }, { name = "pycryptodome", specifier = "==3.22.0" }, { name = "pyyaml", specifier = "==6.0.2" }, { name = "pyzipper", specifier = "==0.4.0" },