From 13053da97fbe612757e68cc418854edba059e926 Mon Sep 17 00:00:00 2001 From: Chris Eager Date: Mon, 15 Mar 2021 21:22:00 -0500 Subject: [PATCH] Add Twilio Verify experiment to AccountController --- .../textsecuregcm/WhisperServerService.java | 6 +- .../auth/StoredVerificationCode.java | 25 +- .../controllers/AccountController.java | 74 +++- .../storage/PendingAccounts.java | 24 +- .../storage/PendingAccountsManager.java | 3 +- .../textsecuregcm/storage/PendingDevices.java | 9 +- .../StoredVerificationCodeRowMapper.java | 5 +- service/src/main/resources/accountsdb.xml | 8 + .../controllers/AccountControllerTest.java | 327 +++++++++++++++--- .../tests/storage/PendingAccountsTest.java | 54 ++- 10 files changed, 442 insertions(+), 93 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 6d4c7e314..75c04a5ca 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -138,6 +138,7 @@ import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient; import org.whispersystems.textsecuregcm.sms.SmsSender; import org.whispersystems.textsecuregcm.sms.TwilioSmsSender; +import org.whispersystems.textsecuregcm.sms.TwilioVerifyExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.AbusiveHostRules; import org.whispersystems.textsecuregcm.storage.Account; @@ -382,6 +383,9 @@ public class WhisperServerService extends Application getTwilioVerificationSid() { + return Optional.ofNullable(twilioVerificationSid); + } + public boolean isValid(String theirCodeString) { if (timestamp + TimeUnit.MINUTES.toMillis(10) < System.currentTimeMillis()) { return false; @@ -52,10 +66,9 @@ public class StoredVerificationCode { return false; } - byte[] ourCode = code.getBytes(); + byte[] ourCode = code.getBytes(); byte[] theirCode = theirCodeString.getBytes(); return MessageDigest.isEqual(ourCode, theirCode); } - } 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 11bb31155..e2e4296d1 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -22,6 +22,7 @@ import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.UUID; +import java.util.concurrent.CompletableFuture; import javax.validation.Valid; import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; @@ -64,6 +65,7 @@ import org.whispersystems.textsecuregcm.push.GCMSender; import org.whispersystems.textsecuregcm.push.GcmMessage; import org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient; import org.whispersystems.textsecuregcm.sms.SmsSender; +import org.whispersystems.textsecuregcm.sms.TwilioVerifyExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.AbusiveHostRule; import org.whispersystems.textsecuregcm.storage.AbusiveHostRules; @@ -99,11 +101,15 @@ public class AccountController { private static final String ACCOUNT_CREATE_COUNTER_NAME = name(AccountController.class, "create"); private static final String ACCOUNT_VERIFY_COUNTER_NAME = name(AccountController.class, "verify"); + private static final String TWILIO_VERIFY_ERROR_COUNTER_NAME = name(AccountController.class, "twilioVerifyError"); + 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"; private static final String VERFICATION_TRANSPORT_TAG_NAME = "transport"; + private static final String VERIFY_EXPERIMENT_TAG_NAME = "twilioVerify"; + private final PendingAccountsManager pendingAccounts; private final AccountsManager accounts; private final UsernamesManager usernames; @@ -120,6 +126,8 @@ public class AccountController { private final APNSender apnSender; private final ExternalServiceCredentialGenerator backupServiceCredentialGenerator; + private final TwilioVerifyExperimentEnrollmentManager verifyExperimentEnrollmentManager; + public AccountController(PendingAccountsManager pendingAccounts, AccountsManager accounts, UsernamesManager usernames, @@ -134,7 +142,8 @@ public class AccountController { RecaptchaClient recaptchaClient, GCMSender gcmSender, APNSender apnSender, - ExternalServiceCredentialGenerator backupServiceCredentialGenerator) + ExternalServiceCredentialGenerator backupServiceCredentialGenerator, + TwilioVerifyExperimentEnrollmentManager verifyExperimentEnrollmentManager) { this.pendingAccounts = pendingAccounts; this.accounts = accounts; @@ -151,6 +160,7 @@ public class AccountController { this.gcmSender = gcmSender; this.apnSender = apnSender; this.backupServiceCredentialGenerator = backupServiceCredentialGenerator; + this.verifyExperimentEnrollmentManager = verifyExperimentEnrollmentManager; } @Timed @@ -246,27 +256,64 @@ public class AccountController { VerificationCode verificationCode = generateVerificationCode(number); StoredVerificationCode storedVerificationCode = new StoredVerificationCode(verificationCode.getVerificationCode(), - System.currentTimeMillis(), - storedChallenge.map(StoredVerificationCode::getPushCode).orElse(null)); + System.currentTimeMillis(), + storedChallenge.map(StoredVerificationCode::getPushCode).orElse(null), + storedChallenge.flatMap(StoredVerificationCode::getTwilioVerificationSid).orElse(null)); pendingAccounts.store(number, storedVerificationCode); + final List languageRanges; + + try { + languageRanges = acceptLanguage.map(Locale.LanguageRange::parse).orElse(Collections.emptyList()); + } catch (final IllegalArgumentException e) { + return Response.status(400).build(); + } + + final boolean enrolledInVerifyExperiment = verifyExperimentEnrollmentManager.isEnrolled(client, number, languageRanges, transport); + final CompletableFuture> sendVerificationWithTwilioVerifyFuture; + if (testDevices.containsKey(number)) { // noop + sendVerificationWithTwilioVerifyFuture = CompletableFuture.completedFuture(Optional.empty()); } else if (transport.equals("sms")) { - smsSender.deliverSmsVerification(number, client, verificationCode.getVerificationCodeDisplay()); - } else if (transport.equals("voice")) { - final List languageRanges; - try { - languageRanges = acceptLanguage.map(Locale.LanguageRange::parse).orElse(Collections.emptyList()); - } catch (final IllegalArgumentException e) { - return Response.status(400).build(); + if (enrolledInVerifyExperiment) { + sendVerificationWithTwilioVerifyFuture = smsSender.deliverSmsVerificationWithTwilioVerify(number, client, verificationCode.getVerificationCode(), languageRanges); + } else { + smsSender.deliverSmsVerification(number, client, verificationCode.getVerificationCodeDisplay()); + sendVerificationWithTwilioVerifyFuture = CompletableFuture.completedFuture(Optional.empty()); + } + } else if (transport.equals("voice")) { + + if (enrolledInVerifyExperiment) { + sendVerificationWithTwilioVerifyFuture = smsSender.deliverVoxVerificationWithTwilioVerify(number, verificationCode.getVerificationCode(), languageRanges); + } else { + smsSender.deliverVoxVerification(number, verificationCode.getVerificationCode(), languageRanges); + sendVerificationWithTwilioVerifyFuture = CompletableFuture.completedFuture(Optional.empty()); } - smsSender.deliverVoxVerification(number, verificationCode.getVerificationCode(), languageRanges); + } else { + sendVerificationWithTwilioVerifyFuture = CompletableFuture.completedFuture(Optional.empty()); } + sendVerificationWithTwilioVerifyFuture.whenComplete((maybeVerificationSid, throwable) -> { + if (throwable != null) { + Metrics.counter(TWILIO_VERIFY_ERROR_COUNTER_NAME).increment(); + + logger.warn("Error with Twilio Verify", throwable); + return; + } + maybeVerificationSid.ifPresent(twilioVerificationSid -> { + StoredVerificationCode storedVerificationCodeWithVerificationSid = new StoredVerificationCode( + storedVerificationCode.getCode(), + storedVerificationCode.getTimestamp(), + storedVerificationCode.getPushCode(), + twilioVerificationSid); + pendingAccounts.store(number, storedVerificationCodeWithVerificationSid); + }); + }); + metricRegistry.meter(name(AccountController.class, "create", Util.getCountryCode(number))).mark(); { @@ -274,6 +321,7 @@ public class AccountController { tags.add(Tag.of(COUNTRY_CODE_TAG_NAME, Util.getCountryCode(number))); tags.add(Tag.of(VERFICATION_TRANSPORT_TAG_NAME, transport)); tags.add(UserAgentTagUtil.getPlatformTag(userAgent)); + tags.add(Tag.of(VERIFY_EXPERIMENT_TAG_NAME, String.valueOf(enrolledInVerifyExperiment))); Metrics.counter(ACCOUNT_CREATE_COUNTER_NAME, tags).increment(); } @@ -311,6 +359,9 @@ public class AccountController { throw new WebApplicationException(Response.status(403).build()); } + storedVerificationCode.flatMap(StoredVerificationCode::getTwilioVerificationSid) + .ifPresent(smsSender::reportVerificationSucceeded); + Optional existingAccount = accounts.get(number); Optional existingRegistrationLock = existingAccount.map(Account::getRegistrationLock); Optional existingBackupCredentials = existingAccount.map(Account::getUuid) @@ -345,6 +396,7 @@ public class AccountController { final List tags = new ArrayList<>(); tags.add(Tag.of(COUNTRY_CODE_TAG_NAME, Util.getCountryCode(number))); tags.add(UserAgentTagUtil.getPlatformTag(userAgent)); + tags.add(Tag.of(VERIFY_EXPERIMENT_TAG_NAME, String.valueOf(storedVerificationCode.get().getTwilioVerificationSid().isPresent()))); Metrics.counter(ACCOUNT_VERIFY_COUNTER_NAME, tags).increment(); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccounts.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccounts.java index 73bdbc156..83815a2b8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccounts.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccounts.java @@ -4,17 +4,17 @@ */ package org.whispersystems.textsecuregcm.storage; +import static com.codahale.metrics.MetricRegistry.name; + import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.SharedMetricRegistries; import com.codahale.metrics.Timer; +import java.util.Optional; +import com.google.common.annotations.VisibleForTesting; import org.whispersystems.textsecuregcm.auth.StoredVerificationCode; import org.whispersystems.textsecuregcm.storage.mappers.StoredVerificationCodeRowMapper; import org.whispersystems.textsecuregcm.util.Constants; -import java.util.Optional; - -import static com.codahale.metrics.MetricRegistry.name; - public class PendingAccounts { private final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); @@ -30,17 +30,23 @@ public class PendingAccounts { this.database.getDatabase().registerRowMapper(new StoredVerificationCodeRowMapper()); } - public void insert(String number, String verificationCode, long timestamp, String pushCode) { + @VisibleForTesting + public void insert (String number, String verificationCode, long timestamp, String pushCode) { + insert(number, verificationCode, timestamp, pushCode, null); + } + + public void insert(String number, String verificationCode, long timestamp, String pushCode, String twilioVerificationSid) { database.use(jdbi -> jdbi.useHandle(handle -> { try (Timer.Context ignored = insertTimer.time()) { - handle.createUpdate("INSERT INTO pending_accounts (number, verification_code, timestamp, push_code) " + - "VALUES (:number, :verification_code, :timestamp, :push_code) " + + handle.createUpdate("INSERT INTO pending_accounts (number, verification_code, timestamp, push_code, twilio_verification_sid) " + + "VALUES (:number, :verification_code, :timestamp, :push_code, :twilio_verification_sid) " + "ON CONFLICT(number) DO UPDATE " + - "SET verification_code = EXCLUDED.verification_code, timestamp = EXCLUDED.timestamp, push_code = EXCLUDED.push_code") + "SET verification_code = EXCLUDED.verification_code, timestamp = EXCLUDED.timestamp, push_code = EXCLUDED.push_code, twilio_verification_sid = EXCLUDED.twilio_verification_sid") .bind("verification_code", verificationCode) .bind("timestamp", timestamp) .bind("number", number) .bind("push_code", pushCode) + .bind("twilio_verification_sid", twilioVerificationSid) .execute(); } })); @@ -49,7 +55,7 @@ public class PendingAccounts { public Optional getCodeForNumber(String number) { return database.with(jdbi ->jdbi.withHandle(handle -> { try (Timer.Context ignored = getCodeForNumberTimer.time()) { - return handle.createQuery("SELECT verification_code, timestamp, push_code FROM pending_accounts WHERE number = :number") + return handle.createQuery("SELECT verification_code, timestamp, push_code, twilio_verification_sid FROM pending_accounts WHERE number = :number") .bind("number", number) .mapTo(StoredVerificationCode.class) .findFirst(); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccountsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccountsManager.java index 788d3c6aa..5b8adf156 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccountsManager.java @@ -34,7 +34,8 @@ public class PendingAccountsManager { public void store(String number, StoredVerificationCode code) { memcacheSet(number, code); - pendingAccounts.insert(number, code.getCode(), code.getTimestamp(), code.getPushCode()); + pendingAccounts.insert(number, code.getCode(), code.getTimestamp(), code.getPushCode(), + code.getTwilioVerificationSid().orElse(null)); } public void remove(String number) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevices.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevices.java index 142dfe7f1..3aa455cf3 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevices.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevices.java @@ -4,17 +4,16 @@ */ package org.whispersystems.textsecuregcm.storage; +import static com.codahale.metrics.MetricRegistry.name; + import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.SharedMetricRegistries; import com.codahale.metrics.Timer; +import java.util.Optional; import org.whispersystems.textsecuregcm.auth.StoredVerificationCode; import org.whispersystems.textsecuregcm.storage.mappers.StoredVerificationCodeRowMapper; import org.whispersystems.textsecuregcm.util.Constants; -import java.util.Optional; - -import static com.codahale.metrics.MetricRegistry.name; - public class PendingDevices { private final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); @@ -45,7 +44,7 @@ public class PendingDevices { public Optional getCodeForNumber(String number) { return database.with(jdbi -> jdbi.withHandle(handle -> { try (Timer.Context timer = getCodeForNumberTimer.time()) { - return handle.createQuery("SELECT verification_code, timestamp, NULL as push_code FROM pending_devices WHERE number = :number") + return handle.createQuery("SELECT verification_code, timestamp, NULL as push_code, NULL as twilio_verification_sid FROM pending_devices WHERE number = :number") .bind("number", number) .mapTo(StoredVerificationCode.class) .findFirst(); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/mappers/StoredVerificationCodeRowMapper.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/mappers/StoredVerificationCodeRowMapper.java index 9a38c9261..a427a36ec 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/mappers/StoredVerificationCodeRowMapper.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/mappers/StoredVerificationCodeRowMapper.java @@ -17,7 +17,8 @@ public class StoredVerificationCodeRowMapper implements RowMapper + + + + + + + + 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 c9d8c3263..b1dc9ce69 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 @@ -7,19 +7,10 @@ package org.whispersystems.textsecuregcm.tests.controllers; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.Mockito.clearInvocations; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.eq; -import static org.mockito.Mockito.isA; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.reset; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; import com.google.common.collect.ImmutableSet; import io.dropwizard.auth.PolymorphicAuthValueFactoryProvider; @@ -34,6 +25,7 @@ import java.util.Locale; import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.stream.Stream; import javax.ws.rs.client.Entity; @@ -48,6 +40,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials; @@ -76,6 +69,7 @@ import org.whispersystems.textsecuregcm.push.GCMSender; import org.whispersystems.textsecuregcm.push.GcmMessage; import org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient; import org.whispersystems.textsecuregcm.sms.SmsSender; +import org.whispersystems.textsecuregcm.sms.TwilioVerifyExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.AbusiveHostRule; import org.whispersystems.textsecuregcm.storage.AbusiveHostRules; @@ -139,6 +133,9 @@ class AccountControllerTest { private static DynamicConfigurationManager dynamicConfigurationManager = mock(DynamicConfigurationManager.class); + private static TwilioVerifyExperimentEnrollmentManager verifyExperimentEnrollmentManager = mock( + TwilioVerifyExperimentEnrollmentManager.class); + private byte[] registration_lock_key = new byte[32]; private static ExternalServiceCredentialGenerator storageCredentialGenerator = new ExternalServiceCredentialGenerator(new byte[32], new byte[32], false); @@ -163,7 +160,8 @@ class AccountControllerTest { recaptchaClient, gcmSender, apnSender, - storageCredentialGenerator)) + storageCredentialGenerator, + verifyExperimentEnrollmentManager)) .build(); @@ -267,7 +265,8 @@ class AccountControllerTest { recaptchaClient, gcmSender, apnSender, - usernamesManager); + usernamesManager, + verifyExperimentEnrollmentManager); } @Test @@ -328,8 +327,18 @@ class AccountControllerTest { verifyNoMoreInteractions(gcmSender); } - @Test - void testSendCode() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendCode(final boolean enrolledInVerifyExperiment) throws Exception { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + if (enrolledInVerifyExperiment) { + when(smsSender.deliverSmsVerificationWithTwilioVerify(anyString(), any(), anyString(), anyList())) + .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid"))); + } + Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER)) @@ -340,12 +349,35 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(200); - verify(smsSender).deliverSmsVerification(eq(SENDER), eq(Optional.empty()), anyString()); + if (enrolledInVerifyExperiment) { + ArgumentCaptor storedVerificationCodeArgumentCaptor = ArgumentCaptor + .forClass(StoredVerificationCode.class); + + verify(smsSender).deliverSmsVerificationWithTwilioVerify(eq(SENDER), eq(Optional.empty()), anyString(), eq(Collections.emptyList())); + verify(pendingAccountsManager, times(2)).store(eq(SENDER), storedVerificationCodeArgumentCaptor.capture()); + + assertThat(storedVerificationCodeArgumentCaptor.getValue().getTwilioVerificationSid()) + .isEqualTo(Optional.of("VerificationSid")); + + } else { + verify(smsSender).deliverSmsVerification(eq(SENDER), eq(Optional.empty()), anyString()); + } + verifyNoMoreInteractions(smsSender); verify(abusiveHostRules).getAbusiveHostRulesFor(eq(NICE_HOST)); } - @Test - public void testSendCodeVoiceNoLocale() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + public void testSendCodeVoiceNoLocale(final 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)) @@ -356,12 +388,26 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(200); - verify(smsSender).deliverVoxVerification(eq(SENDER), anyString(), eq(Collections.emptyList())); + if (enrolledInVerifyExperiment) { + verify(smsSender).deliverVoxVerificationWithTwilioVerify(eq(SENDER), anyString(), eq(Collections.emptyList())); + } else { + verify(smsSender).deliverVoxVerification(eq(SENDER), anyString(), eq(Collections.emptyList())); + } verify(abusiveHostRules).getAbusiveHostRulesFor(eq(NICE_HOST)); } - @Test - public void testSendCodeVoiceSingleLocale() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + public void testSendCodeVoiceSingleLocale(final 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)) @@ -373,12 +419,27 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(200); - verify(smsSender).deliverVoxVerification(eq(SENDER), anyString(), eq(Locale.LanguageRange.parse("pt-BR"))); + if (enrolledInVerifyExperiment) { + verify(smsSender) + .deliverVoxVerificationWithTwilioVerify(eq(SENDER), anyString(), eq(Locale.LanguageRange.parse("pt-BR"))); + } else { + verify(smsSender).deliverVoxVerification(eq(SENDER), anyString(), eq(Locale.LanguageRange.parse("pt-BR"))); + } verify(abusiveHostRules).getAbusiveHostRulesFor(eq(NICE_HOST)); } - @Test - public void testSendCodeVoiceMultipleLocales() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + public void testSendCodeVoiceMultipleLocales(final 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)) @@ -390,7 +451,13 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(200); - verify(smsSender).deliverVoxVerification(eq(SENDER), anyString(), eq(Locale.LanguageRange.parse("en-US;q=1, ar-US;q=0.9, fa-US;q=0.8, zh-Hans-US;q=0.7, ru-RU;q=0.6, zh-Hant-US;q=0.5"))); + if (enrolledInVerifyExperiment) { + verify(smsSender).deliverVoxVerificationWithTwilioVerify(eq(SENDER), anyString(), eq(Locale.LanguageRange + .parse("en-US;q=1, ar-US;q=0.9, fa-US;q=0.8, zh-Hans-US;q=0.7, ru-RU;q=0.6, zh-Hant-US;q=0.5"))); + } else { + verify(smsSender).deliverVoxVerification(eq(SENDER), anyString(), eq(Locale.LanguageRange + .parse("en-US;q=1, ar-US;q=0.9, fa-US;q=0.8, zh-Hans-US;q=0.7, ru-RU;q=0.6, zh-Hant-US;q=0.5"))); + } verify(abusiveHostRules).getAbusiveHostRulesFor(eq(NICE_HOST)); } @@ -408,11 +475,22 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(400); verify(smsSender, never()).deliverVoxVerification(eq(SENDER), anyString(), any()); + verify(smsSender, never()).deliverVoxVerificationWithTwilioVerify(eq(SENDER), anyString(), any()); verify(abusiveHostRules).getAbusiveHostRulesFor(eq(NICE_HOST)); } - @Test - void testSendCodeWithValidPreauth() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendCodeWithValidPreauth(final boolean enrolledInVerifyExperiment) throws Exception { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + if (enrolledInVerifyExperiment) { + when(smsSender.deliverSmsVerificationWithTwilioVerify(anyString(), any(), anyString(), anyList())) + .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid"))); + } + Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER_PREAUTH)) @@ -423,7 +501,12 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(200); - verify(smsSender).deliverSmsVerification(eq(SENDER_PREAUTH), eq(Optional.empty()), anyString()); + if (enrolledInVerifyExperiment) { + verify(smsSender).deliverSmsVerificationWithTwilioVerify(eq(SENDER_PREAUTH), eq(Optional.empty()), anyString(), + eq(Collections.emptyList())); + } else { + verify(smsSender).deliverSmsVerification(eq(SENDER_PREAUTH), eq(Optional.empty()), anyString()); + } verify(abusiveHostRules).getAbusiveHostRulesFor(eq(NICE_HOST)); } @@ -455,11 +538,22 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(402); verify(smsSender, never()).deliverSmsVerification(eq(SENDER_PREAUTH), eq(Optional.empty()), anyString()); + verify(smsSender, never()).deliverSmsVerificationWithTwilioVerify(eq(SENDER_PREAUTH), eq(Optional.empty()), anyString(), anyList()); } - @Test - void testSendiOSCode() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendiOSCode(final boolean enrolledInVerifyExperiment) throws Exception { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + if (enrolledInVerifyExperiment) { + when(smsSender.deliverSmsVerificationWithTwilioVerify(anyString(), any(), anyString(), anyList())) + .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid"))); + } + Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER)) @@ -471,11 +565,26 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(200); - verify(smsSender).deliverSmsVerification(eq(SENDER), eq(Optional.of("ios")), anyString()); + if (enrolledInVerifyExperiment) { + verify(smsSender).deliverSmsVerificationWithTwilioVerify(eq(SENDER), eq(Optional.of("ios")), anyString(), + eq(Collections.emptyList())); + } else { + verify(smsSender).deliverSmsVerification(eq(SENDER), eq(Optional.of("ios")), anyString()); + } } - @Test - void testSendAndroidNgCode() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendAndroidNgCode(final boolean enrolledInVerifyExperiment) throws Exception { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + if (enrolledInVerifyExperiment) { + when(smsSender.deliverSmsVerificationWithTwilioVerify(anyString(), any(), anyString(), anyList())) + .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid"))); + } + Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER)) @@ -487,11 +596,26 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(200); - verify(smsSender).deliverSmsVerification(eq(SENDER), eq(Optional.of("android-ng")), anyString()); + if (enrolledInVerifyExperiment) { + verify(smsSender).deliverSmsVerificationWithTwilioVerify(eq(SENDER), eq(Optional.of("android-ng")), anyString(), + eq(Collections.emptyList())); + } else { + verify(smsSender).deliverSmsVerification(eq(SENDER), eq(Optional.of("android-ng")), anyString()); + } } - @Test - void testSendAbusiveHost() { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendAbusiveHost(final boolean enrolledInVerifyExperiment) { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + if (enrolledInVerifyExperiment) { + when(smsSender.deliverSmsVerificationWithTwilioVerify(anyString(), any(), anyString(), anyList())) + .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid"))); + } + Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER)) @@ -506,8 +630,18 @@ class AccountControllerTest { verifyNoMoreInteractions(smsSender); } - @Test - void testSendAbusiveHostWithValidCaptcha() throws IOException { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendAbusiveHostWithValidCaptcha(final boolean enrolledInVerifyExperiment) throws IOException { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + if (enrolledInVerifyExperiment) { + when(smsSender.deliverSmsVerificationWithTwilioVerify(anyString(), any(), anyString(), anyList())) + .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid"))); + } + Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER)) @@ -520,11 +654,25 @@ class AccountControllerTest { verifyNoMoreInteractions(abusiveHostRules); verify(recaptchaClient).verify(eq(VALID_CAPTCHA_TOKEN), eq(ABUSIVE_HOST)); - verify(smsSender).deliverSmsVerification(eq(SENDER), eq(Optional.empty()), anyString()); + if (enrolledInVerifyExperiment) { + verify(smsSender).deliverSmsVerificationWithTwilioVerify(eq(SENDER), eq(Optional.empty()), anyString(), + eq(Collections.emptyList())); + } else { + verify(smsSender).deliverSmsVerification(eq(SENDER), eq(Optional.empty()), anyString()); + } } - @Test - void testSendAbusiveHostWithInvalidCaptcha() { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendAbusiveHostWithInvalidCaptcha(final boolean enrolledInVerifyExperiment) { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + if (enrolledInVerifyExperiment) { + when(smsSender.deliverSmsVerificationWithTwilioVerify(anyString(), any(), anyString(), anyList())) + .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid"))); + } Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER)) @@ -540,8 +688,18 @@ class AccountControllerTest { verifyNoMoreInteractions(smsSender); } - @Test - void testSendRateLimitedHostAutoBlock() { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendRateLimitedHostAutoBlock(final boolean enrolledInVerifyExperiment) { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + if (enrolledInVerifyExperiment) { + when(smsSender.deliverSmsVerificationWithTwilioVerify(anyString(), any(), anyString(), anyList())) + .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid"))); + } + Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER)) @@ -560,8 +718,18 @@ class AccountControllerTest { verifyNoMoreInteractions(smsSender); } - @Test - void testSendRateLimitedPrefixAutoBlock() { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendRateLimitedPrefixAutoBlock(final boolean enrolledInVerifyExperiment) { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + if (enrolledInVerifyExperiment) { + when(smsSender.deliverSmsVerificationWithTwilioVerify(anyString(), any(), anyString(), anyList())) + .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid"))); + } + Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER_OVER_PREFIX)) @@ -580,8 +748,18 @@ class AccountControllerTest { verifyNoMoreInteractions(smsSender); } - @Test - void testSendRateLimitedHostNoAutoBlock() { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendRateLimitedHostNoAutoBlock(final boolean enrolledInVerifyExperiment) { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + if (enrolledInVerifyExperiment) { + when(smsSender.deliverSmsVerificationWithTwilioVerify(anyString(), any(), anyString(), anyList())) + .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid"))); + } + Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER)) @@ -600,8 +778,13 @@ class AccountControllerTest { } - @Test - void testSendMultipleHost() { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendMultipleHost(final boolean enrolledInVerifyExperiment) { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER)) @@ -619,8 +802,13 @@ class AccountControllerTest { } - @Test - void testSendRestrictedHostOut() { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendRestrictedHostOut(final boolean enrolledInVerifyExperiment) { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + Response response = resources.getJerseyTest() .target(String.format("/v1/accounts/sms/code/%s", SENDER)) @@ -635,8 +823,18 @@ class AccountControllerTest { verifyNoMoreInteractions(smsSender); } - @Test - void testSendRestrictedIn() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSendRestrictedIn(final boolean enrolledInVerifyExperiment) throws Exception { + + when(verifyExperimentEnrollmentManager.isEnrolled(any(), anyString(), anyList(), anyString())) + .thenReturn(enrolledInVerifyExperiment); + + if (enrolledInVerifyExperiment) { + when(smsSender.deliverSmsVerificationWithTwilioVerify(anyString(), any(), anyString(), anyList())) + .thenReturn(CompletableFuture.completedFuture(Optional.of("VerificationSid"))); + } + final String number = "+12345678901"; final String challenge = "challenge"; @@ -652,11 +850,24 @@ class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(200); - verify(smsSender).deliverSmsVerification(eq(number), eq(Optional.empty()), anyString()); + if (enrolledInVerifyExperiment) { + verify(smsSender).deliverSmsVerificationWithTwilioVerify(eq(number), eq(Optional.empty()), anyString(), + eq(Collections.emptyList())); + } else { + verify(smsSender).deliverSmsVerification(eq(number), eq(Optional.empty()), anyString()); + } + + verifyNoMoreInteractions(smsSender); } - @Test - void testVerifyCode() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testVerifyCode(final boolean enrolledInVerifyExperiment) throws Exception { + if (enrolledInVerifyExperiment) { + when(pendingAccountsManager.getCodeForNumber(SENDER)).thenReturn( + Optional.of(new StoredVerificationCode("1234", System.currentTimeMillis(), "1234-push", "VerificationSid")));; + } + AccountCreationResult result = resources.getJerseyTest() .target(String.format("/v1/accounts/code/%s", "1234")) @@ -674,6 +885,10 @@ class AccountControllerTest { verify(directoryQueue, times(1)).refreshRegisteredUser(argThat(account -> SENDER.equals(account.getNumber()))); assertThat(accountArgumentCaptor.getValue().isDiscoverableByPhoneNumber()).isTrue(); + + if (enrolledInVerifyExperiment) { + verify(smsSender).reportVerificationSucceeded("VerificationSid"); + } } @Test diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingAccountsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingAccountsTest.java index c36b9cc3e..2cf30ddcc 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingAccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingAccountsTest.java @@ -75,6 +75,28 @@ class PendingAccountsTest { assertThat(resultSet.next()).isFalse(); } + @Test + void testStoreWithTwilioVerificationSid() throws SQLException { + pendingAccounts.insert("+14151112222", null, 1111, null, "id1"); + + PreparedStatement statement = db.getTestDatabase().getConnection() + .prepareStatement("SELECT * FROM pending_accounts WHERE number = ?"); + statement.setString(1, "+14151112222"); + + ResultSet resultSet = statement.executeQuery(); + + if (resultSet.next()) { + assertThat(resultSet.getString("verification_code")).isNull(); + assertThat(resultSet.getLong("timestamp")).isEqualTo(1111); + assertThat(resultSet.getString("push_code")).isNull(); + assertThat(resultSet.getString("twilio_verification_sid")).isEqualTo("id1"); + } else { + throw new AssertionError("no results"); + } + + assertThat(resultSet.next()).isFalse(); + } + @Test void testRetrieve() throws Exception { pendingAccounts.insert("+14151112222", "4321", 2222, null); @@ -106,6 +128,23 @@ class PendingAccountsTest { assertThat(missingCode.isPresent()).isFalse(); } + @Test + void testRetrieveWithTwilioVerificationSid() throws Exception { + pendingAccounts.insert("+14151112222", "4321", 2222, "bar", "id1"); + pendingAccounts.insert("+14151113333", "1212", 5555, "bang", "id2"); + + Optional verificationCode = pendingAccounts.getCodeForNumber("+14151112222"); + + assertThat(verificationCode).isPresent(); + assertThat(verificationCode.get().getCode()).isEqualTo("4321"); + assertThat(verificationCode.get().getTimestamp()).isEqualTo(2222); + assertThat(verificationCode.get().getPushCode()).isEqualTo("bar"); + assertThat(verificationCode.get().getTwilioVerificationSid()).contains("id1"); + + Optional missingCode = pendingAccounts.getCodeForNumber("+11111111111"); + assertThat(missingCode).isNotPresent(); + } + @Test void testOverwrite() throws Exception { pendingAccounts.insert("+14151112222", "4321", 2222, null); @@ -131,6 +170,19 @@ class PendingAccountsTest { assertThat(verificationCode.get().getPushCode()).isEqualTo("bang"); } + @Test + void testOverwriteWithTwilioVerificationSid() throws Exception { + pendingAccounts.insert("+14151112222", "4321", 2222, "bar", "id1"); + pendingAccounts.insert("+14151112222", "4444", 3333, "bang", "id2"); + + Optional verificationCode = pendingAccounts.getCodeForNumber("+14151112222"); + + assertThat(verificationCode.isPresent()).isTrue(); + assertThat(verificationCode.get().getCode()).isEqualTo("4444"); + assertThat(verificationCode.get().getTimestamp()).isEqualTo(3333); + assertThat(verificationCode.get().getPushCode()).isEqualTo("bang"); + assertThat(verificationCode.get().getTwilioVerificationSid()).contains("id2"); + } @Test void testVacuum() { @@ -167,6 +219,4 @@ class PendingAccountsTest { assertThat(verificationCode.get().getTimestamp()).isEqualTo(5555); assertThat(verificationCode.get().getPushCode()).isNull(); } - - }