From 4e358b891f8036294c13f6a17b8489835384959a Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Wed, 16 Nov 2022 17:19:00 -0500 Subject: [PATCH] Retire `StoredVerificationCode#twilioVerificationSid` --- .../auth/StoredVerificationCode.java | 1 - .../controllers/AccountController.java | 3 +- .../controllers/DeviceController.java | 2 +- .../auth/StoredVerificationCodeTest.java | 7 +-- .../controllers/AccountControllerTest.java | 48 +++++++++---------- .../storage/VerificationCodeStoreTest.java | 9 ++-- .../controllers/DeviceControllerTest.java | 2 +- 7 files changed, 35 insertions(+), 37 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCode.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCode.java index e04c62ca3..4976ce66f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCode.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCode.java @@ -13,7 +13,6 @@ import org.whispersystems.textsecuregcm.util.Util; public record StoredVerificationCode(String code, long timestamp, String pushCode, - @Nullable String twilioVerificationSid, @Nullable byte[] sessionId) { public static final Duration EXPIRATION = Duration.ofMinutes(10); 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 64aaad58f..2361c6fc9 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -244,7 +244,7 @@ public class AccountController { String pushChallenge = generatePushChallenge(); StoredVerificationCode storedVerificationCode = - new StoredVerificationCode(null, clock.millis(), pushChallenge, null, null); + new StoredVerificationCode(null, clock.millis(), pushChallenge, null); pendingAccounts.store(number, storedVerificationCode); pushNotificationManager.sendRegistrationChallengeNotification(pushToken, tokenType, storedVerificationCode.pushCode()); @@ -352,7 +352,6 @@ public class AccountController { final StoredVerificationCode storedVerificationCode = new StoredVerificationCode(null, clock.millis(), maybeStoredVerificationCode.map(StoredVerificationCode::pushCode).orElse(null), - null, sessionId); pendingAccounts.store(number, storedVerificationCode); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java index 1a4edd33d..87299577d 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java @@ -133,7 +133,7 @@ public class DeviceController { VerificationCode verificationCode = generateVerificationCode(); StoredVerificationCode storedVerificationCode = - new StoredVerificationCode(verificationCode.getVerificationCode(), System.currentTimeMillis(), null, null, null); + new StoredVerificationCode(verificationCode.getVerificationCode(), System.currentTimeMillis(), null, null); pendingDevices.store(account.getNumber(), storedVerificationCode); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCodeTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCodeTest.java index 7caae76ec..091e70d81 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCodeTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/auth/StoredVerificationCodeTest.java @@ -22,9 +22,10 @@ class StoredVerificationCodeTest { private static Stream isValid() { return Stream.of( - Arguments.of(new StoredVerificationCode("code", System.currentTimeMillis(), null, null, null), "code", true), - Arguments.of(new StoredVerificationCode("code", System.currentTimeMillis(), null, null, null), "incorrect", false), - Arguments.of(new StoredVerificationCode("", System.currentTimeMillis(), null, null, null), "", false) + 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 551f5cee1..03940615a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java @@ -235,15 +235,15 @@ class AccountControllerTest { when(senderTransfer.getUuid()).thenReturn(SENDER_TRANSFER_UUID); when(senderTransfer.getNumber()).thenReturn(SENDER_TRANSFER); - when(pendingAccountsManager.getCodeForNumber(SENDER)).thenReturn(Optional.of(new StoredVerificationCode("1234", System.currentTimeMillis(), "1234-push", null, null))); + when(pendingAccountsManager.getCodeForNumber(SENDER)).thenReturn(Optional.of(new StoredVerificationCode("1234", System.currentTimeMillis(), "1234-push", null))); when(pendingAccountsManager.getCodeForNumber(SENDER_OLD)).thenReturn(Optional.empty()); - when(pendingAccountsManager.getCodeForNumber(SENDER_PIN)).thenReturn(Optional.of(new StoredVerificationCode("333333", System.currentTimeMillis(), null, null, null))); - when(pendingAccountsManager.getCodeForNumber(SENDER_REG_LOCK)).thenReturn(Optional.of(new StoredVerificationCode("666666", System.currentTimeMillis(), null, null, null))); - when(pendingAccountsManager.getCodeForNumber(SENDER_OVER_PIN)).thenReturn(Optional.of(new StoredVerificationCode("444444", System.currentTimeMillis(), null, null, null))); - when(pendingAccountsManager.getCodeForNumber(SENDER_OVER_PREFIX)).thenReturn(Optional.of(new StoredVerificationCode("777777", System.currentTimeMillis(), "1234-push", null, null))); - when(pendingAccountsManager.getCodeForNumber(SENDER_PREAUTH)).thenReturn(Optional.of(new StoredVerificationCode("555555", System.currentTimeMillis(), "validchallenge", null, null))); - when(pendingAccountsManager.getCodeForNumber(SENDER_HAS_STORAGE)).thenReturn(Optional.of(new StoredVerificationCode("666666", System.currentTimeMillis(), null, null, null))); - when(pendingAccountsManager.getCodeForNumber(SENDER_TRANSFER)).thenReturn(Optional.of(new StoredVerificationCode("1234", System.currentTimeMillis(), null, null, null))); + when(pendingAccountsManager.getCodeForNumber(SENDER_PIN)).thenReturn(Optional.of(new StoredVerificationCode("333333", System.currentTimeMillis(), null, null))); + when(pendingAccountsManager.getCodeForNumber(SENDER_REG_LOCK)).thenReturn(Optional.of(new StoredVerificationCode("666666", System.currentTimeMillis(), null, null))); + when(pendingAccountsManager.getCodeForNumber(SENDER_OVER_PIN)).thenReturn(Optional.of(new StoredVerificationCode("444444", System.currentTimeMillis(), null, null))); + when(pendingAccountsManager.getCodeForNumber(SENDER_OVER_PREFIX)).thenReturn(Optional.of(new StoredVerificationCode("777777", System.currentTimeMillis(), "1234-push", null))); + when(pendingAccountsManager.getCodeForNumber(SENDER_PREAUTH)).thenReturn(Optional.of(new StoredVerificationCode("555555", System.currentTimeMillis(), "validchallenge", null))); + when(pendingAccountsManager.getCodeForNumber(SENDER_HAS_STORAGE)).thenReturn(Optional.of(new StoredVerificationCode("666666", System.currentTimeMillis(), null, null))); + when(pendingAccountsManager.getCodeForNumber(SENDER_TRANSFER)).thenReturn(Optional.of(new StoredVerificationCode("1234", 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)); @@ -709,7 +709,7 @@ class AccountControllerTest { final String challenge = "challenge"; when(pendingAccountsManager.getCodeForNumber(RESTRICTED_NUMBER)) - .thenReturn(Optional.of(new StoredVerificationCode("123456", System.currentTimeMillis(), challenge, null, null))); + .thenReturn(Optional.of(new StoredVerificationCode("123456", System.currentTimeMillis(), challenge, null))); Response response = resources.getJerseyTest() @@ -740,7 +740,7 @@ class AccountControllerTest { final String challenge = "challenge"; when(pendingAccountsManager.getCodeForNumber(number)) - .thenReturn(Optional.of(new StoredVerificationCode("123456", System.currentTimeMillis(), challenge, null, null))); + .thenReturn(Optional.of(new StoredVerificationCode("123456", System.currentTimeMillis(), challenge, null))); when(registrationServiceClient.sendRegistrationCode(any(), any(), any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(new byte[16])); @@ -769,7 +769,7 @@ class AccountControllerTest { final String challenge = "challenge"; - when(pendingAccountsManager.getCodeForNumber(SENDER)).thenReturn(Optional.of(new StoredVerificationCode("123456", System.currentTimeMillis(), challenge, null, null))); + when(pendingAccountsManager.getCodeForNumber(SENDER)).thenReturn(Optional.of(new StoredVerificationCode("123456", System.currentTimeMillis(), challenge, null))); when(registrationServiceClient.sendRegistrationCode(any(), any(), any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(new byte[16])); @@ -831,7 +831,7 @@ class AccountControllerTest { when(pendingAccountsManager.getCodeForNumber(SENDER)) .thenReturn(Optional.of( - new StoredVerificationCode("1234", System.currentTimeMillis(), "1234-push", null, sessionId))); + new StoredVerificationCode("1234", System.currentTimeMillis(), "1234-push", sessionId))); when(registrationServiceClient.checkVerificationCode(sessionId, "1234", AccountController.REGISTRATION_RPC_TIMEOUT)) .thenReturn(CompletableFuture.completedFuture(true)); @@ -894,7 +894,7 @@ class AccountControllerTest { when(pendingAccountsManager.getCodeForNumber(SENDER)) .thenReturn(Optional.of( - new StoredVerificationCode("1234", System.currentTimeMillis(), "1234-push", null, sessionId))); + new StoredVerificationCode("1234", System.currentTimeMillis(), "1234-push", sessionId))); when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(false)); @@ -1066,7 +1066,7 @@ class AccountControllerTest { final byte[] sessionId = "session".getBytes(StandardCharsets.UTF_8); when(pendingAccountsManager.getCodeForNumber(number)).thenReturn(Optional.of( - new StoredVerificationCode(null, System.currentTimeMillis(), "push", null, sessionId))); + new StoredVerificationCode(null, System.currentTimeMillis(), "push", sessionId))); when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(true)); @@ -1167,7 +1167,7 @@ class AccountControllerTest { final byte[] sessionId = "session-id".getBytes(StandardCharsets.UTF_8); when(pendingAccountsManager.getCodeForNumber(number)).thenReturn(Optional.of( - new StoredVerificationCode(code, System.currentTimeMillis(), "push", null, sessionId))); + new StoredVerificationCode(code, System.currentTimeMillis(), "push", sessionId))); when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(false)); @@ -1193,7 +1193,7 @@ class AccountControllerTest { final byte[] sessionId = "session-id".getBytes(StandardCharsets.UTF_8); when(pendingAccountsManager.getCodeForNumber(number)).thenReturn(Optional.of( - new StoredVerificationCode(code, System.currentTimeMillis(), "push", null, sessionId))); + new StoredVerificationCode(code, System.currentTimeMillis(), "push", sessionId))); when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(true)); @@ -1227,7 +1227,7 @@ class AccountControllerTest { final byte[] sessionId = "session-id".getBytes(StandardCharsets.UTF_8); when(pendingAccountsManager.getCodeForNumber(number)).thenReturn(Optional.of( - new StoredVerificationCode(code, System.currentTimeMillis(), "push", null, sessionId))); + new StoredVerificationCode(code, System.currentTimeMillis(), "push", sessionId))); when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(true)); @@ -1266,7 +1266,7 @@ class AccountControllerTest { final byte[] sessionId = "session-id".getBytes(StandardCharsets.UTF_8); when(pendingAccountsManager.getCodeForNumber(number)).thenReturn(Optional.of( - new StoredVerificationCode(null, System.currentTimeMillis(), "push", null, sessionId))); + new StoredVerificationCode(null, System.currentTimeMillis(), "push", sessionId))); when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(true)); @@ -1306,7 +1306,7 @@ class AccountControllerTest { final byte[] sessionId = "session-id".getBytes(StandardCharsets.UTF_8); when(pendingAccountsManager.getCodeForNumber(number)).thenReturn(Optional.of( - new StoredVerificationCode(null, System.currentTimeMillis(), "push", null, sessionId))); + new StoredVerificationCode(null, System.currentTimeMillis(), "push", sessionId))); when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(true)); @@ -1358,7 +1358,7 @@ class AccountControllerTest { when(AuthHelper.VALID_ACCOUNT.getDevice(3L)).thenReturn(Optional.of(device3)); when(pendingAccountsManager.getCodeForNumber(number)).thenReturn(Optional.of( - new StoredVerificationCode(null, System.currentTimeMillis(), "push", null, sessionId))); + new StoredVerificationCode(null, System.currentTimeMillis(), "push", sessionId))); when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(true)); @@ -1891,10 +1891,10 @@ class AccountControllerTest { return Stream.of( Arguments.of(null, null, false), Arguments.of("123456", null, false), - Arguments.of(null, new StoredVerificationCode(null, 0, null, null, null), false), - Arguments.of(null, new StoredVerificationCode(null, 0, "123456", null, null), false), - Arguments.of("654321", new StoredVerificationCode(null, 0, "123456", null, null), false), - Arguments.of("123456", new StoredVerificationCode(null, 0, "123456", null, null), true) + Arguments.of(null, new StoredVerificationCode(null, 0, null, null), false), + Arguments.of(null, new StoredVerificationCode(null, 0, "123456", null), false), + Arguments.of("654321", new StoredVerificationCode(null, 0, "123456", null), false), + Arguments.of("123456", new StoredVerificationCode(null, 0, "123456", null), true) ); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStoreTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStoreTest.java index 28b21731a..6728fde95 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStoreTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStoreTest.java @@ -53,8 +53,8 @@ class VerificationCodeStoreTest { void testStoreAndFind() { assertEquals(Optional.empty(), verificationCodeStore.findForNumber(PHONE_NUMBER)); - final StoredVerificationCode originalCode = new StoredVerificationCode("1234", VALID_TIMESTAMP, "abcd", "0987", "session".getBytes(StandardCharsets.UTF_8)); - final StoredVerificationCode secondCode = new StoredVerificationCode("5678", VALID_TIMESTAMP, "efgh", "7890", "changed-session".getBytes(StandardCharsets.UTF_8)); + 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); { @@ -77,13 +77,13 @@ class VerificationCodeStoreTest { void testRemove() { assertEquals(Optional.empty(), verificationCodeStore.findForNumber(PHONE_NUMBER)); - verificationCodeStore.insert(PHONE_NUMBER, new StoredVerificationCode("1234", VALID_TIMESTAMP, "abcd", "0987", "session".getBytes(StandardCharsets.UTF_8))); + 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", "0987", "session".getBytes(StandardCharsets.UTF_8))); + verificationCodeStore.insert(PHONE_NUMBER, new StoredVerificationCode("1234", EXPIRED_TIMESTAMP, "abcd", "session".getBytes(StandardCharsets.UTF_8))); assertFalse(verificationCodeStore.findForNumber(PHONE_NUMBER).isPresent()); } @@ -97,7 +97,6 @@ class VerificationCodeStoreTest { return Objects.equals(first.code(), second.code()) && first.timestamp() == second.timestamp() && Objects.equals(first.pushCode(), second.pushCode()) && - Objects.equals(first.twilioVerificationSid(), second.twilioVerificationSid()) && Arrays.equals(first.sessionId(), second.sessionId()); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java index c38b0df4b..4b910eb38 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java @@ -134,7 +134,7 @@ class DeviceControllerTest { when(account.isPaymentActivationSupported()).thenReturn(false); when(pendingDevicesManager.getCodeForNumber(AuthHelper.VALID_NUMBER)).thenReturn( - Optional.of(new StoredVerificationCode("5678901", System.currentTimeMillis(), null, null, null))); + Optional.of(new StoredVerificationCode("5678901", System.currentTimeMillis(), null, null))); when(pendingDevicesManager.getCodeForNumber(AuthHelper.VALID_NUMBER_TWO)).thenReturn(Optional.empty()); when(accountsManager.getByE164(AuthHelper.VALID_NUMBER)).thenReturn(Optional.of(account)); when(accountsManager.getByE164(AuthHelper.VALID_NUMBER_TWO)).thenReturn(Optional.of(maxedAccount));