Retire verification code storage machinery

service/src/main/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCode.java

@@ -1,30 +0,0 @@
-/*
- * Copyright 2013-2020 Signal Messenger, LLC
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-
-package org.whispersystems.textsecuregcm.auth;
-
-import java.security.MessageDigest;
-import java.time.Duration;
-import javax.annotation.Nullable;
-import org.whispersystems.textsecuregcm.util.Util;
-
-public record StoredVerificationCode(@Nullable String code,
-                                     long timestamp,
-                                     @Nullable String pushCode,
-                                     @Nullable byte[] sessionId) {
-
-  public static final Duration EXPIRATION = Duration.ofMinutes(10);
-
-  public boolean isValid(String theirCodeString) {
-    if (Util.isEmpty(code) || Util.isEmpty(theirCodeString)) {
-      return false;
-    }
-
-    byte[] ourCode = code.getBytes();
-    byte[] theirCode = theirCodeString.getBytes();
-
-    return MessageDigest.isEqual(ourCode, theirCode);
-  }
-}

service/src/main/java/org/whispersystems/textsecuregcm/storage/SerializedExpireableJsonDynamoStore.java

@@ -45,7 +45,7 @@ public abstract class SerializedExpireableJsonDynamoStore<T> {
   private static final String ATTR_SERIALIZED_VALUE = "V";
   private static final String ATTR_TTL = "E";
 
-  private static final Logger log = LoggerFactory.getLogger(VerificationCodeStore.class);
+  private final Logger log = LoggerFactory.getLogger(getClass());
 
   public SerializedExpireableJsonDynamoStore(final DynamoDbAsyncClient dynamoDbClient, final String tableName,
       final Clock clock) {

service/src/main/java/org/whispersystems/textsecuregcm/storage/StoredVerificationCodeManager.java

@@ -1,29 +0,0 @@
-/*
- * Copyright 2013-2020 Signal Messenger, LLC
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-package org.whispersystems.textsecuregcm.storage;
-
-import java.util.Optional;
-import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
-
-public class StoredVerificationCodeManager {
-
-  private final VerificationCodeStore verificationCodeStore;
-
-  public StoredVerificationCodeManager(final VerificationCodeStore verificationCodeStore) {
-    this.verificationCodeStore = verificationCodeStore;
-  }
-
-  public void store(String number, StoredVerificationCode code) {
-    verificationCodeStore.insert(number, code);
-  }
-
-  public void remove(String number) {
-    verificationCodeStore.remove(number);
-  }
-
-  public Optional<StoredVerificationCode> getCodeForNumber(String number) {
-    return verificationCodeStore.findForNumber(number);
-  }
-}

service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStore.java

@@ -1,114 +0,0 @@
-/*
- * Copyright 2013 Signal Messenger, LLC
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-
-package org.whispersystems.textsecuregcm.storage;
-
-import static com.codahale.metrics.MetricRegistry.name;
-
-import com.fasterxml.jackson.core.JsonProcessingException;
-import com.google.common.annotations.VisibleForTesting;
-import io.micrometer.core.instrument.Metrics;
-import io.micrometer.core.instrument.Timer;
-import java.time.Instant;
-import java.util.Map;
-import java.util.Optional;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
-import org.whispersystems.textsecuregcm.util.AttributeValues;
-import org.whispersystems.textsecuregcm.util.SystemMapper;
-import software.amazon.awssdk.services.dynamodb.DynamoDbClient;
-import software.amazon.awssdk.services.dynamodb.model.DeleteItemRequest;
-import software.amazon.awssdk.services.dynamodb.model.GetItemRequest;
-import software.amazon.awssdk.services.dynamodb.model.GetItemResponse;
-import software.amazon.awssdk.services.dynamodb.model.PutItemRequest;
-
-public class VerificationCodeStore {
-
-  private final DynamoDbClient dynamoDbClient;
-  private final String tableName;
-
-  private final Timer insertTimer;
-  private final Timer getTimer;
-  private final Timer removeTimer;
-
-  @VisibleForTesting
-  static final String KEY_E164 = "P";
-
-  private static final String ATTR_STORED_CODE = "C";
-  private static final String ATTR_TTL = "E";
-
-  private static final Logger log = LoggerFactory.getLogger(VerificationCodeStore.class);
-
-  public VerificationCodeStore(final DynamoDbClient dynamoDbClient, final String tableName) {
-    this.dynamoDbClient = dynamoDbClient;
-    this.tableName = tableName;
-
-    this.insertTimer = Metrics.timer(name(getClass(), "insert"), "table", tableName);
-    this.getTimer = Metrics.timer(name(getClass(), "get"), "table", tableName);
-    this.removeTimer = Metrics.timer(name(getClass(), "remove"), "table", tableName);
-  }
-
-  public void insert(final String number, final StoredVerificationCode verificationCode) {
-    insertTimer.record(() -> {
-      try {
-        dynamoDbClient.putItem(PutItemRequest.builder()
-            .tableName(tableName)
-            .item(Map.of(
-                KEY_E164, AttributeValues.fromString(number),
-                ATTR_STORED_CODE, AttributeValues.fromString(SystemMapper.jsonMapper().writeValueAsString(verificationCode)),
-                ATTR_TTL, AttributeValues.fromLong(getExpirationTimestamp(verificationCode))))
-            .build());
-      } catch (final JsonProcessingException e) {
-        // This should never happen when writing directly to a string except in cases of serious misconfiguration, which
-        // would be caught by tests.
-        throw new AssertionError(e);
-      }
-    });
-  }
-
-  private long getExpirationTimestamp(final StoredVerificationCode storedVerificationCode) {
-    return Instant.ofEpochMilli(storedVerificationCode.timestamp()).plus(StoredVerificationCode.EXPIRATION).getEpochSecond();
-  }
-
-  public Optional<StoredVerificationCode> findForNumber(final String number) {
-    return getTimer.record(() -> {
-      final GetItemResponse response = dynamoDbClient.getItem(GetItemRequest.builder()
-          .tableName(tableName)
-          .consistentRead(true)
-          .key(Map.of(KEY_E164, AttributeValues.fromString(number)))
-          .build());
-
-      try {
-        return response.hasItem()
-            ? filterMaybeExpiredCode(
-            SystemMapper.jsonMapper().readValue(response.item().get(ATTR_STORED_CODE).s(), StoredVerificationCode.class))
-            : Optional.empty();
-      } catch (final JsonProcessingException e) {
-        log.error("Failed to parse stored verification code", e);
-        return Optional.empty();
-      }
-    });
-  }
-
-  private Optional<StoredVerificationCode> filterMaybeExpiredCode(StoredVerificationCode storedVerificationCode) {
-    // It's possible for DynamoDB to return items after their expiration time (although it is very unlikely for small
-    // tables)
-    if (getExpirationTimestamp(storedVerificationCode) < Instant.now().getEpochSecond()) {
-      return Optional.empty();
-    }
-
-    return Optional.of(storedVerificationCode);
-  }
-
-  public void remove(final String number) {
-    removeTimer.record(() -> {
-      dynamoDbClient.deleteItem(DeleteItemRequest.builder()
-          .tableName(tableName)
-          .key(Map.of(KEY_E164, AttributeValues.fromString(number)))
-          .build());
-    });
-  }
-}

service/src/test/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCodeTest.java

@@ -1,30 +0,0 @@
-/*
- * Copyright 2013-2021 Signal Messenger, LLC
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-
-package org.whispersystems.textsecuregcm.auth;
-
-import static org.junit.jupiter.api.Assertions.assertEquals;
-
-import java.util.stream.Stream;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.Arguments;
-import org.junit.jupiter.params.provider.MethodSource;
-
-class StoredVerificationCodeTest {
-
-  @ParameterizedTest
-  @MethodSource
-  void isValid(final StoredVerificationCode storedVerificationCode, final String code, final boolean expectValid) {
-    assertEquals(expectValid, storedVerificationCode.isValid(code));
-  }
-
-  private static Stream<Arguments> isValid() {
-    return Stream.of(
-        Arguments.of(new StoredVerificationCode("code", System.currentTimeMillis(), null, null), "code", true),
-        Arguments.of(new StoredVerificationCode("code", System.currentTimeMillis(), null, null), "incorrect", false),
-        Arguments.of(new StoredVerificationCode("", System.currentTimeMillis(), null, null), "", false)
-    );
-  }
-}

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

@@ -57,7 +57,6 @@ import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount;
 import org.whispersystems.textsecuregcm.auth.DisabledPermittedAuthenticatedAccount;
 import org.whispersystems.textsecuregcm.auth.SaltedTokenHash;
 import org.whispersystems.textsecuregcm.auth.StoredRegistrationLock;
-import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
 import org.whispersystems.textsecuregcm.auth.TurnTokenGenerator;
 import org.whispersystems.textsecuregcm.entities.AccountAttributes;
 import org.whispersystems.textsecuregcm.entities.AccountIdentifierResponse;
@@ -83,7 +82,6 @@ import org.whispersystems.textsecuregcm.spam.ScoreThresholdProvider;
 import org.whispersystems.textsecuregcm.storage.Account;
 import org.whispersystems.textsecuregcm.storage.AccountsManager;
 import org.whispersystems.textsecuregcm.storage.RegistrationRecoveryPasswordsManager;
-import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager;
 import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableException;
 import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundException;
 import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
@@ -124,7 +122,6 @@ class AccountControllerTest {
   private static final String NICE_HOST                = "127.0.0.1";
   private static final String RATE_LIMITED_IP_HOST     = "10.0.0.1";
 
-  private static StoredVerificationCodeManager pendingAccountsManager = mock(StoredVerificationCodeManager.class);
   private static AccountsManager accountsManager = mock(AccountsManager.class);
   private static RateLimiters rateLimiters = mock(RateLimiters.class);
   private static RateLimiter rateLimiter = mock(RateLimiter.class);
@@ -204,24 +201,6 @@ class AccountControllerTest {
     when(senderTransfer.getUuid()).thenReturn(SENDER_TRANSFER_UUID);
     when(senderTransfer.getNumber()).thenReturn(SENDER_TRANSFER);
 
-    when(pendingAccountsManager.getCodeForNumber(SENDER)).thenReturn(
-        Optional.of(new StoredVerificationCode(null, System.currentTimeMillis(), "1234-push", null)));
-    when(pendingAccountsManager.getCodeForNumber(SENDER_OLD)).thenReturn(Optional.empty());
-    when(pendingAccountsManager.getCodeForNumber(SENDER_PIN)).thenReturn(
-        Optional.of(new StoredVerificationCode(null, System.currentTimeMillis(), null, null)));
-    when(pendingAccountsManager.getCodeForNumber(SENDER_REG_LOCK)).thenReturn(
-        Optional.of(new StoredVerificationCode(null, System.currentTimeMillis(), null, null)));
-    when(pendingAccountsManager.getCodeForNumber(SENDER_OVER_PIN)).thenReturn(
-        Optional.of(new StoredVerificationCode(null, System.currentTimeMillis(), null, null)));
-    when(pendingAccountsManager.getCodeForNumber(SENDER_OVER_PREFIX)).thenReturn(
-        Optional.of(new StoredVerificationCode(null, System.currentTimeMillis(), "1234-push", null)));
-    when(pendingAccountsManager.getCodeForNumber(SENDER_PREAUTH)).thenReturn(
-        Optional.of(new StoredVerificationCode(null, System.currentTimeMillis(), "validchallenge", null)));
-    when(pendingAccountsManager.getCodeForNumber(SENDER_HAS_STORAGE)).thenReturn(
-        Optional.of(new StoredVerificationCode(null, System.currentTimeMillis(), null, null)));
-    when(pendingAccountsManager.getCodeForNumber(SENDER_TRANSFER)).thenReturn(
-        Optional.of(new StoredVerificationCode(null, System.currentTimeMillis(), null, null)));
-
     when(accountsManager.getByE164(eq(SENDER_PIN))).thenReturn(Optional.of(senderPinAccount));
     when(accountsManager.getByE164(eq(SENDER_REG_LOCK))).thenReturn(Optional.of(senderRegLockAccount));
     when(accountsManager.getByE164(eq(SENDER_OVER_PIN))).thenReturn(Optional.of(senderPinAccount));
@@ -244,7 +223,6 @@ class AccountControllerTest {
   @AfterEach
   void teardown() {
     reset(
-        pendingAccountsManager,
         accountsManager,
         rateLimiters,
         rateLimiter,

service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamoDbExtensionSchema.java

@@ -316,15 +316,6 @@ public final class DynamoDbExtensionSchema {
             .build()),
         List.of(), List.of()),
 
-    VERIFICATION_CODES("verification_codes_test",
-        VerificationCodeStore.KEY_E164,
-        null,
-        List.of(AttributeDefinition.builder()
-            .attributeName(VerificationCodeStore.KEY_E164)
-            .attributeType(ScalarAttributeType.S)
-            .build()),
-        List.of(), List.of()),
-    
     VERIFICATION_SESSIONS("verification_sessions_test",
         VerificationSessions.KEY_KEY,
         null,

service/src/test/java/org/whispersystems/textsecuregcm/storage/StoredVerificationCodeManagerTest.java

@@ -1,62 +0,0 @@
-/*
- * Copyright 2013-2021 Signal Messenger, LLC
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-
-package org.whispersystems.textsecuregcm.storage;
-
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-import java.util.Optional;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
-
-class StoredVerificationCodeManagerTest {
-
-  private VerificationCodeStore verificationCodeStore;
-
-  private StoredVerificationCodeManager storedVerificationCodeManager;
-
-  @BeforeEach
-  void setUp() {
-    verificationCodeStore = mock(VerificationCodeStore.class);
-
-    storedVerificationCodeManager = new StoredVerificationCodeManager(verificationCodeStore);
-  }
-
-  @Test
-  void store() {
-    final String number = "+18005551234";
-    final StoredVerificationCode code = mock(StoredVerificationCode.class);
-
-    storedVerificationCodeManager.store(number, code);
-
-    verify(verificationCodeStore).insert(number, code);
-  }
-
-  @Test
-  void remove() {
-    final String number = "+18005551234";
-
-    storedVerificationCodeManager.remove(number);
-
-    verify(verificationCodeStore).remove(number);
-  }
-
-  @Test
-  void getCodeForNumber() {
-    final String number = "+18005551234";
-
-    when(verificationCodeStore.findForNumber(number)).thenReturn(Optional.empty());
-    assertEquals(Optional.empty(), storedVerificationCodeManager.getCodeForNumber(number));
-
-    final StoredVerificationCode storedVerificationCode = mock(StoredVerificationCode.class);
-
-    when(verificationCodeStore.findForNumber(number)).thenReturn(Optional.of(storedVerificationCode));
-    assertEquals(Optional.of(storedVerificationCode), storedVerificationCodeManager.getCodeForNumber(number));
-  }
-}

service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStoreTest.java

@@ -1,97 +0,0 @@
-/*
- * Copyright 2013-2021 Signal Messenger, LLC
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-
-package org.whispersystems.textsecuregcm.storage;
-
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
-import java.nio.charset.StandardCharsets;
-import java.time.Duration;
-import java.time.Instant;
-import java.util.Arrays;
-import java.util.Objects;
-import java.util.Optional;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
-import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
-import org.whispersystems.textsecuregcm.storage.DynamoDbExtensionSchema.Tables;
-
-class VerificationCodeStoreTest {
-
-  private VerificationCodeStore verificationCodeStore;
-
-  private static final String PHONE_NUMBER = "+14151112222";
-
-  private static final long VALID_TIMESTAMP = Instant.now().toEpochMilli();
-  private static final long EXPIRED_TIMESTAMP = Instant.now().minus(StoredVerificationCode.EXPIRATION).minus(
-      Duration.ofHours(1)).toEpochMilli();
-
-  @RegisterExtension
-  static final DynamoDbExtension DYNAMO_DB_EXTENSION = new DynamoDbExtension(Tables.VERIFICATION_CODES);
-
-  @BeforeEach
-  void setUp() {
-    verificationCodeStore = new VerificationCodeStore(
-        DYNAMO_DB_EXTENSION.getDynamoDbClient(), Tables.VERIFICATION_CODES.tableName());
-  }
-
-  @Test
-  void testStoreAndFind() {
-    assertEquals(Optional.empty(), verificationCodeStore.findForNumber(PHONE_NUMBER));
-
-    final StoredVerificationCode originalCode = new StoredVerificationCode("1234", VALID_TIMESTAMP, "abcd",
-        "session".getBytes(StandardCharsets.UTF_8));
-    final StoredVerificationCode secondCode = new StoredVerificationCode("5678", VALID_TIMESTAMP, "efgh",
-        "changed-session".getBytes(StandardCharsets.UTF_8));
-
-    verificationCodeStore.insert(PHONE_NUMBER, originalCode);
-    {
-      final Optional<StoredVerificationCode> maybeCode = verificationCodeStore.findForNumber(PHONE_NUMBER);
-
-      assertTrue(maybeCode.isPresent());
-      assertTrue(storedVerificationCodesAreEqual(originalCode, maybeCode.get()));
-    }
-
-    verificationCodeStore.insert(PHONE_NUMBER, secondCode);
-    {
-      final Optional<StoredVerificationCode> maybeCode = verificationCodeStore.findForNumber(PHONE_NUMBER);
-
-      assertTrue(maybeCode.isPresent());
-      assertTrue(storedVerificationCodesAreEqual(secondCode, maybeCode.get()));
-    }
-  }
-
-  @Test
-  void testRemove() {
-    assertEquals(Optional.empty(), verificationCodeStore.findForNumber(PHONE_NUMBER));
-
-    verificationCodeStore.insert(PHONE_NUMBER,
-        new StoredVerificationCode("1234", VALID_TIMESTAMP, "abcd", "session".getBytes(StandardCharsets.UTF_8)));
-    assertTrue(verificationCodeStore.findForNumber(PHONE_NUMBER).isPresent());
-
-    verificationCodeStore.remove(PHONE_NUMBER);
-    assertFalse(verificationCodeStore.findForNumber(PHONE_NUMBER).isPresent());
-
-    verificationCodeStore.insert(PHONE_NUMBER,
-        new StoredVerificationCode("1234", EXPIRED_TIMESTAMP, "abcd", "session".getBytes(StandardCharsets.UTF_8)));
-    assertFalse(verificationCodeStore.findForNumber(PHONE_NUMBER).isPresent());
-  }
-
-  private static boolean storedVerificationCodesAreEqual(final StoredVerificationCode first, final StoredVerificationCode second) {
-    if (first == null && second == null) {
-      return true;
-    } else if (first == null || second == null) {
-      return false;
-    }
-
-    return Objects.equals(first.code(), second.code()) &&
-        first.timestamp() == second.timestamp() &&
-        Objects.equals(first.pushCode(), second.pushCode()) &&
-        Arrays.equals(first.sessionId(), second.sessionId());
-  }
-}