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.
This commit is contained in:
Ravi Khadiwala 2022-07-08 11:03:24 -05:00 committed by Jon Chambers
parent 0fdfdabf2a
commit a45d95905e
2 changed files with 29 additions and 9 deletions

View File

@ -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);

View File

@ -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