From a45d95905e74abeebe49e3b40d7d993b49326256 Mon Sep 17 00:00:00 2001
From: Ravi Khadiwala <ravi@signal.org>
Date: Fri, 8 Jul 2022 11:03:24 -0500
Subject: [PATCH] Be permissive in account-create accept-language

Currently, if we fail to parse a user's accept-language in account
creation, creation will fail. While it's a suboptimal experience to get
a verify code in the wrong language, it might be better than not being
able to sign up at all.
---
 .../controllers/AccountController.java        | 14 ++++++++---
 .../controllers/AccountControllerTest.java    | 24 ++++++++++++++-----
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java
index 1d4f01194..3718f80cf 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java
@@ -74,6 +74,7 @@ import org.whispersystems.textsecuregcm.entities.RegistrationLock;
 import org.whispersystems.textsecuregcm.entities.RegistrationLockFailure;
 import org.whispersystems.textsecuregcm.entities.StaleDevices;
 import org.whispersystems.textsecuregcm.limits.RateLimiters;
+import org.whispersystems.textsecuregcm.metrics.MetricsUtil;
 import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
 import org.whispersystems.textsecuregcm.push.APNSender;
 import org.whispersystems.textsecuregcm.push.ApnMessage;
@@ -121,6 +122,8 @@ public class AccountController {
 
   private static final String TWILIO_VERIFY_ERROR_COUNTER_NAME = name(AccountController.class, "twilioVerifyError");
 
+  private static final String INVALID_ACCEPT_LANGUAGE_COUNTER_NAME = name(AccountController.class, "invalidAcceptLanguage");
+
   private static final String CHALLENGE_PRESENT_TAG_NAME = "present";
   private static final String CHALLENGE_MATCH_TAG_NAME = "matches";
   private static final String COUNTRY_CODE_TAG_NAME = "countryCode";
@@ -266,12 +269,17 @@ public class AccountController {
 
     pendingAccounts.store(number, storedVerificationCode);
 
-    final List<Locale.LanguageRange> languageRanges;
-
+    List<Locale.LanguageRange> languageRanges;
     try {
       languageRanges = acceptLanguage.map(Locale.LanguageRange::parse).orElse(Collections.emptyList());
     } catch (final IllegalArgumentException e) {
-      return Response.status(400).build();
+      logger.debug("Could not get acceptable languages; Accept-Language: {}; User-Agent: {}",
+          acceptLanguage.orElse(""),
+          userAgent,
+          e);
+
+      Metrics.counter(INVALID_ACCEPT_LANGUAGE_COUNTER_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))).increment();
+      languageRanges = Collections.emptyList();
     }
 
     final boolean enrolledInVerifyExperiment = verifyExperimentEnrollmentManager.isEnrolled(client, number, languageRanges, transport);
diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java
index 71c21cceb..e0c15322d 100644
--- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java
+++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java
@@ -636,8 +636,17 @@ class AccountControllerTest {
     verify(abusiveHostRules).isBlocked(eq(NICE_HOST));
   }
 
-  @Test
-  void testSendCodeVoiceInvalidLocale() throws Exception {
+  @ParameterizedTest
+  @ValueSource(booleans = {false, true})
+  void testSendCodeVoiceInvalidLocale(boolean enrolledInVerifyExperiment) throws Exception {
+    when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString()))
+        .thenReturn(enrolledInVerifyExperiment);
+
+    if (enrolledInVerifyExperiment) {
+      when(smsSender.deliverVoxVerificationWithTwilioVerify(anyString(), anyString(), anyList()))
+          .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid")));
+    }
+
     Response response =
         resources.getJerseyTest()
             .target(String.format("/v1/accounts/voice/code/%s", SENDER))
@@ -647,11 +656,14 @@ class AccountControllerTest {
             .header("X-Forwarded-For", NICE_HOST)
             .get();
 
-    assertThat(response.getStatus()).isEqualTo(400);
+    // Should still send a code, just with no accept language
+    assertThat(response.getStatus()).isEqualTo(200);
 
-    verify(smsSender, never()).deliverVoxVerification(eq(SENDER), anyString(), any());
-    verify(smsSender, never()).deliverVoxVerificationWithTwilioVerify(eq(SENDER), anyString(), any());
-    verify(abusiveHostRules).isBlocked(eq(NICE_HOST));
+    if (enrolledInVerifyExperiment) {
+      verify(smsSender).deliverVoxVerificationWithTwilioVerify(eq(SENDER), anyString(), eq(Collections.emptyList()));
+    } else {
+      verify(smsSender).deliverVoxVerification(eq(SENDER), anyString(), eq(Collections.emptyList()));
+    }
   }
 
   @ParameterizedTest