From 935e268decda61d5e9450b7d0bc962041e509794 Mon Sep 17 00:00:00 2001 From: Chris Eager Date: Mon, 28 Feb 2022 09:49:31 -0800 Subject: [PATCH] Parameterize sitekey --- service/config/sample.yml | 1 - .../textsecuregcm/WhisperServerService.java | 3 +- .../RecaptchaV2Configuration.java | 8 +-- .../recaptcha/EnterpriseRecaptchaClient.java | 51 +++++++++++---- .../TransitionalRecaptchaClient.java | 26 +------- .../EnterpriseRecaptchaClientTest.java | 63 +++++++++++++++++++ .../TransitionalRecaptchaClientTest.java | 22 +++---- 7 files changed, 115 insertions(+), 59 deletions(-) create mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/EnterpriseRecaptchaClientTest.java diff --git a/service/config/sample.yml b/service/config/sample.yml index 1a2e55a36..8a54b45ab 100644 --- a/service/config/sample.yml +++ b/service/config/sample.yml @@ -246,7 +246,6 @@ recaptcha: secret: unset recaptchaV2: - siteKey: unset scoreFloor: 1.0 projectPath: projects/example credentialConfigurationJson: "{ }" # service account configuration for backend authentication diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 2cca1bec1..9b49d74e8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2021 Signal Messenger, LLC + * Copyright 2013-2022 Signal Messenger, LLC * SPDX-License-Identifier: AGPL-3.0-only */ package org.whispersystems.textsecuregcm; @@ -477,7 +477,6 @@ public class WhisperServerService extends Application + * For action to be optional, there is a strong assumption that the token will never contain a {@value SEPARATOR}. + * Observation suggests {@code token} is base-64 encoded. In practice, an action should always be present, but we + * don’t need to be strict. + */ + static String[] parseInputToken(final String input) { + String[] keyActionAndToken = input.split("\\" + SEPARATOR, 3); + + if (keyActionAndToken.length == 1) { + throw new BadRequestException("too few parts"); + } + + if (keyActionAndToken.length == 2) { + // there was no ":" delimiter; assume we only have a token + return new String[]{keyActionAndToken[0], null, keyActionAndToken[1]}; + } + + return keyActionAndToken; } - public boolean verify(final String token, final String ip, @Nullable final String expectedAction) { + @Override + public boolean verify(final String input, final String ip) { + final String[] parts = parseInputToken(input); + + final String sitekey = parts[0]; + final String expectedAction = parts[1]; + final String token = parts[2]; + Event.Builder eventBuilder = Event.newBuilder() - .setSiteKey(siteKey) + .setSiteKey(sitekey) .setToken(token) .setUserIpAddress(ip); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/recaptcha/TransitionalRecaptchaClient.java b/service/src/main/java/org/whispersystems/textsecuregcm/recaptcha/TransitionalRecaptchaClient.java index 4ca8a8158..55b5a5974 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/recaptcha/TransitionalRecaptchaClient.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/recaptcha/TransitionalRecaptchaClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 Signal Messenger, LLC + * Copyright 2021-2022 Signal Messenger, LLC * SPDX-License-Identifier: AGPL-3.0-only */ @@ -12,9 +12,7 @@ import javax.annotation.Nonnull; public class TransitionalRecaptchaClient implements RecaptchaClient { @VisibleForTesting - static final String SEPARATOR = "."; - @VisibleForTesting - static final String V2_PREFIX = "signal-recaptcha-v2" + SEPARATOR; + static final String V2_PREFIX = "signal-recaptcha-v2" + EnterpriseRecaptchaClient.SEPARATOR; private final LegacyRecaptchaClient legacyRecaptchaClient; private final EnterpriseRecaptchaClient enterpriseRecaptchaClient; @@ -29,28 +27,10 @@ public class TransitionalRecaptchaClient implements RecaptchaClient { @Override public boolean verify(@Nonnull final String token, @Nonnull final String ip) { if (token.startsWith(V2_PREFIX)) { - final String[] actionAndToken = parseV2ActionAndToken(token.substring(V2_PREFIX.length())); - return enterpriseRecaptchaClient.verify(actionAndToken[1], ip, actionAndToken[0]); + return enterpriseRecaptchaClient.verify(token.substring(V2_PREFIX.length()), ip); } else { return legacyRecaptchaClient.verify(token, ip); } } - /** - * Parses the token and action (if any) from {@code input}. The expected input format is: {@code [action:]token}. - *

- * For action to be optional, there is a strong assumption that the token will never contain a {@value SEPARATOR}. - * Observation suggests {@code token} is base-64 encoded. In practice, an action should always be present, but we - * don’t need to be strict. - */ - static String[] parseV2ActionAndToken(final String input) { - String[] actionAndToken = input.split("\\" + SEPARATOR, 2); - - if (actionAndToken.length == 1) { - // there was no ":" delimiter; assume we only have a token - return new String[]{null, actionAndToken[0]}; - } - - return actionAndToken; - } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/EnterpriseRecaptchaClientTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/EnterpriseRecaptchaClientTest.java new file mode 100644 index 000000000..572aeccb1 --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/EnterpriseRecaptchaClientTest.java @@ -0,0 +1,63 @@ +/* + * Copyright 2022 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.recaptcha; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.whispersystems.textsecuregcm.recaptcha.EnterpriseRecaptchaClient.SEPARATOR; + +import java.util.stream.Stream; +import javax.annotation.Nullable; +import javax.ws.rs.BadRequestException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +class EnterpriseRecaptchaClientTest { + + private static final String SITE_KEY = "site-key"; + private static final String TOKEN = "some-token"; + + @ParameterizedTest + @MethodSource + void parseInputToken(final String input, final String expectedToken, final String siteKey, + @Nullable final String expectedAction) { + + final String[] parts = EnterpriseRecaptchaClient.parseInputToken(input); + + assertEquals(siteKey, parts[0]); + assertEquals(expectedAction, parts[1]); + assertEquals(expectedToken, parts[2]); + } + + @Test + void parseInputTokenBadRequest() { + assertThrows(BadRequestException.class, () -> { + EnterpriseRecaptchaClient.parseInputToken(TOKEN); + }); + } + + static Stream parseInputToken() { + return Stream.of( + Arguments.of( + String.join(SEPARATOR, SITE_KEY, TOKEN), + TOKEN, + SITE_KEY, + null), + Arguments.of( + String.join(SEPARATOR, SITE_KEY, "an-action", TOKEN), + TOKEN, + SITE_KEY, + "an-action"), + Arguments.of( + String.join(SEPARATOR, SITE_KEY, "an-action", TOKEN, "something-else"), + TOKEN + SEPARATOR + "something-else", + SITE_KEY, + "an-action") + ); + } +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/TransitionalRecaptchaClientTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/TransitionalRecaptchaClientTest.java index 1e2862da9..a2735796b 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/TransitionalRecaptchaClientTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/TransitionalRecaptchaClientTest.java @@ -8,7 +8,7 @@ package org.whispersystems.textsecuregcm.recaptcha; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; -import static org.whispersystems.textsecuregcm.recaptcha.TransitionalRecaptchaClient.SEPARATOR; +import static org.whispersystems.textsecuregcm.recaptcha.EnterpriseRecaptchaClient.SEPARATOR; import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; @@ -36,8 +36,7 @@ class TransitionalRecaptchaClientTest { @ParameterizedTest @MethodSource - void testVerify(final String inputToken, final boolean expectLegacy, final String expectedToken, - final String expectedAction) { + void testVerify(final String inputToken, final boolean expectLegacy, final String expectedToken) { transitionalRecaptchaClient.verify(inputToken, IP_ADDRESS); @@ -46,7 +45,7 @@ class TransitionalRecaptchaClientTest { verify(legacyRecaptchaClient).verify(expectedToken, IP_ADDRESS); } else { verifyNoInteractions(legacyRecaptchaClient); - verify(enterpriseRecaptchaClient).verify(expectedToken, IP_ADDRESS, expectedAction); + verify(enterpriseRecaptchaClient).verify(expectedToken, IP_ADDRESS); } } @@ -56,23 +55,20 @@ class TransitionalRecaptchaClientTest { Arguments.of( TOKEN, true, - TOKEN, - null), + TOKEN), Arguments.of( String.join(SEPARATOR, PREFIX, TOKEN), false, - TOKEN, - null), + TOKEN), Arguments.of( - String.join(SEPARATOR, PREFIX, "an-action", TOKEN), + String.join(SEPARATOR, PREFIX, "site-key", "an-action", TOKEN), false, - TOKEN, + String.join(SEPARATOR, "site-key", "an-action", TOKEN), "an-action"), Arguments.of( - String.join(SEPARATOR, PREFIX, "an-action", TOKEN, "something-else"), + String.join(SEPARATOR, PREFIX, "site-key", "an-action", TOKEN, "something-else"), false, - TOKEN + SEPARATOR + "something-else", - "an-action") + "site-key" + SEPARATOR + "an-action" + SEPARATOR + TOKEN + SEPARATOR + "something-else") ); }