Stop checking for stored verification codes when linking devices

service/config/sample.yml

@@ -112,8 +112,6 @@ dynamoDbTables:
   messages:
     tableName: Example_Messages
     expiration: P30D # Duration of time until rows expire
-  pendingDevices:
-    tableName: Example_PendingDevices
   phoneNumberIdentifiers:
     tableName: Example_PhoneNumberIdentifiers
   profiles:

service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java

@@ -115,14 +115,14 @@ import org.whispersystems.textsecuregcm.controllers.VerificationController;
 import org.whispersystems.textsecuregcm.currency.CoinMarketCapClient;
 import org.whispersystems.textsecuregcm.currency.CurrencyConversionManager;
 import org.whispersystems.textsecuregcm.currency.FixerClient;
-import org.whispersystems.textsecuregcm.grpc.GrpcServerManagedWrapper;
-import org.whispersystems.textsecuregcm.grpc.UserAgentInterceptor;
 import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager;
 import org.whispersystems.textsecuregcm.filters.RemoteDeprecationFilter;
 import org.whispersystems.textsecuregcm.filters.RequestStatisticsFilter;
 import org.whispersystems.textsecuregcm.filters.TimestampResponseFilter;
-import org.whispersystems.textsecuregcm.grpc.KeysGrpcService;
+import org.whispersystems.textsecuregcm.grpc.GrpcServerManagedWrapper;
 import org.whispersystems.textsecuregcm.grpc.KeysAnonymousGrpcService;
+import org.whispersystems.textsecuregcm.grpc.KeysGrpcService;
+import org.whispersystems.textsecuregcm.grpc.UserAgentInterceptor;
 import org.whispersystems.textsecuregcm.limits.CardinalityEstimator;
 import org.whispersystems.textsecuregcm.limits.PushChallengeManager;
 import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager;
@@ -189,9 +189,7 @@ import org.whispersystems.textsecuregcm.storage.RemoteConfigs;
 import org.whispersystems.textsecuregcm.storage.RemoteConfigsManager;
 import org.whispersystems.textsecuregcm.storage.ReportMessageDynamoDb;
 import org.whispersystems.textsecuregcm.storage.ReportMessageManager;
-import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager;
 import org.whispersystems.textsecuregcm.storage.SubscriptionManager;
-import org.whispersystems.textsecuregcm.storage.VerificationCodeStore;
 import org.whispersystems.textsecuregcm.storage.VerificationSessionManager;
 import org.whispersystems.textsecuregcm.storage.VerificationSessions;
 import org.whispersystems.textsecuregcm.subscriptions.BraintreeManager;
@@ -352,8 +350,6 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
     ReportMessageDynamoDb reportMessageDynamoDb = new ReportMessageDynamoDb(dynamoDbClient,
         config.getDynamoDbTables().getReportMessage().getTableName(),
         config.getReportMessageConfiguration().getReportTtl());
-    VerificationCodeStore pendingDevices = new VerificationCodeStore(dynamoDbClient,
-        config.getDynamoDbTables().getPendingDevices().getTableName());
     RegistrationRecoveryPasswords registrationRecoveryPasswords = new RegistrationRecoveryPasswords(
         config.getDynamoDbTables().getRegistrationRecovery().getTableName(),
         config.getDynamoDbTables().getRegistrationRecovery().getExpiration(),
@@ -507,7 +503,6 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
         storageServiceExecutor, storageServiceRetryExecutor, config.getSecureStorageServiceConfiguration());
     ClientPresenceManager clientPresenceManager = new ClientPresenceManager(clientPresenceCluster, recurringJobExecutor,
         keyspaceNotificationDispatchExecutor);
-    StoredVerificationCodeManager pendingDevicesManager = new StoredVerificationCodeManager(pendingDevices);
     ProfilesManager profilesManager = new ProfilesManager(profiles, cacheCluster);
     MessagesCache messagesCache = new MessagesCache(messagesCluster, messagesCluster,
         keyspaceNotificationDispatchExecutor, messageDeliveryScheduler, messageDeletionAsyncExecutor, clock);
@@ -756,7 +751,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
         new CallLinkController(rateLimiters, genericZkSecretParams),
         new CertificateController(new CertificateGenerator(config.getDeliveryCertificate().certificate().value(), config.getDeliveryCertificate().ecPrivateKey(), config.getDeliveryCertificate().expiresDays()), zkAuthOperations, genericZkSecretParams, clock),
         new ChallengeController(rateLimitChallengeManager),
-        new DeviceController(pendingDevicesManager, config.getLinkDeviceSecretConfiguration().secret().value(), accountsManager, messagesManager, keys, rateLimiters,
+        new DeviceController(config.getLinkDeviceSecretConfiguration().secret().value(), accountsManager, messagesManager, keys, rateLimiters,
             rateLimitersCluster, config.getMaxDevices(), clock),
         new DirectoryV2Controller(directoryV2CredentialsGenerator),
         new DonationController(clock, zkReceiptOperations, redeemedReceiptsManager, accountsManager, config.getBadges(),

service/src/main/java/org/whispersystems/textsecuregcm/configuration/DynamoDbTables.java

@@ -56,7 +56,6 @@ public class DynamoDbTables {
   private final Table kemKeys;
   private final Table kemLastResortKeys;
   private final TableWithExpiration messages;
-  private final Table pendingDevices;
   private final Table phoneNumberIdentifiers;
   private final Table profiles;
   private final Table pushChallenge;
@@ -78,7 +77,6 @@ public class DynamoDbTables {
       @JsonProperty("pqKeys") final Table kemKeys,
       @JsonProperty("pqLastResortKeys") final Table kemLastResortKeys,
       @JsonProperty("messages") final TableWithExpiration messages,
-      @JsonProperty("pendingDevices") final Table pendingDevices,
       @JsonProperty("phoneNumberIdentifiers") final Table phoneNumberIdentifiers,
       @JsonProperty("profiles") final Table profiles,
       @JsonProperty("pushChallenge") final Table pushChallenge,
@@ -99,7 +97,6 @@ public class DynamoDbTables {
     this.kemKeys = kemKeys;
     this.kemLastResortKeys = kemLastResortKeys;
     this.messages = messages;
-    this.pendingDevices = pendingDevices;
     this.phoneNumberIdentifiers = phoneNumberIdentifiers;
     this.profiles = profiles;
     this.pushChallenge = pushChallenge;
@@ -171,12 +168,6 @@ public class DynamoDbTables {
     return messages;
   }
 
-  @NotNull
-  @Valid
-  public Table getPendingDevices() {
-    return pendingDevices;
-  }
-
   @NotNull
   @Valid
   public Table getPhoneNumberIdentifiers() {

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

@@ -66,7 +66,6 @@ import org.whispersystems.textsecuregcm.storage.Device;
 import org.whispersystems.textsecuregcm.storage.Device.DeviceCapabilities;
 import org.whispersystems.textsecuregcm.storage.KeysManager;
 import org.whispersystems.textsecuregcm.storage.MessagesManager;
-import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager;
 import org.whispersystems.textsecuregcm.util.Pair;
 import org.whispersystems.textsecuregcm.util.Util;
 import org.whispersystems.textsecuregcm.util.VerificationCode;
@@ -77,7 +76,6 @@ public class DeviceController {
 
   static final int MAX_DEVICES = 6;
 
-  private final StoredVerificationCodeManager pendingDevices;
   private final Key verificationTokenKey;
   private final AccountsManager       accounts;
   private final MessagesManager       messages;
@@ -93,15 +91,13 @@ public class DeviceController {
   @VisibleForTesting
   static final Duration TOKEN_EXPIRATION_DURATION = Duration.ofMinutes(10);
 
-  public DeviceController(StoredVerificationCodeManager pendingDevices,
+  public DeviceController(byte[] linkDeviceSecret,
-      byte[] linkDeviceSecret,
       AccountsManager accounts,
       MessagesManager messages,
       KeysManager keys,
       RateLimiters rateLimiters,
       FaultTolerantRedisCluster usedTokenCluster,
       Map<String, Integer> maxDeviceConfiguration, final Clock clock) {
-    this.pendingDevices = pendingDevices;
     this.verificationTokenKey = new SecretKeySpec(linkDeviceSecret, VERIFICATION_TOKEN_ALGORITHM);
     this.accounts = accounts;
     this.messages = messages;
@@ -202,8 +198,7 @@ public class DeviceController {
       @Context ContainerRequest containerRequest)
       throws RateLimitExceededException, DeviceLimitExceededException {
 
-    final Pair<Account, Device> accountAndDevice = createDevice(authorizationHeader.getUsername(),
+    final Pair<Account, Device> accountAndDevice = createDevice(authorizationHeader.getPassword(),
-        authorizationHeader.getPassword(),
         verificationCode,
         accountAttributes,
         containerRequest,
@@ -237,8 +232,7 @@ public class DeviceController {
                                    @Context ContainerRequest containerRequest)
       throws RateLimitExceededException, DeviceLimitExceededException {
 
-    final Pair<Account, Device> accountAndDevice = createDevice(authorizationHeader.getUsername(),
+    final Pair<Account, Device> accountAndDevice = createDevice(authorizationHeader.getPassword(),
-        authorizationHeader.getPassword(),
         linkDeviceRequest.verificationCode(),
         linkDeviceRequest.accountAttributes(),
         containerRequest,
@@ -362,28 +356,20 @@ public class DeviceController {
     return isDowngrade;
   }
 
-  private Pair<Account, Device> createDevice(final String phoneNumber,
+  private Pair<Account, Device> createDevice(final String password,
-                                     final String password,
                                      final String verificationCode,
                                      final AccountAttributes accountAttributes,
                                      final ContainerRequest containerRequest,
                                      final Optional<DeviceActivationRequest> maybeDeviceActivationRequest)
       throws RateLimitExceededException, DeviceLimitExceededException {
 
-    rateLimiters.getVerifyDeviceLimiter().validate(phoneNumber);
-
     final Optional<UUID> maybeAciFromToken = checkVerificationToken(verificationCode);
 
     final Account account = maybeAciFromToken.flatMap(accounts::getByAccountIdentifier)
-        .or(() -> {
-          final boolean verificationCodeValid = pendingDevices.getCodeForNumber(phoneNumber)
-              .map(storedVerificationCode -> storedVerificationCode.isValid(verificationCode))
-              .orElse(false);
-
-          return verificationCodeValid ? accounts.getByE164(phoneNumber) : Optional.empty();
-        })
         .orElseThrow(ForbiddenException::new);
 
+    rateLimiters.getVerifyDeviceLimiter().validate(account.getUuid());
+
     maybeDeviceActivationRequest.ifPresent(deviceActivationRequest -> {
       assert deviceActivationRequest.aciSignedPreKey().isPresent();
       assert deviceActivationRequest.pniSignedPreKey().isPresent();
@@ -468,8 +454,6 @@ public class DeviceController {
       a.addDevice(device);
     });
 
-    pendingDevices.remove(phoneNumber);
-
     if (maybeAciFromToken.isPresent()) {
       usedTokenCluster.useCluster(connection ->
           connection.sync().set(getUsedTokenKey(verificationCode), "", new SetArgs().ex(TOKEN_EXPIRATION_DURATION)));

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

@@ -25,7 +25,6 @@ import org.signal.libsignal.protocol.ecc.Curve;
 import org.signal.libsignal.protocol.ecc.ECKeyPair;
 import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount;
 import org.whispersystems.textsecuregcm.auth.DisabledPermittedAuthenticatedAccount;
-import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
 import org.whispersystems.textsecuregcm.auth.WebsocketRefreshApplicationEventListener;
 import org.whispersystems.textsecuregcm.entities.*;
 import org.whispersystems.textsecuregcm.limits.RateLimiter;
@@ -34,6 +33,8 @@ import org.whispersystems.textsecuregcm.mappers.DeviceLimitExceededExceptionMapp
 import org.whispersystems.textsecuregcm.push.ClientPresenceManager;
 import org.whispersystems.textsecuregcm.storage.*;
 import org.whispersystems.textsecuregcm.storage.Device.DeviceCapabilities;
+import org.whispersystems.textsecuregcm.storage.KeysManager;
+import org.whispersystems.textsecuregcm.storage.MessagesManager;
 import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
 import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
 import org.whispersystems.textsecuregcm.tests.util.KeysHelper;
@@ -61,7 +62,6 @@ import static org.mockito.Mockito.*;
 @ExtendWith(DropwizardExtensionsSupport.class)
 class DeviceControllerTest {
 
-  private static StoredVerificationCodeManager pendingDevicesManager = mock(StoredVerificationCodeManager.class);
   private static AccountsManager accountsManager = mock(AccountsManager.class);
   private static MessagesManager messagesManager = mock(MessagesManager.class);
   private static KeysManager keysManager = mock(KeysManager.class);
@@ -75,7 +75,7 @@ class DeviceControllerTest {
   private static Map<String, Integer> deviceConfiguration = new HashMap<>();
   private static TestClock testClock = TestClock.now();
 
-  private static DeviceController deviceController = new DeviceController(pendingDevicesManager,
+  private static DeviceController deviceController = new DeviceController(
       generateLinkDeviceSecret(),
       accountsManager,
       messagesManager,
@@ -125,7 +125,6 @@ class DeviceControllerTest {
     when(account.isGiftBadgesSupported()).thenReturn(true);
     when(account.isPaymentActivationSupported()).thenReturn(false);
 
-    when(pendingDevicesManager.getCodeForNumber(any())).thenReturn(Optional.empty());
     when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(account));
     when(accountsManager.getByE164(AuthHelper.VALID_NUMBER)).thenReturn(Optional.of(account));
     when(accountsManager.getByE164(AuthHelper.VALID_NUMBER_TWO)).thenReturn(Optional.of(maxedAccount));
@@ -139,7 +138,6 @@ class DeviceControllerTest {
   @AfterEach
   void teardown() {
     reset(
-        pendingDevicesManager,
         accountsManager,
         messagesManager,
         keysManager,
@@ -179,36 +177,10 @@ class DeviceControllerTest {
 
     assertThat(response.getDeviceId()).isEqualTo(42L);
 
-    verify(pendingDevicesManager).remove(AuthHelper.VALID_NUMBER);
     verify(messagesManager).clear(eq(AuthHelper.VALID_UUID), eq(42L));
     verify(commands).set(anyString(), anyString(), any());
   }
 
-  @Test
-  void validDeviceRegisterTestStoredCode() {
-    final Device existingDevice = mock(Device.class);
-    when(existingDevice.getId()).thenReturn(Device.MASTER_ID);
-    when(account.getDevices()).thenReturn(List.of(existingDevice));
-
-    final String storedCode = "5678901";
-
-    when(pendingDevicesManager.getCodeForNumber(AuthHelper.VALID_NUMBER)).thenReturn(
-        Optional.of(new StoredVerificationCode(storedCode, System.currentTimeMillis(), null, null)));
-
-    final DeviceResponse response = resources.getJerseyTest()
-        .target("/v1/devices/" + storedCode)
-        .request()
-        .header("Authorization", AuthHelper.getProvisioningAuthHeader(AuthHelper.VALID_NUMBER, "password1"))
-        .put(Entity.entity(new AccountAttributes(false, 1234, null,
-                    null, true, null),
-                MediaType.APPLICATION_JSON_TYPE),
-            DeviceResponse.class);
-
-    assertThat(response.getDeviceId()).isEqualTo(42L);
-
-    verify(commands, never()).set(anyString(), anyString(), any());
-  }
-
   @Test
   void validDeviceRegisterTestSignedTokenUsed() {
     when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(account));
@@ -339,7 +311,6 @@ class DeviceControllerTest {
     expectedGcmToken.ifPresentOrElse(expectedToken -> assertEquals(expectedToken, device.getGcmId()),
         () -> assertNull(device.getGcmId()));
 
-    verify(pendingDevicesManager).remove(AuthHelper.VALID_NUMBER);
     verify(messagesManager).clear(eq(AuthHelper.VALID_UUID), eq(42L));
     verify(keysManager).storeEcSignedPreKeys(AuthHelper.VALID_UUID, Map.of(response.getDeviceId(), aciSignedPreKey.get()));
     verify(keysManager).storeEcSignedPreKeys(AuthHelper.VALID_PNI, Map.of(response.getDeviceId(), pniSignedPreKey.get()));
@@ -349,61 +320,6 @@ class DeviceControllerTest {
   }
 
 
-  @ParameterizedTest
-  @MethodSource("linkDeviceAtomic")
-  @SuppressWarnings("OptionalUsedAsFieldOrParameterType")
-  void linkDeviceAtomicWithStoredCode(final boolean fetchesMessages,
-      final Optional<ApnRegistrationId> apnRegistrationId,
-      final Optional<GcmRegistrationId> gcmRegistrationId,
-      final Optional<String> expectedApnsToken,
-      final Optional<String> expectedApnsVoipToken,
-      final Optional<String> expectedGcmToken) {
-
-    when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(account));
-
-    final Device existingDevice = mock(Device.class);
-    when(existingDevice.getId()).thenReturn(Device.MASTER_ID);
-    when(AuthHelper.VALID_ACCOUNT.getDevices()).thenReturn(List.of(existingDevice));
-
-    final Optional<ECSignedPreKey> aciSignedPreKey;
-    final Optional<ECSignedPreKey> pniSignedPreKey;
-    final Optional<KEMSignedPreKey> aciPqLastResortPreKey;
-    final Optional<KEMSignedPreKey> pniPqLastResortPreKey;
-
-    final ECKeyPair aciIdentityKeyPair = Curve.generateKeyPair();
-    final ECKeyPair pniIdentityKeyPair = Curve.generateKeyPair();
-
-    aciSignedPreKey = Optional.of(KeysHelper.signedECPreKey(1, aciIdentityKeyPair));
-    pniSignedPreKey = Optional.of(KeysHelper.signedECPreKey(2, pniIdentityKeyPair));
-    aciPqLastResortPreKey = Optional.of(KeysHelper.signedKEMPreKey(3, aciIdentityKeyPair));
-    pniPqLastResortPreKey = Optional.of(KeysHelper.signedKEMPreKey(4, pniIdentityKeyPair));
-
-    when(account.getIdentityKey()).thenReturn(new IdentityKey(aciIdentityKeyPair.getPublicKey()));
-    when(account.getPhoneNumberIdentityKey()).thenReturn(new IdentityKey(pniIdentityKeyPair.getPublicKey()));
-
-    when(keysManager.storeEcSignedPreKeys(any(), any())).thenReturn(CompletableFuture.completedFuture(null));
-    when(keysManager.storePqLastResort(any(), any())).thenReturn(CompletableFuture.completedFuture(null));
-
-    final String storedCode = "5678901";
-
-    when(pendingDevicesManager.getCodeForNumber(AuthHelper.VALID_NUMBER)).thenReturn(
-        Optional.of(new StoredVerificationCode(storedCode, System.currentTimeMillis(), null, null)));
-
-    final LinkDeviceRequest request = new LinkDeviceRequest(storedCode,
-        new AccountAttributes(fetchesMessages, 1234, null, null, true, null),
-        new DeviceActivationRequest(aciSignedPreKey, pniSignedPreKey, aciPqLastResortPreKey, pniPqLastResortPreKey, apnRegistrationId, gcmRegistrationId));
-
-    final DeviceResponse response = resources.getJerseyTest()
-        .target("/v1/devices/link")
-        .request()
-        .header("Authorization", AuthHelper.getProvisioningAuthHeader(AuthHelper.VALID_NUMBER, "password1"))
-        .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE), DeviceResponse.class);
-
-    assertThat(response.getDeviceId()).isEqualTo(42L);
-
-    verify(commands, never()).set(anyString(), anyString(), any());
-  }
-
   private static Stream<Arguments> linkDeviceAtomic() {
     final String apnsToken = "apns-token";
     final String apnsVoipToken = "apns-voip-token";