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 053c31964..7e174f66a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProvider.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProvider.java @@ -22,12 +22,12 @@ import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.util.Pair; /** - * This {@link WebsocketRefreshRequirementProvider} observes intra-request changes in {@link Account#isEnabled()} and + * This {@link WebsocketRefreshRequirementProvider} observes intra-request changes in * {@link Device#hasMessageDeliveryChannel()}. *

- * 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. + * If a change in 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. * * @see AuthenticatedAccount */ @@ -48,9 +48,8 @@ public class AuthEnablementRefreshRequirementProvider implements WebsocketRefres @Override public void handleRequestFiltered(final RequestEvent requestEvent) { if (requestEvent.getUriInfo().getMatchedResourceMethod().getInvocable().getHandlingMethod().getAnnotation(ChangesDeviceEnabledState.class) != null) { - // The authenticated principal, if any, will be available after filters have run. - // Now that the account is known, capture a snapshot of `isEnabled` for the account's devices before carrying out - // the request’s business logic. + // The authenticated principal, if any, will be available after filters have run. Now that the account is known, + // capture a snapshot of the account's devices before carrying out the request’s business logic. ContainerRequestUtil.getAuthenticatedAccount(requestEvent.getContainerRequest()).ifPresent(account -> setAccount(requestEvent.getContainerRequest(), account)); } @@ -66,8 +65,8 @@ public class AuthEnablementRefreshRequirementProvider implements WebsocketRefres @Override public List> handleRequestFinished(final RequestEvent requestEvent) { - // Now that the request is finished, check whether `isEnabled` changed for any of the devices. If the value did - // change or if a devices was added or removed, all devices must disconnect and reauthenticate. + // Now that the request is finished, check whether `hasMessageDeliveryChannel` changed for any of the devices. If + // the value did change or if a devices was added or removed, all devices must disconnect and reauthenticate. if (requestEvent.getContainerRequest().getProperty(DEVICES_ENABLED) != null) { @SuppressWarnings("unchecked") final Map initialDevicesEnabled = 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 21435bbe9..aa6997677 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/OptionalAccess.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/OptionalAccess.java @@ -18,6 +18,8 @@ import java.util.Optional; @SuppressWarnings("OptionalUsedAsFieldOrParameterType") public class OptionalAccess { + public static String ALL_DEVICES_SELECTOR = "*"; + public static void verify(Optional requestAccount, Optional accessKey, Optional targetAccount, @@ -26,12 +28,12 @@ public class OptionalAccess { try { verify(requestAccount, accessKey, targetAccount); - if (!deviceSelector.equals("*")) { + if (!ALL_DEVICES_SELECTOR.equals(deviceSelector)) { byte deviceId = Byte.parseByte(deviceSelector); Optional targetDevice = targetAccount.get().getDevice(deviceId); - if (targetDevice.isPresent() && targetDevice.get().hasMessageDeliveryChannel()) { + if (targetDevice.isPresent()) { return; } @@ -48,11 +50,10 @@ public class OptionalAccess { public static void verify(Optional requestAccount, Optional accessKey, - Optional targetAccount) - { + Optional targetAccount) { if (requestAccount.isPresent()) { - // Authenticated requests are never unauthorized; if the target exists and is enabled, return OK, otherwise throw not-found. - if (targetAccount.isPresent() && targetAccount.get().isEnabled()) { + // Authenticated requests are never unauthorized; if the target exists, return OK, otherwise throw not-found. + if (targetAccount.isPresent()) { return; } else { throw new NotFoundException(); @@ -63,7 +64,7 @@ public class OptionalAccess { // has unrestricted unidentified access, callers need to supply a fake access key. Likewise, if // the target account does not exist, we *also* report unauthorized here (*not* not-found, // since that would provide a free exists check). - if (accessKey.isEmpty() || !targetAccount.map(Account::isEnabled).orElse(false)) { + if (accessKey.isEmpty() || targetAccount.isEmpty()) { throw new NotAuthorizedException(Response.Status.UNAUTHORIZED); } 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 2c517902c..dafea9314 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java @@ -313,7 +313,7 @@ public class KeysController { @ApiResponse(responseCode = "200", description = "Indicates at least one prekey was available for at least one requested device.", useReturnTypeSchema = true) @ApiResponse(responseCode = "400", description = "A group send endorsement and other authorization (account authentication or unidentified-access key) were both provided.") @ApiResponse(responseCode = "401", description = "Account authentication check failed and unidentified-access key or group send endorsement token was not supplied or invalid.") - @ApiResponse(responseCode = "404", description = "Requested identity or device does not exist, is not active, or has no available prekeys.") + @ApiResponse(responseCode = "404", description = "Requested identity or device does not exist or device has no available prekeys.") @ApiResponse(responseCode = "429", description = "Rate limit exceeded.", headers = @Header( name = "Retry-After", description = "If present, a positive integer indicating the number of seconds before a subsequent attempt could succeed")) @@ -440,11 +440,11 @@ public class KeysController { private List parseDeviceId(String deviceId, Account account) { if (deviceId.equals("*")) { - return account.getDevices().stream().filter(Device::hasMessageDeliveryChannel).toList(); + return account.getDevices(); } try { byte id = Byte.parseByte(deviceId); - return account.getDevice(id).filter(Device::hasMessageDeliveryChannel).map(List::of).orElse(List.of()); + return account.getDevice(id).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/controllers/MessageController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java index 768d9a267..091f7fd0e 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java @@ -369,7 +369,7 @@ public class MessageController { OptionalAccess.verify(source.map(AuthenticatedAccount::getAccount), accessKey, destination); } - boolean needsSync = !isSyncMessage && source.isPresent() && source.get().getAccount().hasEnabledLinkedDevice(); + boolean needsSync = !isSyncMessage && source.isPresent() && source.get().getAccount().getDevices().size() > 1; // We return 200 when stories are sent to a non-existent account. Since story sends bypass OptionalAccess.verify // we leak information about whether a destination UUID exists if we return any other code (e.g. 404) from 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 792e31a90..526d2f786 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,6 @@ class KeysGrpcHelper { : Flux.from(Mono.justOrEmpty(targetAccount.getDevice(targetDeviceId))); return devices - .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 1678e33da..959211179 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java @@ -303,12 +303,6 @@ public class Account { .allMatch(device -> device.getCapabilities() != null && predicate.test(device.getCapabilities())); } - public boolean isEnabled() { - requireNotStale(); - - return getPrimaryDevice().hasMessageDeliveryChannel(); - } - public byte getNextDeviceId() { requireNotStale(); @@ -325,14 +319,6 @@ public class Account { return candidateId; } - public boolean hasEnabledLinkedDevice() { - requireNotStale(); - - return devices.stream() - .filter(d -> Device.PRIMARY_ID != d.getId()) - .anyMatch(Device::hasMessageDeliveryChannel); - } - public void setIdentityKey(final IdentityKey identityKey) { requireNotStale(); @@ -503,12 +489,6 @@ public class Account { this.discoverableByPhoneNumber = discoverableByPhoneNumber; } - public boolean shouldBeVisibleInDirectory() { - requireNotStale(); - - return isEnabled() && isDiscoverableByPhoneNumber(); - } - public int getVersion() { requireNotStale(); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java index 56036c77b..c9ffae3ce 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java @@ -443,7 +443,7 @@ public class Accounts extends AbstractDynamoDbStore { .expressionAttributeValues(Map.of( ":number", numberAttr, ":data", accountDataAttributeValue(account), - ":cds", AttributeValues.fromBool(account.shouldBeVisibleInDirectory()), + ":cds", AttributeValues.fromBool(account.isDiscoverableByPhoneNumber()), ":pni", pniAttr, ":version", AttributeValues.fromInt(account.getVersion()), ":version_increment", AttributeValues.fromInt(1))) @@ -924,7 +924,7 @@ public class Accounts extends AbstractDynamoDbStore { final Map attrValues = new HashMap<>(Map.of( ":data", accountDataAttributeValue(account), - ":cds", AttributeValues.fromBool(account.shouldBeVisibleInDirectory()), + ":cds", AttributeValues.fromBool(account.isDiscoverableByPhoneNumber()), ":version", AttributeValues.fromInt(account.getVersion()), ":version_increment", AttributeValues.fromInt(1))); @@ -1359,7 +1359,7 @@ public class Accounts extends AbstractDynamoDbStore { ATTR_PNI_UUID, pniUuidAttr, ATTR_ACCOUNT_DATA, accountDataAttributeValue(account), ATTR_VERSION, AttributeValues.fromInt(account.getVersion()), - ATTR_CANONICALLY_DISCOVERABLE, AttributeValues.fromBool(account.shouldBeVisibleInDirectory()))); + ATTR_CANONICALLY_DISCOVERABLE, AttributeValues.fromBool(account.isDiscoverableByPhoneNumber()))); // Add the UAK if it's in the account account.getUnidentifiedAccessKey() 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 52cc6917c..bb24ff387 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,6 @@ public class AccountsManager { account.getDevices() .stream() - .filter(Device::hasMessageDeliveryChannel) .forEach(device -> device.setPhoneNumberIdentityRegistrationId(pniRegistrationIds.get(device.getId()))); account.setPhoneNumberIdentityKey(pniIdentityKey); 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 c13cfced1..ad26b0ec4 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,6 @@ public class DestinationDeviceValidator { final Set excludedDeviceIds) throws MismatchedDevicesException { final Set accountDeviceIds = account.getDevices().stream() - .filter(Device::hasMessageDeliveryChannel) .map(Device::getId) .filter(deviceId -> !excludedDeviceIds.contains(deviceId)) .collect(Collectors.toSet()); @@ -97,6 +96,12 @@ public class DestinationDeviceValidator { final Set missingDeviceIds = new HashSet<>(accountDeviceIds); missingDeviceIds.removeAll(messageDeviceIds); + // Temporarily "excuse" missing devices if they're missing a message delivery channel as a transitional measure + missingDeviceIds.removeAll(account.getDevices().stream() + .filter(device -> !device.hasMessageDeliveryChannel()) + .map(Device::getId) + .collect(Collectors.toSet())); + final Set extraDeviceIds = new HashSet<>(messageDeviceIds); extraDeviceIds.removeAll(accountDeviceIds); 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 19071dde3..cf01132f2 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/auth/AccountAuthenticatorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/auth/AccountAuthenticatorTest.java @@ -161,7 +161,6 @@ class AccountAuthenticatorTest { when(accountsManager.getByAccountIdentifier(uuid)).thenReturn(Optional.of(account)); when(account.getUuid()).thenReturn(uuid); when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); - when(account.isEnabled()).thenReturn(true); when(device.getId()).thenReturn(deviceId); when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getAuthTokenHash()).thenReturn(credentials); @@ -191,7 +190,6 @@ class AccountAuthenticatorTest { when(accountsManager.getByAccountIdentifier(uuid)).thenReturn(Optional.of(account)); when(account.getUuid()).thenReturn(uuid); when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); - when(account.isEnabled()).thenReturn(true); when(device.getId()).thenReturn(deviceId); when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getAuthTokenHash()).thenReturn(credentials); @@ -224,7 +222,6 @@ class AccountAuthenticatorTest { when(accountsManager.getByAccountIdentifier(uuid)).thenReturn(Optional.of(account)); when(account.getUuid()).thenReturn(uuid); when(account.getDevice(deviceId)).thenReturn(Optional.of(authenticatedDevice)); - when(account.isEnabled()).thenReturn(accountEnabled); when(authenticatedDevice.getId()).thenReturn(deviceId); when(authenticatedDevice.hasMessageDeliveryChannel()).thenReturn(deviceEnabled); when(authenticatedDevice.getAuthTokenHash()).thenReturn(credentials); @@ -260,7 +257,6 @@ class AccountAuthenticatorTest { when(accountsManager.getByAccountIdentifier(uuid)).thenReturn(Optional.of(account)); when(account.getUuid()).thenReturn(uuid); when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); - when(account.isEnabled()).thenReturn(true); when(device.getId()).thenReturn(deviceId); when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getAuthTokenHash()).thenReturn(credentials); @@ -297,7 +293,6 @@ class AccountAuthenticatorTest { when(accountsManager.getByAccountIdentifier(uuid)).thenReturn(Optional.of(account)); when(account.getUuid()).thenReturn(uuid); when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); - when(account.isEnabled()).thenReturn(true); when(device.getId()).thenReturn(deviceId); when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getAuthTokenHash()).thenReturn(credentials); @@ -325,7 +320,6 @@ class AccountAuthenticatorTest { when(accountsManager.getByAccountIdentifier(uuid)).thenReturn(Optional.of(account)); when(account.getUuid()).thenReturn(uuid); when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); - when(account.isEnabled()).thenReturn(true); when(device.getId()).thenReturn(deviceId); when(device.hasMessageDeliveryChannel()).thenReturn(true); when(device.getAuthTokenHash()).thenReturn(credentials); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/auth/OptionalAccessTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/auth/OptionalAccessTest.java index 06f96671b..c5c9c6852 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/auth/OptionalAccessTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/auth/OptionalAccessTest.java @@ -5,152 +5,141 @@ package org.whispersystems.textsecuregcm.auth; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.nio.charset.StandardCharsets; import java.util.Base64; +import java.util.List; import java.util.Optional; +import java.util.OptionalInt; import javax.ws.rs.WebApplicationException; -import org.junit.jupiter.api.Test; +import org.apache.commons.lang3.RandomStringUtils; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.whispersystems.textsecuregcm.storage.Account; +import org.whispersystems.textsecuregcm.storage.Device; class OptionalAccessTest { - @Test - void testUnidentifiedMissingTarget() { - try { - OptionalAccess.verify(Optional.empty(), Optional.empty(), Optional.empty()); - throw new AssertionError("should fail"); - } catch (WebApplicationException e) { - assertEquals(e.getResponse().getStatus(), 401); - } + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") + @ParameterizedTest + @MethodSource + void verify(final Optional requestAccount, + final Optional accessKey, + final Optional targetAccount, + final String deviceSelector, + final OptionalInt expectedStatusCode) { + + expectedStatusCode.ifPresentOrElse(statusCode -> { + final WebApplicationException webApplicationException = assertThrows(WebApplicationException.class, + () -> OptionalAccess.verify(requestAccount, accessKey, targetAccount, deviceSelector)); + + assertEquals(statusCode, webApplicationException.getResponse().getStatus()); + }, () -> assertDoesNotThrow(() -> OptionalAccess.verify(requestAccount, accessKey, targetAccount, deviceSelector))); } - @Test - void testUnidentifiedMissingTargetDevice() { - Account account = mock(Account.class); - when(account.isEnabled()).thenReturn(true); - when(account.getDevice(eq((byte) 10))).thenReturn(Optional.empty()); - when(account.getUnidentifiedAccessKey()).thenReturn(Optional.of("1234".getBytes())); + private static List verify() { + final String unidentifiedAccessKey = RandomStringUtils.randomAlphanumeric(16); - try { - OptionalAccess.verify(Optional.empty(), Optional.of(new Anonymous(Base64.getEncoder().encodeToString("1234".getBytes()))), Optional.of(account), "10"); - } catch (WebApplicationException e) { - assertEquals(e.getResponse().getStatus(), 401); - } - } + final Anonymous correctUakHeader = + new Anonymous(Base64.getEncoder().encodeToString(unidentifiedAccessKey.getBytes())); - @Test - void testUnidentifiedBadTargetDevice() { - Account account = mock(Account.class); - when(account.isEnabled()).thenReturn(true); - when(account.getDevice(eq((byte) 10))).thenReturn(Optional.empty()); - when(account.getUnidentifiedAccessKey()).thenReturn(Optional.of("1234".getBytes())); + final Anonymous incorrectUakHeader = + new Anonymous(Base64.getEncoder().encodeToString((unidentifiedAccessKey + "-incorrect").getBytes())); - try { - OptionalAccess.verify(Optional.empty(), Optional.of(new Anonymous(Base64.getEncoder().encodeToString("1234".getBytes()))), Optional.of(account), "$$"); - } catch (WebApplicationException e) { - assertEquals(e.getResponse().getStatus(), 422); - } - } + final Account targetAccount = mock(Account.class); + when(targetAccount.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(mock(Device.class))); + when(targetAccount.getUnidentifiedAccessKey()) + .thenReturn(Optional.of(unidentifiedAccessKey.getBytes(StandardCharsets.UTF_8))); + final Account allowAllTargetAccount = mock(Account.class); + when(allowAllTargetAccount.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(mock(Device.class))); + when(allowAllTargetAccount.isUnrestrictedUnidentifiedAccess()).thenReturn(true); - @Test - void testUnidentifiedBadCode() { - Account account = mock(Account.class); - when(account.isEnabled()).thenReturn(true); - when(account.getUnidentifiedAccessKey()).thenReturn(Optional.of("1234".getBytes())); + final Account noUakTargetAccount = mock(Account.class); + when(noUakTargetAccount.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(mock(Device.class))); + when(noUakTargetAccount.getUnidentifiedAccessKey()).thenReturn(Optional.empty()); - try { - OptionalAccess.verify(Optional.empty(), Optional.of(new Anonymous(Base64.getEncoder().encodeToString("5678".getBytes()))), Optional.of(account)); - throw new AssertionError("should fail"); - } catch (WebApplicationException e) { - assertEquals(e.getResponse().getStatus(), 401); - } - } + final Account inactiveTargetAccount = mock(Account.class); + when(inactiveTargetAccount.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(mock(Device.class))); + when(inactiveTargetAccount.getUnidentifiedAccessKey()) + .thenReturn(Optional.of(unidentifiedAccessKey.getBytes(StandardCharsets.UTF_8))); - @Test - void testIdentifiedMissingTarget() { - Account account = mock(Account.class); - when(account.isEnabled()).thenReturn(true); + return List.of( + // Unidentified caller; correct UAK + Arguments.of(Optional.empty(), + Optional.of(correctUakHeader), + Optional.of(targetAccount), + OptionalAccess.ALL_DEVICES_SELECTOR, + OptionalInt.empty()), - try { - OptionalAccess.verify(Optional.of(account), Optional.empty(), Optional.empty()); - throw new AssertionError("should fail"); - } catch (WebApplicationException e) { - assertEquals(e.getResponse().getStatus(), 404); - } - } + // Identified caller; no UAK needed + Arguments.of(Optional.of(mock(Account.class)), + Optional.empty(), + Optional.of(targetAccount), + OptionalAccess.ALL_DEVICES_SELECTOR, + OptionalInt.empty()), - @Test - void testUnsolicitedBadTarget() { - Account account = mock(Account.class); - when(account.isUnrestrictedUnidentifiedAccess()).thenReturn(false); - when(account.isEnabled()).thenReturn(true); + // Unidentified caller; target account not found + Arguments.of(Optional.empty(), + Optional.empty(), + Optional.empty(), + OptionalAccess.ALL_DEVICES_SELECTOR, + OptionalInt.of(401)), - try { - OptionalAccess.verify(Optional.empty(), Optional.empty(), Optional.of(account)); - throw new AssertionError("should fail"); - } catch (WebApplicationException e) { - assertEquals(e.getResponse().getStatus(), 401); - } - } + // Identified caller; target account not found + Arguments.of(Optional.of(mock(Account.class)), + Optional.empty(), + Optional.empty(), + OptionalAccess.ALL_DEVICES_SELECTOR, + OptionalInt.of(404)), - @Test - void testUnsolicitedGoodTarget() { - Account account = mock(Account.class); - Anonymous random = mock(Anonymous.class); - when(account.isUnrestrictedUnidentifiedAccess()).thenReturn(true); - when(account.isEnabled()).thenReturn(true); - OptionalAccess.verify(Optional.empty(), Optional.of(random), Optional.of(account)); - } + // Unidentified caller; target account found, but target device not found + Arguments.of(Optional.empty(), + Optional.of(correctUakHeader), + Optional.of(targetAccount), + String.valueOf(Device.PRIMARY_ID + 1), + OptionalInt.of(401)), - @Test - void testUnidentifiedGoodTarget() { - Account account = mock(Account.class); - when(account.getUnidentifiedAccessKey()).thenReturn(Optional.of("1234".getBytes())); - when(account.isEnabled()).thenReturn(true); - OptionalAccess.verify(Optional.empty(), Optional.of(new Anonymous(Base64.getEncoder().encodeToString("1234".getBytes()))), Optional.of(account)); - } + // Unidentified caller; target account found, but incorrect UAK provided + Arguments.of(Optional.empty(), + Optional.of(incorrectUakHeader), + Optional.of(targetAccount), + OptionalAccess.ALL_DEVICES_SELECTOR, + OptionalInt.of(401)), - @Test - void testUnidentifiedTargetMissingAccessKey() { - Account account = mock(Account.class); - when(account.getUnidentifiedAccessKey()).thenReturn(Optional.empty()); - when(account.isEnabled()).thenReturn(true); - try { - OptionalAccess.verify( - Optional.empty(), - Optional.of(new Anonymous(Base64.getEncoder().encodeToString("1234".getBytes()))), - Optional.of(account)); - throw new AssertionError("should fail"); - } catch (WebApplicationException e) { - assertEquals(e.getResponse().getStatus(), 401); - } - } + // Unidentified caller; target account found, but has no UAK + Arguments.of(Optional.empty(), + Optional.of(correctUakHeader), + Optional.of(noUakTargetAccount), + OptionalAccess.ALL_DEVICES_SELECTOR, + OptionalInt.of(401)), - @Test - void testUnidentifiedInactive() { - Account account = mock(Account.class); - when(account.getUnidentifiedAccessKey()).thenReturn(Optional.of("1234".getBytes())); - when(account.isEnabled()).thenReturn(false); + // Unidentified caller; target account found, allows unrestricted unidentified access + Arguments.of(Optional.empty(), + Optional.of(incorrectUakHeader), + Optional.of(allowAllTargetAccount), + OptionalAccess.ALL_DEVICES_SELECTOR, + OptionalInt.empty()), - try { - OptionalAccess.verify(Optional.empty(), Optional.of(new Anonymous(Base64.getEncoder().encodeToString("1234".getBytes()))), Optional.of(account)); - throw new AssertionError(); - } catch (WebApplicationException e) { - assertEquals(e.getResponse().getStatus(), 401); - } - } + // Unidentified caller; target account found, but inactive + Arguments.of(Optional.empty(), + Optional.of(correctUakHeader), + Optional.of(inactiveTargetAccount), + OptionalAccess.ALL_DEVICES_SELECTOR, + OptionalInt.empty()), - @Test - void testIdentifiedGoodTarget() { - Account source = mock(Account.class); - Account target = mock(Account.class); - when(target.isEnabled()).thenReturn(true); - OptionalAccess.verify(Optional.of(source), Optional.empty(), Optional.of(target)); + // Malformed device ID + Arguments.of(Optional.empty(), + Optional.of(correctUakHeader), + Optional.of(targetAccount), + "not a valid identifier", + OptionalInt.of(422)) + ); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DeviceControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DeviceControllerTest.java index 28df19261..6f7306cef 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DeviceControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DeviceControllerTest.java @@ -143,7 +143,6 @@ class DeviceControllerTest { when(account.getNumber()).thenReturn(AuthHelper.VALID_NUMBER); when(account.getUuid()).thenReturn(AuthHelper.VALID_UUID); when(account.getPhoneNumberIdentifier()).thenReturn(AuthHelper.VALID_PNI); - when(account.isEnabled()).thenReturn(false); when(account.isPaymentActivationSupported()).thenReturn(false); when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(account)); 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 27bab634c..f7db14d1d 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java @@ -240,7 +240,6 @@ class KeysControllerTest { when(existsAccount.getDevice(sampleDevice4Id)).thenReturn(Optional.of(sampleDevice4)); when(existsAccount.getDevice((byte) 22)).thenReturn(Optional.empty()); when(existsAccount.getDevices()).thenReturn(allDevices); - when(existsAccount.isEnabled()).thenReturn(true); when(existsAccount.getIdentityKey(IdentityType.ACI)).thenReturn(IDENTITY_KEY); when(existsAccount.getIdentityKey(IdentityType.PNI)).thenReturn(PNI_IDENTITY_KEY); when(existsAccount.getNumber()).thenReturn(EXISTS_NUMBER); @@ -676,7 +675,7 @@ class KeysControllerTest { .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) .get(PreKeyResponse.class); - assertThat(results.getDevicesCount()).isEqualTo(3); + assertThat(results.getDevicesCount()).isEqualTo(4); assertThat(results.getIdentityKey()).isEqualTo(existsAccount.getIdentityKey(IdentityType.ACI)); ECSignedPreKey signedPreKey = results.getDevice(SAMPLE_DEVICE_ID).getSignedPreKey(); @@ -711,12 +710,15 @@ class KeysControllerTest { verify(KEYS).takeEC(EXISTS_UUID, SAMPLE_DEVICE_ID); verify(KEYS).takeEC(EXISTS_UUID, SAMPLE_DEVICE_ID2); + verify(KEYS).takeEC(EXISTS_UUID, SAMPLE_DEVICE_ID3); verify(KEYS).takeEC(EXISTS_UUID, SAMPLE_DEVICE_ID4); verify(KEYS).takePQ(EXISTS_UUID, SAMPLE_DEVICE_ID); verify(KEYS).takePQ(EXISTS_UUID, SAMPLE_DEVICE_ID2); + verify(KEYS).takePQ(EXISTS_UUID, SAMPLE_DEVICE_ID3); verify(KEYS).takePQ(EXISTS_UUID, SAMPLE_DEVICE_ID4); verify(KEYS).getEcSignedPreKey(EXISTS_UUID, SAMPLE_DEVICE_ID); verify(KEYS).getEcSignedPreKey(EXISTS_UUID, SAMPLE_DEVICE_ID2); + verify(KEYS).getEcSignedPreKey(EXISTS_UUID, SAMPLE_DEVICE_ID3); verify(KEYS).getEcSignedPreKey(EXISTS_UUID, SAMPLE_DEVICE_ID4); verifyNoMoreInteractions(KEYS); } @@ -746,7 +748,7 @@ class KeysControllerTest { .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) .get(PreKeyResponse.class); - assertThat(results.getDevicesCount()).isEqualTo(3); + assertThat(results.getDevicesCount()).isEqualTo(4); assertThat(results.getIdentityKey()).isEqualTo(existsAccount.getIdentityKey(IdentityType.ACI)); ECSignedPreKey signedPreKey = results.getDevice(SAMPLE_DEVICE_ID).getSignedPreKey(); @@ -789,10 +791,13 @@ class KeysControllerTest { verify(KEYS).takePQ(EXISTS_UUID, SAMPLE_DEVICE_ID); verify(KEYS).takeEC(EXISTS_UUID, SAMPLE_DEVICE_ID2); verify(KEYS).takePQ(EXISTS_UUID, SAMPLE_DEVICE_ID2); + verify(KEYS).takeEC(EXISTS_UUID, SAMPLE_DEVICE_ID3); + verify(KEYS).takePQ(EXISTS_UUID, SAMPLE_DEVICE_ID3); verify(KEYS).takeEC(EXISTS_UUID, SAMPLE_DEVICE_ID4); verify(KEYS).takePQ(EXISTS_UUID, SAMPLE_DEVICE_ID4); verify(KEYS).getEcSignedPreKey(EXISTS_UUID, SAMPLE_DEVICE_ID); verify(KEYS).getEcSignedPreKey(EXISTS_UUID, SAMPLE_DEVICE_ID2); + verify(KEYS).getEcSignedPreKey(EXISTS_UUID, SAMPLE_DEVICE_ID3); verify(KEYS).getEcSignedPreKey(EXISTS_UUID, SAMPLE_DEVICE_ID4); verifyNoMoreInteractions(KEYS); } 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 bc6cc87f5..7a80fafd0 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java @@ -1584,18 +1584,20 @@ class MessageControllerTest { void sendMultiRecipientMessageMismatchedDevices(final ServiceIdentifier serviceIdentifier) throws JsonProcessingException { + final byte extraDeviceId = MULTI_DEVICE_ID3 + 1; + final List recipients = List.of( new Recipient(serviceIdentifier, MULTI_DEVICE_ID1, MULTI_DEVICE_REG_ID1, new byte[48]), new Recipient(serviceIdentifier, MULTI_DEVICE_ID2, MULTI_DEVICE_REG_ID2, new byte[48]), - new Recipient(serviceIdentifier, MULTI_DEVICE_ID3, MULTI_DEVICE_REG_ID3, new byte[48])); + new Recipient(serviceIdentifier, MULTI_DEVICE_ID3, MULTI_DEVICE_REG_ID3, new byte[48]), + new Recipient(serviceIdentifier, extraDeviceId, 1234, new byte[48])); // initialize our binary payload and create an input stream - byte[] buffer = new byte[2048]; - // InputStream stream = initializeMultiPayload(recipientUUID, buffer); - InputStream stream = initializeMultiPayload(recipients, buffer, true); + final byte[] buffer = new byte[2048]; + final InputStream stream = initializeMultiPayload(recipients, buffer, true); // set up the entity to use in our PUT request - Entity entity = Entity.entity(stream, MultiRecipientMessageProvider.MEDIA_TYPE); + final Entity entity = Entity.entity(stream, MultiRecipientMessageProvider.MEDIA_TYPE); // start building the request final Invocation.Builder invocationBuilder = resources @@ -1619,7 +1621,7 @@ class MessageControllerTest { .constructCollectionType(List.class, AccountMismatchedDevices.class)); assertEquals(List.of(new AccountMismatchedDevices(serviceIdentifier, - new MismatchedDevices(Collections.emptyList(), List.of(MULTI_DEVICE_ID3)))), + new MismatchedDevices(Collections.emptyList(), List.of(extraDeviceId)))), mismatchedDevices); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java index 3e9f9710e..a36421b32 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java @@ -199,7 +199,6 @@ class ProfileControllerTest { when(profileAccount.getIdentityKey(IdentityType.PNI)).thenReturn(ACCOUNT_TWO_PHONE_NUMBER_IDENTITY_KEY); when(profileAccount.getUuid()).thenReturn(AuthHelper.VALID_UUID_TWO); when(profileAccount.getPhoneNumberIdentifier()).thenReturn(AuthHelper.VALID_PNI_TWO); - when(profileAccount.isEnabled()).thenReturn(true); when(profileAccount.getCurrentProfileVersion()).thenReturn(Optional.empty()); when(profileAccount.getUsernameHash()).thenReturn(Optional.of(USERNAME_HASH)); when(profileAccount.getUnidentifiedAccessKey()).thenReturn(Optional.of(UNIDENTIFIED_ACCESS_KEY)); @@ -209,7 +208,6 @@ class ProfileControllerTest { when(capabilitiesAccount.getUuid()).thenReturn(AuthHelper.VALID_UUID); when(capabilitiesAccount.getIdentityKey(IdentityType.ACI)).thenReturn(ACCOUNT_IDENTITY_KEY); when(capabilitiesAccount.getIdentityKey(IdentityType.PNI)).thenReturn(ACCOUNT_PHONE_NUMBER_IDENTITY_KEY); - when(capabilitiesAccount.isEnabled()).thenReturn(true); when(accountsManager.getByServiceIdentifier(any())).thenReturn(Optional.empty()); @@ -1012,7 +1010,6 @@ class ProfileControllerTest { final Account account = mock(Account.class); when(account.getUuid()).thenReturn(AuthHelper.VALID_UUID); when(account.getCurrentProfileVersion()).thenReturn(Optional.of(versionHex("version"))); - when(account.isEnabled()).thenReturn(true); when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(account)); when(profilesManager.get(any(), any())).thenReturn(Optional.empty()); @@ -1168,7 +1165,6 @@ class ProfileControllerTest { final Account account = mock(Account.class); when(account.getUuid()).thenReturn(AuthHelper.VALID_UUID); when(account.getCurrentProfileVersion()).thenReturn(Optional.of(version)); - when(account.isEnabled()).thenReturn(true); when(account.getUnidentifiedAccessKey()).thenReturn(Optional.of(UNIDENTIFIED_ACCESS_KEY)); final Instant expiration = Instant.now().plus(org.whispersystems.textsecuregcm.util.ProfileHelper.EXPIRING_PROFILE_KEY_CREDENTIAL_EXPIRATION) @@ -1234,7 +1230,6 @@ class ProfileControllerTest { final Account account = mock(Account.class); when(account.getUuid()).thenReturn(AuthHelper.VALID_UUID); - when(account.isEnabled()).thenReturn(true); when(account.getUnidentifiedAccessKey()).thenReturn(Optional.of(UNIDENTIFIED_ACCESS_KEY)); when(accountsManager.getByServiceIdentifier(new AciServiceIdentifier(AuthHelper.VALID_UUID))).thenReturn(Optional.of(account)); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/KeysAnonymousGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/KeysAnonymousGrpcServiceTest.java index 1ae81f07b..04a66701e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/KeysAnonymousGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/KeysAnonymousGrpcServiceTest.java @@ -9,8 +9,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyByte; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; import static org.whispersystems.textsecuregcm.grpc.GrpcTestUtils.assertStatusException; import com.google.protobuf.ByteString; @@ -26,7 +26,6 @@ import java.util.Map; import java.util.Optional; import java.util.UUID; import java.util.concurrent.CompletableFuture; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountTest.java index af3af0a24..634de1071 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountTest.java @@ -96,32 +96,6 @@ class AccountTest { } - @Test - void testIsEnabled() { - final Device enabledPrimaryDevice = mock(Device.class); - final Device enabledLinkedDevice = mock(Device.class); - final Device disabledPrimaryDevice = mock(Device.class); - final Device disabledLinkedDevice = mock(Device.class); - - when(enabledPrimaryDevice.hasMessageDeliveryChannel()).thenReturn(true); - when(enabledLinkedDevice.hasMessageDeliveryChannel()).thenReturn(true); - when(disabledPrimaryDevice.hasMessageDeliveryChannel()).thenReturn(false); - when(disabledLinkedDevice.hasMessageDeliveryChannel()).thenReturn(false); - - when(enabledPrimaryDevice.getId()).thenReturn(Device.PRIMARY_ID); - final byte deviceId2 = 2; - when(enabledLinkedDevice.getId()).thenReturn(deviceId2); - when(disabledPrimaryDevice.getId()).thenReturn(Device.PRIMARY_ID); - when(disabledLinkedDevice.getId()).thenReturn(deviceId2); - - assertTrue(AccountsHelper.generateTestAccount("+14151234567", List.of(enabledPrimaryDevice)).isEnabled()); - assertTrue(AccountsHelper.generateTestAccount("+14151234567", List.of(enabledPrimaryDevice, enabledLinkedDevice)).isEnabled()); - assertTrue(AccountsHelper.generateTestAccount("+14151234567", List.of(enabledPrimaryDevice, disabledLinkedDevice)).isEnabled()); - assertFalse(AccountsHelper.generateTestAccount("+14151234567", List.of(disabledPrimaryDevice)).isEnabled()); - assertFalse(AccountsHelper.generateTestAccount("+14151234567", List.of(disabledPrimaryDevice, enabledLinkedDevice)).isEnabled()); - assertFalse(AccountsHelper.generateTestAccount("+14151234567", List.of(disabledPrimaryDevice, disabledLinkedDevice)).isEnabled()); - } - @Test void testIsTransferSupported() { final Device transferCapablePrimaryDevice = mock(Device.class); @@ -308,49 +282,4 @@ class AccountTest { final JsonFilter jsonFilterAnnotation = (JsonFilter) maybeJsonFilterAnnotation.get(); assertEquals(Account.class.getSimpleName(), jsonFilterAnnotation.value()); } - - @ParameterizedTest - @MethodSource - public void testHasEnabledLinkedDevice(final Account account, final boolean expect) { - assertEquals(expect, account.hasEnabledLinkedDevice()); - } - - static Stream testHasEnabledLinkedDevice() { - final Device enabledPrimary = mock(Device.class); - when(enabledPrimary.hasMessageDeliveryChannel()).thenReturn(true); - when(enabledPrimary.getId()).thenReturn(Device.PRIMARY_ID); - - final Device disabledPrimary = mock(Device.class); - when(disabledPrimary.getId()).thenReturn(Device.PRIMARY_ID); - - final byte linked1DeviceId = Device.PRIMARY_ID + 1; - final Device enabledLinked1 = mock(Device.class); - when(enabledLinked1.hasMessageDeliveryChannel()).thenReturn(true); - when(enabledLinked1.getId()).thenReturn(linked1DeviceId); - - final Device disabledLinked1 = mock(Device.class); - when(disabledLinked1.getId()).thenReturn(linked1DeviceId); - - final byte linked2DeviceId = Device.PRIMARY_ID + 2; - final Device enabledLinked2 = mock(Device.class); - when(enabledLinked2.hasMessageDeliveryChannel()).thenReturn(true); - when(enabledLinked2.getId()).thenReturn(linked2DeviceId); - - final Device disabledLinked2 = mock(Device.class); - when(disabledLinked2.getId()).thenReturn(linked2DeviceId); - - return Stream.of( - Arguments.of(AccountsHelper.generateTestAccount("+14155550123", List.of(enabledPrimary)), false), - Arguments.of(AccountsHelper.generateTestAccount("+14155550123", List.of(enabledPrimary, disabledLinked1)), - false), - Arguments.of(AccountsHelper.generateTestAccount("+14155550123", - List.of(enabledPrimary, disabledLinked1, disabledLinked2)), false), - Arguments.of(AccountsHelper.generateTestAccount("+14155550123", - List.of(enabledPrimary, enabledLinked1, disabledLinked2)), true), - Arguments.of(AccountsHelper.generateTestAccount("+14155550123", - List.of(enabledPrimary, disabledLinked1, enabledLinked2)), true), - Arguments.of(AccountsHelper.generateTestAccount("+14155550123", - List.of(disabledLinked2, enabledLinked1, enabledLinked2)), true) - ); - } } 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 91a61b076..46948092c 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -1066,11 +1066,13 @@ class AccountsManagerTest { final ECKeyPair identityKeyPair = Curve.generateKeyPair(); final Map newSignedKeys = Map.of( Device.PRIMARY_ID, KeysHelper.signedECPreKey(1, identityKeyPair), - deviceId2, KeysHelper.signedECPreKey(2, identityKeyPair)); + deviceId2, KeysHelper.signedECPreKey(2, identityKeyPair), + deviceId3, KeysHelper.signedECPreKey(3, identityKeyPair)); final Map newSignedPqKeys = Map.of( - Device.PRIMARY_ID, KeysHelper.signedKEMPreKey(3, identityKeyPair), - deviceId2, KeysHelper.signedKEMPreKey(4, identityKeyPair)); - final Map newRegistrationIds = Map.of(Device.PRIMARY_ID, 201, deviceId2, 202); + Device.PRIMARY_ID, KeysHelper.signedKEMPreKey(4, identityKeyPair), + deviceId2, KeysHelper.signedKEMPreKey(5, identityKeyPair), + deviceId3, KeysHelper.signedKEMPreKey(6, identityKeyPair)); + final Map newRegistrationIds = Map.of(Device.PRIMARY_ID, 201, deviceId2, 202, deviceId3, 203); final Account existingAccount = AccountsHelper.generateTestAccount(targetNumber, existingAccountUuid, targetPni, new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); when(accounts.getByE164(targetNumber)).thenReturn(Optional.of(existingAccount)); @@ -1097,7 +1099,9 @@ class AccountsManagerTest { verify(keysManager).getPqEnabledDevices(uuid); verify(keysManager).buildWriteItemForEcSignedPreKey(eq(newPni), eq(Device.PRIMARY_ID), any()); verify(keysManager).buildWriteItemForEcSignedPreKey(eq(newPni), eq(deviceId2), any()); + verify(keysManager).buildWriteItemForEcSignedPreKey(eq(newPni), eq(deviceId3), any()); verify(keysManager).buildWriteItemForLastResortKey(eq(newPni), eq(Device.PRIMARY_ID), any()); + verify(keysManager).buildWriteItemForLastResortKey(eq(newPni), eq(deviceId3), any()); verifyNoMoreInteractions(keysManager); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java index e10487f27..b8c74beb5 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java @@ -315,7 +315,7 @@ class AccountsTest { Accounts.ATTR_ACCOUNT_E164, AttributeValues.fromString(account.getNumber()), Accounts.ATTR_ACCOUNT_DATA, AttributeValues.fromByteArray(SystemMapper.jsonMapper().writeValueAsBytes(account)), Accounts.ATTR_VERSION, AttributeValues.fromInt(account.getVersion()), - Accounts.ATTR_CANONICALLY_DISCOVERABLE, AttributeValues.fromBool(account.shouldBeVisibleInDirectory()))) + Accounts.ATTR_CANONICALLY_DISCOVERABLE, AttributeValues.fromBool(account.isDiscoverableByPhoneNumber()))) .build()) .build(); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java index 041d4518f..42ccef8aa 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java @@ -145,12 +145,10 @@ public class AccountsHelper { case "getDevices" -> when(updatedAccount.getDevices()).thenAnswer(stubbing); case "getDevice" -> when(updatedAccount.getDevice(stubbing.getInvocation().getArgument(0))).thenAnswer(stubbing); case "getPrimaryDevice" -> when(updatedAccount.getPrimaryDevice()).thenAnswer(stubbing); - case "isEnabled" -> when(updatedAccount.isEnabled()).thenAnswer(stubbing); case "isDiscoverableByPhoneNumber" -> when(updatedAccount.isDiscoverableByPhoneNumber()).thenAnswer(stubbing); case "getNextDeviceId" -> when(updatedAccount.getNextDeviceId()).thenAnswer(stubbing); case "isPaymentActivationSupported" -> when(updatedAccount.isPaymentActivationSupported()).thenAnswer(stubbing); case "isDeleteSyncSupported" -> when(updatedAccount.isDeleteSyncSupported()).thenAnswer(stubbing); - case "hasEnabledLinkedDevice" -> when(updatedAccount.hasEnabledLinkedDevice()).thenAnswer(stubbing); case "getRegistrationLock" -> when(updatedAccount.getRegistrationLock()).thenAnswer(stubbing); case "getIdentityKey" -> when(updatedAccount.getIdentityKey(stubbing.getInvocation().getArgument(0))).thenAnswer(stubbing); 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 ec39cc176..53b4a40c4 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 @@ -159,8 +159,6 @@ public class AuthHelper { when(UNDISCOVERABLE_ACCOUNT.getDevices()).thenReturn(List.of(UNDISCOVERABLE_DEVICE)); when(VALID_ACCOUNT_3.getDevices()).thenReturn(List.of(VALID_DEVICE_3_PRIMARY, VALID_DEVICE_3_LINKED)); - when(VALID_ACCOUNT_TWO.hasEnabledLinkedDevice()).thenReturn(true); - when(VALID_ACCOUNT.getNumber()).thenReturn(VALID_NUMBER); when(VALID_ACCOUNT.getUuid()).thenReturn(VALID_UUID); when(VALID_ACCOUNT.getPhoneNumberIdentifier()).thenReturn(VALID_PNI); @@ -180,11 +178,6 @@ public class AuthHelper { when(VALID_ACCOUNT_3.getIdentifier(IdentityType.ACI)).thenReturn(VALID_UUID_3); when(VALID_ACCOUNT_3.getIdentifier(IdentityType.PNI)).thenReturn(VALID_PNI_3); - when(VALID_ACCOUNT.isEnabled()).thenReturn(true); - when(VALID_ACCOUNT_TWO.isEnabled()).thenReturn(true); - when(UNDISCOVERABLE_ACCOUNT.isEnabled()).thenReturn(true); - when(VALID_ACCOUNT_3.isEnabled()).thenReturn(true); - when(VALID_ACCOUNT.isDiscoverableByPhoneNumber()).thenReturn(true); when(VALID_ACCOUNT_TWO.isDiscoverableByPhoneNumber()).thenReturn(true); when(UNDISCOVERABLE_ACCOUNT.isDiscoverableByPhoneNumber()).thenReturn(false); @@ -284,7 +277,6 @@ public class AuthHelper { when(account.getPrimaryDevice()).thenReturn(device); when(account.getNumber()).thenReturn(number); when(account.getUuid()).thenReturn(uuid); - when(account.isEnabled()).thenReturn(true); when(accountsManager.getByE164(number)).thenReturn(Optional.of(account)); when(accountsManager.getByAccountIdentifier(uuid)).thenReturn(Optional.of(account)); } 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 b80a8f3f4..63d89f570 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/util/DestinationDeviceValidatorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/util/DestinationDeviceValidatorTest.java @@ -119,35 +119,44 @@ class DestinationDeviceValidatorTest { return account; } - static Stream validateCompleteDeviceListSource() { + static Stream validateCompleteDeviceList() { final byte id1 = 1; final byte id2 = 2; final byte id3 = 3; return Stream.of( + // Device IDs provided for all enabled devices arguments( mockAccountWithDeviceAndEnabled(Map.of(id1, true, id2, false, id3, true)), Set.of(id1, id3), null, null, Collections.emptySet()), + + // Device ID provided for disabled device arguments( mockAccountWithDeviceAndEnabled(Map.of(id1, true, id2, false, id3, true)), Set.of(id1, id2, id3), null, - Set.of(id2), + null, Collections.emptySet()), + + // Device ID omitted for enabled device arguments( mockAccountWithDeviceAndEnabled(Map.of(id1, true, id2, false, id3, true)), Set.of(id1), Set.of(id3), null, Collections.emptySet()), + + // Device ID included for disabled device, omitted for enabled device arguments( mockAccountWithDeviceAndEnabled(Map.of(id1, true, id2, false, id3, true)), Set.of(id1, id2), Set.of(id3), - Set.of(id2), + null, Collections.emptySet()), + + // Device ID omitted for enabled device, included for device in excluded list arguments( mockAccountWithDeviceAndEnabled(Map.of(id1, true, id2, false, id3, true)), Set.of(id1), @@ -155,13 +164,17 @@ class DestinationDeviceValidatorTest { Set.of(id1), Set.of(id1) ), + + // Device ID omitted for enabled device, included for disabled device, omitted for excluded device arguments( mockAccountWithDeviceAndEnabled(Map.of(id1, true, id2, false, id3, true)), Set.of(id2), Set.of(id3), - Set.of(id2), + null, Set.of(id1) ), + + // Device ID included for enabled device, omitted for excluded device arguments( mockAccountWithDeviceAndEnabled(Map.of(id1, true, id2, false, id3, true)), Set.of(id3), @@ -173,8 +186,8 @@ class DestinationDeviceValidatorTest { } @ParameterizedTest - @MethodSource("validateCompleteDeviceListSource") - void testValidateCompleteDeviceList( + @MethodSource + void validateCompleteDeviceList( Account account, Set deviceIds, Collection expectedMissingDeviceIds, diff --git a/service/src/test/resources/fixtures/current_message_extra_device.json b/service/src/test/resources/fixtures/current_message_extra_device.json index cd9b8fcb8..3d6712bb8 100644 --- a/service/src/test/resources/fixtures/current_message_extra_device.json +++ b/service/src/test/resources/fixtures/current_message_extra_device.json @@ -10,5 +10,11 @@ "destinationDeviceId" : 3, "content" : "Zm9vYmFyego", "timestamp" : 1234 - }] + }, + { + "type" : 1, + "destinationDeviceId" : 4, + "content" : "Zm9vYmFyego", + "timestamp" : 1234 + }] } diff --git a/service/src/test/resources/fixtures/missing_device_response2.json b/service/src/test/resources/fixtures/missing_device_response2.json index 960900cc5..9203abd6e 100644 --- a/service/src/test/resources/fixtures/missing_device_response2.json +++ b/service/src/test/resources/fixtures/missing_device_response2.json @@ -1,4 +1,4 @@ { "missingDevices" : [2], - "extraDevices" : [3] -} \ No newline at end of file + "extraDevices" : [4] +}