From 3a4a55c2452725147a0fc77ede05a84db419413d Mon Sep 17 00:00:00 2001 From: Chris Eager Date: Fri, 3 Jan 2025 12:17:13 -0600 Subject: [PATCH] Reject old-format Benin numbers, which are now undeliverable --- .../textsecuregcm/WhisperServerService.java | 2 ++ .../controllers/VerificationController.java | 3 +- ...oletePhoneNumberFormatExceptionMapper.java | 18 +++++++++++ .../ObsoletePhoneNumberFormatException.java | 15 ++++++++++ .../textsecuregcm/util/Util.java | 13 ++++---- .../VerificationControllerTest.java | 30 +++++++++++++++++-- .../textsecuregcm/util/UtilTest.java | 22 +++++++++----- 7 files changed, 83 insertions(+), 20 deletions(-) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/mappers/ObsoletePhoneNumberFormatExceptionMapper.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/ObsoletePhoneNumberFormatException.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 911a10c7d..537f7222b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/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() diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java index 07cb66b15..e2294f077 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java +++ b/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 pushTokenAndType = validateAndExtractPushToken( request.getUpdateVerificationSessionRequest()); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/mappers/ObsoletePhoneNumberFormatExceptionMapper.java b/service/src/main/java/org/whispersystems/textsecuregcm/mappers/ObsoletePhoneNumberFormatExceptionMapper.java new file mode 100644 index 000000000..59d401612 --- /dev/null +++ b/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 { + + 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(); + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/ObsoletePhoneNumberFormatException.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/ObsoletePhoneNumberFormatException.java new file mode 100644 index 000000000..fcd0948f8 --- /dev/null +++ b/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; + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java index a2c51ccac..ed03adace 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java +++ b/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 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 getCanonicalNumber(List 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; } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java index e279a73c6..b310111d4 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java +++ b/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 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, diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/util/UtilTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/util/UtilTest.java index f5d6e76cd..53f3df301 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/util/UtilTest.java +++ b/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,9 +95,13 @@ private static Stream 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 exception) + throws Exception { + if (exception == null) { + assertTrue(expectedBeninNumber.exactlySameAs(Util.canonicalizePhoneNumber(beninNumber))); + } else { + assertThrows(exception, () -> Util.canonicalizePhoneNumber(beninNumber)); + } } private static Stream normalizeBeninPhoneNumber() throws NumberParseException { @@ -103,9 +109,9 @@ private static Stream normalizeBeninPhoneNumber() throws NumberParseE 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) ); } }