From 2f5574760154afc400953b56efcfae55faaea801 Mon Sep 17 00:00:00 2001 From: Jon Chambers <63609320+jon-signal@users.noreply.github.com> Date: Fri, 7 Jun 2024 13:39:11 -0400 Subject: [PATCH] Remove expiration check from `Device#isEnabled()` --- ...hEnablementRefreshRequirementProvider.java | 4 +-- .../auth/ContainerRequestUtil.java | 2 +- .../textsecuregcm/auth/OptionalAccess.java | 2 +- .../controllers/KeysController.java | 4 +-- .../textsecuregcm/grpc/KeysGrpcHelper.java | 2 +- .../textsecuregcm/storage/Account.java | 6 ++-- .../storage/AccountsManager.java | 2 +- .../textsecuregcm/storage/Device.java | 7 ++-- .../storage/MessagePersister.java | 2 +- .../util/DestinationDeviceValidator.java | 2 +- ...rocessPushNotificationFeedbackCommand.java | 2 +- .../auth/AccountAuthenticatorTest.java | 12 +++---- ...blementRefreshRequirementProviderTest.java | 8 ++--- .../auth/ContainerRequestUtilTest.java | 2 +- .../controllers/KeysControllerTest.java | 8 ++--- .../controllers/MessageControllerTest.java | 18 ++++------ .../grpc/KeysGrpcServiceTest.java | 2 +- .../textsecuregcm/storage/AccountTest.java | 30 ++++++++--------- .../storage/ChangeNumberManagerTest.java | 16 ++++----- .../textsecuregcm/storage/DeviceTest.java | 33 ++++--------------- .../storage/MessagePersisterTest.java | 28 ++++++++-------- .../textsecuregcm/tests/util/AuthHelper.java | 10 +++--- .../tests/util/DevicesHelper.java | 7 ++-- .../util/DestinationDeviceValidatorTest.java | 2 +- ...ssPushNotificationFeedbackCommandTest.java | 16 ++++----- 25 files changed, 99 insertions(+), 128 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProvider.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProvider.java index 3385f3621..053c31964 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProvider.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProvider.java @@ -23,9 +23,9 @@ import org.whispersystems.textsecuregcm.util.Pair; /** * This {@link WebsocketRefreshRequirementProvider} observes intra-request changes in {@link Account#isEnabled()} and - * {@link Device#isEnabled()}. + * {@link Device#hasMessageDeliveryChannel()}. *

- * If a change in {@link Account#isEnabled()} or any associated {@link Device#isEnabled()} is observed, then any active + * If a change in {@link Account#isEnabled()} or any associated {@link Device#hasMessageDeliveryChannel()} is observed, then any active * WebSocket connections for the account must be closed in order for clients to get a refreshed * {@link io.dropwizard.auth.Auth} object with a current device list. * diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/ContainerRequestUtil.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/ContainerRequestUtil.java index eb2ecf029..666afbe6c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/ContainerRequestUtil.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/ContainerRequestUtil.java @@ -17,7 +17,7 @@ import java.util.stream.Collectors; class ContainerRequestUtil { private static Map buildDevicesEnabledMap(final Account account) { - return account.getDevices().stream().collect(Collectors.toMap(Device::getId, Device::isEnabled)); + return account.getDevices().stream().collect(Collectors.toMap(Device::getId, Device::hasMessageDeliveryChannel)); } /** diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/OptionalAccess.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/OptionalAccess.java index 9a6edb87b..21435bbe9 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/OptionalAccess.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/OptionalAccess.java @@ -31,7 +31,7 @@ public class OptionalAccess { Optional targetDevice = targetAccount.get().getDevice(deviceId); - if (targetDevice.isPresent() && targetDevice.get().isEnabled()) { + if (targetDevice.isPresent() && targetDevice.get().hasMessageDeliveryChannel()) { return; } 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 f5b1f636c..2c517902c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java @@ -440,11 +440,11 @@ public class KeysController { private List parseDeviceId(String deviceId, Account account) { if (deviceId.equals("*")) { - return account.getDevices().stream().filter(Device::isEnabled).toList(); + return account.getDevices().stream().filter(Device::hasMessageDeliveryChannel).toList(); } try { byte id = Byte.parseByte(deviceId); - return account.getDevice(id).filter(Device::isEnabled).map(List::of).orElse(List.of()); + return account.getDevice(id).filter(Device::hasMessageDeliveryChannel).map(List::of).orElse(List.of()); } catch (NumberFormatException e) { throw new WebApplicationException(Response.status(422).build()); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/KeysGrpcHelper.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/KeysGrpcHelper.java index 249e234c2..792e31a90 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/KeysGrpcHelper.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/KeysGrpcHelper.java @@ -39,7 +39,7 @@ class KeysGrpcHelper { : Flux.from(Mono.justOrEmpty(targetAccount.getDevice(targetDeviceId))); return devices - .filter(Device::isEnabled) + .filter(Device::hasMessageDeliveryChannel) .switchIfEmpty(Mono.error(Status.NOT_FOUND.asException())) .flatMap(device -> Flux.merge( Mono.fromFuture(() -> keysManager.takeEC(targetAccount.getIdentifier(identityType), device.getId())), diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java index 77660d489..87009c56c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java @@ -296,14 +296,14 @@ public class Account { requireNotStale(); return devices.stream() - .filter(Device::isEnabled) + .filter(Device::hasMessageDeliveryChannel) .allMatch(device -> device.getCapabilities() != null && predicate.test(device.getCapabilities())); } public boolean isEnabled() { requireNotStale(); - return getPrimaryDevice().isEnabled(); + return getPrimaryDevice().hasMessageDeliveryChannel(); } public byte getNextDeviceId() { @@ -327,7 +327,7 @@ public class Account { return devices.stream() .filter(d -> Device.PRIMARY_ID != d.getId()) - .anyMatch(Device::isEnabled); + .anyMatch(Device::hasMessageDeliveryChannel); } public void setIdentityKey(final IdentityKey identityKey) { 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 1e74a04d5..52cc6917c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -490,7 +490,7 @@ public class AccountsManager { account.getDevices() .stream() - .filter(Device::isEnabled) + .filter(Device::hasMessageDeliveryChannel) .forEach(device -> device.setPhoneNumberIdentityRegistrationId(pniRegistrationIds.get(device.getId()))); account.setPhoneNumberIdentityKey(pniIdentityKey); 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 a7f1c759f..db41de17e 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java @@ -11,7 +11,6 @@ import com.fasterxml.jackson.databind.annotation.JsonSerialize; import java.time.Duration; import java.util.List; import java.util.OptionalInt; -import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.IntStream; import javax.annotation.Nullable; @@ -199,10 +198,8 @@ public class Device { this.capabilities = capabilities; } - public boolean isEnabled() { - boolean hasChannel = fetchesMessages || StringUtils.isNotEmpty(getApnId()) || StringUtils.isNotEmpty(getGcmId()); - - return (id == PRIMARY_ID && hasChannel) || (id != PRIMARY_ID && hasChannel && !isExpired()); + public boolean hasMessageDeliveryChannel() { + return fetchesMessages || StringUtils.isNotEmpty(getApnId()) || StringUtils.isNotEmpty(getGcmId()); } public boolean isExpired() { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java index d189f352d..fc1a515ef 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java @@ -245,7 +245,7 @@ public class MessagePersister implements Managed { // its messages) is unlinked final Device deviceToDelete = account.getDevices() .stream() - .filter(d -> !d.isPrimary() && !d.isEnabled()) + .filter(d -> !d.isPrimary() && !d.hasMessageDeliveryChannel()) .min(Comparator.comparing(Device::getLastSeen)) .or(() -> Flux.fromIterable(account.getDevices()) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/DestinationDeviceValidator.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/DestinationDeviceValidator.java index 38c8c43c4..c13cfced1 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/DestinationDeviceValidator.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/DestinationDeviceValidator.java @@ -89,7 +89,7 @@ public class DestinationDeviceValidator { final Set excludedDeviceIds) throws MismatchedDevicesException { final Set accountDeviceIds = account.getDevices().stream() - .filter(Device::isEnabled) + .filter(Device::hasMessageDeliveryChannel) .map(Device::getId) .filter(deviceId -> !excludedDeviceIds.contains(deviceId)) .collect(Collectors.toSet()); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/ProcessPushNotificationFeedbackCommand.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/ProcessPushNotificationFeedbackCommand.java index 04f7a42fd..2dcf0fedc 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/ProcessPushNotificationFeedbackCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/ProcessPushNotificationFeedbackCommand.java @@ -130,7 +130,7 @@ public class ProcessPushNotificationFeedbackCommand extends AbstractSinglePassCr @VisibleForTesting boolean deviceNeedsUpdate(final Device device) { - return pushFeedbackIntervalElapsed(device) && (device.isEnabled() || device.getLastSeen() > device.getUninstalledFeedbackTimestamp()); + return pushFeedbackIntervalElapsed(device) && (device.hasMessageDeliveryChannel() || device.getLastSeen() > device.getUninstalledFeedbackTimestamp()); } @VisibleForTesting diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/auth/AccountAuthenticatorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/auth/AccountAuthenticatorTest.java index c54f5bd17..19071dde3 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/auth/AccountAuthenticatorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/auth/AccountAuthenticatorTest.java @@ -163,7 +163,7 @@ class AccountAuthenticatorTest { when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); when(account.isEnabled()).thenReturn(true); when(device.getId()).thenReturn(deviceId); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getAuthTokenHash()).thenReturn(credentials); when(credentials.verify(password)).thenReturn(true); when(credentials.getVersion()).thenReturn(SaltedTokenHash.CURRENT_VERSION); @@ -193,7 +193,7 @@ class AccountAuthenticatorTest { when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); when(account.isEnabled()).thenReturn(true); when(device.getId()).thenReturn(deviceId); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getAuthTokenHash()).thenReturn(credentials); when(credentials.verify(password)).thenReturn(true); when(credentials.getVersion()).thenReturn(SaltedTokenHash.CURRENT_VERSION); @@ -226,7 +226,7 @@ class AccountAuthenticatorTest { when(account.getDevice(deviceId)).thenReturn(Optional.of(authenticatedDevice)); when(account.isEnabled()).thenReturn(accountEnabled); when(authenticatedDevice.getId()).thenReturn(deviceId); - when(authenticatedDevice.isEnabled()).thenReturn(deviceEnabled); + when(authenticatedDevice.hasMessageDeliveryChannel()).thenReturn(deviceEnabled); when(authenticatedDevice.getAuthTokenHash()).thenReturn(credentials); when(credentials.verify(password)).thenReturn(true); when(credentials.getVersion()).thenReturn(SaltedTokenHash.CURRENT_VERSION); @@ -262,7 +262,7 @@ class AccountAuthenticatorTest { when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); when(account.isEnabled()).thenReturn(true); when(device.getId()).thenReturn(deviceId); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getAuthTokenHash()).thenReturn(credentials); when(credentials.verify(password)).thenReturn(true); when(credentials.getVersion()).thenReturn(SaltedTokenHash.Version.V1); @@ -299,7 +299,7 @@ class AccountAuthenticatorTest { when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); when(account.isEnabled()).thenReturn(true); when(device.getId()).thenReturn(deviceId); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getAuthTokenHash()).thenReturn(credentials); when(credentials.verify(password)).thenReturn(true); when(credentials.getVersion()).thenReturn(SaltedTokenHash.CURRENT_VERSION); @@ -327,7 +327,7 @@ class AccountAuthenticatorTest { when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); when(account.isEnabled()).thenReturn(true); when(device.getId()).thenReturn(deviceId); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getAuthTokenHash()).thenReturn(credentials); when(credentials.verify(password)).thenReturn(true); when(credentials.getVersion()).thenReturn(SaltedTokenHash.CURRENT_VERSION); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProviderTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProviderTest.java index bc3a913e2..aa4e07336 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProviderTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProviderTest.java @@ -136,7 +136,7 @@ class AuthEnablementRefreshRequirementProviderTest { void testDeviceEnabledChanged(final Map initialEnabled, final Map finalEnabled) { assert initialEnabled.size() == finalEnabled.size(); - assert account.getPrimaryDevice().isEnabled(); + assert account.getPrimaryDevice().hasMessageDeliveryChannel(); initialEnabled.forEach((deviceId, enabled) -> DevicesHelper.setEnabled(account.getDevice(deviceId).orElseThrow(), enabled)); @@ -177,7 +177,7 @@ class AuthEnablementRefreshRequirementProviderTest { @Test void testDeviceAdded() { - assert account.getPrimaryDevice().isEnabled(); + assert account.getPrimaryDevice().hasMessageDeliveryChannel(); final int initialDeviceCount = account.getDevices().size(); @@ -204,7 +204,7 @@ class AuthEnablementRefreshRequirementProviderTest { @ParameterizedTest @ValueSource(ints = {1, 2}) void testDeviceRemoved(final int removedDeviceCount) { - assert account.getPrimaryDevice().isEnabled(); + assert account.getPrimaryDevice().hasMessageDeliveryChannel(); final List initialDeviceIds = account.getDevices().stream().map(Device::getId).toList(); @@ -367,7 +367,7 @@ class AuthEnablementRefreshRequirementProviderTest { DevicesHelper.setEnabled(device, enabled); - assert device.isEnabled() == enabled; + assert device.hasMessageDeliveryChannel() == enabled; return String.format("Set account to %s", enabled); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/auth/ContainerRequestUtilTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/auth/ContainerRequestUtilTest.java index e0a87d3f0..cef8b85a2 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/auth/ContainerRequestUtilTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/auth/ContainerRequestUtilTest.java @@ -35,7 +35,7 @@ public class ContainerRequestUtilTest { .forEach(id -> { final Device device = mock(Device.class); when(device.getId()).thenReturn((byte) id); - when(device.isEnabled()).thenReturn(id != disabledDeviceId); + when(device.hasMessageDeliveryChannel()).thenReturn(id != disabledDeviceId); devices.add(device); }); 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 25b03efb4..27bab634c 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java @@ -221,10 +221,10 @@ class KeysControllerTest { 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.isEnabled()).thenReturn(true); - when(sampleDevice2.isEnabled()).thenReturn(true); - when(sampleDevice3.isEnabled()).thenReturn(false); - when(sampleDevice4.isEnabled()).thenReturn(true); + when(sampleDevice.hasMessageDeliveryChannel()).thenReturn(true); + when(sampleDevice2.hasMessageDeliveryChannel()).thenReturn(true); + when(sampleDevice3.hasMessageDeliveryChannel()).thenReturn(false); + when(sampleDevice4.hasMessageDeliveryChannel()).thenReturn(true); when(sampleDevice.getId()).thenReturn(sampleDeviceId); when(sampleDevice2.getId()).thenReturn(sampleDevice2Id); when(sampleDevice3.getId()).thenReturn(sampleDevice3Id); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java index e6f2407d4..bc6cc87f5 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java @@ -58,7 +58,6 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; -import java.util.concurrent.TimeUnit; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -211,13 +210,13 @@ class MessageControllerTest { @BeforeEach void setup() { final List singleDeviceList = List.of( - generateTestDevice(SINGLE_DEVICE_ID1, SINGLE_DEVICE_REG_ID1, SINGLE_DEVICE_PNI_REG_ID1, System.currentTimeMillis(), System.currentTimeMillis()) + generateTestDevice(SINGLE_DEVICE_ID1, SINGLE_DEVICE_REG_ID1, SINGLE_DEVICE_PNI_REG_ID1, true) ); final List multiDeviceList = List.of( - generateTestDevice(MULTI_DEVICE_ID1, MULTI_DEVICE_REG_ID1, MULTI_DEVICE_PNI_REG_ID1, System.currentTimeMillis(), System.currentTimeMillis()), - generateTestDevice(MULTI_DEVICE_ID2, MULTI_DEVICE_REG_ID2, MULTI_DEVICE_PNI_REG_ID2, System.currentTimeMillis(), System.currentTimeMillis()), - generateTestDevice(MULTI_DEVICE_ID3, MULTI_DEVICE_REG_ID3, MULTI_DEVICE_PNI_REG_ID3, System.currentTimeMillis(), System.currentTimeMillis() - TimeUnit.DAYS.toMillis(31)) + generateTestDevice(MULTI_DEVICE_ID1, MULTI_DEVICE_REG_ID1, MULTI_DEVICE_PNI_REG_ID1, true), + generateTestDevice(MULTI_DEVICE_ID2, MULTI_DEVICE_REG_ID2, MULTI_DEVICE_PNI_REG_ID2, true), + generateTestDevice(MULTI_DEVICE_ID3, MULTI_DEVICE_REG_ID3, MULTI_DEVICE_PNI_REG_ID3, false) ); Account singleDeviceAccount = AccountsHelper.generateTestAccount(SINGLE_DEVICE_RECIPIENT, SINGLE_DEVICE_UUID, SINGLE_DEVICE_PNI, singleDeviceList, UNIDENTIFIED_ACCESS_BYTES); @@ -260,14 +259,12 @@ class MessageControllerTest { } private static Device generateTestDevice(final byte id, final int registrationId, final int pniRegistrationId, - final long createdAt, final long lastSeen) { + final boolean enabled) { final Device device = new Device(); device.setId(id); device.setRegistrationId(registrationId); device.setPhoneNumberIdentityRegistrationId(pniRegistrationId); - device.setCreated(createdAt); - device.setLastSeen(lastSeen); - device.setGcmId("isgcm"); + device.setFetchesMessages(enabled); return device; } @@ -1125,8 +1122,7 @@ class MessageControllerTest { IntStream.range(1, devicesPerRecipient + 1) .mapToObj( d -> generateTestDevice( - (byte) d, 100 + d, 10 * d, System.currentTimeMillis(), - System.currentTimeMillis())) + (byte) d, 100 + d, 10 * d, true)) .collect(Collectors.toList()); final UUID aci = new UUID(0L, (long) i); final UUID pni = new UUID(1L, (long) i); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/KeysGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/KeysGrpcServiceTest.java index ff89cd503..748820eac 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/KeysGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/KeysGrpcServiceTest.java @@ -489,7 +489,7 @@ class KeysGrpcServiceTest extends SimpleBaseGrpcTest testHasEnabledLinkedDevice() { final Device enabledPrimary = mock(Device.class); - when(enabledPrimary.isEnabled()).thenReturn(true); + when(enabledPrimary.hasMessageDeliveryChannel()).thenReturn(true); when(enabledPrimary.getId()).thenReturn(Device.PRIMARY_ID); final Device disabledPrimary = mock(Device.class); @@ -304,7 +304,7 @@ class AccountTest { final byte linked1DeviceId = Device.PRIMARY_ID + 1; final Device enabledLinked1 = mock(Device.class); - when(enabledLinked1.isEnabled()).thenReturn(true); + when(enabledLinked1.hasMessageDeliveryChannel()).thenReturn(true); when(enabledLinked1.getId()).thenReturn(linked1DeviceId); final Device disabledLinked1 = mock(Device.class); @@ -312,7 +312,7 @@ class AccountTest { final byte linked2DeviceId = Device.PRIMARY_ID + 2; final Device enabledLinked2 = mock(Device.class); - when(enabledLinked2.isEnabled()).thenReturn(true); + when(enabledLinked2.hasMessageDeliveryChannel()).thenReturn(true); when(enabledLinked2.getId()).thenReturn(linked2DeviceId); final Device disabledLinked2 = mock(Device.class); 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 0d895efd3..1238e8904 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java @@ -134,7 +134,7 @@ public class ChangeNumberManagerTest { when(account.getPhoneNumberIdentifier()).thenReturn(pni); final Device d2 = mock(Device.class); - when(d2.isEnabled()).thenReturn(true); + when(d2.hasMessageDeliveryChannel()).thenReturn(true); final byte deviceId2 = 2; when(d2.getId()).thenReturn(deviceId2); @@ -181,7 +181,7 @@ public class ChangeNumberManagerTest { when(account.getPhoneNumberIdentifier()).thenReturn(pni); final Device d2 = mock(Device.class); - when(d2.isEnabled()).thenReturn(true); + when(d2.hasMessageDeliveryChannel()).thenReturn(true); final byte deviceId2 = 2; when(d2.getId()).thenReturn(deviceId2); @@ -228,7 +228,7 @@ public class ChangeNumberManagerTest { when(account.getPhoneNumberIdentifier()).thenReturn(pni); final Device d2 = mock(Device.class); - when(d2.isEnabled()).thenReturn(true); + when(d2.hasMessageDeliveryChannel()).thenReturn(true); final byte deviceId2 = 2; when(d2.getId()).thenReturn(deviceId2); @@ -273,7 +273,7 @@ public class ChangeNumberManagerTest { when(account.getPhoneNumberIdentifier()).thenReturn(pni); final Device d2 = mock(Device.class); - when(d2.isEnabled()).thenReturn(true); + when(d2.hasMessageDeliveryChannel()).thenReturn(true); final byte deviceId2 = 2; when(d2.getId()).thenReturn(deviceId2); @@ -316,7 +316,7 @@ public class ChangeNumberManagerTest { when(account.getPhoneNumberIdentifier()).thenReturn(pni); final Device d2 = mock(Device.class); - when(d2.isEnabled()).thenReturn(true); + when(d2.hasMessageDeliveryChannel()).thenReturn(true); final byte deviceId2 = 2; when(d2.getId()).thenReturn(deviceId2); @@ -361,7 +361,7 @@ public class ChangeNumberManagerTest { for (byte i = 1; i <= 3; i++) { final Device device = mock(Device.class); when(device.getId()).thenReturn(i); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getRegistrationId()).thenReturn((int) i); devices.add(device); @@ -400,7 +400,7 @@ public class ChangeNumberManagerTest { for (byte i = 1; i <= 3; i++) { final Device device = mock(Device.class); when(device.getId()).thenReturn(i); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getRegistrationId()).thenReturn((int) i); devices.add(device); @@ -439,7 +439,7 @@ public class ChangeNumberManagerTest { for (byte i = 1; i <= 3; i++) { final Device device = mock(Device.class); when(device.getId()).thenReturn(i); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getRegistrationId()).thenReturn((int) i); devices.add(device); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/DeviceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/DeviceTest.java index 87529d7c7..9619abe48 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/DeviceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/DeviceTest.java @@ -19,41 +19,22 @@ class DeviceTest { @ParameterizedTest @MethodSource - void testIsEnabled(final boolean primary, final boolean fetchesMessages, final String apnId, final String gcmId, - final Duration timeSinceLastSeen, final boolean expectEnabled) { - - final long lastSeen = System.currentTimeMillis() - timeSinceLastSeen.toMillis(); + void testHasMessageDeliveryChannel(final boolean fetchesMessages, final String apnId, final String gcmId, final boolean expectEnabled) { final Device device = new Device(); - device.setId(primary ? Device.PRIMARY_ID : Device.PRIMARY_ID + 1); device.setFetchesMessages(fetchesMessages); device.setApnId(apnId); device.setGcmId(gcmId); - device.setCreated(lastSeen); - device.setLastSeen(lastSeen); - assertEquals(expectEnabled, device.isEnabled()); + assertEquals(expectEnabled, device.hasMessageDeliveryChannel()); } - private static Stream testIsEnabled() { + private static Stream testHasMessageDeliveryChannel() { return Stream.of( - // primary fetchesMessages apnId gcmId lastSeen expectEnabled - Arguments.of(true, false, null, null, Duration.ofDays(60), false), - Arguments.of(true, false, null, null, Duration.ofDays(1), false), - Arguments.of(true, false, null, "gcm-id", Duration.ofDays(60), true), - Arguments.of(true, false, null, "gcm-id", Duration.ofDays(1), true), - Arguments.of(true, false, "apn-id", null, Duration.ofDays(60), true), - Arguments.of(true, false, "apn-id", null, Duration.ofDays(1), true), - Arguments.of(true, true, null, null, Duration.ofDays(60), true), - Arguments.of(true, true, null, null, Duration.ofDays(1), true), - Arguments.of(false, false, null, null, Duration.ofDays(60), false), - Arguments.of(false, false, null, null, Duration.ofDays(1), false), - Arguments.of(false, false, null, "gcm-id", Duration.ofDays(60), false), - Arguments.of(false, false, null, "gcm-id", Duration.ofDays(1), true), - Arguments.of(false, false, "apn-id", null, Duration.ofDays(60), false), - Arguments.of(false, false, "apn-id", null, Duration.ofDays(1), true), - Arguments.of(false, true, null, null, Duration.ofDays(60), false), - Arguments.of(false, true, null, null, Duration.ofDays(1), true) + Arguments.of(false, null, null, false), + Arguments.of(false, null, "gcm-id", true), + Arguments.of(false, "apn-id", null, true), + Arguments.of(true, null, null, true) ); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagePersisterTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagePersisterTest.java index 4bfb7bbcc..716311a39 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagePersisterTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagePersisterTest.java @@ -258,23 +258,23 @@ class MessagePersisterTest { final Device primary = mock(Device.class); when(primary.getId()).thenReturn((byte) 1); when(primary.isPrimary()).thenReturn(true); - when(primary.isEnabled()).thenReturn(true); + when(primary.hasMessageDeliveryChannel()).thenReturn(true); final Device activeA = mock(Device.class); when(activeA.getId()).thenReturn((byte) 2); - when(activeA.isEnabled()).thenReturn(true); + when(activeA.hasMessageDeliveryChannel()).thenReturn(true); final Device inactiveB = mock(Device.class); final byte inactiveId = 3; when(inactiveB.getId()).thenReturn(inactiveId); - when(inactiveB.isEnabled()).thenReturn(false); + when(inactiveB.hasMessageDeliveryChannel()).thenReturn(false); final Device inactiveC = mock(Device.class); when(inactiveC.getId()).thenReturn((byte) 4); - when(inactiveC.isEnabled()).thenReturn(false); + when(inactiveC.hasMessageDeliveryChannel()).thenReturn(false); final Device activeD = mock(Device.class); when(activeD.getId()).thenReturn((byte) 5); - when(activeD.isEnabled()).thenReturn(true); + when(activeD.hasMessageDeliveryChannel()).thenReturn(true); final Device destination = mock(Device.class); when(destination.getId()).thenReturn(DESTINATION_DEVICE_ID); - when(destination.isEnabled()).thenReturn(true); + when(destination.hasMessageDeliveryChannel()).thenReturn(true); when(destinationAccount.getDevices()).thenReturn(List.of(primary, activeA, inactiveB, inactiveC, activeD, destination)); @@ -302,27 +302,27 @@ class MessagePersisterTest { final byte primaryId = 1; when(primary.getId()).thenReturn(primaryId); when(primary.isPrimary()).thenReturn(true); - when(primary.isEnabled()).thenReturn(true); + when(primary.hasMessageDeliveryChannel()).thenReturn(true); when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(primary))) .thenReturn(Mono.just(4L)); final Device deviceA = mock(Device.class); final byte deviceIdA = 2; when(deviceA.getId()).thenReturn(deviceIdA); - when(deviceA.isEnabled()).thenReturn(true); + when(deviceA.hasMessageDeliveryChannel()).thenReturn(true); when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(deviceA))) .thenReturn(Mono.empty()); final Device deviceB = mock(Device.class); final byte deviceIdB = 3; when(deviceB.getId()).thenReturn(deviceIdB); - when(deviceB.isEnabled()).thenReturn(true); + when(deviceB.hasMessageDeliveryChannel()).thenReturn(true); when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(deviceB))) .thenReturn(Mono.just(2L)); final Device destination = mock(Device.class); when(destination.getId()).thenReturn(DESTINATION_DEVICE_ID); - when(destination.isEnabled()).thenReturn(true); + when(destination.hasMessageDeliveryChannel()).thenReturn(true); when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(destination))) .thenReturn(Mono.just(5L)); @@ -352,27 +352,27 @@ class MessagePersisterTest { final byte primaryId = 1; when(primary.getId()).thenReturn(primaryId); when(primary.isPrimary()).thenReturn(true); - when(primary.isEnabled()).thenReturn(true); + when(primary.hasMessageDeliveryChannel()).thenReturn(true); when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(primary))) .thenReturn(Mono.just(1L)); final Device deviceA = mock(Device.class); final byte deviceIdA = 2; when(deviceA.getId()).thenReturn(deviceIdA); - when(deviceA.isEnabled()).thenReturn(true); + when(deviceA.hasMessageDeliveryChannel()).thenReturn(true); when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(deviceA))) .thenReturn(Mono.just(3L)); final Device deviceB = mock(Device.class); final byte deviceIdB = 2; when(deviceB.getId()).thenReturn(deviceIdB); - when(deviceB.isEnabled()).thenReturn(true); + when(deviceB.hasMessageDeliveryChannel()).thenReturn(true); when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(deviceB))) .thenReturn(Mono.empty()); final Device destination = mock(Device.class); when(destination.getId()).thenReturn(DESTINATION_DEVICE_ID); - when(destination.isEnabled()).thenReturn(true); + when(destination.hasMessageDeliveryChannel()).thenReturn(true); when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(destination))) .thenReturn(Mono.just(2L)); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java index 3a0786575..ec39cc176 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java @@ -138,11 +138,11 @@ public class AuthHelper { when(VALID_DEVICE_3_PRIMARY.getId()).thenReturn(Device.PRIMARY_ID); when(VALID_DEVICE_3_LINKED.getId()).thenReturn((byte) 2); - when(VALID_DEVICE.isEnabled()).thenReturn(true); - when(VALID_DEVICE_TWO.isEnabled()).thenReturn(true); + when(VALID_DEVICE.hasMessageDeliveryChannel()).thenReturn(true); + when(VALID_DEVICE_TWO.hasMessageDeliveryChannel()).thenReturn(true); when(UNDISCOVERABLE_DEVICE.isPrimary()).thenReturn(true); - when(VALID_DEVICE_3_PRIMARY.isEnabled()).thenReturn(true); - when(VALID_DEVICE_3_LINKED.isEnabled()).thenReturn(true); + when(VALID_DEVICE_3_PRIMARY.hasMessageDeliveryChannel()).thenReturn(true); + when(VALID_DEVICE_3_LINKED.hasMessageDeliveryChannel()).thenReturn(true); when(VALID_ACCOUNT.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(VALID_DEVICE)); when(VALID_ACCOUNT.getPrimaryDevice()).thenReturn(VALID_DEVICE); @@ -279,7 +279,7 @@ public class AuthHelper { when(device.getAuthTokenHash()).thenReturn(saltedTokenHash); when(device.isPrimary()).thenReturn(true); when(device.getId()).thenReturn(Device.PRIMARY_ID); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(account.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(device)); when(account.getPrimaryDevice()).thenReturn(device); when(account.getNumber()).thenReturn(number); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/DevicesHelper.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/DevicesHelper.java index 82d837e8f..1f5181553 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/DevicesHelper.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/DevicesHelper.java @@ -6,9 +6,7 @@ package org.whispersystems.textsecuregcm.tests.util; import java.util.Random; -import org.signal.libsignal.protocol.ecc.Curve; import org.whispersystems.textsecuregcm.storage.Device; -import org.whispersystems.textsecuregcm.util.Util; public class DevicesHelper { @@ -48,13 +46,12 @@ public class DevicesHelper { public static void setEnabled(Device device, boolean enabled) { if (enabled) { device.setGcmId("testGcmId" + RANDOM.nextLong()); - device.setLastSeen(Util.todayInMillis()); } else { - device.setLastSeen(0); + device.setGcmId(null); } // fail fast, to guard against a change to the isEnabled() implementation causing unexpected test behavior - assert enabled == device.isEnabled(); + assert enabled == device.hasMessageDeliveryChannel(); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/util/DestinationDeviceValidatorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/util/DestinationDeviceValidatorTest.java index b78c6a71a..b80a8f3f4 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/util/DestinationDeviceValidatorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/util/DestinationDeviceValidatorTest.java @@ -107,7 +107,7 @@ class DestinationDeviceValidatorTest { enabledStateByDeviceId.forEach((deviceId, enabled) -> { final Device device = mock(Device.class); - when(device.isEnabled()).thenReturn(enabled); + when(device.hasMessageDeliveryChannel()).thenReturn(enabled); when(device.getId()).thenReturn(deviceId); when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/workers/ProcessPushNotificationFeedbackCommandTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/workers/ProcessPushNotificationFeedbackCommandTest.java index ca498c49d..95eee6b32 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/workers/ProcessPushNotificationFeedbackCommandTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/workers/ProcessPushNotificationFeedbackCommandTest.java @@ -98,7 +98,7 @@ class ProcessPushNotificationFeedbackCommandTest { final Account accountWithActiveDevice = mock(Account.class); { final Device device = mock(Device.class); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(accountWithActiveDevice.getDevices()).thenReturn(List.of(device)); } @@ -106,7 +106,7 @@ class ProcessPushNotificationFeedbackCommandTest { final Account accountWithUninstalledDevice = mock(Account.class); { final Device uninstalledDevice = mock(Device.class); - when(uninstalledDevice.isEnabled()).thenReturn(true); + when(uninstalledDevice.hasMessageDeliveryChannel()).thenReturn(true); when(uninstalledDevice.getUninstalledFeedbackTimestamp()) .thenReturn(clock.instant().minus(ProcessPushNotificationFeedbackCommand.MAX_TOKEN_REFRESH_DELAY.multipliedBy(2)).toEpochMilli()); @@ -116,7 +116,7 @@ class ProcessPushNotificationFeedbackCommandTest { final Account accountWithAlreadyDisabledUninstalledDevice = mock(Account.class); { final Device previouslyDisabledUninstalledDevice = mock(Device.class); - when(previouslyDisabledUninstalledDevice.isEnabled()).thenReturn(false); + when(previouslyDisabledUninstalledDevice.hasMessageDeliveryChannel()).thenReturn(false); when(previouslyDisabledUninstalledDevice.getUninstalledFeedbackTimestamp()) .thenReturn(clock.instant().minus(ProcessPushNotificationFeedbackCommand.MAX_TOKEN_REFRESH_DELAY.multipliedBy(2)).toEpochMilli()); @@ -196,7 +196,7 @@ class ProcessPushNotificationFeedbackCommandTest { { // Device is active, enabled, and has no push feedback final Device device = mock(Device.class); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getUninstalledFeedbackTimestamp()).thenReturn(0L); when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli()); @@ -206,7 +206,7 @@ class ProcessPushNotificationFeedbackCommandTest { { // Device is active, but not enabled, and has no push feedback final Device device = mock(Device.class); - when(device.isEnabled()).thenReturn(false); + when(device.hasMessageDeliveryChannel()).thenReturn(false); when(device.getUninstalledFeedbackTimestamp()).thenReturn(0L); when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli()); @@ -216,7 +216,7 @@ class ProcessPushNotificationFeedbackCommandTest { { // Device is active, enabled, and has "mature" push feedback final Device device = mock(Device.class); - when(device.isEnabled()).thenReturn(true); + when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getUninstalledFeedbackTimestamp()).thenReturn(maturePushFeedbackTimestamp); when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli()); @@ -226,7 +226,7 @@ class ProcessPushNotificationFeedbackCommandTest { { // Device is active, but not enabled, and has "mature" push feedback final Device device = mock(Device.class); - when(device.isEnabled()).thenReturn(false); + when(device.hasMessageDeliveryChannel()).thenReturn(false); when(device.getUninstalledFeedbackTimestamp()).thenReturn(maturePushFeedbackTimestamp); when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli()); @@ -236,7 +236,7 @@ class ProcessPushNotificationFeedbackCommandTest { { // Device is inactive, not enabled, and has "mature" push feedback final Device device = mock(Device.class); - when(device.isEnabled()).thenReturn(false); + when(device.hasMessageDeliveryChannel()).thenReturn(false); when(device.getUninstalledFeedbackTimestamp()).thenReturn(maturePushFeedbackTimestamp); when(device.getLastSeen()).thenReturn(maturePushFeedbackTimestamp);