From 0414da8c324588ec1669b1fd6b649164298deeb4 Mon Sep 17 00:00:00 2001 From: Katherine Date: Wed, 12 Jun 2024 13:54:06 -0400 Subject: [PATCH] Add delete sync capability --- .../java/org/signal/integration/TestUser.java | 2 +- .../controllers/DeviceController.java | 6 ++ .../controllers/ProfileController.java | 4 +- .../entities/UserCapabilities.java | 9 ++- .../grpc/DevicesGrpcService.java | 3 +- .../textsecuregcm/grpc/ProfileGrpcHelper.java | 3 +- .../textsecuregcm/storage/Account.java | 4 ++ .../textsecuregcm/storage/Device.java | 2 +- .../main/proto/org/signal/chat/device.proto | 1 + .../main/proto/org/signal/chat/profile.proto | 4 ++ .../controllers/DeviceControllerTest.java | 59 ++++++++++++++++++- .../controllers/ProfileControllerTest.java | 11 ++-- .../RegistrationControllerTest.java | 14 ++--- .../grpc/DevicesGrpcServiceTest.java | 7 ++- .../grpc/ProfileAnonymousGrpcServiceTest.java | 4 +- .../grpc/ProfileGrpcServiceTest.java | 2 +- ...ccountCreationDeletionIntegrationTest.java | 7 ++- .../textsecuregcm/storage/AccountTest.java | 27 ++++++++- ...ntsManagerChangeNumberIntegrationTest.java | 2 +- ...ConcurrentModificationIntegrationTest.java | 2 +- .../storage/AccountsManagerTest.java | 4 +- .../AddRemoveDeviceIntegrationTest.java | 6 +- .../tests/util/AccountsHelper.java | 1 + 23 files changed, 144 insertions(+), 40 deletions(-) diff --git a/integration-tests/src/main/java/org/signal/integration/TestUser.java b/integration-tests/src/main/java/org/signal/integration/TestUser.java index 88bab99e3..8a7dbeff5 100644 --- a/integration-tests/src/main/java/org/signal/integration/TestUser.java +++ b/integration-tests/src/main/java/org/signal/integration/TestUser.java @@ -126,7 +126,7 @@ public class TestUser { } public AccountAttributes accountAttributes() { - return new AccountAttributes(true, registrationId, pniRegistrationId, "".getBytes(StandardCharsets.UTF_8), "", true, new Device.DeviceCapabilities(false, false, false)) + return new AccountAttributes(true, registrationId, pniRegistrationId, "".getBytes(StandardCharsets.UTF_8), "", true, new Device.DeviceCapabilities(false, false, false, false)) .withUnidentifiedAccessKey(unidentifiedAccessKey) .withRecoveryPassword(registrationPassword); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java index 8a381e538..9b7770728 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java @@ -232,6 +232,8 @@ public class DeviceController { if (capabilities == null) { throw new WebApplicationException(Response.status(422, "Missing device capabilities").build()); + } else if (isCapabilityDowngrade(account, capabilities)) { + throw new WebApplicationException(Response.status(409).build()); } final String signalAgent; @@ -387,6 +389,10 @@ public class DeviceController { return Optional.of(aci); } + private static boolean isCapabilityDowngrade(Account account, DeviceCapabilities capabilities) { + return account.isDeleteSyncSupported() && !capabilities.deleteSync(); + } + private static String getUsedTokenKey(final String token) { return "usedToken::" + token; } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java index 1fa24ad21..8d435d5b9 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java @@ -447,7 +447,7 @@ public class ProfileController { return new BaseProfileResponse(account.getIdentityKey(IdentityType.ACI), account.getUnidentifiedAccessKey().map(UnidentifiedAccessChecksum::generateFor).orElse(null), account.isUnrestrictedUnidentifiedAccess(), - new UserCapabilities(), + UserCapabilities.createForAccount(account), profileBadgeConverter.convert( getAcceptableLanguagesForRequest(containerRequestContext), account.getBadges(), @@ -459,7 +459,7 @@ public class ProfileController { return new BaseProfileResponse(account.getIdentityKey(IdentityType.PNI), null, false, - new UserCapabilities(), + UserCapabilities.createForAccount(account), Collections.emptyList(), new PniServiceIdentifier(account.getPhoneNumberIdentifier())); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/UserCapabilities.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/UserCapabilities.java index 0b2387651..55192e1ec 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/UserCapabilities.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/UserCapabilities.java @@ -5,11 +5,14 @@ package org.whispersystems.textsecuregcm.entities; +import org.whispersystems.textsecuregcm.storage.Account; + public record UserCapabilities( // TODO: Remove the paymentActivation capability entirely sometime soon after 2024-06-30 - boolean paymentActivation) { + boolean paymentActivation, + boolean deleteSync) { - public UserCapabilities() { - this(true); + public static UserCapabilities createForAccount(final Account account) { + return new UserCapabilities(true, account.isDeleteSyncSupported()); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcService.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcService.java index 0c6cd0227..9954e36a3 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcService.java @@ -201,7 +201,8 @@ public class DevicesGrpcService extends ReactorDevicesGrpc.DevicesImplBase { d -> d.setCapabilities(new Device.DeviceCapabilities( request.getStorage(), request.getTransfer(), - request.getPaymentActivation()))))) + request.getPaymentActivation(), + request.getDeleteSync()))))) .thenReturn(SetCapabilitiesResponse.newBuilder().build()); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ProfileGrpcHelper.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ProfileGrpcHelper.java index ede2d78f6..c48775416 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ProfileGrpcHelper.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ProfileGrpcHelper.java @@ -83,6 +83,7 @@ public class ProfileGrpcHelper { static UserCapabilities buildUserCapabilities(final org.whispersystems.textsecuregcm.entities.UserCapabilities capabilities) { return UserCapabilities.newBuilder() .setPaymentActivation(capabilities.paymentActivation()) + .setDeleteSync(capabilities.deleteSync()) .build(); } @@ -104,7 +105,7 @@ public class ProfileGrpcHelper { final ProfileBadgeConverter profileBadgeConverter) { final GetUnversionedProfileResponse.Builder responseBuilder = GetUnversionedProfileResponse.newBuilder() .setIdentityKey(ByteString.copyFrom(targetAccount.getIdentityKey(targetIdentifier.identityType()).serialize())) - .setCapabilities(buildUserCapabilities(new org.whispersystems.textsecuregcm.entities.UserCapabilities())); + .setCapabilities(buildUserCapabilities(org.whispersystems.textsecuregcm.entities.UserCapabilities.createForAccount(targetAccount))); switch (targetIdentifier.identityType()) { case ACI -> { 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 87009c56c..80b4d7c2d 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java @@ -292,6 +292,10 @@ public class Account { return allEnabledDevicesHaveCapability(DeviceCapabilities::paymentActivation); } + public boolean isDeleteSyncSupported() { + return allEnabledDevicesHaveCapability(DeviceCapabilities::deleteSync); + } + private boolean allEnabledDevicesHaveCapability(final Predicate predicate) { requireNotStale(); 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 db41de17e..24c5d1073 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java @@ -248,6 +248,6 @@ public class Device { return this.userAgent; } - public record DeviceCapabilities(boolean storage, boolean transfer, boolean paymentActivation) { + public record DeviceCapabilities(boolean storage, boolean transfer, boolean paymentActivation, boolean deleteSync) { } } diff --git a/service/src/main/proto/org/signal/chat/device.proto b/service/src/main/proto/org/signal/chat/device.proto index 9fb4a8146..a6541a2f8 100644 --- a/service/src/main/proto/org/signal/chat/device.proto +++ b/service/src/main/proto/org/signal/chat/device.proto @@ -148,6 +148,7 @@ message SetCapabilitiesRequest { bool storage = 1; bool transfer = 2; bool paymentActivation = 3; + bool deleteSync = 4; } message SetCapabilitiesResponse {} diff --git a/service/src/main/proto/org/signal/chat/profile.proto b/service/src/main/proto/org/signal/chat/profile.proto index 1f1dab8f8..790d946b8 100644 --- a/service/src/main/proto/org/signal/chat/profile.proto +++ b/service/src/main/proto/org/signal/chat/profile.proto @@ -322,6 +322,10 @@ message UserCapabilities { * Whether all devices linked to the account support MobileCoin payments. */ bool payment_activation = 1; + /** + * Whether all devices linked to the account support delete syncing + */ + bool delete_sync = 2; } message Badge { 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 29d17dd02..28df19261 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DeviceControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DeviceControllerTest.java @@ -218,7 +218,8 @@ class DeviceControllerTest { when(asyncCommands.set(any(), any(), any())).thenReturn(MockRedisFuture.completedFuture(null)); - final AccountAttributes accountAttributes = new AccountAttributes(fetchesMessages, 1234, 5678, null, null, true, new DeviceCapabilities(true, true, true)); + final AccountAttributes accountAttributes = new AccountAttributes(fetchesMessages, 1234, 5678, null, + null, true, new DeviceCapabilities(true, true, true, false)); final LinkDeviceRequest request = new LinkDeviceRequest(deviceCode.verificationCode(), accountAttributes, @@ -264,6 +265,58 @@ class DeviceControllerTest { ); } + @ParameterizedTest + @MethodSource + void deviceDowngradeDeleteSync(final boolean accountSupportsDeleteSync, final boolean deviceSupportsDeleteSync, final int expectedStatus) { + when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(account)); + when(accountsManager.addDevice(any(), any())) + .thenReturn(CompletableFuture.completedFuture(new Pair<>(mock(Account.class), mock(Device.class)))); + + final Device primaryDevice = mock(Device.class); + when(primaryDevice.getId()).thenReturn(Device.PRIMARY_ID); + when(AuthHelper.VALID_ACCOUNT.getDevices()).thenReturn(List.of(primaryDevice)); + + final ECSignedPreKey aciSignedPreKey; + final ECSignedPreKey pniSignedPreKey; + final KEMSignedPreKey aciPqLastResortPreKey; + final KEMSignedPreKey pniPqLastResortPreKey; + + final ECKeyPair aciIdentityKeyPair = Curve.generateKeyPair(); + final ECKeyPair pniIdentityKeyPair = Curve.generateKeyPair(); + + aciSignedPreKey = KeysHelper.signedECPreKey(1, aciIdentityKeyPair); + pniSignedPreKey = KeysHelper.signedECPreKey(2, pniIdentityKeyPair); + aciPqLastResortPreKey = KeysHelper.signedKEMPreKey(3, aciIdentityKeyPair); + pniPqLastResortPreKey = KeysHelper.signedKEMPreKey(4, pniIdentityKeyPair); + + when(account.getIdentityKey(IdentityType.ACI)).thenReturn(new IdentityKey(aciIdentityKeyPair.getPublicKey())); + when(account.getIdentityKey(IdentityType.PNI)).thenReturn(new IdentityKey(pniIdentityKeyPair.getPublicKey())); + when(account.isDeleteSyncSupported()).thenReturn(accountSupportsDeleteSync); + + when(asyncCommands.set(any(), any(), any())).thenReturn(MockRedisFuture.completedFuture(null)); + + final LinkDeviceRequest request = new LinkDeviceRequest(deviceController.generateVerificationToken(AuthHelper.VALID_UUID), + new AccountAttributes(false, 1234, 5678, null, null, true, new DeviceCapabilities(true, true, true, deviceSupportsDeleteSync)), + new DeviceActivationRequest(aciSignedPreKey, pniSignedPreKey, aciPqLastResortPreKey, pniPqLastResortPreKey, Optional.empty(), Optional.of(new GcmRegistrationId("gcm-id")))); + + try (final Response response = resources.getJerseyTest() + .target("/v1/devices/link") + .request() + .header("Authorization", AuthHelper.getProvisioningAuthHeader(AuthHelper.VALID_NUMBER, "password1")) + .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE))) { + + assertEquals(expectedStatus, response.getStatus()); + } + } + + private static List deviceDowngradeDeleteSync() { + return List.of( + Arguments.of(true, true, 200), + Arguments.of(true, false, 409), + Arguments.of(false, true, 200), + Arguments.of(false, false, 200)); + } + @Test void linkDeviceAtomicBadCredentials() { when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(account)); @@ -633,7 +686,7 @@ class DeviceControllerTest { when(asyncCommands.set(any(), any(), any())).thenReturn(MockRedisFuture.completedFuture(null)); final LinkDeviceRequest request = new LinkDeviceRequest(deviceCode.verificationCode(), - new AccountAttributes(false, registrationId, pniRegistrationId, null, null, true, new DeviceCapabilities(true, true, true)), + new AccountAttributes(false, registrationId, pniRegistrationId, null, null, true, new DeviceCapabilities(true, true, true, false)), new DeviceActivationRequest(aciSignedPreKey, pniSignedPreKey, aciPqLastResortPreKey, pniPqLastResortPreKey, Optional.of(new ApnRegistrationId("apn", null)), Optional.empty())); try (final Response response = resources.getJerseyTest() @@ -692,7 +745,7 @@ class DeviceControllerTest { @Test void putCapabilitiesSuccessTest() { - final DeviceCapabilities deviceCapabilities = new DeviceCapabilities(true, true, true); + final DeviceCapabilities deviceCapabilities = new DeviceCapabilities(true, true, true, false); final Response response = resources .getJerseyTest() .target("/v1/devices/capabilities") 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 0e2f080bc..3e9f9710e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java @@ -144,6 +144,7 @@ class ProfileControllerTest { private DynamicPaymentsConfiguration dynamicPaymentsConfiguration; private Account profileAccount; + private Account capabilitiesAccount; private static final ResourceExtension resources = ResourceExtension.builder() .addProperty(ServerProperties.UNWRAP_COMPLETION_STAGE_IN_WRITER_ENABLE, Boolean.TRUE) @@ -203,12 +204,11 @@ class ProfileControllerTest { when(profileAccount.getUsernameHash()).thenReturn(Optional.of(USERNAME_HASH)); when(profileAccount.getUnidentifiedAccessKey()).thenReturn(Optional.of(UNIDENTIFIED_ACCESS_KEY)); - Account capabilitiesAccount = mock(Account.class); + capabilitiesAccount = mock(Account.class); 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.isPaymentActivationSupported()).thenReturn(false); when(capabilitiesAccount.isEnabled()).thenReturn(true); when(accountsManager.getByServiceIdentifier(any())).thenReturn(Optional.empty()); @@ -439,14 +439,17 @@ class ProfileControllerTest { assertThat(response.getStatus()).isEqualTo(401); } - @Test - void testProfileCapabilities() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testProfileCapabilities(final boolean isDeleteSyncSupported) { + when(capabilitiesAccount.isDeleteSyncSupported()).thenReturn(isDeleteSyncSupported); final BaseProfileResponse profile = resources.getJerseyTest() .target("/v1/profile/" + AuthHelper.VALID_UUID) .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) .get(BaseProfileResponse.class); + assertEquals(isDeleteSyncSupported, profile.getCapabilities().deleteSync()); assertThat(profile.getCapabilities().paymentActivation()).isTrue(); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RegistrationControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RegistrationControllerTest.java index 113971d09..bd1eb3516 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RegistrationControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RegistrationControllerTest.java @@ -477,10 +477,10 @@ class RegistrationControllerTest { } final AccountAttributes fetchesMessagesAccountAttributes = - new AccountAttributes(true, 1, 1, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false)); + new AccountAttributes(true, 1, 1, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false, false)); final AccountAttributes pushAccountAttributes = - new AccountAttributes(false, 1, 1, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false)); + new AccountAttributes(false, 1, 1, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false, false)); return Stream.of( // "Fetches messages" is true, but an APNs token is provided @@ -566,7 +566,7 @@ class RegistrationControllerTest { } final AccountAttributes accountAttributes = - new AccountAttributes(true, 1, 1, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false)); + new AccountAttributes(true, 1, 1, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false, false)); return Stream.of( // Signed PNI EC pre-key is missing @@ -736,13 +736,13 @@ class RegistrationControllerTest { final int registrationId = 1; final int pniRegistrationId = 2; - final Device.DeviceCapabilities deviceCapabilities = new Device.DeviceCapabilities(false, false, false); + final Device.DeviceCapabilities deviceCapabilities = new Device.DeviceCapabilities(false, false, false, false); final AccountAttributes fetchesMessagesAccountAttributes = - new AccountAttributes(true, registrationId, pniRegistrationId, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false)); + new AccountAttributes(true, registrationId, pniRegistrationId, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false, false)); final AccountAttributes pushAccountAttributes = - new AccountAttributes(false, registrationId, pniRegistrationId, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false)); + new AccountAttributes(false, registrationId, pniRegistrationId, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false, false)); final String apnsToken = "apns-token"; final String apnsVoipToken = "apns-voip-token"; @@ -857,7 +857,7 @@ class RegistrationControllerTest { final IdentityKey pniIdentityKey = new IdentityKey(pniIdentityKeyPair.getPublicKey()); final AccountAttributes accountAttributes = new AccountAttributes(true, registrationId, pniRegistrationId, "name".getBytes(StandardCharsets.UTF_8), "reglock", - true, new Device.DeviceCapabilities(true, true, true)); + true, new Device.DeviceCapabilities(true, true, true, false)); final RegistrationRequest request = new RegistrationRequest( Base64.getEncoder().encodeToString(sessionId.getBytes(StandardCharsets.UTF_8)), diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcServiceTest.java index ca0532958..277112725 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcServiceTest.java @@ -393,7 +393,8 @@ class DevicesGrpcServiceTest extends SimpleBaseGrpcTest 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" ->