Accept a captcha score threshold for challenges from the spam filter
This commit is contained in:
parent
9c053e20da
commit
fd19299ae0
|
@ -167,6 +167,7 @@ import org.whispersystems.textsecuregcm.s3.PostPolicyGenerator;
|
|||
import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient;
|
||||
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecovery2Client;
|
||||
import org.whispersystems.textsecuregcm.spam.FilterSpam;
|
||||
import org.whispersystems.textsecuregcm.spam.PushChallengeConfigProvider;
|
||||
import org.whispersystems.textsecuregcm.spam.RateLimitChallengeListener;
|
||||
import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider;
|
||||
import org.whispersystems.textsecuregcm.spam.ScoreThresholdProvider;
|
||||
|
@ -848,9 +849,14 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
|
|||
private void registerProviders(Environment environment,
|
||||
WebSocketEnvironment<AuthenticatedAccount> webSocketEnvironment,
|
||||
WebSocketEnvironment<AuthenticatedAccount> provisioningEnvironment) {
|
||||
environment.jersey().register(ScoreThresholdProvider.ScoreThresholdFeature.class);
|
||||
webSocketEnvironment.jersey().register(ScoreThresholdProvider.ScoreThresholdFeature.class);
|
||||
provisioningEnvironment.jersey().register(ScoreThresholdProvider.ScoreThresholdFeature.class);
|
||||
List.of(
|
||||
ScoreThresholdProvider.ScoreThresholdFeature.class,
|
||||
PushChallengeConfigProvider.PushChallengeConfigFeature.class)
|
||||
.forEach(feature -> {
|
||||
environment.jersey().register(feature);
|
||||
webSocketEnvironment.jersey().register(feature);
|
||||
provisioningEnvironment.jersey().register(feature);
|
||||
});
|
||||
}
|
||||
|
||||
private void registerExceptionMappers(Environment environment,
|
||||
|
|
|
@ -36,10 +36,15 @@ import org.whispersystems.textsecuregcm.entities.AnswerRecaptchaChallengeRequest
|
|||
import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager;
|
||||
import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
|
||||
import org.whispersystems.textsecuregcm.push.NotPushRegisteredException;
|
||||
import org.whispersystems.textsecuregcm.spam.Extract;
|
||||
import org.whispersystems.textsecuregcm.spam.FilterSpam;
|
||||
import org.whispersystems.textsecuregcm.spam.PushChallengeConfig;
|
||||
import org.whispersystems.textsecuregcm.spam.ScoreThreshold;
|
||||
import org.whispersystems.textsecuregcm.util.HeaderUtils;
|
||||
|
||||
@Path("/v1/challenge")
|
||||
@Tag(name = "Challenge")
|
||||
@FilterSpam
|
||||
public class ChallengeController {
|
||||
|
||||
private final RateLimitChallengeManager rateLimitChallengeManager;
|
||||
|
@ -74,7 +79,9 @@ public class ChallengeController {
|
|||
public Response handleChallengeResponse(@Auth final AuthenticatedAccount auth,
|
||||
@Valid final AnswerChallengeRequest answerRequest,
|
||||
@HeaderParam(HttpHeaders.X_FORWARDED_FOR) final String forwardedFor,
|
||||
@HeaderParam(HttpHeaders.USER_AGENT) final String userAgent) throws RateLimitExceededException, IOException {
|
||||
@HeaderParam(HttpHeaders.USER_AGENT) final String userAgent,
|
||||
@Extract final ScoreThreshold captchaScoreThreshold,
|
||||
@Extract final PushChallengeConfig pushChallengeConfig) throws RateLimitExceededException, IOException {
|
||||
|
||||
Tags tags = Tags.of(UserAgentTagUtil.getPlatformTag(userAgent));
|
||||
|
||||
|
@ -82,6 +89,9 @@ public class ChallengeController {
|
|||
if (answerRequest instanceof final AnswerPushChallengeRequest pushChallengeRequest) {
|
||||
tags = tags.and(CHALLENGE_TYPE_TAG, "push");
|
||||
|
||||
if (!pushChallengeConfig.pushPermitted()) {
|
||||
return Response.status(429).build();
|
||||
}
|
||||
rateLimitChallengeManager.answerPushChallenge(auth.getAccount(), pushChallengeRequest.getChallenge());
|
||||
} else if (answerRequest instanceof AnswerRecaptchaChallengeRequest recaptchaChallengeRequest) {
|
||||
tags = tags.and(CHALLENGE_TYPE_TAG, "recaptcha");
|
||||
|
@ -91,7 +101,8 @@ public class ChallengeController {
|
|||
auth.getAccount(),
|
||||
recaptchaChallengeRequest.getCaptcha(),
|
||||
mostRecentProxy,
|
||||
userAgent);
|
||||
userAgent,
|
||||
captchaScoreThreshold.getScoreThreshold());
|
||||
|
||||
if (!success) {
|
||||
return Response.status(428).build();
|
||||
|
@ -150,7 +161,11 @@ public class ChallengeController {
|
|||
@ApiResponse(responseCode = "429", description = "Too many attempts", headers = @Header(
|
||||
name = "Retry-After",
|
||||
description = "If present, an positive integer indicating the number of seconds before a subsequent attempt could succeed"))
|
||||
public Response requestPushChallenge(@Auth final AuthenticatedAccount auth) {
|
||||
public Response requestPushChallenge(@Auth final AuthenticatedAccount auth,
|
||||
@Extract PushChallengeConfig pushChallengeConfig) {
|
||||
if (!pushChallengeConfig.pushPermitted()) {
|
||||
return Response.status(429).build();
|
||||
}
|
||||
try {
|
||||
rateLimitChallengeManager.sendPushChallenge(auth.getAccount());
|
||||
return Response.status(200).build();
|
||||
|
|
|
@ -14,6 +14,8 @@ import java.io.IOException;
|
|||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
|
||||
import org.whispersystems.textsecuregcm.captcha.Action;
|
||||
import org.whispersystems.textsecuregcm.captcha.CaptchaChecker;
|
||||
import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
|
||||
|
@ -64,12 +66,12 @@ public class RateLimitChallengeManager {
|
|||
}
|
||||
}
|
||||
|
||||
public boolean answerRecaptchaChallenge(final Account account, final String captcha, final String mostRecentProxyIp, final String userAgent)
|
||||
public boolean answerRecaptchaChallenge(final Account account, final String captcha, final String mostRecentProxyIp, final String userAgent, final Optional<Float> scoreThreshold)
|
||||
throws RateLimitExceededException, IOException {
|
||||
|
||||
rateLimiters.getRecaptchaChallengeAttemptLimiter().validate(account.getUuid());
|
||||
|
||||
final boolean challengeSuccess = captchaChecker.verify(Action.CHALLENGE, captcha, mostRecentProxyIp).isValid();
|
||||
final boolean challengeSuccess = captchaChecker.verify(Action.CHALLENGE, captcha, mostRecentProxyIp).isValid(scoreThreshold);
|
||||
|
||||
final Tags tags = Tags.of(
|
||||
Tag.of(SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account.getNumber())),
|
||||
|
|
|
@ -0,0 +1,44 @@
|
|||
/*
|
||||
* Copyright 2023 Signal Messenger, LLC
|
||||
* SPDX-License-Identifier: AGPL-3.0-only
|
||||
*/
|
||||
|
||||
package org.whispersystems.textsecuregcm.spam;
|
||||
|
||||
import java.util.Optional;
|
||||
|
||||
import org.glassfish.jersey.server.ContainerRequest;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
/**
|
||||
* A PushChallengeConfig may be provided by an upstream request filter. If request contains a
|
||||
* property for PROPERTY_NAME it can be forwarded to a downstream filter to indicate whether
|
||||
* push-token challenges can be used in place of captchas when evaluating whether a request should
|
||||
* be allowed to continue.
|
||||
*/
|
||||
public class PushChallengeConfig {
|
||||
private static final Logger logger = LoggerFactory.getLogger(PushChallengeConfig.class);
|
||||
|
||||
public static final String PROPERTY_NAME = "pushChallengePermitted";
|
||||
|
||||
/**
|
||||
* A score threshold in the range [0, 1.0]
|
||||
*/
|
||||
private final boolean pushPermitted;
|
||||
|
||||
/**
|
||||
* Extract an optional score threshold parameter provided by an upstream request filter
|
||||
*/
|
||||
public PushChallengeConfig(final ContainerRequest containerRequest) {
|
||||
this.pushPermitted = Optional
|
||||
.ofNullable(containerRequest.getProperty(PROPERTY_NAME))
|
||||
.filter(obj -> obj instanceof Boolean)
|
||||
.map(obj -> (Boolean) obj)
|
||||
.orElse(true); // not a typo! true is the default
|
||||
}
|
||||
|
||||
public boolean pushPermitted() {
|
||||
return pushPermitted;
|
||||
}
|
||||
}
|
|
@ -0,0 +1,60 @@
|
|||
/*
|
||||
* Copyright 2023 Signal Messenger, LLC
|
||||
* SPDX-License-Identifier: AGPL-3.0-only
|
||||
*/
|
||||
|
||||
package org.whispersystems.textsecuregcm.spam;
|
||||
|
||||
import java.util.function.Function;
|
||||
import javax.inject.Singleton;
|
||||
import javax.ws.rs.core.Feature;
|
||||
import javax.ws.rs.core.FeatureContext;
|
||||
import org.glassfish.jersey.internal.inject.AbstractBinder;
|
||||
import org.glassfish.jersey.server.ContainerRequest;
|
||||
import org.glassfish.jersey.server.model.Parameter;
|
||||
import org.glassfish.jersey.server.spi.internal.ValueParamProvider;
|
||||
|
||||
/**
|
||||
* Parses a {@link PushChallengeConfig} out of a {@link ContainerRequest} to provide to jersey resources.
|
||||
*
|
||||
* A request filter may enrich a ContainerRequest with a PushChallengeConfig by providing a float
|
||||
* property with the name {@link PushChallengeConfig#PROPERTY_NAME}. This indicates whether push
|
||||
* challenges may be considered when evaluating whether a request should proceed.
|
||||
*
|
||||
* A resource can consume a PushChallengeConfig with by annotating a PushChallengeConfig parameter with {@link Extract}
|
||||
*/
|
||||
public class PushChallengeConfigProvider implements ValueParamProvider {
|
||||
|
||||
/**
|
||||
* Configures the PushChallengeConfigProvider
|
||||
*/
|
||||
public static class PushChallengeConfigFeature implements Feature {
|
||||
@Override
|
||||
public boolean configure(FeatureContext context) {
|
||||
context.register(new AbstractBinder() {
|
||||
@Override
|
||||
protected void configure() {
|
||||
bind(PushChallengeConfigProvider.class)
|
||||
.to(ValueParamProvider.class)
|
||||
.in(Singleton.class);
|
||||
}
|
||||
});
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public Function<ContainerRequest, ?> getValueProvider(final Parameter parameter) {
|
||||
if (parameter.getRawType().equals(PushChallengeConfig.class)
|
||||
&& parameter.isAnnotationPresent(Extract.class)) {
|
||||
return PushChallengeConfig::new;
|
||||
}
|
||||
return null;
|
||||
|
||||
}
|
||||
|
||||
@Override
|
||||
public PriorityType getPriority() {
|
||||
return Priority.HIGH;
|
||||
}
|
||||
}
|
|
@ -22,18 +22,29 @@ import io.dropwizard.testing.junit5.DropwizardExtensionsSupport;
|
|||
import io.dropwizard.testing.junit5.ResourceExtension;
|
||||
import java.io.IOException;
|
||||
import java.time.Duration;
|
||||
import java.util.Optional;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import javax.ws.rs.client.Entity;
|
||||
import javax.ws.rs.container.ContainerRequestContext;
|
||||
import javax.ws.rs.container.ContainerRequestFilter;
|
||||
import javax.ws.rs.core.Feature;
|
||||
import javax.ws.rs.core.FeatureContext;
|
||||
import javax.ws.rs.core.Response;
|
||||
import org.glassfish.jersey.test.grizzly.GrizzlyWebTestContainerFactory;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
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.ValueSource;
|
||||
import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount;
|
||||
import org.whispersystems.textsecuregcm.auth.DisabledPermittedAuthenticatedAccount;
|
||||
import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager;
|
||||
import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper;
|
||||
import org.whispersystems.textsecuregcm.push.NotPushRegisteredException;
|
||||
import org.whispersystems.textsecuregcm.spam.PushChallengeConfigProvider;
|
||||
import org.whispersystems.textsecuregcm.spam.ScoreThreshold;
|
||||
import org.whispersystems.textsecuregcm.spam.ScoreThresholdProvider;
|
||||
import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
|
||||
import org.whispersystems.textsecuregcm.util.SystemMapper;
|
||||
|
||||
|
@ -44,10 +55,24 @@ class ChallengeControllerTest {
|
|||
|
||||
private static final ChallengeController challengeController = new ChallengeController(rateLimitChallengeManager);
|
||||
|
||||
private static final AtomicReference<Float> scoreThreshold = new AtomicReference<>();
|
||||
|
||||
private static final ResourceExtension EXTENSION = ResourceExtension.builder()
|
||||
.addProvider(AuthHelper.getAuthFilter())
|
||||
.addProvider(new PolymorphicAuthValueFactoryProvider.Binder<>(
|
||||
Set.of(AuthenticatedAccount.class, DisabledPermittedAuthenticatedAccount.class)))
|
||||
.addProvider(ScoreThresholdProvider.ScoreThresholdFeature.class)
|
||||
.addProvider(PushChallengeConfigProvider.PushChallengeConfigFeature.class)
|
||||
.addProvider(new Feature() {
|
||||
public boolean configure(FeatureContext featureContext) {
|
||||
featureContext.register(new ContainerRequestFilter() {
|
||||
public void filter(ContainerRequestContext requestContext) {
|
||||
requestContext.setProperty(ScoreThreshold.PROPERTY_NAME, scoreThreshold.get());
|
||||
}
|
||||
});
|
||||
return true;
|
||||
}
|
||||
})
|
||||
.setMapper(SystemMapper.jsonMapper())
|
||||
.setTestContainerFactory(new GrizzlyWebTestContainerFactory())
|
||||
.addResource(new RateLimitExceededExceptionMapper())
|
||||
|
@ -57,6 +82,7 @@ class ChallengeControllerTest {
|
|||
@AfterEach
|
||||
void teardown() {
|
||||
reset(rateLimitChallengeManager);
|
||||
scoreThreshold.set(null);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -99,8 +125,9 @@ class ChallengeControllerTest {
|
|||
assertEquals(String.valueOf(retryAfter.toSeconds()), response.getHeaderString("Retry-After"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testHandleRecaptcha() throws RateLimitExceededException, IOException {
|
||||
@ParameterizedTest
|
||||
@ValueSource(booleans = { true, false } )
|
||||
void testHandleRecaptcha(boolean hasThreshold) throws RateLimitExceededException, IOException {
|
||||
final String recaptchaChallengeJson = """
|
||||
{
|
||||
"type": "recaptcha",
|
||||
|
@ -109,9 +136,13 @@ class ChallengeControllerTest {
|
|||
}
|
||||
""";
|
||||
|
||||
when(rateLimitChallengeManager.answerRecaptchaChallenge(any(), any(), any(), any()))
|
||||
when(rateLimitChallengeManager.answerRecaptchaChallenge(any(), any(), any(), any(), any()))
|
||||
.thenReturn(true);
|
||||
|
||||
|
||||
if (hasThreshold) {
|
||||
scoreThreshold.set(Float.valueOf(0.5f));
|
||||
}
|
||||
final Response response = EXTENSION.target("/v1/challenge")
|
||||
.request()
|
||||
.header(HttpHeaders.X_FORWARDED_FOR, "10.0.0.1")
|
||||
|
@ -120,7 +151,7 @@ class ChallengeControllerTest {
|
|||
|
||||
assertEquals(200, response.getStatus());
|
||||
|
||||
verify(rateLimitChallengeManager).answerRecaptchaChallenge(eq(AuthHelper.VALID_ACCOUNT), eq("The value of the solved captcha token"), eq("10.0.0.1"), anyString());
|
||||
verify(rateLimitChallengeManager).answerRecaptchaChallenge(eq(AuthHelper.VALID_ACCOUNT), eq("The value of the solved captcha token"), eq("10.0.0.1"), anyString(), eq(hasThreshold ? Optional.of(0.5f) : Optional.empty()));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -132,8 +163,7 @@ class ChallengeControllerTest {
|
|||
"captcha": "The value of the solved captcha token"
|
||||
}
|
||||
""";
|
||||
|
||||
when(rateLimitChallengeManager.answerRecaptchaChallenge(eq(AuthHelper.VALID_ACCOUNT), eq("The value of the solved captcha token"), eq("10.0.0.1"), anyString()))
|
||||
when(rateLimitChallengeManager.answerRecaptchaChallenge(eq(AuthHelper.VALID_ACCOUNT), eq("The value of the solved captcha token"), eq("10.0.0.1"), anyString(), any()))
|
||||
.thenReturn(false);
|
||||
|
||||
final Response response = EXTENSION.target("/v1/challenge")
|
||||
|
@ -157,7 +187,7 @@ class ChallengeControllerTest {
|
|||
|
||||
final Duration retryAfter = Duration.ofMinutes(17);
|
||||
doThrow(new RateLimitExceededException(retryAfter, true)).when(rateLimitChallengeManager)
|
||||
.answerRecaptchaChallenge(any(), any(), any(), any());
|
||||
.answerRecaptchaChallenge(any(), any(), any(), any(), any());
|
||||
|
||||
final Response response = EXTENSION.target("/v1/challenge")
|
||||
.request()
|
||||
|
|
|
@ -13,9 +13,14 @@ import static org.mockito.Mockito.verifyNoInteractions;
|
|||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Optional;
|
||||
import java.util.UUID;
|
||||
import java.util.stream.Stream;
|
||||
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.params.ParameterizedTest;
|
||||
import org.junit.jupiter.params.provider.Arguments;
|
||||
import org.junit.jupiter.params.provider.MethodSource;
|
||||
import org.junit.jupiter.params.provider.ValueSource;
|
||||
import org.whispersystems.textsecuregcm.captcha.Action;
|
||||
import org.whispersystems.textsecuregcm.captcha.AssessmentResult;
|
||||
|
@ -27,6 +32,8 @@ import org.whispersystems.textsecuregcm.storage.Account;
|
|||
|
||||
class RateLimitChallengeManagerTest {
|
||||
|
||||
private static final float DEFAULT_SCORE_THRESHOLD = 0.1f;
|
||||
|
||||
private PushChallengeManager pushChallengeManager;
|
||||
private CaptchaChecker captchaChecker;
|
||||
private RateLimiters rateLimiters;
|
||||
|
@ -71,27 +78,39 @@ class RateLimitChallengeManagerTest {
|
|||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@ValueSource(booleans = {true, false})
|
||||
void answerRecaptchaChallenge(final boolean successfulChallenge) throws RateLimitExceededException, IOException {
|
||||
@MethodSource
|
||||
void answerRecaptchaChallenge(Optional<Float> scoreThreshold, float actualScore, boolean expectSuccess)
|
||||
throws RateLimitExceededException, IOException {
|
||||
final Account account = mock(Account.class);
|
||||
when(account.getNumber()).thenReturn("+18005551234");
|
||||
when(account.getUuid()).thenReturn(UUID.randomUUID());
|
||||
|
||||
when(captchaChecker.verify(eq(Action.CHALLENGE), any(), any()))
|
||||
.thenReturn(successfulChallenge
|
||||
? AssessmentResult.alwaysValid()
|
||||
: AssessmentResult.invalid());
|
||||
.thenReturn(AssessmentResult.fromScore(actualScore, DEFAULT_SCORE_THRESHOLD));
|
||||
|
||||
when(rateLimiters.getRecaptchaChallengeAttemptLimiter()).thenReturn(mock(RateLimiter.class));
|
||||
when(rateLimiters.getRecaptchaChallengeSuccessLimiter()).thenReturn(mock(RateLimiter.class));
|
||||
when(rateLimiters.getRateLimitResetLimiter()).thenReturn(mock(RateLimiter.class));
|
||||
|
||||
rateLimitChallengeManager.answerRecaptchaChallenge(account, "captcha", "10.0.0.1", "Test User-Agent");
|
||||
rateLimitChallengeManager.answerRecaptchaChallenge(account, "captcha", "10.0.0.1", "Test User-Agent", scoreThreshold);
|
||||
|
||||
if (successfulChallenge) {
|
||||
if (expectSuccess) {
|
||||
verify(rateLimitChallengeListener).handleRateLimitChallengeAnswered(account, ChallengeType.CAPTCHA);
|
||||
} else {
|
||||
verifyNoInteractions(rateLimitChallengeListener);
|
||||
}
|
||||
}
|
||||
|
||||
private static Stream<Arguments> answerRecaptchaChallenge() {
|
||||
return Stream.of(
|
||||
Arguments.of(Optional.empty(), 0.5f, true),
|
||||
Arguments.of(Optional.empty(), 0.1f, true),
|
||||
Arguments.of(Optional.empty(), 0.0f, false),
|
||||
Arguments.of(Optional.of(0.1f), 0.5f, true),
|
||||
Arguments.of(Optional.of(0.1f), 0.1f, true),
|
||||
Arguments.of(Optional.of(0.1f), 0.0f, false),
|
||||
Arguments.of(Optional.of(0.3f), 0.5f, true),
|
||||
Arguments.of(Optional.of(0.3f), 0.1f, false),
|
||||
Arguments.of(Optional.of(0.3f), 0.0f, false));
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue