If push challenge times out, don't try again.

2025-12-24 04:58:45 +00:00 · 2023-03-13 09:50:54 -04:00
parent f24d82bf04
commit 9087f427a5
6 changed files with 48 additions and 11 deletions
--- a/app/src/main/java/org/thoughtcrime/securesms/registration/PushChallengeRequest.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/registration/PushChallengeRequest.java
@@ -38,7 +38,7 @@ public final class PushChallengeRequest {
                                                          @NonNull Optional<String> fcmToken,
                                                          long timeoutMs)
  {
-    if (!fcmToken.isPresent()) {
+    if (fcmToken.isEmpty() || fcmToken.get().isEmpty()) {
      Log.w(TAG, "Push challenge not requested, as no FCM token was present");
      return Optional.empty();
    }
--- a/app/src/main/java/org/thoughtcrime/securesms/registration/RegistrationSessionProcessor.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/registration/RegistrationSessionProcessor.kt
@@ -13,6 +13,7 @@ import org.whispersystems.signalservice.api.util.Preconditions
 import org.whispersystems.signalservice.internal.ServiceResponse
 import org.whispersystems.signalservice.internal.ServiceResponseProcessor
 import org.whispersystems.signalservice.internal.push.RegistrationSessionMetadataResponse
+import org.whispersystems.signalservice.internal.push.RegistrationSessionState
 import kotlin.time.Duration.Companion.seconds

 /**
@@ -27,7 +28,7 @@ sealed class RegistrationSessionProcessor(response: ServiceResponse<Registration
  }

  public override fun captchaRequired(): Boolean {
-    return super.captchaRequired() || (hasResult() && CAPTCHA_KEY == getChallenge())
+    return super.captchaRequired() || (hasResult() && CAPTCHA_KEY == getChallenge(emptyList()))
  }

  public override fun rateLimit(): Boolean {
@@ -38,8 +39,9 @@ sealed class RegistrationSessionProcessor(response: ServiceResponse<Registration
    return super.getError()
  }

-  fun pushChallengeRequired(): Boolean {
-    return PUSH_CHALLENGE_KEY == getChallenge()
+  fun pushChallengeTimedOut(): Boolean {
+    val state: RegistrationSessionState = response.result.get().state ?: return false
+    return state.pushChallengeTimedOut
  }

  fun isTokenRejected(): Boolean {
@@ -107,9 +109,15 @@ sealed class RegistrationSessionProcessor(response: ServiceResponse<Registration
    return result.body.allowedToRequestCode
  }

-  fun getChallenge(): String? {
+  /**
+   * Parse the response body for the server requested challenges that the client may submit.
+   *
+   * @param exclusions a collection of keys to ignore, used when they've already tried and failed
+   * @return the next challenge
+   */
+  fun getChallenge(exclusions: Collection<String>): String? {
    Preconditions.checkState(hasResult(), "This can only be called when result is present!")
-    return result.body.requestedInformation.firstOrNull { REQUESTABLE_INFORMATION.contains(it) }
+    return result.body.requestedInformation.filterNot { exclusions.contains(it) }.firstOrNull { REQUESTABLE_INFORMATION.contains(it) }
  }

  fun isVerified(): Boolean {
--- a/app/src/main/java/org/thoughtcrime/securesms/registration/VerifyAccountRepository.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/registration/VerifyAccountRepository.kt
@@ -22,6 +22,7 @@ import org.whispersystems.signalservice.api.push.SignalServiceAddress
 import org.whispersystems.signalservice.api.push.exceptions.NoSuchSessionException
 import org.whispersystems.signalservice.internal.ServiceResponse
 import org.whispersystems.signalservice.internal.push.RegistrationSessionMetadataResponse
+import org.whispersystems.signalservice.internal.push.RegistrationSessionState
 import java.io.IOException
 import java.util.Locale
 import java.util.Optional
@@ -84,7 +85,9 @@ class VerifyAccountRepository(private val context: Application) {
    return if (challenge != null) {
      accountManager.submitPushChallengeToken(response.result.get().body.id, challenge)
    } else {
-      response
+      val registrationSessionState = RegistrationSessionState(pushChallengeTimedOut = true)
+      val rawResponse: RegistrationSessionMetadataResponse = response.result.get()
+      ServiceResponse.forResult(rawResponse.copy(state = registrationSessionState), 200, null)
    }
  }

--- a/app/src/main/java/org/thoughtcrime/securesms/registration/viewmodel/BaseRegistrationViewModel.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/registration/viewmodel/BaseRegistrationViewModel.java
@@ -25,6 +25,8 @@ import org.thoughtcrime.securesms.registration.VerifyResponseWithSuccessfulKbs;
 import org.thoughtcrime.securesms.registration.VerifyResponseWithoutKbs;
 import org.whispersystems.signalservice.internal.ServiceResponse;

+import java.util.ArrayList;
+import java.util.List;
 import java.util.Objects;

 import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers;
@@ -43,6 +45,7 @@ public abstract class BaseRegistrationViewModel extends ViewModel {
  private static final String STATE_REGISTRATION_SECRET     = "REGISTRATION_SECRET";
  private static final String STATE_VERIFICATION_CODE       = "TEXT_CODE_ENTERED";
  private static final String STATE_CAPTCHA                 = "CAPTCHA";
+  private static final String STATE_PUSH_TIMED_OUT          = "PUSH_TIMED_OUT";
  private static final String STATE_INCORRECT_CODE_ATTEMPTS = "STATE_INCORRECT_CODE_ATTEMPTS";
  private static final String STATE_REQUEST_RATE_LIMITER    = "REQUEST_RATE_LIMITER";
  private static final String STATE_KBS_TOKEN               = "KBS_TOKEN";
@@ -71,6 +74,7 @@ public abstract class BaseRegistrationViewModel extends ViewModel {
    setInitialDefaultValue(STATE_INCORRECT_CODE_ATTEMPTS, 0);
    setInitialDefaultValue(STATE_REQUEST_RATE_LIMITER, new LocalCodeRequestRateLimiter(60_000));
    setInitialDefaultValue(STATE_RECOVERY_PASSWORD, SignalStore.kbsValues().getRecoveryPassword());
+    setInitialDefaultValue(STATE_PUSH_TIMED_OUT, false);
  }

  protected <T> void setInitialDefaultValue(@NonNull String key, @Nullable T initialValue) {
@@ -172,6 +176,18 @@ public abstract class BaseRegistrationViewModel extends ViewModel {
    return savedState.getLiveData(STATE_INCORRECT_CODE_ATTEMPTS, 0);
  }

+  public void markPushChallengeTimedOut() {
+    savedState.set(STATE_PUSH_TIMED_OUT, true);
+  }
+
+  public List<String> getExcludedChallenges() {
+    ArrayList<String> challengeKeys = new ArrayList<>();
+    if (Boolean.TRUE.equals(savedState.get(STATE_PUSH_TIMED_OUT))) {
+      challengeKeys.add(RegistrationSessionProcessor.PUSH_CHALLENGE_KEY);
+    }
+    return challengeKeys;
+  }
+
  public @Nullable TokenData getKeyBackupCurrentToken() {
    return savedState.get(STATE_KBS_TOKEN);
  }
@@ -268,7 +284,12 @@ public abstract class BaseRegistrationViewModel extends ViewModel {
        .flatMap(processor -> {
          if (processor.isInvalidSession()) {
            return verifyAccountRepository.requestValidSession(e164, getRegistrationSecret(), mcc, mnc)
-                                          .map(RegistrationSessionProcessor.RegistrationSessionProcessorForSession::new);
+                                          .map(RegistrationSessionProcessor.RegistrationSessionProcessorForSession::new)
+                                          .doOnSuccess(createSessionProcessor -> {
+                                            if (createSessionProcessor.pushChallengeTimedOut()) {
+                                              markPushChallengeTimedOut();
+                                            }
+                                          });
          } else {
            return Single.just(processor);
          }
@@ -289,7 +310,7 @@ public abstract class BaseRegistrationViewModel extends ViewModel {
      return verifyAccountRepository.verifyCaptcha(sessionId, captcha, e164, getRegistrationSecret())
                                    .map(RegistrationSessionProcessor.RegistrationSessionProcessorForSession::new);
    } else {
-      String challenge = processor.getChallenge();
+      String challenge = processor.getChallenge(getExcludedChallenges());
      Log.d(TAG, "Handling challenge of type " + challenge);
      if (challenge != null) {
        switch (challenge) {
--- a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java
+++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java
@@ -2625,7 +2625,7 @@ public class PushServiceSocket {
    RegistrationSessionMetadataHeaders responseHeaders = new RegistrationSessionMetadataHeaders(serverDeliveredTimestamp);
    RegistrationSessionMetadataJson responseBody = JsonUtil.fromJson(readBodyString(response), RegistrationSessionMetadataJson.class);

-    return new RegistrationSessionMetadataResponse(responseHeaders, responseBody);
+    return new RegistrationSessionMetadataResponse(responseHeaders, responseBody, null);
  }

  public static final class GroupHistory {
--- a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/RegistrationSessionMetadataResponse.kt
+++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/RegistrationSessionMetadataResponse.kt
@@ -8,7 +8,8 @@ import com.fasterxml.jackson.annotation.JsonProperty
 */
 data class RegistrationSessionMetadataResponse(
  val headers: RegistrationSessionMetadataHeaders,
-  val body: RegistrationSessionMetadataJson
+  val body: RegistrationSessionMetadataJson,
+  val state: RegistrationSessionState?,
 )

 data class RegistrationSessionMetadataHeaders(
@@ -32,3 +33,7 @@ data class RegistrationSessionMetadataJson(
    return requestedInformation.contains("captcha")
  }
 }
+
+data class RegistrationSessionState(
+  var pushChallengeTimedOut: Boolean,
+)