FindMy.py's submit does two calls: the check, which passed, and a Grand Slam + * re-authentication, which 503s. The message is the one that actually arrives across the + * bridge, because that string is what the app has to pattern-match on - FindMy.py folds + * every non-OK status into {@code UnhandledProtocolError} carrying only the number. + * + *
Reported as issue #168; the desktop exporter fixed its half in #169.
+ */
+ public FakeAppleAuthService whereAppleTakesTheCodeThenFails() {
+ this.codeFailsWith = new RuntimeException(
+ "com.chaquo.python.PyException: findmy.errors.UnhandledProtocolError:"
+ + " Error response for GSA request: 503");
+ return this;
+ }
+
@Override
public Observable The submit is two calls, and only the second failed. FindMy.py checks the code - which
+ * passes - and then runs a Grand Slam re-authentication, which can 503 on its own. By then Apple
+ * has consumed the code.
+ *
+ * So the screen's old behaviour was the one thing guaranteed to fail. It cleared the box
+ * and asked for the code again; the code is spent, so the next attempt returns
+ * {@code InvalidCredentialsError} - which reads as a typo - and the failure counter climbs until
+ * the screen advises changing the Anisette server. Anisette had no part in it, and changing it
+ * forces a re-login against a different machine identity (AGENTS.md rule 4): somebody is sent to
+ * fix something that was never broken.
+ *
+ * Reported as
+ * #168. The desktop
+ * exporter fixed the same bug through the same library in #169, and this is deliberately reasoned
+ * the same way rather than invented differently.
+ *
+ * The arithmetic of the waits lives in {@code ACodeAppleAlreadyTookTest} on the JVM, including
+ * the test that goes red if either is shortened. This covers what the screen does.
+ */
+@LargeTest
+@RunWith(AndroidJUnit4.class)
+public class ACodeAppleTookAndThenFailedOnTest {
+
+ private static final String EMAIL = "someone@example.com";
+ private static final String PASSWORD = "hunter2";
+ private static final String A_CODE = "222222";
+
+ private FakeAppleAuthService apple;
+ private ActivityScenario The distinction the whole change turns on: this is not a wrong code, and a screen that
+ * says "Two-Factor Authentication failed" over an empty box invites the one action that
+ * cannot work.
+ */
+ @Test
+ public void itSaysAppleTookTheCodeRatherThanBlamingTheUser() {
+ this.getToTheCodeBoxAndSubmit();
+
+ Eventually.check(() -> onView(withId(R.id.verification_code_error_message))
+ .check(matches(isDisplayed())));
+
+ // **Asserted against the old wording, not merely the absence of "Anisette".** The old
+ // message does not mention Anisette on a first attempt either, so a test that only
+ // checked for that passed with the fix removed - it proved nothing about the case it was
+ // written for. What must be gone is the sentence that blames the code.
+ final String blamesTheCode = getInstrumentation().getTargetContext()
+ .getString(R.string.twofactor_failed_x, "").trim();
+
+ Eventually.check(() -> onView(withId(R.id.verification_code_error_message))
+ .check(matches(not(withText(containsString(blamesTheCode))))));
+ onView(withId(R.id.verification_code_error_message))
+ .check(matches(not(withText(containsString("Anisette")))));
+ }
+
+ /**
+ * And it does not count as a failed attempt.
+ *
+ * This is the assertion with teeth. The counter is what eventually produces the
+ * change-your-Anisette-server advice, so a fault on Apple's side reaching it turns one bad
+ * afternoon into a re-login against a different machine identity.
+ */
+ @Test
+ public void itDoesNotCountTowardsTheAnisetteAdvice() {
+ this.getToTheCodeBoxAndSubmit();
+ for (int attempt = 0; attempt < 3; attempt++) {
+ this.submitTheCode();
+ }
+
+ // Four goes is past HINT_DIFFERENT_ANISETTE_SERVER_AFTER_FAILED_2FACODES, so if this were
+ // being counted as a rejected code the hint would be on screen by now.
+ onView(withId(R.id.verification_code_error_message))
+ .check(matches(not(withText(containsString("Anisette")))));
+ }
+
+ /**
+ * Nobody is asked to sign in again.
+ *
+ * The account is still in its second-factor state, so a new code needs only a request on
+ * the chosen method. Sending them back to the email and password screen would be a second
+ * thing that looks broken.
+ */
+ @Test
+ public void theAppleIdAndPasswordAreNotAskedForAgain() {
+ this.getToTheCodeBoxAndSubmit();
+
+ Eventually.check(() -> onView(withId(R.id.login_2fa_container))
+ .check(matches(isDisplayed())));
+ onView(withId(R.id.login_maininfo_container)).check(matches(not(isDisplayed())));
+
+ assertEquals("signing in again was not needed and must not happen",
+ 1, this.apple.timesCalled("login"));
+ }
+
+ private void getToTheCodeBoxAndSubmit() {
+ onView(withId(R.id.email_or_phone_input_field)).perform(replaceText(EMAIL));
+ onView(withId(R.id.password_input_field))
+ .perform(replaceText(PASSWORD), closeSoftKeyboard());
+
+ Eventually.perform("the sign in button", () -> this.apple.timesCalled("login") > 0,
+ () -> onView(withId(R.id.login_button_main)).perform(click()));
+
+ Eventually.check(() -> onView(withText(containsString(FakeAppleAuthService.PHONE_ONE)))
+ .check(matches(isDisplayed())));
+ onView(withText(containsString(FakeAppleAuthService.PHONE_ONE))).perform(click());
+
+ this.submitTheCode();
+ }
+
+ private void submitTheCode() {
+ final long before = this.apple.timesCalled("submitCode");
+
+ Eventually.check(() -> onView(withId(R.id.twofactorauth_textinput_1))
+ .check(matches(isDisplayed())));
+
+ // replaceText rather than typeText: the boxes move focus as they fill, and a paste is
+ // what people actually do with a code. See AGENTS.md on Espresso.
+ Eventually.perform("the code", () -> this.apple.timesCalled("submitCode") > before,
+ () -> onView(withId(R.id.twofactorauth_textinput_1)).perform(replaceText(A_CODE)));
+ }
+}
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 5a47b334..b02f4a69 100644
--- a/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java
+++ b/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java
@@ -72,6 +72,7 @@
import dev.wander.android.opentagviewer.ui.settings.SharedMainSettingsManager;
import dev.wander.android.opentagviewer.util.android.AppCryptographyUtil;
import dev.wander.android.opentagviewer.util.android.PropertiesUtil;
+import dev.wander.android.opentagviewer.util.rx.ACodeAppleAlreadyTook;
import dev.wander.android.opentagviewer.viewmodel.AppleLoginViewModel;
import dev.wander.android.opentagviewer.viewmodel.LoginActivityState;
import dev.wander.android.opentagviewer.viewmodel.LoginActivityState.PAGE;
@@ -1013,6 +1014,104 @@ public void onClickBackTo2FAMethodChoice(View view) {
this.show2FAChoiceScreen(Direction.BACK);
}
+ /**
+ * How many times a spent code has been waited out on this screen. Never reset on purpose:
+ * two goes at it is the budget for one sign-in, not per code.
+ */
+ private int spentCodeRecoveries = 0;
+
+ /** Cancelled if the screen goes away mid-wait, so a dead activity is not written to. */
+ private final Handler waitingForApple = new Handler(Looper.getMainLooper());
+
+ /**
+ * Apple took the code and then failed. Wait, then ask for a new one - without a prompt.
+ *
+ * There is no question worth asking, so none is asked. Re-typing cannot work and
+ * waiting is the only option, so a dialog would offer a choice between one real answer and a
+ * wrong one. The exporter reached the same conclusion and has a test asserting the user is
+ * never prompted.
+ *
+ * The wait is shown counting down. Two minutes of a still screen on a phone is
+ * indistinguishable from a hang, and gets force-quit.
+ *
+ * And the new code is requested after the wait, never before. Both orders look
+ * right in a diff; Apple's codes expire, so one fetched first is two minutes stale by the
+ * time it is typed.
+ *
+ * The Apple ID and password are not asked for again. The account is still in its
+ * second-factor state, so requesting on the chosen method is all that is needed.
+ */
+ private void recoverFromApppleTakingTheCode(
+ final FrameLayout errorBox, final TextView errorText) {
+
+ final long wait = ACodeAppleAlreadyTook.waitBefore(this.spentCodeRecoveries);
+ this.spentCodeRecoveries++;
+
+ this.hideLoading();
+ this.showPage(R.id.login_2fa_container, Direction.BACK);
+ this.twoFactorEntryManager.clear();
+ this.twoFactorAuthChoiceBackButton.setEnabled(true);
+ errorBox.setVisibility(VISIBLE);
+
+ if (wait < 0) {
+ // Out of goes. Say whose fault it is, and warn about the password refusal - it
+ // happened in the one observed recovery, and unwarned it reads as a second,
+ // unrelated problem.
+ Log.w(TAG, "Apple did not recover after waiting it out twice");
+ errorText.setText(R.string.twofactor_apple_did_not_recover);
+ return;
+ }
+
+ Log.i(TAG, "Apple took the code and then failed; waiting " + wait
+ + "ms before asking for a new one");
+ errorText.setText(R.string.twofactor_apple_took_the_code);
+ this.countDownThenAskForANewCode(wait, errorBox, errorText);
+ }
+
+ private void countDownThenAskForANewCode(
+ final long remaining, final FrameLayout errorBox, final TextView errorText) {
+ if (this.isFinishing() || this.isDestroyed()) {
+ return;
+ }
+
+ if (remaining <= 0) {
+ final var chosen = this.getUiState().getChosenAuthMethod();
+ if (chosen == null) {
+ errorText.setText(R.string.twofactor_apple_did_not_recover);
+ return;
+ }
+
+ var async = this.authService.requestCode(chosen)
+ .observeOn(AndroidSchedulers.mainThread())
+ .subscribe(
+ () -> {
+ // The box is already empty and the prompt above it still says
+ // where the code was sent, so the error line's job is done.
+ Log.i(TAG, "Asked Apple for a fresh code after the wait");
+ errorBox.setVisibility(GONE);
+ },
+ error -> {
+ Log.e(TAG, "Asking for a fresh code failed too", error);
+ errorText.setText(R.string.twofactor_apple_did_not_recover);
+ });
+ return;
+ }
+
+ errorText.setText(this.getString(
+ R.string.twofactor_waiting_seconds, (int) Math.ceil(remaining / 1000.0)));
+
+ this.waitingForApple.postDelayed(
+ () -> this.countDownThenAskForANewCode(remaining - 1000L, errorBox, errorText),
+ 1000L);
+ }
+
+ @Override
+ protected void onDestroy() {
+ // Or a countdown outlives the screen and writes to views that are gone.
+ this.waitingForApple.removeCallbacksAndMessages(null);
+ super.onDestroy();
+ }
+
private void on2FAAuthCodeFilled(final String authCode) {
if (!REGEX_2FA_CODE.matcher(authCode).matches()) {
Log.w(TAG, "2FA Auth code from callback was invalid: " + authCode);
@@ -1055,6 +1154,17 @@ private void on2FAAuthCodeFilled(final String authCode) {
}, error -> {
// I really would like to handle this error separately from the one above, hence the nesting above.
Log.e(TAG, "Failed to authenticate using auth code " + authCode, error);
+
+ // **Apple taking the code and then failing is not a wrong code**, and must be told
+ // apart before anything counts it as one. See ACodeAppleAlreadyTook: the submit does
+ // two calls, the first succeeded, and the code is spent - so returning to the code
+ // box is the one action guaranteed to fail, and the attempt counter would eventually
+ // advise changing the Anisette server for a fault Anisette had no part in.
+ if (ACodeAppleAlreadyTook.spentIt(error)) {
+ this.recoverFromApppleTakingTheCode(twoFactorErrorMessage, errorMessageText);
+ return;
+ }
+
var state = this.getUiState();
final int failedLoginAttemptCount = state.getFailed2FAAttemptCount() + 1;
state.setFailed2FAAttemptCount(failedLoginAttemptCount);
diff --git a/app/src/main/java/dev/wander/android/opentagviewer/util/rx/ACodeAppleAlreadyTook.java b/app/src/main/java/dev/wander/android/opentagviewer/util/rx/ACodeAppleAlreadyTook.java
new file mode 100644
index 00000000..aeaccd0f
--- /dev/null
+++ b/app/src/main/java/dev/wander/android/opentagviewer/util/rx/ACodeAppleAlreadyTook.java
@@ -0,0 +1,90 @@
+package dev.wander.android.opentagviewer.util.rx;
+
+import java.util.concurrent.TimeUnit;
+
+/**
+ * The 503 that arrives after Apple has accepted a verification code.
+ *
+ * Two calls hide behind one submit. FindMy.py's {@code td_2fa_submit} first sends the
+ * code - that is the check, and it passes - and then runs a full Grand Slam re-authentication.
+ * The second half can fail on its own, and when it does the code has already been consumed.
+ *
+ * So the one thing the screen used to do is the one thing that cannot work. It sent the
+ * user back to an empty code box holding a code Apple has already spent. Typing it again returns
+ * {@code InvalidCredentialsError} - a different error, which reads as "you typed it wrong"
+ * - and the attempt counter climbs until the screen advises changing the Anisette server. That
+ * advice is wrong here and expensive: Anisette had nothing to do with it, and changing it forces
+ * a re-login against a different machine identity (AGENTS.md rule 4). Somebody is sent to fix
+ * something that was never broken.
+ *
+ * Reported as
+ * #168 and reproduced
+ * since. The desktop exporter fixed the same bug through the same library in #169; this is the
+ * app's half, deliberately reasoned the same way rather than invented differently.
+ */
+public final class ACodeAppleAlreadyTook {
+
+ /**
+ * How long to wait before asking for a new code, first time and second.
+ *
+ * These are a measurement, not round numbers, and they are the part most likely to be
+ * quietly lowered. The one observed recovery went: the 503; then a whole manual round -
+ * re-typing the Apple ID and password, choosing delivery, waiting for a code, typing it -
+ * which was refused at the password step; then another round, which worked. A manual
+ * round is the better part of a minute, so roughly a minute after the 503 the account was
+ * still being refused, and what eventually worked was about two rounds out.
+ *
+ * A first wait materially under a minute is known to be too short. {@code
+ * ACodeAppleAlreadyTookTest} goes red if either number drops, with the reasoning attached, so
+ * that lowering them has to be a decision rather than a tidy-up.
+ *
+ * The honest caveat, carried over from the exporter's write-up: that middle refusal was on
+ * the password call rather than the 2FA call, so it does not strictly prove a new code would
+ * have been rejected at that instant. It is the only measurement there is, and it points one
+ * way.
+ */
+ public static final long[] WAITS_MS = {
+ TimeUnit.SECONDS.toMillis(60),
+ TimeUnit.SECONDS.toMillis(120),
+ };
+
+ private ACodeAppleAlreadyTook() {
+ }
+
+ /**
+ * Whether this failure spent the user's code.
+ *
+ * A wide net, on purpose. Everything it catches happened after the submit
+ * returned, so the code is gone in all of them - and the cost of being wrong runs one way.
+ * Treating a spent code as a typo sends somebody back to type it again, which cannot work and
+ * ends in bad advice about Anisette; treating a typo as a spent code costs a wait and a fresh
+ * code, which is inconvenient and correct.
+ *
+ * Matched on the name because that is what survives the bridge: the failure arrives from
+ * Chaquopy as a {@code PyException} whose message carries the Python class. FindMy.py folds
+ * every non-OK status into {@code UnhandledProtocolError} with only the number in it, so
+ * there is nothing better to match on until the fork grows a transient-failure type - see the
+ * handover note, which explains why that was left out of the first fix.
+ */
+ public static boolean spentIt(final Throwable error) {
+ for (Throwable cause = error; cause != null; cause = cause.getCause()) {
+ final String message = cause.getMessage();
+ if (message != null && message.contains("UnhandledProtocolError")) {
+ return true;
+ }
+ if (cause.getCause() == cause) {
+ break;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * @param attempt how many times this has already been waited out, from zero.
+ * @return how long to wait before asking Apple for a new code, or -1 when there is no attempt
+ * left and the user should be told plainly that it did not clear.
+ */
+ public static long waitBefore(final int attempt) {
+ return attempt >= 0 && attempt < WAITS_MS.length ? WAITS_MS[attempt] : -1;
+ }
+}
diff --git a/app/src/main/res/values-de/strings.xml b/app/src/main/res/values-de/strings.xml
index e3262fa4..eb846b7f 100644
--- a/app/src/main/res/values-de/strings.xml
+++ b/app/src/main/res/values-de/strings.xml
@@ -310,4 +310,7 @@ Du kannst das jetzt einrichten oder jederzeit später in den Einstellungen.Auf dem Stand deines Apple-Kontos
The difference decides whether somebody is sent back to retype a code that can never be
+ * accepted, and eventually told to change their Anisette server for a fault that had nothing to
+ * do with Anisette.
+ */
+public class ACodeAppleAlreadyTookTest {
+
+ /** How the failure actually arrives: a Chaquopy PyException carrying the Python class name. */
+ private static Throwable fromTheBridge(final String message) {
+ return new RuntimeException("com.chaquo.python.PyException: " + message);
+ }
+
+ @Test
+ public void a503AfterTheCodeWasTakenIsRecognised() {
+ assertTrue(ACodeAppleAlreadyTook.spentIt(fromTheBridge(
+ "UnhandledProtocolError: Error response for GSA request: 503")));
+ }
+
+ @Test
+ public void itIsFoundThroughAWrappingException() {
+ final Throwable wrapped = new IllegalStateException("submitting the code failed",
+ fromTheBridge("UnhandledProtocolError: Error response for GSA request: 503"));
+
+ assertTrue("the bridge's exception is usually wrapped by the time a screen sees it",
+ ACodeAppleAlreadyTook.spentIt(wrapped));
+ }
+
+ /**
+ * The one it must not claim. A rejected code is a typo, and the screen's existing
+ * behaviour - clear the box, let them try again - is exactly right for it.
+ */
+ @Test
+ public void aRejectedCodeIsNotThis() {
+ assertFalse(ACodeAppleAlreadyTook.spentIt(fromTheBridge(
+ "InvalidCredentialsError: The verification code was not accepted")));
+ }
+
+ @Test
+ public void nothingAtAllIsNotThis() {
+ assertFalse(ACodeAppleAlreadyTook.spentIt(null));
+ assertFalse(ACodeAppleAlreadyTook.spentIt(new RuntimeException()));
+ }
+
+ /** A cycle in the cause chain must not hang the screen. See ICloudFailures, same guard. */
+ @Test(timeout = 2000)
+ public void aSelfReferencingCauseDoesNotSpin() {
+ final Throwable loop = new RuntimeException("something") {
+ @Override
+ public synchronized Throwable getCause() {
+ return this;
+ }
+ };
+
+ assertFalse(ACodeAppleAlreadyTook.spentIt(loop));
+ }
+
+ // ---------------------------------------------------------------- the waits
+
+ /**
+ * These numbers are a measurement, and this test exists so lowering them is a decision.
+ *
+ * The one observed recovery: the 503, then a full manual round - Apple ID, password,
+ * choose delivery, wait, type the code - which was refused at the password step, then another
+ * round that worked. A round is the better part of a minute, so the account was still
+ * refusing about a minute after the 503, and what worked was roughly two rounds out.
+ *
+ * So a first wait materially under a minute is known to be too short. If this test is in
+ * the way, the thing to change is the evidence, not the constant.
+ */
+ @Test
+ public void theFirstWaitIsNotShorterThanAMinute() {
+ assertTrue("a wait under a minute is known to be too short - see the class comment",
+ ACodeAppleAlreadyTook.waitBefore(0) >= TimeUnit.SECONDS.toMillis(60));
+ }
+
+ @Test
+ public void theSecondWaitIsLongerStill() {
+ assertTrue("the second attempt has to reach further out than the first",
+ ACodeAppleAlreadyTook.waitBefore(1) >= TimeUnit.SECONDS.toMillis(120));
+ }
+
+ @Test
+ public void thereAreExactlyTwoAttemptsAndThenItGivesUp() {
+ assertEquals(2, ACodeAppleAlreadyTook.WAITS_MS.length);
+ assertEquals("past the last wait there is nothing left to try",
+ -1, ACodeAppleAlreadyTook.waitBefore(2));
+ assertEquals(-1, ACodeAppleAlreadyTook.waitBefore(99));
+ }
+
+ @Test
+ public void aNonsenseAttemptNumberDoesNotWait() {
+ assertEquals(-1, ACodeAppleAlreadyTook.waitBefore(-1));
+ }
+}