diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCode.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCode.java deleted file mode 100644 index da610fd59..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCode.java +++ /dev/null @@ -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); - } -} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/SerializedExpireableJsonDynamoStore.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/SerializedExpireableJsonDynamoStore.java index adab40b3f..d2e80ad50 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/SerializedExpireableJsonDynamoStore.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/SerializedExpireableJsonDynamoStore.java @@ -45,7 +45,7 @@ public abstract class SerializedExpireableJsonDynamoStore { 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) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/StoredVerificationCodeManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/StoredVerificationCodeManager.java deleted file mode 100644 index c4c73128c..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/StoredVerificationCodeManager.java +++ /dev/null @@ -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 getCodeForNumber(String number) { - return verificationCodeStore.findForNumber(number); - } -} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStore.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStore.java deleted file mode 100644 index 7d38b5060..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStore.java +++ /dev/null @@ -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 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 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()); - }); - } -} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCodeTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCodeTest.java deleted file mode 100644 index 130f27bbc..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCodeTest.java +++ /dev/null @@ -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 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) - ); - } -} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java index 009e7d23f..483befe66 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java +++ b/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, diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamoDbExtensionSchema.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamoDbExtensionSchema.java index 2ca680404..8946dba2e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamoDbExtensionSchema.java +++ b/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, diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/StoredVerificationCodeManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/StoredVerificationCodeManagerTest.java deleted file mode 100644 index 9636d9783..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/StoredVerificationCodeManagerTest.java +++ /dev/null @@ -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)); - } -} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStoreTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStoreTest.java deleted file mode 100644 index 94fa6f3bb..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStoreTest.java +++ /dev/null @@ -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 maybeCode = verificationCodeStore.findForNumber(PHONE_NUMBER); - - assertTrue(maybeCode.isPresent()); - assertTrue(storedVerificationCodesAreEqual(originalCode, maybeCode.get())); - } - - verificationCodeStore.insert(PHONE_NUMBER, secondCode); - { - final Optional 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()); - } -}