Remove an obsolete client version check when changing phone numbers

signalapp · Aug 16, 2024 · fc3e547 · fc3e547
1 parent e4f9f94
commit fc3e547
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 71 deletions.
diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2.java
@@ -8,9 +8,7 @@
 import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name;
 
 import com.google.common.net.HttpHeaders;
-import com.vdurmont.semver4j.Semver;
 import io.dropwizard.auth.Auth;
-import io.micrometer.core.instrument.Counter;
 import io.micrometer.core.instrument.Metrics;
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Tags;
@@ -54,10 +52,6 @@
 import org.whispersystems.textsecuregcm.storage.Account;
 import org.whispersystems.textsecuregcm.storage.AccountsManager;
 import org.whispersystems.textsecuregcm.storage.ChangeNumberManager;
-import org.whispersystems.textsecuregcm.util.ua.ClientPlatform;
-import org.whispersystems.textsecuregcm.util.ua.UnrecognizedUserAgentException;
-import org.whispersystems.textsecuregcm.util.ua.UserAgent;
-import org.whispersystems.textsecuregcm.util.ua.UserAgentUtil;
 import org.whispersystems.websocket.auth.Mutable;
 import org.whispersystems.websocket.auth.ReadOnly;
 
@@ -68,20 +62,18 @@ public class AccountControllerV2 {
   private static final String CHANGE_NUMBER_COUNTER_NAME = name(AccountControllerV2.class, "changeNumber");
   private static final String VERIFICATION_TYPE_TAG_NAME = "verification";
 
-  private static final Counter ANDROID_CHANGE_NUMBER_REJECTED_COUNTER =
-      Metrics.counter(name(AccountControllerV2.class, "androidChangeNumberRejected"));
-
   private final AccountsManager accountsManager;
   private final ChangeNumberManager changeNumberManager;
   private final PhoneVerificationTokenManager phoneVerificationTokenManager;
   private final RegistrationLockVerificationManager registrationLockVerificationManager;
   private final RateLimiters rateLimiters;
 
-  private static final Semver MINIMUM_ANDROID_CHANGE_NUMBER_VERSION = new Semver("6.46.7");
-
-  public AccountControllerV2(final AccountsManager accountsManager, final ChangeNumberManager changeNumberManager,
+  public AccountControllerV2(final AccountsManager accountsManager,
+      final ChangeNumberManager changeNumberManager,
       final PhoneVerificationTokenManager phoneVerificationTokenManager,
-      final RegistrationLockVerificationManager registrationLockVerificationManager, final RateLimiters rateLimiters) {
+      final RegistrationLockVerificationManager registrationLockVerificationManager,
+      final RateLimiters rateLimiters) {
+
     this.accountsManager = accountsManager;
     this.changeNumberManager = changeNumberManager;
     this.phoneVerificationTokenManager = phoneVerificationTokenManager;
@@ -113,17 +105,6 @@ public AccountIdentityResponse changeNumber(@Mutable @Auth final AuthenticatedDe
       throw new ForbiddenException();
     }
 
-    // We can remove this check after old versions of the Android app expire on or after 2024-05-08
-    try {
-      final UserAgent userAgent = UserAgentUtil.parseUserAgentString(userAgentString);
-
-      if (userAgent.getPlatform().equals(ClientPlatform.ANDROID) && userAgent.getVersion().isLowerThan(MINIMUM_ANDROID_CHANGE_NUMBER_VERSION)) {
-        ANDROID_CHANGE_NUMBER_REJECTED_COUNTER.increment();
-        throw new WebApplicationException(499);
-      }
-    } catch (final UnrecognizedUserAgentException ignored) {
-    }
-
     if (!request.isSignatureValidOnEachSignedPreKey(userAgentString)) {
       throw new WebApplicationException("Invalid signature", 422);
     }
@@ -135,8 +116,8 @@ public AccountIdentityResponse changeNumber(@Mutable @Auth final AuthenticatedDe
 
       rateLimiters.getRegistrationLimiter().validate(number);
 
-      final PhoneVerificationRequest.VerificationType verificationType = phoneVerificationTokenManager.verify(number,
-          request);
+      final PhoneVerificationRequest.VerificationType verificationType =
+          phoneVerificationTokenManager.verify(number, request);
 
       final Optional<Account> existingAccount = accountsManager.getByE164(number);
 

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java
@@ -26,7 +26,6 @@
 import org.whispersystems.textsecuregcm.entities.MessageProtos.Envelope;
 import org.whispersystems.textsecuregcm.identity.AciServiceIdentifier;
 import org.whispersystems.textsecuregcm.push.MessageSender;
-import org.whispersystems.textsecuregcm.push.NotPushRegisteredException;
 import org.whispersystems.textsecuregcm.util.DestinationDeviceValidator;
 
 public class ChangeNumberManager {

diff --git a/...e/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2Test.java b/...e/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2Test.java
@@ -56,7 +56,6 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
-import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.EnumSource;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.mockito.ArgumentCaptor;
@@ -419,49 +418,6 @@ void recoveryPasswordManagerVerificationFalse() {
       }
     }
 
-    @ParameterizedTest
-    @CsvSource({
-        "Signal-Android/4.68.3, true",
-        "Signal-Android/6.46.7, false",
-        "Signal-Android/6.46.8, false",
-        "Signal-Desktop/1.2.3 Linux, false",
-        "Signal-iOS/3.9.0 iOS/14.2, false",
-        "Not a real user-agent string, false"
-    })
-    void changeNumberVersionEnforced(final String userAgentString, final boolean expectBlocked) throws Exception {
-
-      when(registrationServiceClient.getSession(any(), any()))
-          .thenReturn(CompletableFuture.completedFuture(
-              Optional.of(new RegistrationServiceSession(new byte[16], NEW_NUMBER, true, null, null, null,
-                  SESSION_EXPIRATION_SECONDS))));
-
-      try (final Response response = resources.getJerseyTest()
-              .target("/v2/accounts/number")
-              .request()
-              .header(HttpHeaders.AUTHORIZATION,
-                  AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
-              .header(HttpHeaders.USER_AGENT, userAgentString)
-              .put(Entity.entity(
-                  new ChangeNumberRequest(encodeSessionId("session"), null, NEW_NUMBER, "123", new IdentityKey(Curve.generateKeyPair().getPublicKey()),
-                      Collections.emptyList(),
-                      Collections.emptyMap(), null, Collections.emptyMap()),
-                  MediaType.APPLICATION_JSON_TYPE))) {
-
-        if (expectBlocked) {
-          assertEquals(499, response.getStatus());
-
-          verify(changeNumberManager, never())
-              .changeNumber(eq(AuthHelper.VALID_ACCOUNT), eq(NEW_NUMBER), any(), any(), any(), any(), any());
-        } else {
-          assertEquals(200, response.getStatus());
-
-          verify(changeNumberManager)
-              .changeNumber(eq(AuthHelper.VALID_ACCOUNT), eq(NEW_NUMBER), any(), any(), any(), any(), any());
-
-        }
-      }
-    }
-
     /**
      * Valid request JSON with the given Recovery Password
      */