Stop asking for a verification code Apple has already spent

app/src/androidTest/java/dev/wander/android/opentagviewer/python/FakeAppleAuthService.java

@@ -124,6 +124,23 @@ public FakeAppleAuthService thatRejectsTheCode(final String message) {
         return this;
     }
 
+    /**
+     * <b>Apple takes the code and then fails to finish - so the code is spent.</b>
+     *
+     * <p>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.
+     *
+     * <p>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<PythonAuthResponse> login(
             final String emailOrPhone, final String password, final String anisetteServerUrl,

app/src/androidTest/java/dev/wander/android/opentagviewer/ui/login/ACodeAppleTookAndThenFailedOnTest.java

@@ -0,0 +1,190 @@
+package dev.wander.android.opentagviewer.ui.login;
+
+import static androidx.test.espresso.Espresso.onView;
+import static androidx.test.espresso.action.ViewActions.click;
+import static androidx.test.espresso.action.ViewActions.closeSoftKeyboard;
+import static androidx.test.espresso.action.ViewActions.replaceText;
+import static androidx.test.espresso.assertion.ViewAssertions.matches;
+import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
+import static androidx.test.espresso.matcher.ViewMatchers.withId;
+import static androidx.test.espresso.matcher.ViewMatchers.withText;
+import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+import static org.junit.Assert.assertEquals;
+
+import android.app.Activity;
+import android.app.Instrumentation.ActivityResult;
+
+import androidx.test.core.app.ActivityScenario;
+import androidx.test.espresso.intent.Intents;
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+import androidx.test.filters.LargeTest;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import dev.wander.android.opentagviewer.AppleLoginActivity;
+import dev.wander.android.opentagviewer.Eventually;
+import dev.wander.android.opentagviewer.MapsActivity;
+import dev.wander.android.opentagviewer.R;
+import dev.wander.android.opentagviewer.anisette.FakeAnisetteSource;
+import dev.wander.android.opentagviewer.db.AccountBeaconsForTests;
+import dev.wander.android.opentagviewer.python.AppDependencies;
+import dev.wander.android.opentagviewer.python.FakeAppleAuthService;
+
+/**
+ * The 503 that arrives <i>after</i> Apple has accepted the code.
+ *
+ * <p><b>The submit is two calls, and only the second failed.</b> 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.
+ *
+ * <p><b>So the screen's old behaviour was the one thing guaranteed to fail.</b> 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.
+ *
+ * <p>Reported as
+ * <a href="https://github.com/parawanderer/OpenTagViewer/issues/168">#168</a>. 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.
+ *
+ * <p>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<AppleLoginActivity> scenario;
+
+    @Before
+    public void replaceApple() {
+        AccountBeaconsForTests.forgetThemAll();
+
+        this.apple = FakeAppleAuthService.wantsTwoFactor().whereAppleTakesTheCodeThenFails();
+        AppDependencies.replaceAuthService(this.apple);
+        AppDependencies.replaceAnisette(settings -> FakeAnisetteSource.ready());
+
+        Intents.init();
+        Intents.intending(androidx.test.espresso.intent.matcher.IntentMatchers.hasComponent(
+                        MapsActivity.class.getName()))
+                .respondWith(new ActivityResult(Activity.RESULT_OK, null));
+
+        this.scenario = ActivityScenario.launch(AppleLoginActivity.class);
+    }
+
+    @After
+    public void putItBack() {
+        if (this.scenario != null) {
+            this.scenario.close();
+        }
+        Intents.release();
+        AppDependencies.reset();
+        getInstrumentation().waitForIdleSync();
+        AccountBeaconsForTests.forgetThemAll();
+    }
+
+    /**
+     * <b>It says Apple took the code, rather than blaming the code.</b>
+     *
+     * <p>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")))));
+    }
+
+    /**
+     * <b>And it does not count as a failed attempt.</b>
+     *
+     * <p>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")))));
+    }
+
+    /**
+     * <b>Nobody is asked to sign in again.</b>
+     *
+     * <p>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)));
+    }
+}

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.
+     *
+     * <p><b>There is no question worth asking, so none is asked.</b> 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.
+     *
+     * <p><b>The wait is shown counting down.</b> Two minutes of a still screen on a phone is
+     * indistinguishable from a hang, and gets force-quit.
+     *
+     * <p><b>And the new code is requested after the wait, never before.</b> 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.
+     *
+     * <p>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);