From b280c768a4b53cc833ae1a1bf5d8c43bd9dff339 Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Fri, 26 Aug 2022 11:10:02 -0400 Subject: [PATCH] Allow signup captchas to target CLDR two-letter region codes --- .../dynamic/DynamicCaptchaConfiguration.java | 17 +++++- .../controllers/AccountController.java | 6 ++- .../textsecuregcm/util/Util.java | 9 ++++ .../controllers/AccountControllerTest.java | 52 ++++++++++++++++++- 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicCaptchaConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicCaptchaConfiguration.java index f992018c6..db7cb43b3 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicCaptchaConfiguration.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicCaptchaConfiguration.java @@ -21,6 +21,14 @@ public class DynamicCaptchaConfiguration { @NotNull private Set signupCountryCodes = Collections.emptySet(); + @JsonProperty + @NotNull + private Set signupRegions = Collections.emptySet(); + + public BigDecimal getScoreFloor() { + return scoreFloor; + } + public Set getSignupCountryCodes() { return signupCountryCodes; } @@ -30,7 +38,12 @@ public class DynamicCaptchaConfiguration { this.signupCountryCodes = numbers; } - public BigDecimal getScoreFloor() { - return scoreFloor; + @VisibleForTesting + public void setSignupRegions(final Set signupRegions) { + this.signupRegions = signupRegions; + } + + public Set getSignupRegions() { + return signupRegions; } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index 3d5ad81fb..6a68016cf 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -776,6 +776,8 @@ public class AccountController { } final String countryCode = Util.getCountryCode(number); + final String region = Util.getRegion(number); + if (captchaToken.isPresent()) { boolean validToken = recaptchaClient.verify(captchaToken.get(), sourceHost); @@ -822,7 +824,9 @@ public class AccountController { DynamicCaptchaConfiguration captchaConfig = dynamicConfigurationManager.getConfiguration() .getCaptchaConfiguration(); - boolean countryFiltered = captchaConfig.getSignupCountryCodes().contains(countryCode); + + boolean countryFiltered = captchaConfig.getSignupCountryCodes().contains(countryCode) || + captchaConfig.getSignupRegions().contains(region); if (abusiveHostRules.isBlocked(sourceHost)) { blockedHostMeter.mark(); 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 413327330..684aac88a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java @@ -79,6 +79,15 @@ public class Util { else return "0"; } + public static String getRegion(final String number) { + try { + final PhoneNumber phoneNumber = PHONE_NUMBER_UTIL.parse(number, null); + return PHONE_NUMBER_UTIL.getRegionCodeForNumber(phoneNumber); + } catch (final NumberParseException e) { + return "ZZ"; + } + } + public static String getNumberPrefix(String number) { String countryCode = getCountryCode(number); int remaining = number.length() - (1 + countryCode.length()); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java index af335a7b1..813a5154b 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -52,6 +52,7 @@ import org.junit.jupiter.api.Test; 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.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; @@ -984,7 +985,8 @@ class AccountControllerTest { .thenReturn(enrolledInVerifyExperiment); final String challenge = "challenge"; - when(pendingAccountsManager.getCodeForNumber(RESTRICTED_NUMBER)).thenReturn(Optional.of(new StoredVerificationCode("123456", System.currentTimeMillis(), challenge, null))); + when(pendingAccountsManager.getCodeForNumber(RESTRICTED_NUMBER)) + .thenReturn(Optional.of(new StoredVerificationCode("123456", System.currentTimeMillis(), challenge, null))); Response response = resources.getJerseyTest() @@ -1000,6 +1002,54 @@ class AccountControllerTest { verifyNoMoreInteractions(smsSender); } + @ParameterizedTest + @CsvSource({ + "+12025550123, true, true", + "+12025550123, false, true", + "+12505550199, true, false", + "+12505550199, false, false", + }) + void testRestrictedRegion(final String number, final boolean enrolledInVerifyExperiment, final boolean expectSendCode) { + final DynamicConfiguration dynamicConfiguration = mock(DynamicConfiguration.class); + when(dynamicConfigurationManager.getConfiguration()).thenReturn(dynamicConfiguration); + + final DynamicCaptchaConfiguration signupCaptchaConfig = new DynamicCaptchaConfiguration(); + signupCaptchaConfig.setSignupRegions(Set.of("CA")); + + when(dynamicConfiguration.getCaptchaConfiguration()).thenReturn(signupCaptchaConfig); + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + final String challenge = "challenge"; + when(pendingAccountsManager.getCodeForNumber(number)) + .thenReturn(Optional.of(new StoredVerificationCode("123456", System.currentTimeMillis(), challenge, null))); + + when(smsSender.deliverSmsVerificationWithTwilioVerify(any(), any(), any(), any())) + .thenReturn(CompletableFuture.completedFuture(Optional.empty())); + + Response response = + resources.getJerseyTest() + .target(String.format("/v1/accounts/sms/code/%s", number)) + .queryParam("challenge", challenge) + .request() + .header("X-Forwarded-For", NICE_HOST) + .get(); + + if (expectSendCode) { + assertThat(response.getStatus()).isEqualTo(200); + + if (enrolledInVerifyExperiment) { + verify(smsSender).deliverSmsVerificationWithTwilioVerify(eq(number), any(), any(), any()); + } else { + verify(smsSender).deliverSmsVerification(eq(number), any(), any()); + } + } else { + assertThat(response.getStatus()).isEqualTo(402); + verifyNoMoreInteractions(smsSender); + } + } + @ParameterizedTest @ValueSource(booleans = {false, true}) void testSendRestrictedIn(final boolean enrolledInVerifyExperiment) throws Exception {