From 33fb7a72de543c6bf04711a6ddcba7a77393d909 Mon Sep 17 00:00:00 2001 From: Ehren Kret Date: Fri, 30 Jul 2021 13:50:16 -0700 Subject: [PATCH] Use RecaptchaClient interface --- .../textsecuregcm/WhisperServerService.java | 14 +++++++++--- .../controllers/AccountController.java | 10 ++++----- .../limits/RateLimitChallengeManager.java | 16 +++++++------- .../limits/RateLimitChallengeManagerTest.java | 10 ++++----- .../controllers/AccountControllerTest.java | 22 +++++++++---------- 5 files changed, 40 insertions(+), 32 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index ab9fb55c3..75bcc2561 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -132,7 +132,9 @@ import org.whispersystems.textsecuregcm.push.GCMSender; import org.whispersystems.textsecuregcm.push.MessageSender; import org.whispersystems.textsecuregcm.push.ProvisioningManager; import org.whispersystems.textsecuregcm.push.ReceiptSender; +import org.whispersystems.textsecuregcm.recaptcha.EnterpriseRecaptchaClient; import org.whispersystems.textsecuregcm.recaptcha.LegacyRecaptchaClient; +import org.whispersystems.textsecuregcm.recaptcha.TransitionalRecaptchaClient; import org.whispersystems.textsecuregcm.redis.ConnectionEventLogger; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; import org.whispersystems.textsecuregcm.redis.ReplicatedJedisPool; @@ -467,9 +469,12 @@ public class WhisperServerService extends Application commonControllers = List.of( 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 b7f0dc5ba..ac61b0c34 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -63,7 +63,7 @@ import org.whispersystems.textsecuregcm.push.APNSender; import org.whispersystems.textsecuregcm.push.ApnMessage; import org.whispersystems.textsecuregcm.push.GCMSender; import org.whispersystems.textsecuregcm.push.GcmMessage; -import org.whispersystems.textsecuregcm.recaptcha.LegacyRecaptchaClient; +import org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient; import org.whispersystems.textsecuregcm.sms.SmsSender; import org.whispersystems.textsecuregcm.sms.TwilioVerifyExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.storage.AbusiveHostRule; @@ -116,7 +116,7 @@ public class AccountController { private final DynamicConfigurationManager dynamicConfigurationManager; private final TurnTokenGenerator turnTokenGenerator; private final Map testDevices; - private final LegacyRecaptchaClient legacyRecaptchaClient; + private final RecaptchaClient recaptchaClient; private final GCMSender gcmSender; private final APNSender apnSender; private final ExternalServiceCredentialGenerator backupServiceCredentialGenerator; @@ -132,7 +132,7 @@ public class AccountController { DynamicConfigurationManager dynamicConfigurationManager, TurnTokenGenerator turnTokenGenerator, Map testDevices, - LegacyRecaptchaClient legacyRecaptchaClient, + RecaptchaClient recaptchaClient, GCMSender gcmSender, APNSender apnSender, ExternalServiceCredentialGenerator backupServiceCredentialGenerator, @@ -147,7 +147,7 @@ public class AccountController { this.dynamicConfigurationManager = dynamicConfigurationManager; this.testDevices = testDevices; this.turnTokenGenerator = turnTokenGenerator; - this.legacyRecaptchaClient = legacyRecaptchaClient; + this.recaptchaClient = recaptchaClient; this.gcmSender = gcmSender; this.apnSender = apnSender; this.backupServiceCredentialGenerator = backupServiceCredentialGenerator; @@ -595,7 +595,7 @@ public class AccountController { { if (captchaToken.isPresent()) { - boolean validToken = legacyRecaptchaClient.verify(captchaToken.get(), requester); + boolean validToken = recaptchaClient.verify(captchaToken.get(), requester); if (validToken) { captchaSuccessMeter.mark(); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java index 1bd98cd9c..e2adac3d2 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java @@ -1,13 +1,15 @@ package org.whispersystems.textsecuregcm.limits; +import static com.codahale.metrics.MetricRegistry.name; + import com.vdurmont.semver4j.Semver; +import io.micrometer.core.instrument.Metrics; import java.util.ArrayList; import java.util.List; import java.util.Optional; -import io.micrometer.core.instrument.Metrics; import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException; import org.whispersystems.textsecuregcm.push.NotPushRegisteredException; -import org.whispersystems.textsecuregcm.recaptcha.LegacyRecaptchaClient; +import org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.util.Util; @@ -15,12 +17,10 @@ import org.whispersystems.textsecuregcm.util.ua.UnrecognizedUserAgentException; import org.whispersystems.textsecuregcm.util.ua.UserAgent; import org.whispersystems.textsecuregcm.util.ua.UserAgentUtil; -import static com.codahale.metrics.MetricRegistry.name; - public class RateLimitChallengeManager { private final PushChallengeManager pushChallengeManager; - private final LegacyRecaptchaClient legacyRecaptchaClient; + private final RecaptchaClient recaptchaClient; private final PreKeyRateLimiter preKeyRateLimiter; private final UnsealedSenderRateLimiter unsealedSenderRateLimiter; @@ -39,14 +39,14 @@ public class RateLimitChallengeManager { public RateLimitChallengeManager( final PushChallengeManager pushChallengeManager, - final LegacyRecaptchaClient legacyRecaptchaClient, + final RecaptchaClient recaptchaClient, final PreKeyRateLimiter preKeyRateLimiter, final UnsealedSenderRateLimiter unsealedSenderRateLimiter, final RateLimiters rateLimiters, final DynamicConfigurationManager dynamicConfigurationManager) { this.pushChallengeManager = pushChallengeManager; - this.legacyRecaptchaClient = legacyRecaptchaClient; + this.recaptchaClient = recaptchaClient; this.preKeyRateLimiter = preKeyRateLimiter; this.unsealedSenderRateLimiter = unsealedSenderRateLimiter; this.rateLimiters = rateLimiters; @@ -69,7 +69,7 @@ public class RateLimitChallengeManager { rateLimiters.getRecaptchaChallengeAttemptLimiter().validate(account.getNumber()); - final boolean challengeSuccess = legacyRecaptchaClient.verify(captcha, mostRecentProxyIp); + final boolean challengeSuccess = recaptchaClient.verify(captcha, mostRecentProxyIp); Metrics.counter(RECAPTCHA_ATTEMPT_COUNTER_NAME, SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account.getNumber()), diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManagerTest.java index 9b35a0f2f..7202aa780 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManagerTest.java @@ -22,7 +22,7 @@ import org.junit.jupiter.params.provider.ValueSource; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicRateLimitChallengeConfiguration; import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException; -import org.whispersystems.textsecuregcm.recaptcha.LegacyRecaptchaClient; +import org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.util.ua.ClientPlatform; @@ -30,7 +30,7 @@ import org.whispersystems.textsecuregcm.util.ua.ClientPlatform; class RateLimitChallengeManagerTest { private PushChallengeManager pushChallengeManager; - private LegacyRecaptchaClient legacyRecaptchaClient; + private RecaptchaClient recaptchaClient; private PreKeyRateLimiter preKeyRateLimiter; private UnsealedSenderRateLimiter unsealedSenderRateLimiter; private DynamicRateLimitChallengeConfiguration rateLimitChallengeConfiguration; @@ -41,7 +41,7 @@ class RateLimitChallengeManagerTest { @BeforeEach void setUp() { pushChallengeManager = mock(PushChallengeManager.class); - legacyRecaptchaClient = mock(LegacyRecaptchaClient.class); + recaptchaClient = mock(RecaptchaClient.class); preKeyRateLimiter = mock(PreKeyRateLimiter.class); unsealedSenderRateLimiter = mock(UnsealedSenderRateLimiter.class); rateLimiters = mock(RateLimiters.class); @@ -55,7 +55,7 @@ class RateLimitChallengeManagerTest { rateLimitChallengeManager = new RateLimitChallengeManager( pushChallengeManager, - legacyRecaptchaClient, + recaptchaClient, preKeyRateLimiter, unsealedSenderRateLimiter, rateLimiters, @@ -89,7 +89,7 @@ class RateLimitChallengeManagerTest { final Account account = mock(Account.class); when(account.getNumber()).thenReturn("+18005551234"); - when(legacyRecaptchaClient.verify(any(), any())).thenReturn(successfulChallenge); + when(recaptchaClient.verify(any(), any())).thenReturn(successfulChallenge); when(rateLimiters.getRecaptchaChallengeAttemptLimiter()).thenReturn(mock(RateLimiter.class)); when(rateLimiters.getRecaptchaChallengeSuccessLimiter()).thenReturn(mock(RateLimiter.class)); 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 0bc90ae0c..728be8203 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 @@ -75,7 +75,7 @@ import org.whispersystems.textsecuregcm.push.APNSender; import org.whispersystems.textsecuregcm.push.ApnMessage; import org.whispersystems.textsecuregcm.push.GCMSender; import org.whispersystems.textsecuregcm.push.GcmMessage; -import org.whispersystems.textsecuregcm.recaptcha.LegacyRecaptchaClient; +import org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient; import org.whispersystems.textsecuregcm.sms.SmsSender; import org.whispersystems.textsecuregcm.sms.TwilioVerifyExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.storage.AbusiveHostRule; @@ -131,7 +131,7 @@ class AccountControllerTest { private static Account senderRegLockAccount = mock(Account.class); private static Account senderHasStorage = mock(Account.class); private static Account senderTransfer = mock(Account.class); - private static LegacyRecaptchaClient legacyRecaptchaClient = mock(LegacyRecaptchaClient.class); + private static RecaptchaClient recaptchaClient = mock(RecaptchaClient.class); private static GCMSender gcmSender = mock(GCMSender.class); private static APNSender apnSender = mock(APNSender.class); private static UsernamesManager usernamesManager = mock(UsernamesManager.class); @@ -160,7 +160,7 @@ class AccountControllerTest { dynamicConfigurationManager, turnTokenGenerator, new HashMap<>(), - legacyRecaptchaClient, + recaptchaClient, gcmSender, apnSender, storageCredentialGenerator, @@ -241,8 +241,8 @@ class AccountControllerTest { when(abusiveHostRules.getAbusiveHostRulesFor(eq(RESTRICTED_HOST))).thenReturn(Collections.singletonList(new AbusiveHostRule(RESTRICTED_HOST, false, Collections.singletonList("+123")))); when(abusiveHostRules.getAbusiveHostRulesFor(eq(NICE_HOST))).thenReturn(Collections.emptyList()); - when(legacyRecaptchaClient.verify(eq(INVALID_CAPTCHA_TOKEN), anyString())).thenReturn(false); - when(legacyRecaptchaClient.verify(eq(VALID_CAPTCHA_TOKEN), anyString())).thenReturn(true); + when(recaptchaClient.verify(eq(INVALID_CAPTCHA_TOKEN), anyString())).thenReturn(false); + when(recaptchaClient.verify(eq(VALID_CAPTCHA_TOKEN), anyString())).thenReturn(true); doThrow(new RateLimitExceededException(SENDER_OVER_PIN, Duration.ZERO)).when(pinLimiter).validate(eq(SENDER_OVER_PIN)); @@ -273,7 +273,7 @@ class AccountControllerTest { senderRegLockAccount, senderHasStorage, senderTransfer, - legacyRecaptchaClient, + recaptchaClient, gcmSender, apnSender, usernamesManager, @@ -711,7 +711,7 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(200); verifyNoMoreInteractions(abusiveHostRules); - verify(legacyRecaptchaClient).verify(eq(VALID_CAPTCHA_TOKEN), eq(ABUSIVE_HOST)); + verify(recaptchaClient).verify(eq(VALID_CAPTCHA_TOKEN), eq(ABUSIVE_HOST)); if (enrolledInVerifyExperiment) { verify(smsSender).deliverSmsVerificationWithTwilioVerify(eq(SENDER), eq(Optional.empty()), anyString(), eq(Collections.emptyList())); @@ -742,7 +742,7 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(402); verifyNoMoreInteractions(abusiveHostRules); - verify(legacyRecaptchaClient).verify(eq(INVALID_CAPTCHA_TOKEN), eq(ABUSIVE_HOST)); + verify(recaptchaClient).verify(eq(INVALID_CAPTCHA_TOKEN), eq(ABUSIVE_HOST)); verifyNoMoreInteractions(smsSender); } @@ -772,7 +772,7 @@ class AccountControllerTest { verify(abusiveHostRules).setBlockedHost(eq(RATE_LIMITED_IP_HOST), eq("Auto-Block")); verifyNoMoreInteractions(abusiveHostRules); - verifyNoMoreInteractions(legacyRecaptchaClient); + verifyNoMoreInteractions(recaptchaClient); verifyNoMoreInteractions(smsSender); } @@ -802,7 +802,7 @@ class AccountControllerTest { verify(abusiveHostRules).setBlockedHost(eq(RATE_LIMITED_PREFIX_HOST), eq("Auto-Block")); verifyNoMoreInteractions(abusiveHostRules); - verifyNoMoreInteractions(legacyRecaptchaClient); + verifyNoMoreInteractions(recaptchaClient); verifyNoMoreInteractions(smsSender); } @@ -831,7 +831,7 @@ class AccountControllerTest { verify(abusiveHostRules).getAbusiveHostRulesFor(eq(RATE_LIMITED_HOST2)); verifyNoMoreInteractions(abusiveHostRules); - verifyNoMoreInteractions(legacyRecaptchaClient); + verifyNoMoreInteractions(recaptchaClient); verifyNoMoreInteractions(smsSender); }