diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ChallengeController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ChallengeController.java index 9f4f6b777..907b24620 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ChallengeController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ChallengeController.java @@ -20,8 +20,9 @@ import io.swagger.v3.oas.annotations.parameters.RequestBody; import io.swagger.v3.oas.annotations.responses.ApiResponse; import io.swagger.v3.oas.annotations.tags.Tag; import java.io.IOException; -import java.util.NoSuchElementException; +import java.util.Optional; import javax.validation.Valid; +import javax.ws.rs.BadRequestException; import javax.ws.rs.Consumes; import javax.ws.rs.HeaderParam; import javax.ws.rs.POST; @@ -85,19 +86,20 @@ public class ChallengeController { tags = tags.and(CHALLENGE_TYPE_TAG, "push"); rateLimitChallengeManager.answerPushChallenge(auth.getAccount(), pushChallengeRequest.getChallenge()); - } else if (answerRequest instanceof AnswerRecaptchaChallengeRequest) { + } else if (answerRequest instanceof AnswerRecaptchaChallengeRequest recaptchaChallengeRequest) { tags = tags.and(CHALLENGE_TYPE_TAG, "recaptcha"); - try { - final AnswerRecaptchaChallengeRequest recaptchaChallengeRequest = (AnswerRecaptchaChallengeRequest) answerRequest; - final String mostRecentProxy = HeaderUtils.getMostRecentProxy(forwardedFor).orElseThrow(); + final String mostRecentProxy = HeaderUtils.getMostRecentProxy(forwardedFor).orElseThrow(() -> new BadRequestException()); + boolean success = rateLimitChallengeManager.answerRecaptchaChallenge( + auth.getAccount(), + recaptchaChallengeRequest.getCaptcha(), + mostRecentProxy, + userAgent); - rateLimitChallengeManager.answerRecaptchaChallenge(auth.getAccount(), recaptchaChallengeRequest.getCaptcha(), - mostRecentProxy, userAgent); - - } catch (final NoSuchElementException e) { - return Response.status(400).build(); + if (!success) { + return Response.status(428).build(); } + } else { tags = tags.and(CHALLENGE_TYPE_TAG, "unrecognized"); } 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 3625d7308..052576cbe 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java @@ -63,7 +63,7 @@ public class RateLimitChallengeManager { } } - public void 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) throws RateLimitExceededException, IOException { rateLimiters.getRecaptchaChallengeAttemptLimiter().validate(account.getUuid()); @@ -82,6 +82,7 @@ public class RateLimitChallengeManager { rateLimiters.getRecaptchaChallengeSuccessLimiter().validate(account.getUuid()); resetRateLimits(account); } + return challengeSuccess; } private void resetRateLimits(final Account account) throws RateLimitExceededException { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ChallengeControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ChallengeControllerTest.java index 260099751..c91b9dfc3 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ChallengeControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ChallengeControllerTest.java @@ -14,6 +14,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; import com.google.common.net.HttpHeaders; import io.dropwizard.auth.PolymorphicAuthValueFactoryProvider; @@ -108,6 +109,9 @@ class ChallengeControllerTest { } """; + when(rateLimitChallengeManager.answerRecaptchaChallenge(any(), any(), any(), any())) + .thenReturn(true); + final Response response = EXTENSION.target("/v1/challenge") .request() .header(HttpHeaders.X_FORWARDED_FOR, "10.0.0.1") @@ -115,9 +119,32 @@ class ChallengeControllerTest { .put(Entity.json(recaptchaChallengeJson)); 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()); } + @Test + void testHandleInvalidCaptcha() throws RateLimitExceededException, IOException { + final String recaptchaChallengeJson = """ + { + "type": "recaptcha", + "token": "A server-generated token", + "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())) + .thenReturn(false); + + final Response response = EXTENSION.target("/v1/challenge") + .request() + .header(HttpHeaders.X_FORWARDED_FOR, "10.0.0.1") + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.json(recaptchaChallengeJson)); + + assertEquals(428, response.getStatus()); + } + @Test void testHandleRecaptchaRateLimited() throws RateLimitExceededException, IOException { final String recaptchaChallengeJson = """