MirrorMirror
/
Signal-Server
mirror of https://github.com/signalapp/Signal-Server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add platform tag to invalid HCaptcha reason metric

This commit is contained in:
Ameya Lokare 2024-09-04 15:17:50 -07:00
parent 11601fd091
commit d6acfa56c2
11 changed files with 49 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
service/src/main/java/org/whispersystems/textsecuregcm/captcha/CaptchaChecker.java
View File

@ -50,6 +50,7 @@ public class CaptchaChecker {
* @param input expected to contain a prefix indicating the captcha scheme, sitekey, token, and action. The * @param input expected to contain a prefix indicating the captcha scheme, sitekey, token, and action. The
* expected format is {@code version-prefix.sitekey.action.token} * expected format is {@code version-prefix.sitekey.action.token}
* @param ip IP of the solver * @param ip IP of the solver
* @param userAgent User-Agent of the solver
* @return An {@link AssessmentResult} indicating whether the solution should be accepted, and a score that can be * @return An {@link AssessmentResult} indicating whether the solution should be accepted, and a score that can be
* used for metrics * used for metrics
* @throws IOException if there is an error validating the captcha with the underlying service * @throws IOException if there is an error validating the captcha with the underlying service
@ -58,7 +59,8 @@ public class CaptchaChecker {
public AssessmentResult verify( public AssessmentResult verify(
final Action expectedAction, final Action expectedAction,
final String input, final String input,
final String ip) throws IOException { final String ip,
final String userAgent) throws IOException {
final String[] parts = input.split("\\" + SEPARATOR, 4); final String[] parts = input.split("\\" + SEPARATOR, 4);
// we allow missing actions, if we're missing 1 part, assume it's the action // we allow missing actions, if we're missing 1 part, assume it's the action
@ -102,7 +104,7 @@ public class CaptchaChecker {
throw new BadRequestException("invalid captcha site-key"); throw new BadRequestException("invalid captcha site-key");
} }
final AssessmentResult result = client.verify(siteKey, parsedAction, token, ip); final AssessmentResult result = client.verify(siteKey, parsedAction, token, ip, userAgent);
Metrics.counter(ASSESSMENTS_COUNTER_NAME, Metrics.counter(ASSESSMENTS_COUNTER_NAME,
"action", action, "action", action,
"score", result.getScoreString(), "score", result.getScoreString(),

12
service/src/main/java/org/whispersystems/textsecuregcm/captcha/CaptchaClient.java
View File

@ -26,10 +26,11 @@ public interface CaptchaClient {
/** /**
* Verify a provided captcha solution * Verify a provided captcha solution
* *
* @param siteKey identifying string for the captcha service * @param siteKey identifying string for the captcha service
* @param action an action indicating the purpose of the captcha * @param action an action indicating the purpose of the captcha
* @param token the captcha solution that will be verified * @param token the captcha solution that will be verified
* @param ip the ip of the captcha solver * @param ip the ip of the captcha solver
* @param userAgent the User-Agent string of the captcha solver
* @return An {@link AssessmentResult} indicating whether the solution should be accepted * @return An {@link AssessmentResult} indicating whether the solution should be accepted
* @throws IOException if the underlying captcha provider returns an error * @throws IOException if the underlying captcha provider returns an error
*/ */
@ -37,5 +38,6 @@ public interface CaptchaClient {
final String siteKey, final String siteKey,
final Action action, final Action action,
final String token, final String token,
final String ip) throws IOException; final String ip,
final String userAgent) throws IOException;
} }

14
service/src/main/java/org/whispersystems/textsecuregcm/captcha/HCaptchaClient.java
View File

@ -27,6 +27,8 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors; import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledExecutorService;
import javax.ws.rs.core.Response; import javax.ws.rs.core.Response;
import io.micrometer.core.instrument.Tag;
import io.micrometer.core.instrument.Tags;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration; import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
@ -34,6 +36,7 @@ import org.whispersystems.textsecuregcm.configuration.RetryConfiguration;
import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicCaptchaConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicCaptchaConfiguration;
import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
import org.whispersystems.textsecuregcm.http.FaultTolerantHttpClient; import org.whispersystems.textsecuregcm.http.FaultTolerantHttpClient;
import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager;
import org.whispersystems.textsecuregcm.util.ExceptionUtils; import org.whispersystems.textsecuregcm.util.ExceptionUtils;
import org.whispersystems.textsecuregcm.util.SystemMapper; import org.whispersystems.textsecuregcm.util.SystemMapper;
@ -100,7 +103,8 @@ public class HCaptchaClient implements CaptchaClient {
final String siteKey, final String siteKey,
final Action action, final Action action,
final String token, final String token,
final String ip) final String ip,
final String userAgent)
throws IOException { throws IOException {
final DynamicCaptchaConfiguration config = dynamicConfigurationManager.getConfiguration().getCaptchaConfiguration(); final DynamicCaptchaConfiguration config = dynamicConfigurationManager.getConfiguration().getCaptchaConfiguration();
@ -134,9 +138,11 @@ public class HCaptchaClient implements CaptchaClient {
if (!hCaptchaResponse.success) { if (!hCaptchaResponse.success) {
for (String errorCode : hCaptchaResponse.errorCodes) { for (String errorCode : hCaptchaResponse.errorCodes) {
Metrics.counter(INVALID_REASON_COUNTER_NAME, Metrics.counter(INVALID_REASON_COUNTER_NAME, Tags.of(
"action", action.getActionName(), Tag.of("action", action.getActionName()),
"reason", errorCode).increment(); Tag.of("reason", errorCode),
UserAgentTagUtil.getPlatformTag(userAgent)
)).increment();
} }
return AssessmentResult.invalid(); return AssessmentResult.invalid();
} }

4
service/src/main/java/org/whispersystems/textsecuregcm/captcha/RegistrationCaptchaManager.java
View File

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

2
service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java
View File

@ -386,7 +386,7 @@ public class VerificationController {
try { try {
assessmentResult = registrationCaptchaManager.assessCaptcha( assessmentResult = registrationCaptchaManager.assessCaptcha(
Optional.of(updateVerificationSessionRequest.captcha()), sourceHost) Optional.of(updateVerificationSessionRequest.captcha()), sourceHost, userAgent)
.orElseThrow(() -> new ServerErrorException(Response.Status.INTERNAL_SERVER_ERROR)); .orElseThrow(() -> new ServerErrorException(Response.Status.INTERNAL_SERVER_ERROR));
Metrics.counter(CAPTCHA_ATTEMPT_COUNTER_NAME, Tags.of( Metrics.counter(CAPTCHA_ATTEMPT_COUNTER_NAME, Tags.of(

2
service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java
View File

@ -67,7 +67,7 @@ public class RateLimitChallengeManager {
rateLimiters.getCaptchaChallengeAttemptLimiter().validate(account.getUuid()); rateLimiters.getCaptchaChallengeAttemptLimiter().validate(account.getUuid());
final boolean challengeSuccess = captchaChecker.verify(Action.CHALLENGE, captcha, mostRecentProxyIp).isValid(scoreThreshold); final boolean challengeSuccess = captchaChecker.verify(Action.CHALLENGE, captcha, mostRecentProxyIp, userAgent).isValid(scoreThreshold);
final Tags tags = Tags.of( final Tags tags = Tags.of(
Tag.of(SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account.getNumber())), Tag.of(SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account.getNumber())),

21
service/src/test/java/org/whispersystems/textsecuregcm/captcha/CaptchaCheckerTest.java
View File

@ -34,6 +34,7 @@ public class CaptchaCheckerTest {
private static final String PREFIX = "prefix"; private static final String PREFIX = "prefix";
private static final String PREFIX_A = "prefix-a"; private static final String PREFIX_A = "prefix-a";
private static final String PREFIX_B = "prefix-b"; private static final String PREFIX_B = "prefix-b";
private static final String USER_AGENT = "user-agent";
static Stream<Arguments> parseInputToken() { static Stream<Arguments> parseInputToken() {
return Stream.of( return Stream.of(
@ -65,7 +66,7 @@ public class CaptchaCheckerTest {
when(captchaClient.scheme()).thenReturn(prefix); when(captchaClient.scheme()).thenReturn(prefix);
when(captchaClient.validSiteKeys(eq(Action.CHALLENGE))).thenReturn(Collections.singleton(CHALLENGE_SITE_KEY)); when(captchaClient.validSiteKeys(eq(Action.CHALLENGE))).thenReturn(Collections.singleton(CHALLENGE_SITE_KEY));
when(captchaClient.validSiteKeys(eq(Action.REGISTRATION))).thenReturn(Collections.singleton(REG_SITE_KEY)); when(captchaClient.validSiteKeys(eq(Action.REGISTRATION))).thenReturn(Collections.singleton(REG_SITE_KEY));
when(captchaClient.verify(any(), any(), any(), any())).thenReturn(AssessmentResult.invalid()); when(captchaClient.verify(any(), any(), any(), any(), any())).thenReturn(AssessmentResult.invalid());
return captchaClient; return captchaClient;
} }
@ -78,8 +79,8 @@ public class CaptchaCheckerTest {
final String siteKey, final String siteKey,
final Action expectedAction) throws IOException { final Action expectedAction) throws IOException {
final CaptchaClient captchaClient = mockClient(PREFIX); final CaptchaClient captchaClient = mockClient(PREFIX);
new CaptchaChecker(null, List.of(captchaClient)).verify(expectedAction, input, null); new CaptchaChecker(null, List.of(captchaClient)).verify(expectedAction, input, null, USER_AGENT);
verify(captchaClient, times(1)).verify(eq(siteKey), eq(expectedAction), eq(expectedToken), any()); verify(captchaClient, times(1)).verify(eq(siteKey), eq(expectedAction), eq(expectedToken), any(), eq(USER_AGENT));
} }
@ParameterizedTest @ParameterizedTest
@ -106,11 +107,11 @@ public class CaptchaCheckerTest {
final CaptchaClient a = mockClient(PREFIX_A); final CaptchaClient a = mockClient(PREFIX_A);
final CaptchaClient b = mockClient(PREFIX_B); final CaptchaClient b = mockClient(PREFIX_B);
new CaptchaChecker(null, List.of(a, b)).verify(Action.CHALLENGE, ainput, null); new CaptchaChecker(null, List.of(a, b)).verify(Action.CHALLENGE, ainput, null, USER_AGENT);
verify(a, times(1)).verify(any(), any(), any(), any()); verify(a, times(1)).verify(any(), any(), any(), any(), any());
new CaptchaChecker(null, List.of(a, b)).verify(Action.CHALLENGE, binput, null); new CaptchaChecker(null, List.of(a, b)).verify(Action.CHALLENGE, binput, null, USER_AGENT);
verify(b, times(1)).verify(any(), any(), any(), any()); verify(b, times(1)).verify(any(), any(), any(), any(), any());
} }
static Stream<Arguments> badArgs() { static Stream<Arguments> badArgs() {
@ -131,7 +132,7 @@ public class CaptchaCheckerTest {
public void badArgs(final String input) throws IOException { public void badArgs(final String input) throws IOException {
final CaptchaClient cc = mockClient(PREFIX); final CaptchaClient cc = mockClient(PREFIX);
assertThrows(BadRequestException.class, assertThrows(BadRequestException.class,
() -> new CaptchaChecker(null, List.of(cc)).verify(Action.CHALLENGE, input, null)); () -> new CaptchaChecker(null, List.of(cc)).verify(Action.CHALLENGE, input, null, USER_AGENT));
} }
@ -141,8 +142,8 @@ public class CaptchaCheckerTest {
final ShortCodeExpander retriever = mock(ShortCodeExpander.class); final ShortCodeExpander retriever = mock(ShortCodeExpander.class);
when(retriever.retrieve("abc")).thenReturn(Optional.of(TOKEN)); when(retriever.retrieve("abc")).thenReturn(Optional.of(TOKEN));
final String input = String.join(SEPARATOR, PREFIX + "-short", REG_SITE_KEY, "registration", "abc"); final String input = String.join(SEPARATOR, PREFIX + "-short", REG_SITE_KEY, "registration", "abc");
new CaptchaChecker(retriever, List.of(captchaClient)).verify(Action.REGISTRATION, input, null); new CaptchaChecker(retriever, List.of(captchaClient)).verify(Action.REGISTRATION, input, null, USER_AGENT);
verify(captchaClient, times(1)).verify(eq(REG_SITE_KEY), eq(Action.REGISTRATION), eq(TOKEN), any()); verify(captchaClient, times(1)).verify(eq(REG_SITE_KEY), eq(Action.REGISTRATION), eq(TOKEN), any(), any());
} }
} }

9
service/src/test/java/org/whispersystems/textsecuregcm/captcha/HCaptchaClientTest.java
View File

@ -30,6 +30,7 @@ public class HCaptchaClientTest {
private static final String SITE_KEY = "site-key"; private static final String SITE_KEY = "site-key";
private static final String TOKEN = "token"; private static final String TOKEN = "token";
private static final String USER_AGENT = "user-agent";
static Stream<Arguments> captchaProcessed() { static Stream<Arguments> captchaProcessed() {
@ -56,7 +57,7 @@ public class HCaptchaClientTest {
success, 1 - score)); // hCaptcha scores are inverted compared to recaptcha scores. (low score is good) success, 1 - score)); // hCaptcha scores are inverted compared to recaptcha scores. (low score is good)
final AssessmentResult result = new HCaptchaClient("fake", client, mockConfig(true, 0.5)) final AssessmentResult result = new HCaptchaClient("fake", client, mockConfig(true, 0.5))
.verify(SITE_KEY, Action.CHALLENGE, TOKEN, null); .verify(SITE_KEY, Action.CHALLENGE, TOKEN, null, USER_AGENT);
if (!success) { if (!success) {
assertThat(result).isEqualTo(AssessmentResult.invalid()); assertThat(result).isEqualTo(AssessmentResult.invalid());
} else { } else {
@ -68,7 +69,7 @@ public class HCaptchaClientTest {
public void errorResponse() throws IOException, InterruptedException { public void errorResponse() throws IOException, InterruptedException {
final FaultTolerantHttpClient httpClient = mockResponder(503, ""); final FaultTolerantHttpClient httpClient = mockResponder(503, "");
final HCaptchaClient client = new HCaptchaClient("fake", httpClient, mockConfig(true, 0.5)); final HCaptchaClient client = new HCaptchaClient("fake", httpClient, mockConfig(true, 0.5));
assertThrows(IOException.class, () -> client.verify(SITE_KEY, Action.CHALLENGE, TOKEN, null)); assertThrows(IOException.class, () -> client.verify(SITE_KEY, Action.CHALLENGE, TOKEN, null, USER_AGENT));
} }
@Test @Test
@ -77,7 +78,7 @@ public class HCaptchaClientTest {
{"success" : true, "score": 1.1} {"success" : true, "score": 1.1}
"""); """);
final HCaptchaClient client = new HCaptchaClient("fake", httpClient, mockConfig(true, 0.5)); final HCaptchaClient client = new HCaptchaClient("fake", httpClient, mockConfig(true, 0.5));
assertThat(client.verify(SITE_KEY, Action.CHALLENGE, TOKEN, null)).isEqualTo(AssessmentResult.invalid()); assertThat(client.verify(SITE_KEY, Action.CHALLENGE, TOKEN, null, USER_AGENT)).isEqualTo(AssessmentResult.invalid());
} }
@Test @Test
@ -86,7 +87,7 @@ public class HCaptchaClientTest {
{"success" : true, {"success" : true,
"""); """);
final HCaptchaClient client = new HCaptchaClient("fake", httpClient, mockConfig(true, 0.5)); final HCaptchaClient client = new HCaptchaClient("fake", httpClient, mockConfig(true, 0.5));
assertThrows(IOException.class, () -> client.verify(SITE_KEY, Action.CHALLENGE, TOKEN, null)); assertThrows(IOException.class, () -> client.verify(SITE_KEY, Action.CHALLENGE, TOKEN, null, USER_AGENT));
} }
@Test @Test

5
service/src/test/java/org/whispersystems/textsecuregcm/configuration/StubHCaptchaClientFactory.java
View File

@ -49,8 +49,9 @@ public class StubHCaptchaClientFactory implements HCaptchaClientFactory {
} }
@Override @Override
public AssessmentResult verify(final String siteKey, final Action action, final String token, final String ip) public AssessmentResult verify(final String siteKey, final Action action, final String token, final String ip,
throws IOException { final String userAgent)
throws IOException {
return AssessmentResult.alwaysValid(); return AssessmentResult.alwaysValid();
} }
} }

8
service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java
View File

@ -465,7 +465,7 @@ class VerificationControllerTest {
Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(),
registrationServiceSession.expiration())))); registrationServiceSession.expiration()))));
when(registrationCaptchaManager.assessCaptcha(any(), any())) when(registrationCaptchaManager.assessCaptcha(any(), any(), any()))
.thenReturn(Optional.of(AssessmentResult.invalid())); .thenReturn(Optional.of(AssessmentResult.invalid()));
when(verificationSessionManager.update(any(), any())) when(verificationSessionManager.update(any(), any()))
@ -637,7 +637,7 @@ class VerificationControllerTest {
Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(),
registrationServiceSession.expiration())))); registrationServiceSession.expiration()))));
when(registrationCaptchaManager.assessCaptcha(any(), any())) when(registrationCaptchaManager.assessCaptcha(any(), any(), any()))
.thenReturn(Optional.of(AssessmentResult.alwaysValid())); .thenReturn(Optional.of(AssessmentResult.alwaysValid()));
when(verificationSessionManager.update(any(), any())) when(verificationSessionManager.update(any(), any()))
@ -685,7 +685,7 @@ class VerificationControllerTest {
Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(),
registrationServiceSession.expiration())))); registrationServiceSession.expiration()))));
when(registrationCaptchaManager.assessCaptcha(any(), any())) when(registrationCaptchaManager.assessCaptcha(any(), any(), any()))
.thenReturn(Optional.of(AssessmentResult.alwaysValid())); .thenReturn(Optional.of(AssessmentResult.alwaysValid()));
when(verificationSessionManager.update(any(), any())) when(verificationSessionManager.update(any(), any()))
@ -732,7 +732,7 @@ class VerificationControllerTest {
Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(),
registrationServiceSession.expiration())))); registrationServiceSession.expiration()))));
when(registrationCaptchaManager.assessCaptcha(any(), any())) when(registrationCaptchaManager.assessCaptcha(any(), any(), any()))
.thenThrow(new IOException("expected service error")); .thenThrow(new IOException("expected service error"));
when(verificationSessionManager.update(any(), any())) when(verificationSessionManager.update(any(), any()))

2
service/src/test/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManagerTest.java
View File

@ -85,7 +85,7 @@ class RateLimitChallengeManagerTest {
when(account.getNumber()).thenReturn("+18005551234"); when(account.getNumber()).thenReturn("+18005551234");
when(account.getUuid()).thenReturn(UUID.randomUUID()); when(account.getUuid()).thenReturn(UUID.randomUUID());
when(captchaChecker.verify(eq(Action.CHALLENGE), any(), any())) when(captchaChecker.verify(eq(Action.CHALLENGE), any(), any(), any()))
.thenReturn(AssessmentResult.fromScore(actualScore, DEFAULT_SCORE_THRESHOLD)); .thenReturn(AssessmentResult.fromScore(actualScore, DEFAULT_SCORE_THRESHOLD));
when(rateLimiters.getCaptchaChallengeAttemptLimiter()).thenReturn(mock(RateLimiter.class)); when(rateLimiters.getCaptchaChallengeAttemptLimiter()).thenReturn(mock(RateLimiter.class));