Throw checked exceptions from CaptchaChecker

This commit is contained in:
Ravi Khadiwala 2026-06-10 12:03:58 -05:00 committed by Jon Chambers
parent 82e3c16fba
commit 660011017d
11 changed files with 65 additions and 29 deletions

View File

@ -16,9 +16,9 @@ import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.function.Function;
import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.annotation.Nullable;
public class CaptchaChecker {
private static final Logger logger = LoggerFactory.getLogger(CaptchaChecker.class);
@ -53,20 +53,20 @@ public class CaptchaChecker {
* @param userAgent User-Agent of the solver
* @return An {@link AssessmentResult} indicating whether the solution should be accepted, and a score that can be
* used for metrics
* @throws IOException if there is an error validating the captcha with the underlying service
* @throws BadRequestException if input is not in the expected format
* @throws IOException if there is an error validating the captcha with the underlying service
* @throws InvalidCaptchaArgumentException if input is not in the expected format
*/
public AssessmentResult verify(
final Optional<UUID> maybeAci,
final Action expectedAction,
final String input,
final String ip,
@Nullable final String userAgent) throws IOException {
@Nullable final String userAgent) throws IOException, InvalidCaptchaArgumentException {
final String[] parts = input.split("\\" + SEPARATOR, 4);
// we allow missing actions, if we're missing 1 part, assume it's the action
if (parts.length < 4) {
throw new BadRequestException("too few parts");
throw new InvalidCaptchaArgumentException("too few parts");
}
final String prefix = parts[0];
@ -79,30 +79,30 @@ public class CaptchaChecker {
// This is a "short" solution that points to the actual solution. We need to fetch the
// full solution before proceeding
provider = prefix.substring(0, prefix.length() - SHORT_SUFFIX.length());
token = shortCodeExpander.retrieve(token).orElseThrow(() -> new BadRequestException("invalid shortcode"));
token = shortCodeExpander.retrieve(token).orElseThrow(() -> new InvalidCaptchaArgumentException("invalid shortcode"));
}
final CaptchaClient client = this.captchaClientSupplier.apply(provider);
if (client == null) {
throw new BadRequestException("invalid captcha scheme");
throw new InvalidCaptchaArgumentException("invalid captcha scheme");
}
final Action parsedAction = Action.parse(action)
.orElseThrow(() -> {
Metrics.counter(INVALID_ACTION_COUNTER_NAME, "action", action).increment();
return new BadRequestException("invalid captcha action");
Metrics.counter(INVALID_ACTION_COUNTER_NAME).increment();
return new InvalidCaptchaArgumentException("invalid captcha action");
});
if (!parsedAction.equals(expectedAction)) {
Metrics.counter(INVALID_ACTION_COUNTER_NAME, "action", action).increment();
throw new BadRequestException("invalid captcha action");
throw new InvalidCaptchaArgumentException("invalid captcha action");
}
final Set<String> allowedSiteKeys = client.validSiteKeys(parsedAction);
if (!allowedSiteKeys.contains(siteKey)) {
logger.debug("invalid site-key {}, action={}, token={}", siteKey, action, token);
Metrics.counter(INVALID_SITEKEY_COUNTER_NAME, "action", action).increment();
throw new BadRequestException("invalid captcha site-key");
throw new InvalidCaptchaArgumentException("invalid captcha site-key");
}
final AssessmentResult result = client.verify(maybeAci, siteKey, parsedAction, token, ip, userAgent);

View File

@ -0,0 +1,15 @@
/*
* Copyright 2026 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.whispersystems.textsecuregcm.captcha;
/**
* Indicates that a captcha solution was malformed
*/
public class InvalidCaptchaArgumentException extends Exception {
public InvalidCaptchaArgumentException(String message) {
super(message);
}
}

View File

@ -19,7 +19,7 @@ public class RegistrationCaptchaManager {
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
public Optional<AssessmentResult> assessCaptcha(final Optional<UUID> aci, final Optional<String> captcha, final String sourceHost, final String userAgent)
throws IOException {
throws IOException, InvalidCaptchaArgumentException{
return captcha.isPresent()
? Optional.of(captchaChecker.verify(aci, Action.REGISTRATION, captcha.get(), sourceHost, userAgent))
: Optional.empty();

View File

@ -34,6 +34,7 @@ import java.io.IOException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.whispersystems.textsecuregcm.auth.AuthenticatedDevice;
import org.whispersystems.textsecuregcm.captcha.InvalidCaptchaArgumentException;
import org.whispersystems.textsecuregcm.entities.AnswerCaptchaChallengeRequest;
import org.whispersystems.textsecuregcm.entities.AnswerChallengeRequest;
import org.whispersystems.textsecuregcm.entities.AnswerPushChallengeRequest;
@ -126,6 +127,8 @@ public class ChallengeController {
} else {
tags = tags.and(CHALLENGE_TYPE_TAG, "unrecognized");
}
} catch (final InvalidCaptchaArgumentException e) {
return Response.status(Response.Status.BAD_REQUEST.getStatusCode(), e.getMessage()).build();
} catch (final IOException e) {
logger.error("error assessing captcha during challenge response handling", e);
return Response.status(Response.Status.SERVICE_UNAVAILABLE).build();

View File

@ -66,6 +66,7 @@ import org.apache.http.HttpStatus;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.whispersystems.textsecuregcm.captcha.AssessmentResult;
import org.whispersystems.textsecuregcm.captcha.InvalidCaptchaArgumentException;
import org.whispersystems.textsecuregcm.captcha.RegistrationCaptchaManager;
import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
import org.whispersystems.textsecuregcm.entities.CreateVerificationSessionRequest;
@ -497,6 +498,8 @@ public class VerificationController {
} catch (final IOException e) {
logger.error("error assessing captcha during registration verification", e);
throw new ServerErrorException(Response.Status.SERVICE_UNAVAILABLE, e);
} catch (InvalidCaptchaArgumentException e) {
throw new BadRequestException(e);
}
if (assessmentResult.isValid(captchaScoreThreshold)) {

View File

@ -6,6 +6,7 @@ import org.signal.chat.challenge.AnswerChallengeResponse;
import org.signal.chat.challenge.SimpleChallengeGrpc;
import org.whispersystems.textsecuregcm.auth.grpc.AuthenticatedDevice;
import org.whispersystems.textsecuregcm.auth.grpc.AuthenticationUtil;
import org.whispersystems.textsecuregcm.captcha.InvalidCaptchaArgumentException;
import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager;
import org.whispersystems.textsecuregcm.spam.ChallengeConstraintChecker;
@ -40,12 +41,18 @@ public class ChallengeGrpcService extends SimpleChallengeGrpc.ChallengeImplBase
final boolean success = switch (request.getRequestCase()) {
case PUSH -> constraints.pushPermitted() && rateLimitChallengeManager.answerPushChallenge(account,
request.getPush().getChallenge());
case CAPTCHA -> rateLimitChallengeManager.answerCaptchaChallenge(
account,
request.getCaptcha().getCaptcha(),
RequestAttributesUtil.getRemoteAddress().getHostAddress(),
RequestAttributesUtil.getUserAgent().orElse(null),
constraints.captchaScoreThreshold());
case CAPTCHA -> {
try {
yield rateLimitChallengeManager.answerCaptchaChallenge(
account,
request.getCaptcha().getCaptcha(),
RequestAttributesUtil.getRemoteAddress().getHostAddress(),
RequestAttributesUtil.getUserAgent().orElse(null),
constraints.captchaScoreThreshold());
} catch (InvalidCaptchaArgumentException e) {
throw GrpcExceptions.invalidArguments(e.getMessage());
}
}
case REQUEST_NOT_SET -> throw GrpcExceptions.fieldViolation("request", "Must set request type");
};

View File

@ -16,6 +16,7 @@ import java.util.Optional;
import org.whispersystems.textsecuregcm.captcha.Action;
import org.whispersystems.textsecuregcm.captcha.AssessmentResult;
import org.whispersystems.textsecuregcm.captcha.CaptchaChecker;
import org.whispersystems.textsecuregcm.captcha.InvalidCaptchaArgumentException;
import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
import org.whispersystems.textsecuregcm.metrics.CaptchaMetrics;
import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
@ -69,7 +70,8 @@ public class RateLimitChallengeManager {
final String captcha,
final String mostRecentProxyIp,
@Nullable final String userAgent,
final Optional<Float> scoreThreshold) throws RateLimitExceededException, IOException {
final Optional<Float> scoreThreshold)
throws RateLimitExceededException, IOException, InvalidCaptchaArgumentException {
rateLimiters.getCaptchaChallengeAttemptLimiter().validate(account.getUuid());

View File

@ -79,7 +79,7 @@ public class CaptchaCheckerTest {
final String input,
final String expectedToken,
final String siteKey,
final Action expectedAction) throws IOException {
final Action expectedAction) throws IOException, InvalidCaptchaArgumentException {
final CaptchaClient captchaClient = mockClient(PREFIX);
new CaptchaChecker(null, PREFIX -> captchaClient).verify(Optional.empty(), expectedAction, input, null, USER_AGENT);
verify(captchaClient, times(1)).verify(any(), eq(siteKey), eq(expectedAction), eq(expectedToken), any(), eq(USER_AGENT));
@ -103,7 +103,7 @@ public class CaptchaCheckerTest {
}
@Test
public void choose() throws IOException {
public void choose() throws IOException, InvalidCaptchaArgumentException {
String ainput = String.join(SEPARATOR, PREFIX_A, CHALLENGE_SITE_KEY, "challenge", TOKEN);
String binput = String.join(SEPARATOR, PREFIX_B, CHALLENGE_SITE_KEY, "challenge", TOKEN);
final CaptchaClient a = mockClient(PREFIX_A);
@ -134,13 +134,13 @@ public class CaptchaCheckerTest {
@MethodSource
public void badArgs(final String input) throws IOException {
final CaptchaClient cc = mockClient(PREFIX);
assertThrows(BadRequestException.class,
assertThrows(InvalidCaptchaArgumentException.class,
() -> new CaptchaChecker(null, prefix -> PREFIX.equals(prefix) ? cc : null).verify(Optional.of(ACI), Action.CHALLENGE, input, null, USER_AGENT));
}
@Test
public void testShortened() throws IOException {
public void testShortened() throws IOException, InvalidCaptchaArgumentException {
final CaptchaClient captchaClient = mockClient(PREFIX);
final ShortCodeExpander retriever = mock(ShortCodeExpander.class);
when(retriever.retrieve("abc")).thenReturn(Optional.of(TOKEN));

View File

@ -33,6 +33,7 @@ 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.AuthenticatedDevice;
import org.whispersystems.textsecuregcm.captcha.InvalidCaptchaArgumentException;
import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager;
import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper;
import org.whispersystems.textsecuregcm.push.NotPushRegisteredException;
@ -119,7 +120,8 @@ class ChallengeControllerTest {
@ParameterizedTest
@ValueSource(booleans = { true, false } )
void testHandleCaptcha(boolean hasThreshold) throws RateLimitExceededException, IOException {
void testHandleCaptcha(boolean hasThreshold)
throws RateLimitExceededException, IOException, InvalidCaptchaArgumentException {
final String captchaChallengeJson = """
{
"type": "captcha",
@ -149,7 +151,7 @@ class ChallengeControllerTest {
}
@Test
void testHandleInvalidCaptcha() throws RateLimitExceededException, IOException {
void testHandleInvalidCaptcha() throws RateLimitExceededException, IOException, InvalidCaptchaArgumentException {
final String captchaChallengeJson = """
{
"type": "captcha",
@ -170,7 +172,7 @@ class ChallengeControllerTest {
}
@Test
void testHandleCaptchaRateLimited() throws RateLimitExceededException, IOException {
void testHandleCaptchaRateLimited() throws RateLimitExceededException, IOException, InvalidCaptchaArgumentException {
final String captchaChallengeJson = """
{
"type": "captcha",

View File

@ -17,6 +17,7 @@ import org.mockito.Mock;
import org.signal.chat.challenge.AnswerChallengeRequest;
import org.signal.chat.challenge.AnswerChallengeResponse;
import org.signal.chat.challenge.ChallengeGrpc;
import org.whispersystems.textsecuregcm.captcha.InvalidCaptchaArgumentException;
import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager;
import org.whispersystems.textsecuregcm.spam.ChallengeConstraintChecker;
@ -84,7 +85,8 @@ public class ChallengeGrpcServiceTest extends
@ParameterizedTest
@ValueSource(booleans = {true, false})
void handleCaptchaChallengeResponse(final boolean captchaSuccess) throws RateLimitExceededException, IOException {
void handleCaptchaChallengeResponse(final boolean captchaSuccess)
throws RateLimitExceededException, IOException, InvalidCaptchaArgumentException {
when(challengeConstraintChecker.challengeConstraintsGrpc(account)).thenReturn(
new ChallengeConstraintChecker.ChallengeConstraints(true, Optional.empty()));
when(rateLimitChallengeManager.answerCaptchaChallenge(any(), any(), any(), any(), any())).thenReturn(
@ -100,7 +102,8 @@ public class ChallengeGrpcServiceTest extends
}
@Test
void handleCaptchaChallengeResponseRateLimitExceeded() throws RateLimitExceededException, IOException {
void handleCaptchaChallengeResponseRateLimitExceeded()
throws RateLimitExceededException, IOException, InvalidCaptchaArgumentException {
when(challengeConstraintChecker.challengeConstraintsGrpc(account)).thenReturn(
new ChallengeConstraintChecker.ChallengeConstraints(true, Optional.empty()));
final Duration retryAfter = Duration.ofMinutes(1);

View File

@ -26,6 +26,7 @@ import org.junit.jupiter.params.provider.ValueSource;
import org.whispersystems.textsecuregcm.captcha.Action;
import org.whispersystems.textsecuregcm.captcha.AssessmentResult;
import org.whispersystems.textsecuregcm.captcha.CaptchaChecker;
import org.whispersystems.textsecuregcm.captcha.InvalidCaptchaArgumentException;
import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
import org.whispersystems.textsecuregcm.spam.ChallengeType;
import org.whispersystems.textsecuregcm.spam.RateLimitChallengeListener;
@ -80,7 +81,7 @@ class RateLimitChallengeManagerTest {
@ParameterizedTest
@MethodSource
void answerCaptchaChallenge(Optional<Float> scoreThreshold, float actualScore, boolean expectSuccess)
throws RateLimitExceededException, IOException {
throws RateLimitExceededException, IOException, InvalidCaptchaArgumentException {
final Account account = mock(Account.class);
when(account.getNumber()).thenReturn("+18005551234");
when(account.getUuid()).thenReturn(UUID.randomUUID());