Rename obsolete uses of recaptcha

This commit is contained in:
Chris Eager 2024-03-11 13:59:32 -05:00 committed by Chris Eager
parent 0ab2428d87
commit 2c2b5d555e
14 changed files with 56 additions and 84 deletions

View File

@ -31,10 +31,6 @@ public class DynamicCaptchaConfiguration {
@NotNull
private Map<Action, Set<String>> hCaptchaSiteKeys = Collections.emptyMap();
@JsonProperty
@NotNull
private Map<Action, Set<String>> recaptchaSiteKeys = Collections.emptyMap();
@JsonProperty
@NotNull
private Map<Action, BigDecimal> scoreFloorByAction = Collections.emptyMap();
@ -70,14 +66,4 @@ public class DynamicCaptchaConfiguration {
this.hCaptchaSiteKeys = hCaptchaSiteKeys;
}
public Map<Action, Set<String>> getRecaptchaSiteKeys() {
return recaptchaSiteKeys;
}
@VisibleForTesting
public void setRecaptchaSiteKeys(final Map<Action, Set<String>> recaptchaSiteKeys) {
this.recaptchaSiteKeys = recaptchaSiteKeys;
}
}

View File

@ -33,7 +33,7 @@ import javax.ws.rs.core.Response;
import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount;
import org.whispersystems.textsecuregcm.entities.AnswerChallengeRequest;
import org.whispersystems.textsecuregcm.entities.AnswerPushChallengeRequest;
import org.whispersystems.textsecuregcm.entities.AnswerRecaptchaChallengeRequest;
import org.whispersystems.textsecuregcm.entities.AnswerCaptchaChallengeRequest;
import org.whispersystems.textsecuregcm.filters.RemoteAddressFilter;
import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager;
import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
@ -70,7 +70,7 @@ public class ChallengeController {
continue their original operation.
""",
requestBody = @RequestBody(content = {@Content(schema = @Schema(oneOf = {AnswerPushChallengeRequest.class,
AnswerRecaptchaChallengeRequest.class}))})
AnswerCaptchaChallengeRequest.class}))})
)
@ApiResponse(responseCode = "200", description = "Indicates the challenge proof was accepted")
@ApiResponse(responseCode = "413", description = "Too many attempts", headers = @Header(
@ -96,14 +96,14 @@ public class ChallengeController {
return Response.status(429).build();
}
rateLimitChallengeManager.answerPushChallenge(auth.getAccount(), pushChallengeRequest.getChallenge());
} else if (answerRequest instanceof AnswerRecaptchaChallengeRequest recaptchaChallengeRequest) {
tags = tags.and(CHALLENGE_TYPE_TAG, "recaptcha");
} else if (answerRequest instanceof AnswerCaptchaChallengeRequest captchaChallengeRequest) {
tags = tags.and(CHALLENGE_TYPE_TAG, "captcha");
final String remoteAddress = (String) requestContext.getProperty(
RemoteAddressFilter.REMOTE_ADDRESS_ATTRIBUTE_NAME);
boolean success = rateLimitChallengeManager.answerRecaptchaChallenge(
boolean success = rateLimitChallengeManager.answerCaptchaChallenge(
auth.getAccount(),
recaptchaChallengeRequest.getCaptcha(),
captchaChallengeRequest.getCaptcha(),
remoteAddress,
userAgent,
constraints.captchaScoreThreshold());

View File

@ -8,7 +8,7 @@ package org.whispersystems.textsecuregcm.entities;
import io.swagger.v3.oas.annotations.media.Schema;
import javax.validation.constraints.NotBlank;
public class AnswerRecaptchaChallengeRequest extends AnswerChallengeRequest {
public class AnswerCaptchaChallengeRequest extends AnswerChallengeRequest {
@Schema(description = "The value of the token field from the server's 428 response")
@NotBlank

View File

@ -11,8 +11,8 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo;
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type")
@JsonSubTypes({
@JsonSubTypes.Type(value = AnswerPushChallengeRequest.class, name = "rateLimitPushChallenge"),
@JsonSubTypes.Type(value = AnswerRecaptchaChallengeRequest.class, name = "captcha"),
@JsonSubTypes.Type(value = AnswerRecaptchaChallengeRequest.class, name = "recaptcha")
@JsonSubTypes.Type(value = AnswerCaptchaChallengeRequest.class, name = "captcha"),
@JsonSubTypes.Type(value = AnswerCaptchaChallengeRequest.class, name = "recaptcha")
})
public abstract class AnswerChallengeRequest {
}

View File

@ -31,7 +31,8 @@ public class RateLimitChallengeManager {
private final List<RateLimitChallengeListener> rateLimitChallengeListeners;
private static final String RECAPTCHA_ATTEMPT_COUNTER_NAME = name(RateLimitChallengeManager.class, "recaptcha", "attempt");
private static final String CAPTCHA_ATTEMPT_COUNTER_NAME = name(RateLimitChallengeManager.class, "captcha",
"attempt");
private static final String RESET_RATE_LIMIT_EXCEEDED_COUNTER_NAME = name(RateLimitChallengeManager.class, "resetRateLimitExceeded");
private static final String SOURCE_COUNTRY_TAG_NAME = "sourceCountry";
@ -60,10 +61,11 @@ public class RateLimitChallengeManager {
}
}
public boolean answerRecaptchaChallenge(final Account account, final String captcha, final String mostRecentProxyIp, final String userAgent, final Optional<Float> scoreThreshold)
public boolean answerCaptchaChallenge(final Account account, final String captcha, final String mostRecentProxyIp,
final String userAgent, final Optional<Float> scoreThreshold)
throws RateLimitExceededException, IOException {
rateLimiters.getRecaptchaChallengeAttemptLimiter().validate(account.getUuid());
rateLimiters.getCaptchaChallengeAttemptLimiter().validate(account.getUuid());
final boolean challengeSuccess = captchaChecker.verify(Action.CHALLENGE, captcha, mostRecentProxyIp).isValid(scoreThreshold);
@ -73,10 +75,10 @@ public class RateLimitChallengeManager {
UserAgentTagUtil.getPlatformTag(userAgent)
);
Metrics.counter(RECAPTCHA_ATTEMPT_COUNTER_NAME, tags).increment();
Metrics.counter(CAPTCHA_ATTEMPT_COUNTER_NAME, tags).increment();
if (challengeSuccess) {
rateLimiters.getRecaptchaChallengeSuccessLimiter().validate(account.getUuid());
rateLimiters.getCaptchaChallengeSuccessLimiter().validate(account.getUuid());
resetRateLimits(account, ChallengeType.CAPTCHA);
}
return challengeSuccess;

View File

@ -13,7 +13,7 @@ public class RateLimitChallengeOptionManager {
private final RateLimiters rateLimiters;
public static final String OPTION_RECAPTCHA = "recaptcha";
public static final String OPTION_CAPTCHA = "recaptcha";
public static final String OPTION_PUSH_CHALLENGE = "pushChallenge";
public RateLimitChallengeOptionManager(final RateLimiters rateLimiters) {
@ -23,10 +23,10 @@ public class RateLimitChallengeOptionManager {
public List<String> getChallengeOptions(final Account account) {
final List<String> options = new ArrayList<>(2);
if (rateLimiters.getRecaptchaChallengeAttemptLimiter().hasAvailablePermits(account.getUuid(), 1) &&
rateLimiters.getRecaptchaChallengeSuccessLimiter().hasAvailablePermits(account.getUuid(), 1)) {
if (rateLimiters.getCaptchaChallengeAttemptLimiter().hasAvailablePermits(account.getUuid(), 1) &&
rateLimiters.getCaptchaChallengeSuccessLimiter().hasAvailablePermits(account.getUuid(), 1)) {
options.add(OPTION_RECAPTCHA);
options.add(OPTION_CAPTCHA);
}
if (rateLimiters.getPushChallengeAttemptLimiter().hasAvailablePermits(account.getUuid(), 1) &&

View File

@ -44,8 +44,8 @@ public class RateLimiters extends BaseRateLimiters<RateLimiters.For> {
VERIFICATION_PUSH_CHALLENGE("verificationPushChallenge", false, new RateLimiterConfig(5, Duration.ofSeconds(30))),
VERIFICATION_CAPTCHA("verificationCaptcha", false, new RateLimiterConfig(10, Duration.ofSeconds(30))),
RATE_LIMIT_RESET("rateLimitReset", true, new RateLimiterConfig(2, Duration.ofHours(12))),
RECAPTCHA_CHALLENGE_ATTEMPT("recaptchaChallengeAttempt", true, new RateLimiterConfig(10, Duration.ofMinutes(144))),
RECAPTCHA_CHALLENGE_SUCCESS("recaptchaChallengeSuccess", true, new RateLimiterConfig(2, Duration.ofHours(12))),
CAPTCHA_CHALLENGE_ATTEMPT("captchaChallengeAttempt", true, new RateLimiterConfig(10, Duration.ofMinutes(144))),
CAPTCHA_CHALLENGE_SUCCESS("captchaChallengeSuccess", true, new RateLimiterConfig(2, Duration.ofHours(12))),
SET_BACKUP_ID("setBackupId", true, new RateLimiterConfig(2, Duration.ofDays(7))),
PUSH_CHALLENGE_ATTEMPT("pushChallengeAttempt", true, new RateLimiterConfig(10, Duration.ofMinutes(144))),
PUSH_CHALLENGE_SUCCESS("pushChallengeSuccess", true, new RateLimiterConfig(2, Duration.ofHours(12))),
@ -193,12 +193,12 @@ public class RateLimiters extends BaseRateLimiters<RateLimiters.For> {
return forDescriptor(For.RATE_LIMIT_RESET);
}
public RateLimiter getRecaptchaChallengeAttemptLimiter() {
return forDescriptor(For.RECAPTCHA_CHALLENGE_ATTEMPT);
public RateLimiter getCaptchaChallengeAttemptLimiter() {
return forDescriptor(For.CAPTCHA_CHALLENGE_ATTEMPT);
}
public RateLimiter getRecaptchaChallengeSuccessLimiter() {
return forDescriptor(For.RECAPTCHA_CHALLENGE_SUCCESS);
public RateLimiter getCaptchaChallengeSuccessLimiter() {
return forDescriptor(For.CAPTCHA_CHALLENGE_SUCCESS);
}
public RateLimiter getPushChallengeAttemptLimiter() {

View File

@ -1,12 +0,0 @@
/*
* Copyright 2013-2021 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.whispersystems.textsecuregcm.spam;
public enum RateLimitChallengeType {
PUSH_CHALLENGE,
RECAPTCHA
}

View File

@ -269,9 +269,6 @@ class DynamicConfigurationTest {
registration:
- e4ddb6ff-05e7-497b-9a29-b76e7331789c
- 52fdbc88-f246-4705-a7dd-05ad85b93420
recaptchaSiteKeys:
challenge:
- 299068b6-ac78-4288-a90b-2e2ce5a6ddfe
""";
final DynamicCaptchaConfiguration config =
@ -285,9 +282,6 @@ class DynamicConfigurationTest {
assertThat(config.getHCaptchaSiteKeys().get(Action.CHALLENGE)).contains("ab317f2a-2b76-4098-84c9-ecdf8ea44f53");
assertThat(config.getHCaptchaSiteKeys().get(Action.REGISTRATION)).contains("e4ddb6ff-05e7-497b-9a29-b76e7331789c");
assertThat(config.getHCaptchaSiteKeys().get(Action.REGISTRATION)).contains("52fdbc88-f246-4705-a7dd-05ad85b93420");
assertThat(config.getRecaptchaSiteKeys().get(Action.CHALLENGE)).contains("299068b6-ac78-4288-a90b-2e2ce5a6ddfe");
assertThat(config.getRecaptchaSiteKeys().get(Action.REGISTRATION)).isNull();
}
}

View File

@ -114,7 +114,7 @@ class ChallengeControllerTest {
@ParameterizedTest
@ValueSource(booleans = { true, false } )
void testHandleRecaptcha(boolean hasThreshold) throws RateLimitExceededException, IOException {
void testHandleCaptcha(boolean hasThreshold) throws RateLimitExceededException, IOException {
final String captchaChallengeJson = """
{
"type": "captcha",
@ -123,7 +123,7 @@ class ChallengeControllerTest {
}
""";
when(rateLimitChallengeManager.answerRecaptchaChallenge(any(), any(), any(), any(), any()))
when(rateLimitChallengeManager.answerCaptchaChallenge(any(), any(), any(), any(), any()))
.thenReturn(true);
@ -138,7 +138,7 @@ class ChallengeControllerTest {
assertEquals(200, response.getStatus());
verify(rateLimitChallengeManager).answerRecaptchaChallenge(eq(AuthHelper.VALID_ACCOUNT),
verify(rateLimitChallengeManager).answerCaptchaChallenge(eq(AuthHelper.VALID_ACCOUNT),
eq("The value of the solved captcha token"), eq("127.0.0.1"), anyString(),
eq(hasThreshold ? Optional.of(0.5f) : Optional.empty()));
}
@ -152,7 +152,7 @@ class ChallengeControllerTest {
"captcha": "The value of the solved captcha token"
}
""";
when(rateLimitChallengeManager.answerRecaptchaChallenge(eq(AuthHelper.VALID_ACCOUNT),
when(rateLimitChallengeManager.answerCaptchaChallenge(eq(AuthHelper.VALID_ACCOUNT),
eq("The value of the solved captcha token"), eq("127.0.0.1"), anyString(), any()))
.thenReturn(false);
@ -165,7 +165,7 @@ class ChallengeControllerTest {
}
@Test
void testHandleRecaptchaRateLimited() throws RateLimitExceededException, IOException {
void testHandleCaptchaRateLimited() throws RateLimitExceededException, IOException {
final String captchaChallengeJson = """
{
"type": "captcha",
@ -176,7 +176,7 @@ class ChallengeControllerTest {
final Duration retryAfter = Duration.ofMinutes(17);
doThrow(new RateLimitExceededException(retryAfter, true)).when(rateLimitChallengeManager)
.answerRecaptchaChallenge(any(), any(), any(), any(), any());
.answerCaptchaChallenge(any(), any(), any(), any(), any());
final Response response = EXTENSION.target("/v1/challenge")
.request()

View File

@ -48,13 +48,13 @@ class AnswerChallengeRequestTest {
final AnswerChallengeRequest answerChallengeRequest =
SystemMapper.jsonMapper().readValue(captchaChallengeJson, AnswerChallengeRequest.class);
assertTrue(answerChallengeRequest instanceof AnswerRecaptchaChallengeRequest);
assertTrue(answerChallengeRequest instanceof AnswerCaptchaChallengeRequest);
assertEquals("A server-generated token",
((AnswerRecaptchaChallengeRequest) answerChallengeRequest).getToken());
((AnswerCaptchaChallengeRequest) answerChallengeRequest).getToken());
assertEquals("The value of the solved captcha token",
((AnswerRecaptchaChallengeRequest) answerChallengeRequest).getCaptcha());
((AnswerCaptchaChallengeRequest) answerChallengeRequest).getCaptcha());
}
{

View File

@ -79,7 +79,7 @@ class RateLimitChallengeManagerTest {
@ParameterizedTest
@MethodSource
void answerRecaptchaChallenge(Optional<Float> scoreThreshold, float actualScore, boolean expectSuccess)
void answerCaptchaChallenge(Optional<Float> scoreThreshold, float actualScore, boolean expectSuccess)
throws RateLimitExceededException, IOException {
final Account account = mock(Account.class);
when(account.getNumber()).thenReturn("+18005551234");
@ -88,11 +88,11 @@ class RateLimitChallengeManagerTest {
when(captchaChecker.verify(eq(Action.CHALLENGE), any(), any()))
.thenReturn(AssessmentResult.fromScore(actualScore, DEFAULT_SCORE_THRESHOLD));
when(rateLimiters.getRecaptchaChallengeAttemptLimiter()).thenReturn(mock(RateLimiter.class));
when(rateLimiters.getRecaptchaChallengeSuccessLimiter()).thenReturn(mock(RateLimiter.class));
when(rateLimiters.getCaptchaChallengeAttemptLimiter()).thenReturn(mock(RateLimiter.class));
when(rateLimiters.getCaptchaChallengeSuccessLimiter()).thenReturn(mock(RateLimiter.class));
when(rateLimiters.getRateLimitResetLimiter()).thenReturn(mock(RateLimiter.class));
rateLimitChallengeManager.answerRecaptchaChallenge(account, "captcha", "10.0.0.1", "Test User-Agent", scoreThreshold);
rateLimitChallengeManager.answerCaptchaChallenge(account, "captcha", "10.0.0.1", "Test User-Agent", scoreThreshold);
if (expectSuccess) {
verify(rateLimitChallengeListener).handleRateLimitChallengeAnswered(account, ChallengeType.CAPTCHA);
@ -101,7 +101,7 @@ class RateLimitChallengeManagerTest {
}
}
private static Stream<Arguments> answerRecaptchaChallenge() {
private static Stream<Arguments> answerCaptchaChallenge() {
return Stream.of(
Arguments.of(Optional.empty(), 0.5f, true),
Arguments.of(Optional.empty(), 0.1f, true),

View File

@ -42,18 +42,20 @@ class RateLimitChallengeOptionManagerTest {
final boolean expectCaptcha,
final boolean expectPushChallenge) {
final RateLimiter recaptchaChallengeAttemptLimiter = mock(RateLimiter.class);
final RateLimiter recaptchaChallengeSuccessLimiter = mock(RateLimiter.class);
final RateLimiter captchaChallengeAttemptLimiter = mock(RateLimiter.class);
final RateLimiter captchaChallengeSuccessLimiter = mock(RateLimiter.class);
final RateLimiter pushChallengeAttemptLimiter = mock(RateLimiter.class);
final RateLimiter pushChallengeSuccessLimiter = mock(RateLimiter.class);
when(rateLimiters.getRecaptchaChallengeAttemptLimiter()).thenReturn(recaptchaChallengeAttemptLimiter);
when(rateLimiters.getRecaptchaChallengeSuccessLimiter()).thenReturn(recaptchaChallengeSuccessLimiter);
when(rateLimiters.getCaptchaChallengeAttemptLimiter()).thenReturn(captchaChallengeAttemptLimiter);
when(rateLimiters.getCaptchaChallengeSuccessLimiter()).thenReturn(captchaChallengeSuccessLimiter);
when(rateLimiters.getPushChallengeAttemptLimiter()).thenReturn(pushChallengeAttemptLimiter);
when(rateLimiters.getPushChallengeSuccessLimiter()).thenReturn(pushChallengeSuccessLimiter);
when(recaptchaChallengeAttemptLimiter.hasAvailablePermits(any(UUID.class), anyInt())).thenReturn(captchaAttemptPermitted);
when(recaptchaChallengeSuccessLimiter.hasAvailablePermits(any(UUID.class), anyInt())).thenReturn(captchaSuccessPermitted);
when(captchaChallengeAttemptLimiter.hasAvailablePermits(any(UUID.class), anyInt())).thenReturn(
captchaAttemptPermitted);
when(captchaChallengeSuccessLimiter.hasAvailablePermits(any(UUID.class), anyInt())).thenReturn(
captchaSuccessPermitted);
when(pushChallengeAttemptLimiter.hasAvailablePermits(any(UUID.class), anyInt())).thenReturn(pushAttemptPermitted);
when(pushChallengeSuccessLimiter.hasAvailablePermits(any(UUID.class), anyInt())).thenReturn(pushSuccessPermitted);
@ -66,7 +68,7 @@ class RateLimitChallengeOptionManagerTest {
assertEquals(expectedLength, options.size());
if (expectCaptcha) {
assertTrue(options.contains(RateLimitChallengeOptionManager.OPTION_RECAPTCHA));
assertTrue(options.contains(RateLimitChallengeOptionManager.OPTION_CAPTCHA));
}
if (expectPushChallenge) {

View File

@ -122,8 +122,8 @@ public class RateLimitersTest {
final Map<String, RateLimiterConfig> limitsConfigMap = new HashMap<>();
limitsConfigMap.put(RateLimiters.For.RECAPTCHA_CHALLENGE_ATTEMPT.id(), baseConfig);
limitsConfigMap.put(RateLimiters.For.RECAPTCHA_CHALLENGE_SUCCESS.id(), baseConfig);
limitsConfigMap.put(RateLimiters.For.CAPTCHA_CHALLENGE_ATTEMPT.id(), baseConfig);
limitsConfigMap.put(RateLimiters.For.CAPTCHA_CHALLENGE_SUCCESS.id(), baseConfig);
when(configuration.getLimits()).thenReturn(limitsConfigMap);
@ -133,19 +133,19 @@ public class RateLimitersTest {
limitsConfigMap.put(RateLimiters.For.RATE_LIMIT_RESET.id(), initialRateLimiterConfig);
assertEquals(initialRateLimiterConfig, config(limiter));
assertEquals(baseConfig, config(rateLimiters.getRecaptchaChallengeAttemptLimiter()));
assertEquals(baseConfig, config(rateLimiters.getRecaptchaChallengeSuccessLimiter()));
assertEquals(baseConfig, config(rateLimiters.getCaptchaChallengeAttemptLimiter()));
assertEquals(baseConfig, config(rateLimiters.getCaptchaChallengeSuccessLimiter()));
limitsConfigMap.put(RateLimiters.For.RATE_LIMIT_RESET.id(), updatedRateLimiterCongig);
assertEquals(updatedRateLimiterCongig, config(limiter));
assertEquals(baseConfig, config(rateLimiters.getRecaptchaChallengeAttemptLimiter()));
assertEquals(baseConfig, config(rateLimiters.getRecaptchaChallengeSuccessLimiter()));
assertEquals(baseConfig, config(rateLimiters.getCaptchaChallengeAttemptLimiter()));
assertEquals(baseConfig, config(rateLimiters.getCaptchaChallengeSuccessLimiter()));
}
@Test
public void testRateLimiterHasItsPrioritiesStraight() throws Exception {
final RateLimiters.For descriptor = RateLimiters.For.RECAPTCHA_CHALLENGE_ATTEMPT;
final RateLimiters.For descriptor = RateLimiters.For.CAPTCHA_CHALLENGE_ATTEMPT;
final RateLimiterConfig configForDynamic = new RateLimiterConfig(1, Duration.ofMinutes(1));
final RateLimiterConfig configForStatic = new RateLimiterConfig(2, Duration.ofSeconds(30));
final RateLimiterConfig defaultConfig = descriptor.defaultConfig();