From 4c9efdb936f38918fab350a176676c900adfd3bf Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Wed, 6 Dec 2023 17:13:38 -0500 Subject: [PATCH] Revert "Revert "Represent device names as byte arrays"" This reverts commit 45848e7bfe1813924c677ab06402fb921913d8b2. --- .../java/org/signal/integration/TestUser.java | 3 +- .../entities/AccountAttributes.java | 11 ++-- .../textsecuregcm/entities/DeviceInfo.java | 13 ++++- .../textsecuregcm/entities/DeviceName.java | 11 ++-- .../grpc/DevicesGrpcService.java | 6 +-- .../textsecuregcm/storage/Device.java | 10 ++-- .../util/DeviceNameByteArrayAdapter.java | 53 +++++++++++++++++++ ...blementRefreshRequirementProviderTest.java | 7 ++- .../controllers/DeviceControllerTest.java | 5 +- .../RegistrationControllerTest.java | 14 ++--- .../grpc/DevicesGrpcServiceTest.java | 7 +-- .../AccountCreationIntegrationTest.java | 12 +++-- ...ntsManagerChangeNumberIntegrationTest.java | 3 +- ...ConcurrentModificationIntegrationTest.java | 3 +- .../storage/AccountsManagerTest.java | 8 +-- .../textsecuregcm/storage/AccountsTest.java | 6 +-- .../util/DeviceNameByteArrayAdapterTest.java | 51 ++++++++++++++++++ 17 files changed, 177 insertions(+), 46 deletions(-) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/DeviceNameByteArrayAdapter.java create mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/util/DeviceNameByteArrayAdapterTest.java 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 07be08071..8a7dbeff5 100644 --- a/integration-tests/src/main/java/org/signal/integration/TestUser.java +++ b/integration-tests/src/main/java/org/signal/integration/TestUser.java @@ -10,6 +10,7 @@ import static java.util.Objects.requireNonNull; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import java.security.SecureRandom; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; import java.util.Map; @@ -125,7 +126,7 @@ public class TestUser { } public AccountAttributes accountAttributes() { - return new AccountAttributes(true, registrationId, pniRegistrationId, "", "", true, new Device.DeviceCapabilities(false, 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/entities/AccountAttributes.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountAttributes.java index 2236b3572..5ed0c95c9 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountAttributes.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountAttributes.java @@ -8,6 +8,7 @@ import static org.whispersystems.textsecuregcm.util.RegistrationIdValidator.vali import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.google.common.annotations.VisibleForTesting; import java.util.Optional; import javax.annotation.Nullable; @@ -30,8 +31,10 @@ public class AccountAttributes { private int phoneNumberIdentityRegistrationId; @JsonProperty - @Size(max = 204, message = "This field must be less than 50 characters") - private String name; + @JsonSerialize(using = ByteArrayAdapter.Serializing.class) + @JsonDeserialize(using = ByteArrayAdapter.Deserializing.class) + @Size(max = 225) + private byte[] name; @JsonProperty private String registrationLock; @@ -62,7 +65,7 @@ public class AccountAttributes { final boolean fetchesMessages, final int registrationId, final int phoneNumberIdentifierRegistrationId, - final String name, + final byte[] name, final String registrationLock, final boolean discoverableByPhoneNumber, final DeviceCapabilities capabilities) { @@ -87,7 +90,7 @@ public class AccountAttributes { return phoneNumberIdentityRegistrationId; } - public String getName() { + public byte[] getName() { return name; } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/DeviceInfo.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/DeviceInfo.java index 189a0b39b..da66df7d6 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/DeviceInfo.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/DeviceInfo.java @@ -5,5 +5,16 @@ package org.whispersystems.textsecuregcm.entities; -public record DeviceInfo(long id, String name, long lastSeen, long created) { +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import org.whispersystems.textsecuregcm.util.ByteArrayAdapter; + +public record DeviceInfo(long id, + + @JsonSerialize(using = ByteArrayAdapter.Serializing.class) + @JsonDeserialize(using = ByteArrayAdapter.Deserializing.class) + byte[] name, + + long lastSeen, + long created) { } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/DeviceName.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/DeviceName.java index 498c4032b..ebebc1335 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/DeviceName.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/DeviceName.java @@ -6,19 +6,24 @@ package org.whispersystems.textsecuregcm.entities; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import org.whispersystems.textsecuregcm.util.ByteArrayAdapter; import javax.validation.constraints.NotEmpty; import javax.validation.constraints.Size; public class DeviceName { @JsonProperty + @JsonSerialize(using = ByteArrayAdapter.Serializing.class) + @JsonDeserialize(using = ByteArrayAdapter.Deserializing.class) @NotEmpty - @Size(max = 300, message = "This field must be less than 300 characters") - private String deviceName; + @Size(max = 225) + private byte[] deviceName; public DeviceName() {} - public String getDeviceName() { + public byte[] getDeviceName() { return deviceName; } } 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 ea726f92e..fa67e1ff1 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcService.java @@ -60,8 +60,8 @@ public class DevicesGrpcService extends ReactorDevicesGrpc.DevicesImplBase { .reduce(GetDevicesResponse.newBuilder(), (builder, device) -> { final GetDevicesResponse.LinkedDevice.Builder linkedDeviceBuilder = GetDevicesResponse.LinkedDevice.newBuilder(); - if (StringUtils.isNotBlank(device.getName())) { - linkedDeviceBuilder.setName(ByteString.copyFrom(Base64.getDecoder().decode(device.getName()))); + if (device.getName() != null) { + linkedDeviceBuilder.setName(ByteString.copyFrom(device.getName())); } return builder.addDevices(linkedDeviceBuilder @@ -105,7 +105,7 @@ public class DevicesGrpcService extends ReactorDevicesGrpc.DevicesImplBase { return Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier())) .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) .flatMap(account -> Mono.fromFuture(() -> accountsManager.updateDeviceAsync(account, authenticatedDevice.deviceId(), - device -> device.setName(Base64.getEncoder().encodeToString(request.getName().toByteArray()))))) + device -> device.setName(request.getName().toByteArray())))) .thenReturn(SetDeviceNameResponse.newBuilder().build()); } 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 ad66fb7b4..790bce398 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java @@ -7,6 +7,7 @@ package org.whispersystems.textsecuregcm.storage; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; import java.util.List; import java.util.OptionalInt; import java.util.concurrent.TimeUnit; @@ -17,6 +18,7 @@ import org.apache.commons.lang3.StringUtils; import org.whispersystems.textsecuregcm.auth.SaltedTokenHash; import org.whispersystems.textsecuregcm.entities.ECSignedPreKey; import org.whispersystems.textsecuregcm.identity.IdentityType; +import org.whispersystems.textsecuregcm.util.DeviceNameByteArrayAdapter; public class Device { @@ -31,7 +33,9 @@ public class Device { private byte id; @JsonProperty - private String name; + @JsonSerialize(using = DeviceNameByteArrayAdapter.Serializer.class) + @JsonDeserialize(using = DeviceNameByteArrayAdapter.Deserializer.class) + private byte[] name; @JsonProperty private String authToken; @@ -146,11 +150,11 @@ public class Device { this.id = id; } - public String getName() { + public byte[] getName() { return name; } - public void setName(String name) { + public void setName(byte[] name) { this.name = name; } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/DeviceNameByteArrayAdapter.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/DeviceNameByteArrayAdapter.java new file mode 100644 index 000000000..a0758ddc4 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/DeviceNameByteArrayAdapter.java @@ -0,0 +1,53 @@ +package org.whispersystems.textsecuregcm.util; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonSerializer; +import com.fasterxml.jackson.databind.SerializerProvider; +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.Metrics; +import org.whispersystems.textsecuregcm.metrics.MetricsUtil; +import java.io.IOException; +import java.util.Base64; + +/** + * Serializes a byte array as a standard Base64-encoded string and deserializes Base64-encoded strings to byte arrays, + * but treats any string that cannot be parsed as Base64 to {@code null}. + *

+ * Historically, device names were passed around as weakly-typed strings with the expectation that clients would provide + * Base64 strings, but nothing in the server ever verified that was the case. In the absence of strict validation, some + * third-party clients started submitting unencrypted names for devices, and so device names in persistent storage are a + * mix of Base64-encoded device name ciphertexts from first-party clients and plaintext device names from third-party + * clients. This adapter will discard the latter. + */ +public class DeviceNameByteArrayAdapter { + + private static final Counter UNPARSEABLE_DEVICE_NAME_COUNTER = + Metrics.counter(MetricsUtil.name(DeviceNameByteArrayAdapter.class, "unparseableDeviceName")); + + public static class Serializer extends JsonSerializer { + @Override + public void serialize(final byte[] bytes, + final JsonGenerator jsonGenerator, + final SerializerProvider serializerProvider) throws IOException { + + jsonGenerator.writeString(Base64.getEncoder().encodeToString(bytes)); + } + } + + public static class Deserializer extends JsonDeserializer { + @Override + public byte[] deserialize(final JsonParser jsonParser, + final DeserializationContext deserializationContext) throws IOException { + + try { + return Base64.getDecoder().decode(jsonParser.getValueAsString()); + } catch (final IllegalArgumentException e) { + UNPARSEABLE_DEVICE_NAME_COUNTER.increment(); + return null; + } + } + } +} 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 b96d80bed..21a865c58 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProviderTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/auth/AuthEnablementRefreshRequirementProviderTest.java @@ -221,7 +221,10 @@ class AuthEnablementRefreshRequirementProviderTest { final int initialDeviceCount = account.getDevices().size(); - final List addedDeviceNames = List.of("newDevice1", "newDevice2"); + final List addedDeviceNames = List.of( + Base64.getEncoder().encodeToString("newDevice1".getBytes(StandardCharsets.UTF_8)), + Base64.getEncoder().encodeToString("newDevice2".getBytes(StandardCharsets.UTF_8))); + final Response response = resources.getJerseyTest() .target("/v1/test/account/devices") .request() @@ -450,7 +453,7 @@ class AuthEnablementRefreshRequirementProviderTest { @PUT @Path("/account/devices") @ChangesDeviceEnabledState - public String addDevices(@Auth TestPrincipal auth, List deviceNames) { + public String addDevices(@Auth TestPrincipal auth, List deviceNames) { deviceNames.forEach(name -> { final Device device = DevicesHelper.createDevice(auth.getAccount().getNextDeviceId()); 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 029e78db0..a2e52ddf5 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DeviceControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DeviceControllerTest.java @@ -712,11 +712,10 @@ class DeviceControllerTest { .request() .header("Authorization", AuthHelper.getProvisioningAuthHeader(AuthHelper.VALID_NUMBER, "password1")) .put(Entity.entity(new AccountAttributes(false, 1234, 5678, - "this is a really long name that is longer than 80 characters it's so long that it's even longer than 204 characters. that's a lot of characters. we're talking lots and lots and lots of characters. 12345678", - null, true, null), + TestRandomUtil.nextBytes(226), null, true, null), MediaType.APPLICATION_JSON_TYPE)); - assertEquals(response.getStatus(), 422); + assertEquals(422, response.getStatus()); verifyNoMoreInteractions(messagesManager); } 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 34c774bfb..388981350 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", null, true, new Device.DeviceCapabilities(false, 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", null, true, new Device.DeviceCapabilities(false, 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", null, true, new Device.DeviceCapabilities(false, 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 @@ -719,7 +719,7 @@ class RegistrationControllerTest { && a.isUnrestrictedUnidentifiedAccess() == b.isUnrestrictedUnidentifiedAccess() && a.isDiscoverableByPhoneNumber() == b.isDiscoverableByPhoneNumber() && Objects.equals(a.getPhoneNumberIdentityRegistrationId(), b.getPhoneNumberIdentityRegistrationId()) - && Objects.equals(a.getName(), b.getName()) + && Arrays.equals(a.getName(), b.getName()) && Objects.equals(a.getRegistrationLock(), b.getRegistrationLock()) && Arrays.equals(a.getUnidentifiedAccessKey(), b.getUnidentifiedAccessKey()) && Objects.equals(a.getCapabilities(), b.getCapabilities()) @@ -746,10 +746,10 @@ class RegistrationControllerTest { } final AccountAttributes fetchesMessagesAccountAttributes = - new AccountAttributes(true, 1, 1, "test", null, true, new Device.DeviceCapabilities(false, 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", null, true, new Device.DeviceCapabilities(false, false, false, false)); + new AccountAttributes(false, 1, 1, "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"; @@ -861,7 +861,7 @@ class RegistrationControllerTest { final IdentityKey aciIdentityKey = new IdentityKey(aciIdentityKeyPair.getPublicKey()); final IdentityKey pniIdentityKey = new IdentityKey(pniIdentityKeyPair.getPublicKey()); - final AccountAttributes accountAttributes = new AccountAttributes(true, registrationId, pniRegistrationId, "name", "reglock", + final AccountAttributes accountAttributes = new AccountAttributes(true, registrationId, pniRegistrationId, "name".getBytes(StandardCharsets.UTF_8), "reglock", true, new Device.DeviceCapabilities(true, true, true, true)); final RegistrationRequest request = new RegistrationRequest( 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 679a49652..1d38fb9d7 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/DevicesGrpcServiceTest.java @@ -10,7 +10,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyByte; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.whispersystems.textsecuregcm.grpc.GrpcTestUtils.assertStatusException; @@ -21,7 +20,6 @@ import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; -import java.util.Base64; import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -123,8 +121,7 @@ class DevicesGrpcServiceTest extends SimpleBaseGrpcTest badges, @@ -398,7 +400,7 @@ public class AccountCreationIntegrationTest { assertEquals(deliveryChannels.fetchesMessages(), primaryDevice.getFetchesMessages()); assertEquals(registrationId, primaryDevice.getRegistrationId()); assertEquals(pniRegistrationId, primaryDevice.getPhoneNumberIdentityRegistrationId().orElseThrow()); - assertEquals(deviceName, primaryDevice.getName()); + assertArrayEquals(deviceName, primaryDevice.getName()); assertEquals(discoverableByPhoneNumber, account.isDiscoverableByPhoneNumber()); assertEquals(deviceCapabilities, primaryDevice.getCapabilities()); assertEquals(badges, account.getBadges()); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java index dd9692cca..8025b9052 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java @@ -13,6 +13,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.nio.charset.StandardCharsets; import java.time.Clock; import java.util.Map; import java.util.Optional; @@ -185,7 +186,7 @@ class AccountsManagerChangeNumberIntegrationTest { final int rotatedPniRegistrationId = 17; final ECKeyPair rotatedPniIdentityKeyPair = Curve.generateKeyPair(); final ECSignedPreKey rotatedSignedPreKey = KeysHelper.signedECPreKey(1L, rotatedPniIdentityKeyPair); - final AccountAttributes accountAttributes = new AccountAttributes(true, rotatedPniRegistrationId + 1, rotatedPniRegistrationId, "test", null, true, new Device.DeviceCapabilities(false, false, false, false)); + final AccountAttributes accountAttributes = new AccountAttributes(true, rotatedPniRegistrationId + 1, rotatedPniRegistrationId, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false, false)); final Account account = AccountsHelper.createAccount(accountsManager, originalNumber, accountAttributes); account.getPrimaryDevice().setSignedPreKey(KeysHelper.signedECPreKey(1, rotatedPniIdentityKeyPair)); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java index 971755fb9..6dbc936f4 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java @@ -20,6 +20,7 @@ import static org.mockito.Mockito.when; import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.time.Clock; import java.time.Instant; import java.util.ArrayList; @@ -195,7 +196,7 @@ class AccountsManagerConcurrentModificationIntegrationTest { modifyAccount(uuid, account -> account.setRegistrationLock(credentials.hash(), credentials.salt())), modifyAccount(uuid, account -> account.setUnrestrictedUnidentifiedAccess(unrestrictedUnidentifiedAccess)), modifyDevice(uuid, Device.PRIMARY_ID, device -> device.setLastSeen(lastSeen)), - modifyDevice(uuid, Device.PRIMARY_ID, device -> device.setName("deviceName")) + modifyDevice(uuid, Device.PRIMARY_ID, device -> device.setName("deviceName".getBytes(StandardCharsets.UTF_8))) ).join(); final Account managerAccount = accountsManager.getByAccountIdentifier(uuid).orElseThrow(); 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 d16c0b8c8..2457e8ca8 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -900,9 +900,9 @@ class AccountsManagerTest { @SuppressWarnings("unchecked") Consumer unknownDeviceUpdater = mock(Consumer.class); account = accountsManager.updateDevice(account, deviceId, deviceUpdater); - account = accountsManager.updateDevice(account, deviceId, d -> d.setName("deviceName")); + account = accountsManager.updateDevice(account, deviceId, d -> d.setName("deviceName".getBytes(StandardCharsets.UTF_8))); - assertEquals("deviceName", account.getDevice(deviceId).orElseThrow().getName()); + assertArrayEquals("deviceName".getBytes(StandardCharsets.UTF_8), account.getDevice(deviceId).orElseThrow().getName()); verify(deviceUpdater, times(1)).accept(any(Device.class)); @@ -933,9 +933,9 @@ class AccountsManagerTest { @SuppressWarnings("unchecked") Consumer unknownDeviceUpdater = mock(Consumer.class); account = accountsManager.updateDeviceAsync(account, deviceId, deviceUpdater).join(); - account = accountsManager.updateDeviceAsync(account, deviceId, d -> d.setName("deviceName")).join(); + account = accountsManager.updateDeviceAsync(account, deviceId, d -> d.setName("deviceName".getBytes(StandardCharsets.UTF_8))).join(); - assertEquals("deviceName", account.getDevice(deviceId).orElseThrow().getName()); + assertArrayEquals("deviceName".getBytes(StandardCharsets.UTF_8), account.getDevice(deviceId).orElseThrow().getName()); verify(deviceUpdater, times(1)).accept(any(Device.class)); 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 ea135c0b7..7128c9360 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java @@ -499,7 +499,7 @@ class AccountsTest { assertPhoneNumberConstraintExists("+14151112222", account.getUuid()); assertPhoneNumberIdentifierConstraintExists(account.getPhoneNumberIdentifier(), account.getUuid()); - device.setName("foobar"); + device.setName("foobar".getBytes(StandardCharsets.UTF_8)); accounts.update(account); @@ -567,7 +567,7 @@ class AccountsTest { final Account account = generateAccount("+14151112222", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - final String deviceName = "device-name"; + final byte[] deviceName = "device-name".getBytes(StandardCharsets.UTF_8); assertNotEquals(deviceName, accounts.getByAccountIdentifier(account.getUuid()).orElseThrow().getPrimaryDevice().getName()); @@ -593,7 +593,7 @@ class AccountsTest { .build()) .build())).toCompletableFuture().join(); - assertEquals(deviceName, + assertArrayEquals(deviceName, accounts.getByAccountIdentifier(account.getUuid()).orElseThrow().getPrimaryDevice().getName()); assertTrue(DYNAMO_DB_EXTENSION.getDynamoDbClient().getItem(GetItemRequest.builder() diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/util/DeviceNameByteArrayAdapterTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/util/DeviceNameByteArrayAdapterTest.java new file mode 100644 index 000000000..062cebe1b --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/util/DeviceNameByteArrayAdapterTest.java @@ -0,0 +1,51 @@ +package org.whispersystems.textsecuregcm.util; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.SerializerProvider; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.IOException; +import java.util.Base64; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class DeviceNameByteArrayAdapterTest { + + @Test + void serialize() throws IOException { + final byte[] deviceName = TestRandomUtil.nextBytes(16); + final JsonGenerator jsonGenerator = mock(JsonGenerator.class); + + new DeviceNameByteArrayAdapter.Serializer().serialize(deviceName, jsonGenerator, mock(SerializerProvider.class)); + + verify(jsonGenerator).writeString(Base64.getEncoder().encodeToString(deviceName)); + } + + @ParameterizedTest + @MethodSource + void deserialize(final String encodedString, final byte[] expectedBytes) throws IOException { + final JsonParser jsonParser = mock(JsonParser.class); + when(jsonParser.getValueAsString()).thenReturn(encodedString); + + assertArrayEquals(expectedBytes, + new DeviceNameByteArrayAdapter.Deserializer().deserialize(jsonParser, mock(DeserializationContext.class))); + } + + private static List deserialize() { + final byte[] deviceName = TestRandomUtil.nextBytes(16); + + return List.of( + Arguments.of(Base64.getEncoder().encodeToString(deviceName), deviceName), + Arguments.of("This is not a valid Base64 string", null) + ); + } +}