Add platform tag to invalid HCaptcha reason metric

service/src/main/java/org/whispersystems/textsecuregcm/captcha/CaptchaChecker.java

@@ -50,6 +50,7 @@ public CaptchaChecker(
    * @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}
    * @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
    * used for metrics
    * @throws IOException         if there is an error validating the captcha with the underlying service
@@ -58,7 +59,8 @@ public CaptchaChecker(
   public AssessmentResult verify(
       final Action expectedAction,
       final String input,
-      final String ip) throws IOException {
+      final String ip,
+      final String userAgent) throws IOException {
     final String[] parts = input.split("\\" + SEPARATOR, 4);
 
     // we allow missing actions, if we're missing 1 part, assume it's the action
@@ -102,7 +104,7 @@ public AssessmentResult verify(
       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,
             "action", action,
             "score", result.getScoreString(),

service/src/main/java/org/whispersystems/textsecuregcm/captcha/CaptchaClient.java

@@ -26,16 +26,18 @@ public interface CaptchaClient {
   /**
    * Verify a provided captcha solution
    *
-   * @param siteKey identifying string for the captcha service
-   * @param action  an action indicating the purpose of the captcha
-   * @param token   the captcha solution that will be verified
-   * @param ip      the ip of the captcha solver
+   * @param siteKey   identifying string for the captcha service
+   * @param action    an action indicating the purpose of the captcha
+   * @param token     the captcha solution that will be verified
+   * @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
    * @throws IOException if the underlying captcha provider returns an error
    */
   AssessmentResult verify(
       final String siteKey,
       final Action action,
       final String token,
-      final String ip) throws IOException;
+      final String ip,
+      final String userAgent) throws IOException;
 }

service/src/main/java/org/whispersystems/textsecuregcm/captcha/HCaptchaClient.java

@@ -27,13 +27,16 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 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.LoggerFactory;
 import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
 import org.whispersystems.textsecuregcm.configuration.RetryConfiguration;
 import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicCaptchaConfiguration;
 import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
 import org.whispersystems.textsecuregcm.http.FaultTolerantHttpClient;
+import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
 import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager;
 import org.whispersystems.textsecuregcm.util.ExceptionUtils;
 import org.whispersystems.textsecuregcm.util.SystemMapper;
@@ -100,7 +103,8 @@ public AssessmentResult verify(
       final String siteKey,
       final Action action,
       final String token,
-      final String ip)
+      final String ip,
+      final String userAgent)
       throws IOException {
 
     final DynamicCaptchaConfiguration config = dynamicConfigurationManager.getConfiguration().getCaptchaConfiguration();
@@ -134,9 +138,11 @@ public AssessmentResult verify(
 
     if (!hCaptchaResponse.success) {
       for (String errorCode : hCaptchaResponse.errorCodes) {
-        Metrics.counter(INVALID_REASON_COUNTER_NAME,
-            "action", action.getActionName(),
-            "reason", errorCode).increment();
+        Metrics.counter(INVALID_REASON_COUNTER_NAME, Tags.of(
+            Tag.of("action", action.getActionName()),
+            Tag.of("reason", errorCode),
+            UserAgentTagUtil.getPlatformTag(userAgent)
+        )).increment();
       }
       return AssessmentResult.invalid();
     }

service/src/main/java/org/whispersystems/textsecuregcm/captcha/RegistrationCaptchaManager.java

@@ -17,10 +17,10 @@ public RegistrationCaptchaManager(final CaptchaChecker captchaChecker) {
   }
 
   @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 {
     return captcha.isPresent()
-        ? Optional.of(captchaChecker.verify(Action.REGISTRATION, captcha.get(), sourceHost))
+        ? Optional.of(captchaChecker.verify(Action.REGISTRATION, captcha.get(), sourceHost, userAgent))
         : Optional.empty();
   }
 }

service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java

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

service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java

@@ -67,7 +67,7 @@ public boolean answerCaptchaChallenge(final Account account, final String captch
 
     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(
         Tag.of(SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account.getNumber())),

service/src/test/java/org/whispersystems/textsecuregcm/captcha/CaptchaCheckerTest.java

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

service/src/test/java/org/whispersystems/textsecuregcm/captcha/HCaptchaClientTest.java

@@ -30,6 +30,7 @@ public class HCaptchaClientTest {
 
   private static final String SITE_KEY = "site-key";
   private static final String TOKEN = "token";
+  private static final String USER_AGENT = "user-agent";
 
 
   static Stream<Arguments> captchaProcessed() {
@@ -56,7 +57,7 @@ public void captchaProcessed(final boolean success, final float score, final boo
         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))
-        .verify(SITE_KEY, Action.CHALLENGE, TOKEN, null);
+        .verify(SITE_KEY, Action.CHALLENGE, TOKEN, null, USER_AGENT);
     if (!success) {
       assertThat(result).isEqualTo(AssessmentResult.invalid());
     } else {
@@ -68,7 +69,7 @@ public void captchaProcessed(final boolean success, final float score, final boo
   public void errorResponse() throws IOException, InterruptedException {
     final FaultTolerantHttpClient httpClient = mockResponder(503, "");
     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
@@ -77,7 +78,7 @@ public void invalidScore() throws IOException, InterruptedException {
         {"success" : true, "score": 1.1}
         """);
     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
@@ -86,7 +87,7 @@ public void badBody() throws IOException, InterruptedException {
         {"success" : true,
         """);
     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

service/src/test/java/org/whispersystems/textsecuregcm/configuration/StubHCaptchaClientFactory.java

@@ -49,8 +49,9 @@ public Set<String> validSiteKeys(final Action action) {
     }
 
     @Override
-    public AssessmentResult verify(final String siteKey, final Action action, final String token, final String ip)
-        throws IOException {
+    public AssessmentResult verify(final String siteKey, final Action action, final String token, final String ip,
+        final String userAgent)
+    throws IOException {
       return AssessmentResult.alwaysValid();
     }
   }

service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java

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

service/src/test/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManagerTest.java

@@ -85,7 +85,7 @@ void answerCaptchaChallenge(Optional<Float> scoreThreshold, float actualScore, b
     when(account.getNumber()).thenReturn("+18005551234");
     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));
 
     when(rateLimiters.getCaptchaChallengeAttemptLimiter()).thenReturn(mock(RateLimiter.class));