Reject old-format Benin numbers, which are now undeliverable

Author: eager-signal

service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java

@@ -175,6 +175,7 @@
 import org.whispersystems.textsecuregcm.mappers.InvalidWebsocketAddressExceptionMapper;
 import org.whispersystems.textsecuregcm.mappers.JsonMappingExceptionMapper;
 import org.whispersystems.textsecuregcm.mappers.NonNormalizedPhoneNumberExceptionMapper;
+import org.whispersystems.textsecuregcm.mappers.ObsoletePhoneNumberFormatExceptionMapper;
 import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper;
 import org.whispersystems.textsecuregcm.mappers.RegistrationServiceSenderExceptionMapper;
 import org.whispersystems.textsecuregcm.mappers.ServerRejectedExceptionMapper;
@@ -1212,6 +1213,7 @@ private void registerExceptionMappers(Environment environment,
         new ServerRejectedExceptionMapper(),
         new ImpossiblePhoneNumberExceptionMapper(),
         new NonNormalizedPhoneNumberExceptionMapper(),
+        new ObsoletePhoneNumberFormatExceptionMapper(),
         new RegistrationServiceSenderExceptionMapper(),
         new SubscriptionExceptionMapper(),
         new JsonMappingExceptionMapper()

service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java

@@ -95,6 +95,7 @@
 import org.whispersystems.textsecuregcm.storage.RegistrationRecoveryPasswordsManager;
 import org.whispersystems.textsecuregcm.storage.VerificationSessionManager;
 import org.whispersystems.textsecuregcm.util.ExceptionUtils;
+import org.whispersystems.textsecuregcm.util.ObsoletePhoneNumberFormatException;
 import org.whispersystems.textsecuregcm.util.Pair;
 import org.whispersystems.textsecuregcm.util.Util;
 
@@ -173,7 +174,7 @@ public VerificationController(final RegistrationServiceClient registrationServic
       description = "If present, an positive integer indicating the number of seconds before a subsequent attempt could succeed",
       schema = @Schema(implementation = Integer.class)))
   public VerificationSessionResponse createSession(@NotNull @Valid final CreateVerificationSessionRequest request)
-      throws RateLimitExceededException {
+      throws RateLimitExceededException, ObsoletePhoneNumberFormatException {
 
     final Pair<String, PushNotification.TokenType> pushTokenAndType = validateAndExtractPushToken(
         request.getUpdateVerificationSessionRequest());

service/src/main/java/org/whispersystems/textsecuregcm/mappers/ObsoletePhoneNumberFormatExceptionMapper.java

@@ -0,0 +1,18 @@
+package org.whispersystems.textsecuregcm.mappers;
+
+import io.micrometer.core.instrument.Metrics;
+import jakarta.ws.rs.core.Response;
+import jakarta.ws.rs.ext.ExceptionMapper;
+import org.whispersystems.textsecuregcm.metrics.MetricsUtil;
+import org.whispersystems.textsecuregcm.util.ObsoletePhoneNumberFormatException;
+
+public class ObsoletePhoneNumberFormatExceptionMapper implements ExceptionMapper<ObsoletePhoneNumberFormatException> {
+
+  private static final String COUNTER_NAME = MetricsUtil.name(ObsoletePhoneNumberFormatExceptionMapper.class, "errors");
+
+  @Override
+  public Response toResponse(final ObsoletePhoneNumberFormatException exception) {
+    Metrics.counter(COUNTER_NAME, "regionCode", exception.getRegionCode()).increment();
+    return Response.status(499).build();
+  }
+}

service/src/main/java/org/whispersystems/textsecuregcm/util/ObsoletePhoneNumberFormatException.java

@@ -0,0 +1,15 @@
+package org.whispersystems.textsecuregcm.util;
+
+public class ObsoletePhoneNumberFormatException extends Exception {
+
+  private final String regionCode;
+
+  public ObsoletePhoneNumberFormatException(final String regionCode) {
+    super("The provided format is obsolete in %s".formatted(regionCode));
+    this.regionCode = regionCode;
+  }
+
+  public String getRegionCode() {
+    return regionCode;
+  }
+}

service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java

@@ -27,7 +27,6 @@
 import java.util.function.Function;
 import java.util.random.RandomGenerator;
 import java.util.stream.Collectors;
-
 import org.apache.commons.lang3.StringUtils;
 
 public class Util {
@@ -125,7 +124,7 @@ public static List<String> getAlternateForms(final String number) {
     try {
       final PhoneNumber phoneNumber = PHONE_NUMBER_UTIL.parse(number, null);
 
-      // Benin is changing phone number formats from +229 XXXXXXXX to +229 01XXXXXXXX starting on November 30, 2024
+      // Benin changed phone number formats from +229 XXXXXXXX to +229 01XXXXXXXX on November 30, 2024
       if ("BJ".equals(PHONE_NUMBER_UTIL.getRegionCodeForNumber(phoneNumber))) {
         final String nationalSignificantNumber = PHONE_NUMBER_UTIL.getNationalSignificantNumber(phoneNumber);
         final String alternateE164;
@@ -176,7 +175,7 @@ public static Optional<String> getCanonicalNumber(List<String> e164s) {
         throw new IllegalArgumentException("Numbers from different countries cannot be equivalent alternate forms");
       }
       if (regions.contains("BJ")) {
-        // Benin is changing phone number formats from +229 XXXXXXXX to +229 01XXXXXXXX starting on November 30, 2024
+        // Benin changed phone number formats from +229 XXXXXXXX to +229 01XXXXXXXX on November 30, 2024
         // We prefer the longest form for long-term stability
         return e164s.stream().sorted(Comparator.comparingInt(String::length).reversed()).findFirst();
       }
@@ -217,7 +216,7 @@ public static boolean startsWithDecimal(final long number, final long prefix) {
   }
 
   /**
-   * Benin is changing phone number formats from +229 XXXXXXXX to +229 01XXXXXXXX starting on November 30, 2024.
+   * Benin changed phone number formats from +229 XXXXXXXX to +229 01XXXXXXXX on November 30, 2024
    *
    * @param phoneNumber the phone number to check.
    * @return whether the provided phone number is an old-format Benin phone number
@@ -235,11 +234,9 @@ public static boolean isOldFormatBeninPhoneNumber(final Phonenumber.PhoneNumber
    * @return the canonical phone number if applicable, otherwise the original phone number.
    */
   public static Phonenumber.PhoneNumber canonicalizePhoneNumber(final Phonenumber.PhoneNumber phoneNumber)
-      throws NumberParseException {
+      throws NumberParseException, ObsoletePhoneNumberFormatException {
     if (isOldFormatBeninPhoneNumber(phoneNumber)) {
-      // Benin changed phone number formats from +229 XXXXXXXX to +229 01XXXXXXXX starting on November 30, 2024.
-      final String newFormatNumber = "+22901" + PHONE_NUMBER_UTIL.getNationalSignificantNumber(phoneNumber);
-      return PhoneNumberUtil.getInstance().parse(newFormatNumber, null);
+      throw new ObsoletePhoneNumberFormatException("bj");
     }
     return phoneNumber;
   }

service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java

@@ -66,6 +66,7 @@
 import org.whispersystems.textsecuregcm.limits.RateLimiters;
 import org.whispersystems.textsecuregcm.mappers.ImpossiblePhoneNumberExceptionMapper;
 import org.whispersystems.textsecuregcm.mappers.NonNormalizedPhoneNumberExceptionMapper;
+import org.whispersystems.textsecuregcm.mappers.ObsoletePhoneNumberFormatExceptionMapper;
 import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper;
 import org.whispersystems.textsecuregcm.mappers.RegistrationServiceSenderExceptionMapper;
 import org.whispersystems.textsecuregcm.push.PushNotificationManager;
@@ -117,6 +118,7 @@ class VerificationControllerTest {
       .addProvider(new RateLimitExceededExceptionMapper())
       .addProvider(new ImpossiblePhoneNumberExceptionMapper())
       .addProvider(new NonNormalizedPhoneNumberExceptionMapper())
+      .addProvider(new ObsoletePhoneNumberFormatExceptionMapper())
       .addProvider(new RegistrationServiceSenderExceptionMapper())
       .setMapper(SystemMapper.jsonMapper())
       .setTestContainerFactory(new GrizzlyWebTestContainerFactory())
@@ -220,7 +222,7 @@ void createBeninSessionSuccess(final String requestedNumber, final String expect
     when(registrationServiceClient.createRegistrationSession(any(), anyBoolean(), any()))
         .thenReturn(
             CompletableFuture.completedFuture(
-                new RegistrationServiceSession(SESSION_ID, NUMBER, false, null, null, null,
+                new RegistrationServiceSession(SESSION_ID, requestedNumber, false, null, null, null,
                     SESSION_EXPIRATION_SECONDS)));
     when(verificationSessionManager.insert(any(), any()))
         .thenReturn(CompletableFuture.completedFuture(null));
@@ -245,14 +247,36 @@ private static Stream<Arguments> createBeninSessionSuccess() {
     // libphonenumber 8.13.50 and on generate new-format numbers for Benin
     final String newFormatBeninE164 = PhoneNumberUtil.getInstance()
         .format(PhoneNumberUtil.getInstance().getExampleNumber("BJ"), PhoneNumberUtil.PhoneNumberFormat.E164);
-    final String oldFormatBeninE164 = newFormatBeninE164.replaceFirst("01", "");
     return Stream.of(
-        Arguments.of(oldFormatBeninE164, newFormatBeninE164),
         Arguments.of(newFormatBeninE164, newFormatBeninE164),
         Arguments.of(NUMBER, NUMBER)
     );
   }
 
+  @Test
+  void createBeninSessionFailure() {
+    // libphonenumber 8.13.50 and on generate new-format numbers for Benin
+    final String newFormatBeninE164 = PhoneNumberUtil.getInstance()
+        .format(PhoneNumberUtil.getInstance().getExampleNumber("BJ"), PhoneNumberUtil.PhoneNumberFormat.E164);
+    final String oldFormatBeninE164 = newFormatBeninE164.replaceFirst("01", "");
+
+    when(registrationServiceClient.createRegistrationSession(any(), anyBoolean(), any()))
+        .thenReturn(
+            CompletableFuture.completedFuture(
+                new RegistrationServiceSession(SESSION_ID, NUMBER, false, null, null, null,
+                    SESSION_EXPIRATION_SECONDS)));
+    when(verificationSessionManager.insert(any(), any()))
+        .thenReturn(CompletableFuture.completedFuture(null));
+
+    final Invocation.Builder request = resources.getJerseyTest()
+        .target("/v1/verification/session")
+        .request()
+        .header(HttpHeaders.X_FORWARDED_FOR, "127.0.0.1");
+    try (Response response = request.post(Entity.json(createSessionJson(oldFormatBeninE164, "token", "fcm")))) {
+      assertEquals(499, response.getStatus());
+    }
+  }
+
   @ParameterizedTest
   @MethodSource
   void createSessionSuccess(final String pushToken, final String pushTokenType,

service/src/test/java/org/whispersystems/textsecuregcm/util/UtilTest.java

@@ -7,15 +7,17 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.google.i18n.phonenumbers.NumberParseException;
 import com.google.i18n.phonenumbers.PhoneNumberUtil;
+import com.google.i18n.phonenumbers.Phonenumber;
 import java.util.List;
 import java.util.Optional;
-import org.junit.jupiter.api.Test;
 import java.util.stream.Stream;
-import com.google.i18n.phonenumbers.Phonenumber;
+import javax.annotation.Nullable;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.CsvSource;
@@ -93,19 +95,23 @@ private static Stream<Arguments> isOldFormatBeninPhoneNumber4() throws NumberPar
 
   @ParameterizedTest
   @MethodSource
-  void normalizeBeninPhoneNumber(final Phonenumber.PhoneNumber beninNumber, final Phonenumber.PhoneNumber expectedBeninNumber)
-      throws NumberParseException {
-    assertTrue(expectedBeninNumber.exactlySameAs(Util.canonicalizePhoneNumber(beninNumber)));
+  void normalizeBeninPhoneNumber(final Phonenumber.PhoneNumber beninNumber, final Phonenumber.PhoneNumber expectedBeninNumber, @Nullable Class<? extends Throwable> exception)
+      throws Exception {
+    if (exception == null) {
+      assertTrue(expectedBeninNumber.exactlySameAs(Util.canonicalizePhoneNumber(beninNumber)));
+    } else {
+      assertThrows(exception, () -> Util.canonicalizePhoneNumber(beninNumber));
+    }
   }
 
   private static Stream<Arguments> normalizeBeninPhoneNumber() throws NumberParseException {
     final Phonenumber.PhoneNumber oldFormatBeninPhoneNumber = PhoneNumberUtil.getInstance().parse(OLD_FORMAT_BENIN_E164_STRING, null);
     final Phonenumber.PhoneNumber newFormatBeninPhoneNumber = PhoneNumberUtil.getInstance().parse(NEW_FORMAT_BENIN_E164_STRING, null);
     final Phonenumber.PhoneNumber usPhoneNumber = PhoneNumberUtil.getInstance().getExampleNumber("US");
     return Stream.of(
-        Arguments.of(newFormatBeninPhoneNumber, newFormatBeninPhoneNumber),
-        Arguments.of(oldFormatBeninPhoneNumber, newFormatBeninPhoneNumber),
-        Arguments.of(usPhoneNumber, usPhoneNumber)
+        Arguments.of(newFormatBeninPhoneNumber, newFormatBeninPhoneNumber, null),
+        Arguments.of(oldFormatBeninPhoneNumber, null, ObsoletePhoneNumberFormatException.class),
+        Arguments.of(usPhoneNumber, usPhoneNumber, null)
     );
   }
 }