From 13fc0ffbca3524c65ca51dbba18c6d1eb146c715 Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Fri, 2 May 2025 12:28:55 -0400 Subject: [PATCH] Assume that PNI registration IDs are always present on `Device` records --- .../controllers/KeysController.java | 2 +- .../textsecuregcm/push/MessageSender.java | 2 +- .../textsecuregcm/push/ReceiptSender.java | 2 +- .../textsecuregcm/storage/Device.java | 7 +++-- .../controllers/KeysControllerTest.java | 26 +------------------ .../textsecuregcm/push/MessageSenderTest.java | 6 ++--- ...ccountCreationDeletionIntegrationTest.java | 2 +- ...ntsManagerChangeNumberIntegrationTest.java | 4 +-- .../storage/AccountsManagerTest.java | 4 +-- 9 files changed, 13 insertions(+), 42 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java index df16f5ab5..b2b253704 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java @@ -383,7 +383,7 @@ public class KeysController { if (signedEcPreKey != null || unsignedEcPreKey != null || pqPreKey != null) { final int registrationId = switch (targetIdentifier.identityType()) { case ACI -> device.getRegistrationId(); - case PNI -> device.getPhoneNumberIdentityRegistrationId().orElse(device.getRegistrationId()); + case PNI -> device.getPhoneNumberIdentityRegistrationId(); }; responseItems.add( diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java index 70eb5a6b5..c1fa1a885 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java @@ -321,7 +321,7 @@ public class MessageSender { final int expectedRegistrationId = switch (serviceIdentifier.identityType()) { case ACI -> device.getRegistrationId(); - case PNI -> device.getPhoneNumberIdentityRegistrationId().orElseGet(device::getRegistrationId); + case PNI -> device.getPhoneNumberIdentityRegistrationId(); }; return registrationId != expectedRegistrationId; diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/ReceiptSender.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/ReceiptSender.java index 3c19dfaa9..e1c76e708 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/ReceiptSender.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/ReceiptSender.java @@ -63,7 +63,7 @@ public class ReceiptSender { final Map registrationIdsByDeviceId = destinationAccount.getDevices().stream() .collect(Collectors.toMap(Device::getId, device -> switch (destinationIdentifier.identityType()) { case ACI -> device.getRegistrationId(); - case PNI -> device.getPhoneNumberIdentityRegistrationId().orElseGet(device::getRegistrationId); + case PNI -> device.getPhoneNumberIdentityRegistrationId(); })); try { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java index 5e674a9ae..6995b9266 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java @@ -64,9 +64,8 @@ public class Device { @JsonProperty private int registrationId; - @Nullable @JsonProperty("pniRegistrationId") - private Integer phoneNumberIdentityRegistrationId; + private int phoneNumberIdentityRegistrationId; @JsonProperty private long lastSeen; @@ -216,8 +215,8 @@ public class Device { this.registrationId = registrationId; } - public OptionalInt getPhoneNumberIdentityRegistrationId() { - return phoneNumberIdentityRegistrationId != null ? OptionalInt.of(phoneNumberIdentityRegistrationId) : OptionalInt.empty(); + public int getPhoneNumberIdentityRegistrationId() { + return phoneNumberIdentityRegistrationId; } public void setPhoneNumberIdentityRegistrationId(final int phoneNumberIdentityRegistrationId) { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java index 4b8b85fa5..039991d68 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java @@ -38,7 +38,6 @@ import java.time.temporal.ChronoUnit; import java.util.Collections; import java.util.List; import java.util.Optional; -import java.util.OptionalInt; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.stream.IntStream; @@ -219,7 +218,7 @@ class KeysControllerTest { when(sampleDevice2.getRegistrationId()).thenReturn(SAMPLE_REGISTRATION_ID2); when(sampleDevice3.getRegistrationId()).thenReturn(SAMPLE_REGISTRATION_ID2); when(sampleDevice4.getRegistrationId()).thenReturn(SAMPLE_REGISTRATION_ID4); - when(sampleDevice.getPhoneNumberIdentityRegistrationId()).thenReturn(OptionalInt.of(SAMPLE_PNI_REGISTRATION_ID)); + when(sampleDevice.getPhoneNumberIdentityRegistrationId()).thenReturn(SAMPLE_PNI_REGISTRATION_ID); when(sampleDevice.getId()).thenReturn(sampleDeviceId); when(sampleDevice2.getId()).thenReturn(sampleDevice2Id); when(sampleDevice3.getId()).thenReturn(sampleDevice3Id); @@ -437,29 +436,6 @@ class KeysControllerTest { verifyNoMoreInteractions(KEYS); } - @Test - void validSingleRequestByPhoneNumberIdentifierNoPniRegistrationIdTestV2() { - when(sampleDevice.getPhoneNumberIdentityRegistrationId()).thenReturn(OptionalInt.empty()); - - PreKeyResponse result = resources.getJerseyTest() - .target(String.format("/v2/keys/PNI:%s/1", EXISTS_PNI)) - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .get(PreKeyResponse.class); - - assertThat(result.getIdentityKey()).isEqualTo(existsAccount.getIdentityKey(IdentityType.PNI)); - assertThat(result.getDevicesCount()).isEqualTo(1); - assertEquals(SAMPLE_KEY_PNI, result.getDevice(SAMPLE_DEVICE_ID).getPreKey()); - assertThat(result.getDevice(SAMPLE_DEVICE_ID).getPqPreKey()).isEqualTo(SAMPLE_PQ_KEY_PNI); - assertThat(result.getDevice(SAMPLE_DEVICE_ID).getRegistrationId()).isEqualTo(SAMPLE_REGISTRATION_ID); - assertEquals(SAMPLE_SIGNED_PNI_KEY, result.getDevice(SAMPLE_DEVICE_ID).getSignedPreKey()); - - verify(KEYS).takeEC(EXISTS_PNI, SAMPLE_DEVICE_ID); - verify(KEYS).takePQ(EXISTS_PNI, SAMPLE_DEVICE_ID); - verify(KEYS).getEcSignedPreKey(EXISTS_PNI, SAMPLE_DEVICE_ID); - verifyNoMoreInteractions(KEYS); - } - @Test void testGetKeysRateLimited() throws RateLimitExceededException { Duration retryAfter = Duration.ofSeconds(31); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java index 8da5d3f87..f2a73adf9 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java @@ -12,7 +12,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyByte; import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -23,7 +22,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.OptionalInt; import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; @@ -294,12 +292,12 @@ class MessageSenderTest { final Device primaryDevice = mock(Device.class); when(primaryDevice.getId()).thenReturn(primaryDeviceId); when(primaryDevice.getRegistrationId()).thenReturn(primaryDeviceAciRegistrationId); - when(primaryDevice.getPhoneNumberIdentityRegistrationId()).thenReturn(OptionalInt.of(primaryDevicePniRegistrationId)); + when(primaryDevice.getPhoneNumberIdentityRegistrationId()).thenReturn(primaryDevicePniRegistrationId); final Device linkedDevice = mock(Device.class); when(linkedDevice.getId()).thenReturn(linkedDeviceId); when(linkedDevice.getRegistrationId()).thenReturn(linkedDeviceAciRegistrationId); - when(linkedDevice.getPhoneNumberIdentityRegistrationId()).thenReturn(OptionalInt.of(linkedDevicePniRegistrationId)); + when(linkedDevice.getPhoneNumberIdentityRegistrationId()).thenReturn(linkedDevicePniRegistrationId); final Account account = mock(Account.class); when(account.getDevices()).thenReturn(List.of(primaryDevice, linkedDevice)); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java index 57ca4652a..c9e832897 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java @@ -507,7 +507,7 @@ public class AccountCreationDeletionIntegrationTest { assertEquals(signalAgent, primaryDevice.getUserAgent()); assertEquals(deliveryChannels.fetchesMessages(), primaryDevice.getFetchesMessages()); assertEquals(registrationId, primaryDevice.getRegistrationId()); - assertEquals(pniRegistrationId, primaryDevice.getPhoneNumberIdentityRegistrationId().orElseThrow()); + assertEquals(pniRegistrationId, primaryDevice.getPhoneNumberIdentityRegistrationId()); assertArrayEquals(deviceName, primaryDevice.getName()); assertEquals(discoverableByPhoneNumber, account.isDiscoverableByPhoneNumber()); assertEquals(deviceCapabilities, primaryDevice.getCapabilities()); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java index 87e047f7d..3c8c2b357 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java @@ -232,9 +232,7 @@ class AccountsManagerChangeNumberIntegrationTest { assertEquals(Optional.empty(), accountsManager.findRecentlyDeletedAccountIdentifier(secondPni)); assertEquals(pniIdentityKey, updatedAccount.getIdentityKey(IdentityType.PNI)); - - assertEquals(OptionalInt.of(rotatedPniRegistrationId), - updatedAccount.getPrimaryDevice().getPhoneNumberIdentityRegistrationId()); + assertEquals(rotatedPniRegistrationId, updatedAccount.getPrimaryDevice().getPhoneNumberIdentityRegistrationId()); assertEquals(Optional.of(rotatedSignedPreKey), keysManager.getEcSignedPreKey(updatedAccount.getIdentifier(IdentityType.PNI), Device.PRIMARY_ID).join()); 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 1bba9a59a..9ca622db5 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -1013,7 +1013,7 @@ class AccountsManagerTest { assertEquals(signalAgent, device.getUserAgent()); assertEquals(Collections.emptySet(), device.getCapabilities()); assertEquals(aciRegistrationId, device.getRegistrationId()); - assertEquals(pniRegistrationId, device.getPhoneNumberIdentityRegistrationId().getAsInt()); + assertEquals(pniRegistrationId, device.getPhoneNumberIdentityRegistrationId()); assertTrue(device.getFetchesMessages()); assertNull(device.getApnId()); assertNull(device.getGcmId()); @@ -1270,7 +1270,7 @@ class AccountsManagerTest { // PNI stuff should assertEquals(pniIdentityKey, updatedAccount.getIdentityKey(IdentityType.PNI)); assertEquals(newRegistrationIds, - updatedAccount.getDevices().stream().collect(Collectors.toMap(Device::getId, d -> d.getPhoneNumberIdentityRegistrationId().getAsInt()))); + updatedAccount.getDevices().stream().collect(Collectors.toMap(Device::getId, Device::getPhoneNumberIdentityRegistrationId))); verify(accounts).updateTransactionallyAsync(any(), any());