From 9e312cbdfab7df0a3bbbc78ce56802bc6985b1f4 Mon Sep 17 00:00:00 2001 From: Katherine Date: Mon, 2 Dec 2024 07:57:27 -0800 Subject: [PATCH] Normalize Benin phone numbers to the new format before sending to registration service --- .../controllers/VerificationController.java | 2 +- .../textsecuregcm/util/Util.java | 29 ++++++++++ .../VerificationControllerTest.java | 41 ++++++++++++- .../textsecuregcm/util/UtilTest.java | 58 ++++++++++++++++--- 4 files changed, 120 insertions(+), 10 deletions(-) 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 a1208d350..0b8e02be3 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java @@ -160,7 +160,7 @@ public class VerificationController { final Phonenumber.PhoneNumber phoneNumber; try { - phoneNumber = PhoneNumberUtil.getInstance().parse(request.getNumber(), null); + phoneNumber = Util.canonicalizePhoneNumber(PhoneNumberUtil.getInstance().parse(request.getNumber(), null)); } catch (final NumberParseException e) { throw new ServerErrorException("could not parse already validated number", Response.Status.INTERNAL_SERVER_ERROR); } 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 e1fa79dc0..a2c51ccac 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java @@ -7,6 +7,7 @@ package org.whispersystems.textsecuregcm.util; import com.google.i18n.phonenumbers.NumberParseException; import com.google.i18n.phonenumbers.PhoneNumberUtil; import com.google.i18n.phonenumbers.PhoneNumberUtil.PhoneNumberFormat; +import com.google.i18n.phonenumbers.Phonenumber; import com.google.i18n.phonenumbers.Phonenumber.PhoneNumber; import jakarta.ws.rs.core.Response; import java.time.Clock; @@ -215,6 +216,34 @@ public class Util { return workingCopy == prefix; } + /** + * Benin is changing phone number formats from +229 XXXXXXXX to +229 01XXXXXXXX starting on November 30, 2024. + * + * @param phoneNumber the phone number to check. + * @return whether the provided phone number is an old-format Benin phone number + */ + public static boolean isOldFormatBeninPhoneNumber(final Phonenumber.PhoneNumber phoneNumber) { + return "BJ".equals(PHONE_NUMBER_UTIL.getRegionCodeForNumber(phoneNumber)) && + PHONE_NUMBER_UTIL.getNationalSignificantNumber(phoneNumber).length() == 8; + } + + /** + * If applicable, return the canonical form of the provided phone number. + * This is relevant in cases where a numbering authority has changed the numbering format for a region. + * + * @param phoneNumber the phone number to canonicalize. + * @return the canonical phone number if applicable, otherwise the original phone number. + */ + public static Phonenumber.PhoneNumber canonicalizePhoneNumber(final Phonenumber.PhoneNumber phoneNumber) + throws NumberParseException { + 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); + } + return phoneNumber; + } + public static byte[] truncate(byte[] element, int length) { byte[] result = new byte[length]; System.arraycopy(element, 0, result, 0, result.length); 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 7e53916a2..e279a73c6 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java @@ -23,6 +23,7 @@ import static org.mockito.Mockito.when; import com.google.common.net.HttpHeaders; import com.google.i18n.phonenumbers.NumberParseException; import com.google.i18n.phonenumbers.PhoneNumberUtil; +import com.google.i18n.phonenumbers.Phonenumber; import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; import io.dropwizard.testing.junit5.ResourceExtension; import io.grpc.Status; @@ -94,7 +95,6 @@ class VerificationControllerTest { PhoneNumberUtil.PhoneNumberFormat.E164); private static final UUID PNI = UUID.randomUUID(); - private final RegistrationServiceClient registrationServiceClient = mock(RegistrationServiceClient.class); private final VerificationSessionManager verificationSessionManager = mock(VerificationSessionManager.class); private final PushNotificationManager pushNotificationManager = mock(PushNotificationManager.class); @@ -214,6 +214,45 @@ class VerificationControllerTest { } } + @ParameterizedTest + @MethodSource + void createBeninSessionSuccess(final String requestedNumber, final String expectedNumber) { + 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(requestedNumber, "token", "fcm")))) { + assertEquals(HttpStatus.SC_OK, response.getStatus()); + + final ArgumentCaptor phoneNumberArgumentCaptor = ArgumentCaptor.forClass( + Phonenumber.PhoneNumber.class); + verify(registrationServiceClient).createRegistrationSession(phoneNumberArgumentCaptor.capture(), anyBoolean(), any()); + final Phonenumber.PhoneNumber phoneNumber = phoneNumberArgumentCaptor.getValue(); + + assertEquals(expectedNumber, PhoneNumberUtil.getInstance().format(phoneNumber, PhoneNumberUtil.PhoneNumberFormat.E164)); + } + } + + 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) + ); + } + @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 0af2994fe..f5d6e76cd 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/util/UtilTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/util/UtilTest.java @@ -6,17 +6,26 @@ package org.whispersystems.textsecuregcm.util; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import com.google.i18n.phonenumbers.NumberParseException; import com.google.i18n.phonenumbers.PhoneNumberUtil; 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 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.MethodSource; class UtilTest { + // libphonenumber 8.13.50 and on generate new-format numbers for Benin + private static final String NEW_FORMAT_BENIN_E164_STRING = PhoneNumberUtil.getInstance() + .format(PhoneNumberUtil.getInstance().getExampleNumber("BJ"), PhoneNumberUtil.PhoneNumberFormat.E164); + private static final String OLD_FORMAT_BENIN_E164_STRING = NEW_FORMAT_BENIN_E164_STRING.replaceFirst("01", ""); @ParameterizedTest @MethodSource @@ -28,16 +37,10 @@ class UtilTest { final String usE164 = PhoneNumberUtil.getInstance().format( PhoneNumberUtil.getInstance().getExampleNumber("US"), PhoneNumberUtil.PhoneNumberFormat.E164); - // 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 List.of( Arguments.of(usE164, List.of(usE164)), - Arguments.of(newFormatBeninE164, List.of(newFormatBeninE164, oldFormatBeninE164)), - Arguments.of(oldFormatBeninE164, List.of(oldFormatBeninE164, newFormatBeninE164)) + Arguments.of(NEW_FORMAT_BENIN_E164_STRING, List.of(NEW_FORMAT_BENIN_E164_STRING, OLD_FORMAT_BENIN_E164_STRING)), + Arguments.of(OLD_FORMAT_BENIN_E164_STRING, List.of(OLD_FORMAT_BENIN_E164_STRING, NEW_FORMAT_BENIN_E164_STRING)) ); } @@ -66,4 +69,43 @@ class UtilTest { void startsWithDecimal(final long number, final long prefix, final boolean expectStartsWithPrefix) { assertEquals(expectStartsWithPrefix, Util.startsWithDecimal(number, prefix)); } + + @ParameterizedTest + @MethodSource + void isOldFormatBeninPhoneNumber4(final Phonenumber.PhoneNumber beninNumber, final boolean isOldFormatBeninNumber) { + if (isOldFormatBeninNumber) { + assertTrue(Util.isOldFormatBeninPhoneNumber(beninNumber)); + } else { + assertFalse(Util.isOldFormatBeninPhoneNumber(beninNumber)); + } + } + + private static Stream isOldFormatBeninPhoneNumber4() throws NumberParseException { + final Phonenumber.PhoneNumber oldFormatBeninE164 = PhoneNumberUtil.getInstance().parse(OLD_FORMAT_BENIN_E164_STRING, null); + final Phonenumber.PhoneNumber newFormatBeninE164 = PhoneNumberUtil.getInstance().parse(NEW_FORMAT_BENIN_E164_STRING, null); + + return Stream.of( + Arguments.of(oldFormatBeninE164, true), + Arguments.of(newFormatBeninE164, false), + Arguments.of(PhoneNumberUtil.getInstance().getExampleNumber("US"), false) + ); + } + + @ParameterizedTest + @MethodSource + void normalizeBeninPhoneNumber(final Phonenumber.PhoneNumber beninNumber, final Phonenumber.PhoneNumber expectedBeninNumber) + throws NumberParseException { + assertTrue(expectedBeninNumber.exactlySameAs(Util.canonicalizePhoneNumber(beninNumber))); + } + + private static Stream 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) + ); + } }