diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java index c189d07bb..00ca475c0 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -52,7 +52,6 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; -import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; @@ -743,45 +742,6 @@ public class AccountsManager extends RedisPubSubAdapter implemen AccountChangeValidator.NUMBER_CHANGE_VALIDATOR); } - public Account updatePniKeys(final Account account, - final IdentityKey pniIdentityKey, - final Map pniSignedPreKeys, - final Map pniPqLastResortPreKeys, - final Map pniRegistrationIds) throws MismatchedDevicesException { - - try { - return accountLockManager.withLock(Set.of(account.getIdentifier(IdentityType.PNI)), () -> { - validateDevices(account, pniSignedPreKeys, pniPqLastResortPreKeys, pniRegistrationIds); - - final UUID aci = account.getIdentifier(IdentityType.ACI); - final UUID pni = account.getIdentifier(IdentityType.PNI); - - final Collection keyWriteItems = - buildPniKeyWriteItems(pni, pniSignedPreKeys, pniPqLastResortPreKeys); - - return redisDeleteAsync(account) - .thenCompose(ignored -> keysManager.deleteSingleUsePreKeys(pni)) - .thenCompose(ignored -> updateTransactionallyWithRetriesAsync(account, - a -> setPniKeys(a, pniIdentityKey, pniRegistrationIds), - accounts::updateTransactionallyAsync, - () -> accounts.getByAccountIdentifierAsync(aci).thenApply(Optional::orElseThrow), - a -> keyWriteItems, - AccountChangeValidator.GENERAL_CHANGE_VALIDATOR, - MAX_UPDATE_ATTEMPTS)) - .join(); - }, accountLockExecutor); - } catch (final Exception e) { - if (e instanceof MismatchedDevicesException mismatchedDevicesException) { - throw mismatchedDevicesException; - } else if (e instanceof RuntimeException runtimeException) { - throw runtimeException; - } - - logger.error("Unexpected exception when updating PNI key material", e); - throw new RuntimeException(e); - } - } - private Collection buildPniKeyWriteItems( final UUID phoneNumberIdentifier, final Map pniSignedPreKeys, @@ -1120,42 +1080,6 @@ public class AccountsManager extends RedisPubSubAdapter implemen return CompletableFuture.failedFuture(new OptimisticLockRetryLimitExceededException()); } - private CompletionStage updateTransactionallyWithRetriesAsync(final Account account, - final Consumer updater, - final BiFunction, CompletionStage> persister, - final Supplier> retriever, - final Function> additionalWriteItemProvider, - final AccountChangeValidator changeValidator, - final int remainingTries) { - - final Account originalAccount = AccountUtil.cloneAccountAsNotStale(account); - - final Collection additionalWriteItems = additionalWriteItemProvider.apply(account); - updater.accept(account); - - if (remainingTries > 0) { - return persister.apply(account, additionalWriteItems) - .thenApply(ignored -> { - final Account updatedAccount = AccountUtil.cloneAccountAsNotStale(account); - account.markStale(); - - changeValidator.validateChange(originalAccount, updatedAccount); - - return updatedAccount; - }) - .exceptionallyCompose(throwable -> { - if (ExceptionUtils.unwrap(throwable) instanceof ContestedOptimisticLockException) { - return retriever.get().thenCompose(refreshedAccount -> - updateTransactionallyWithRetriesAsync(refreshedAccount, updater, persister, retriever, additionalWriteItemProvider, changeValidator, remainingTries - 1)); - } else { - throw ExceptionUtils.wrap(throwable); - } - }); - } - - return CompletableFuture.failedFuture(new OptimisticLockRetryLimitExceededException()); - } - public Account updateDevice(Account account, byte deviceId, Consumer deviceUpdater) { return update(account, a -> { a.getDevice(deviceId).ifPresent(deviceUpdater); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java index c20048178..6c357ba09 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -59,7 +59,6 @@ import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import java.util.function.Consumer; import java.util.function.Supplier; -import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; import javax.crypto.spec.SecretKeySpec; @@ -1200,101 +1199,6 @@ class AccountsManagerTest { assertThrows(AssertionError.class, () -> accountsManager.update(account, a -> a.setNumber(targetNumber, UUID.randomUUID()))); } - @Test - void testPniUpdate() throws MismatchedDevicesException { - final String number = "+14152222222"; - final byte deviceId2 = 2; - - List devices = List.of(DevicesHelper.createDevice(Device.PRIMARY_ID, 0L, 101), - DevicesHelper.createDevice(deviceId2, 0L, 102)); - - Account account = AccountsHelper.generateTestAccount(number, UUID.randomUUID(), UUID.randomUUID(), devices, new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); - final ECKeyPair identityKeyPair = Curve.generateKeyPair(); - Map newSignedKeys = Map.of( - Device.PRIMARY_ID, KeysHelper.signedECPreKey(1, identityKeyPair), - deviceId2, KeysHelper.signedECPreKey(2, identityKeyPair)); - Map newSignedKemKeys = Map.of( - Device.PRIMARY_ID, KeysHelper.signedKEMPreKey(1, identityKeyPair), - deviceId2, KeysHelper.signedKEMPreKey(2, identityKeyPair)); - Map newRegistrationIds = Map.of(Device.PRIMARY_ID, 201, deviceId2, 202); - - UUID oldUuid = account.getUuid(); - UUID oldPni = account.getPhoneNumberIdentifier(); - - final IdentityKey pniIdentityKey = new IdentityKey(Curve.generateKeyPair().getPublicKey()); - - when(keysManager.storeEcSignedPreKeys(any(), anyByte(), any())).thenReturn(CompletableFuture.completedFuture(null)); - - final Account updatedAccount = accountsManager.updatePniKeys(account, pniIdentityKey, newSignedKeys, newSignedKemKeys, newRegistrationIds); - - // non-PNI stuff should not change - assertEquals(oldUuid, updatedAccount.getUuid()); - assertEquals(number, updatedAccount.getNumber()); - assertEquals(oldPni, updatedAccount.getPhoneNumberIdentifier()); - assertNull(updatedAccount.getIdentityKey(IdentityType.ACI)); - assertEquals(Map.of(Device.PRIMARY_ID, 101, deviceId2, 102), - updatedAccount.getDevices().stream().collect(Collectors.toMap(Device::getId, device -> device.getRegistrationId(IdentityType.ACI)))); - - // PNI stuff should - assertEquals(pniIdentityKey, updatedAccount.getIdentityKey(IdentityType.PNI)); - assertEquals(newRegistrationIds, - updatedAccount.getDevices().stream().collect(Collectors.toMap(Device::getId, device -> device.getRegistrationId(IdentityType.PNI)))); - - verify(accounts).updateTransactionallyAsync(any(), any()); - - verify(keysManager).deleteSingleUsePreKeys(oldPni); - verify(keysManager).buildWriteItemForEcSignedPreKey(eq(oldPni), eq(Device.PRIMARY_ID), any()); - verify(keysManager).buildWriteItemForEcSignedPreKey(eq(oldPni), eq(deviceId2), any()); - verify(keysManager).buildWriteItemForLastResortKey(eq(oldPni), eq(deviceId2), any()); - } - - @Test - void testPniUpdate_incompleteKeys() { - final String number = "+14152222222"; - final byte deviceId2 = 2; - final byte deviceId3 = 3; - List devices = List.of(DevicesHelper.createDevice(Device.PRIMARY_ID, 0L, 101), - DevicesHelper.createDevice(deviceId2, 0L, 102)); - Account account = AccountsHelper.generateTestAccount(number, UUID.randomUUID(), UUID.randomUUID(), devices, new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); - final ECKeyPair identityKeyPair = Curve.generateKeyPair(); - final Map newSignedKeys = Map.of( - deviceId2, KeysHelper.signedECPreKey(1, identityKeyPair), - deviceId3, KeysHelper.signedECPreKey(2, identityKeyPair)); - Map newRegistrationIds = Map.of(Device.PRIMARY_ID, 201, deviceId2, 202); - - final IdentityKey pniIdentityKey = new IdentityKey(Curve.generateKeyPair().getPublicKey()); - - assertThrows(MismatchedDevicesException.class, - () -> accountsManager.updatePniKeys(account, pniIdentityKey, newSignedKeys, null, newRegistrationIds)); - - verifyNoInteractions(accounts); - verifyNoInteractions(keysManager); - } - - @Test - void testPniPqUpdate_incompleteKeys() { - final String number = "+14152222222"; - final byte deviceId2 = 2; - List devices = List.of(DevicesHelper.createDevice(Device.PRIMARY_ID, 0L, 101), - DevicesHelper.createDevice(deviceId2, 0L, 102)); - Account account = AccountsHelper.generateTestAccount(number, UUID.randomUUID(), UUID.randomUUID(), devices, new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); - final ECKeyPair identityKeyPair = Curve.generateKeyPair(); - final Map newSignedKeys = Map.of( - Device.PRIMARY_ID, KeysHelper.signedECPreKey(1, identityKeyPair), - deviceId2, KeysHelper.signedECPreKey(2, identityKeyPair)); - final Map newSignedPqKeys = Map.of( - Device.PRIMARY_ID, KeysHelper.signedKEMPreKey(3, identityKeyPair)); - Map newRegistrationIds = Map.of(Device.PRIMARY_ID, 201, deviceId2, 202); - - final IdentityKey pniIdentityKey = new IdentityKey(Curve.generateKeyPair().getPublicKey()); - - assertThrows(MismatchedDevicesException.class, - () -> accountsManager.updatePniKeys(account, pniIdentityKey, newSignedKeys, newSignedPqKeys, newRegistrationIds)); - - verifyNoInteractions(accounts); - verifyNoInteractions(keysManager); - } - @Test void testReserveUsernameHash() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java index 59e10efa4..712b63ac4 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java @@ -82,25 +82,6 @@ public class ChangeNumberManagerTest { return updatedAccount; }); - - when(accountsManager.updatePniKeys(any(), any(), any(), any(), any())).thenAnswer((Answer)invocation -> { - final Account account = invocation.getArgument(0, Account.class); - - final UUID uuid = account.getUuid(); - final UUID pni = account.getPhoneNumberIdentifier(); - final List devices = account.getDevices(); - - final Account updatedAccount = mock(Account.class); - when(updatedAccount.getUuid()).thenReturn(uuid); - when(updatedAccount.getPhoneNumberIdentifier()).thenReturn(pni); - when(updatedAccount.getDevices()).thenReturn(devices); - for (byte i = 1; i <= 3; i++) { - final Optional d = account.getDevice(i); - when(updatedAccount.getDevice(i)).thenReturn(d); - } - - return updatedAccount; - }); } @Test