Skip to content

Commit

Permalink
Revert "Represent device names as byte arrays"
Browse files Browse the repository at this point in the history
This reverts commit 5ae2e52.
  • Loading branch information
jon-signal committed Dec 6, 2023
1 parent 4fa10e5 commit 45848e7
Show file tree
Hide file tree
Showing 17 changed files with 46 additions and 177 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
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;
Expand Down Expand Up @@ -126,7 +125,7 @@ public UUID pniUuid() {
}

public AccountAttributes accountAttributes() {
return new AccountAttributes(true, registrationId, pniRegistrationId, "".getBytes(StandardCharsets.UTF_8), "", true, new Device.DeviceCapabilities(false, false, false, false))
return new AccountAttributes(true, registrationId, pniRegistrationId, "", "", true, new Device.DeviceCapabilities(false, false, false, false))
.withUnidentifiedAccessKey(unidentifiedAccessKey)
.withRecoveryPassword(registrationPassword);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

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;
Expand All @@ -31,10 +30,8 @@ public class AccountAttributes {
private int phoneNumberIdentityRegistrationId;

@JsonProperty
@JsonSerialize(using = ByteArrayAdapter.Serializing.class)
@JsonDeserialize(using = ByteArrayAdapter.Deserializing.class)
@Size(max = 225)
private byte[] name;
@Size(max = 204, message = "This field must be less than 50 characters")
private String name;

@JsonProperty
private String registrationLock;
Expand Down Expand Up @@ -65,7 +62,7 @@ public AccountAttributes(
final boolean fetchesMessages,
final int registrationId,
final int phoneNumberIdentifierRegistrationId,
final byte[] name,
final String name,
final String registrationLock,
final boolean discoverableByPhoneNumber,
final DeviceCapabilities capabilities) {
Expand All @@ -90,7 +87,7 @@ public int getPhoneNumberIdentityRegistrationId() {
return phoneNumberIdentityRegistrationId;
}

public byte[] getName() {
public String getName() {
return name;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,5 @@

package org.whispersystems.textsecuregcm.entities;

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) {
public record DeviceInfo(long id, String name, long lastSeen, long created) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,19 @@
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 = 225)
private byte[] deviceName;
@Size(max = 300, message = "This field must be less than 300 characters")
private String deviceName;

public DeviceName() {}

public byte[] getDeviceName() {
public String getDeviceName() {
return deviceName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ public Mono<GetDevicesResponse> getDevices(final GetDevicesRequest request) {
.reduce(GetDevicesResponse.newBuilder(), (builder, device) -> {
final GetDevicesResponse.LinkedDevice.Builder linkedDeviceBuilder = GetDevicesResponse.LinkedDevice.newBuilder();

if (device.getName() != null) {
linkedDeviceBuilder.setName(ByteString.copyFrom(device.getName()));
if (StringUtils.isNotBlank(device.getName())) {
linkedDeviceBuilder.setName(ByteString.copyFrom(Base64.getDecoder().decode(device.getName())));
}

return builder.addDevices(linkedDeviceBuilder
Expand Down Expand Up @@ -105,7 +105,7 @@ public Mono<SetDeviceNameResponse> setDeviceName(final SetDeviceNameRequest requ
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(request.getName().toByteArray()))))
device -> device.setName(Base64.getEncoder().encodeToString(request.getName().toByteArray())))))
.thenReturn(SetDeviceNameResponse.newBuilder().build());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

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;
Expand All @@ -18,7 +17,6 @@
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 {

Expand All @@ -33,9 +31,7 @@ public class Device {
private byte id;

@JsonProperty
@JsonSerialize(using = DeviceNameByteArrayAdapter.Serializer.class)
@JsonDeserialize(using = DeviceNameByteArrayAdapter.Deserializer.class)
private byte[] name;
private String name;

@JsonProperty
private String authToken;
Expand Down Expand Up @@ -150,11 +146,11 @@ public void setId(byte id) {
this.id = id;
}

public byte[] getName() {
public String getName() {
return name;
}

public void setName(byte[] name) {
public void setName(String name) {
this.name = name;
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -221,10 +221,7 @@ void testDeviceAdded() {

final int initialDeviceCount = account.getDevices().size();

final List<String> addedDeviceNames = List.of(
Base64.getEncoder().encodeToString("newDevice1".getBytes(StandardCharsets.UTF_8)),
Base64.getEncoder().encodeToString("newDevice2".getBytes(StandardCharsets.UTF_8)));

final List<String> addedDeviceNames = List.of("newDevice1", "newDevice2");
final Response response = resources.getJerseyTest()
.target("/v1/test/account/devices")
.request()
Expand Down Expand Up @@ -453,7 +450,7 @@ public String setEnabled(@Auth TestPrincipal principal, Map<Byte, Boolean> devic
@PUT
@Path("/account/devices")
@ChangesDeviceEnabledState
public String addDevices(@Auth TestPrincipal auth, List<byte[]> deviceNames) {
public String addDevices(@Auth TestPrincipal auth, List<String> deviceNames) {

deviceNames.forEach(name -> {
final Device device = DevicesHelper.createDevice(auth.getAccount().getNextDeviceId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -712,10 +712,11 @@ void longNameTest() {
.request()
.header("Authorization", AuthHelper.getProvisioningAuthHeader(AuthHelper.VALID_NUMBER, "password1"))
.put(Entity.entity(new AccountAttributes(false, 1234, 5678,
TestRandomUtil.nextBytes(226), null, true, null),
"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),
MediaType.APPLICATION_JSON_TYPE));

assertEquals(422, response.getStatus());
assertEquals(response.getStatus(), 422);
verifyNoMoreInteractions(messagesManager);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -477,10 +477,10 @@ static Stream<Arguments> atomicAccountCreationConflictingChannel() {
}

final AccountAttributes fetchesMessagesAccountAttributes =
new AccountAttributes(true, 1, 1, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false, false));
new AccountAttributes(true, 1, 1, "test", 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, false));
new AccountAttributes(false, 1, 1, "test", null, true, new Device.DeviceCapabilities(false, false, false, false));

return Stream.of(
// "Fetches messages" is true, but an APNs token is provided
Expand Down Expand Up @@ -566,7 +566,7 @@ static Stream<Arguments> atomicAccountCreationPartialSignedPreKeys() {
}

final AccountAttributes accountAttributes =
new AccountAttributes(true, 1, 1, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false, false));
new AccountAttributes(true, 1, 1, "test", null, true, new Device.DeviceCapabilities(false, false, false, false));

return Stream.of(
// Signed PNI EC pre-key is missing
Expand Down Expand Up @@ -719,7 +719,7 @@ private static boolean accountAttributesEqual(final AccountAttributes a, final A
&& a.isUnrestrictedUnidentifiedAccess() == b.isUnrestrictedUnidentifiedAccess()
&& a.isDiscoverableByPhoneNumber() == b.isDiscoverableByPhoneNumber()
&& Objects.equals(a.getPhoneNumberIdentityRegistrationId(), b.getPhoneNumberIdentityRegistrationId())
&& Arrays.equals(a.getName(), b.getName())
&& Objects.equals(a.getName(), b.getName())
&& Objects.equals(a.getRegistrationLock(), b.getRegistrationLock())
&& Arrays.equals(a.getUnidentifiedAccessKey(), b.getUnidentifiedAccessKey())
&& Objects.equals(a.getCapabilities(), b.getCapabilities())
Expand All @@ -746,10 +746,10 @@ private static Stream<Arguments> atomicAccountCreationSuccess() {
}

final AccountAttributes fetchesMessagesAccountAttributes =
new AccountAttributes(true, 1, 1, "test".getBytes(StandardCharsets.UTF_8), null, true, new Device.DeviceCapabilities(false, false, false, false));
new AccountAttributes(true, 1, 1, "test", 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, false));
new AccountAttributes(false, 1, 1, "test", null, true, new Device.DeviceCapabilities(false, false, false, false));

final String apnsToken = "apns-token";
final String apnsVoipToken = "apns-voip-token";
Expand Down Expand Up @@ -861,7 +861,7 @@ private static String requestJson(final String sessionId,
final IdentityKey aciIdentityKey = new IdentityKey(aciIdentityKeyPair.getPublicKey());
final IdentityKey pniIdentityKey = new IdentityKey(pniIdentityKeyPair.getPublicKey());

final AccountAttributes accountAttributes = new AccountAttributes(true, registrationId, pniRegistrationId, "name".getBytes(StandardCharsets.UTF_8), "reglock",
final AccountAttributes accountAttributes = new AccountAttributes(true, registrationId, pniRegistrationId, "name", "reglock",
true, new Device.DeviceCapabilities(true, true, true, true));

final RegistrationRequest request = new RegistrationRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
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;
Expand All @@ -20,6 +21,7 @@
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;
Expand Down Expand Up @@ -121,7 +123,8 @@ void getDevices() {
when(linkedDevice.getId()).thenReturn((byte) (Device.PRIMARY_ID + 1));
when(linkedDevice.getCreated()).thenReturn(linkedDeviceCreated.toEpochMilli());
when(linkedDevice.getLastSeen()).thenReturn(linkedDeviceLastSeen.toEpochMilli());
when(linkedDevice.getName()).thenReturn(linkedDeviceName.getBytes(StandardCharsets.UTF_8));
when(linkedDevice.getName())
.thenReturn(Base64.getEncoder().encodeToString(linkedDeviceName.getBytes(StandardCharsets.UTF_8)));

when(authenticatedAccount.getDevices()).thenReturn(List.of(primaryDevice, linkedDevice));

Expand Down Expand Up @@ -189,7 +192,7 @@ void setDeviceName(final byte deviceId) {
.setName(ByteString.copyFrom(deviceName))
.build());

verify(device).setName(deviceName);
verify(device).setName(Base64.getEncoder().encodeToString(deviceName));
}

@ParameterizedTest
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.whispersystems.textsecuregcm.storage;

import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand All @@ -9,7 +8,6 @@
import static org.mockito.Mockito.when;

import com.google.i18n.phonenumbers.PhoneNumberUtil;
import java.nio.charset.StandardCharsets;
import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
Expand Down Expand Up @@ -172,7 +170,7 @@ void createAccount(final DeliveryChannels deliveryChannels,
final String signalAgent = RandomStringUtils.randomAlphabetic(3);
final int registrationId = ThreadLocalRandom.current().nextInt(Device.MAX_REGISTRATION_ID);
final int pniRegistrationId = ThreadLocalRandom.current().nextInt(Device.MAX_REGISTRATION_ID);
final byte[] deviceName = RandomStringUtils.randomAlphabetic(16).getBytes(StandardCharsets.UTF_8);
final String deviceName = RandomStringUtils.randomAlphabetic(16);
final String registrationLockSecret = RandomStringUtils.randomAlphanumeric(16);

final Device.DeviceCapabilities deviceCapabilities = new Device.DeviceCapabilities(
Expand Down Expand Up @@ -266,7 +264,7 @@ void reregisterAccount(final DeliveryChannels deliveryChannels,
final Account originalAccount = accountsManager.create(number,
RandomStringUtils.randomAlphanumeric(16),
"OWI",
new AccountAttributes(true, 1, 1, "name".getBytes(StandardCharsets.UTF_8), "registration-lock", false, new Device.DeviceCapabilities(false, false, false, false)),
new AccountAttributes(true, 1, 1, "name", "registration-lock", false, new Device.DeviceCapabilities(false, false, false, false)),
Collections.emptyList(),
new IdentityKey(aciKeyPair.getPublicKey()),
new IdentityKey(pniKeyPair.getPublicKey()),
Expand All @@ -284,7 +282,7 @@ void reregisterAccount(final DeliveryChannels deliveryChannels,
final String signalAgent = RandomStringUtils.randomAlphabetic(3);
final int registrationId = ThreadLocalRandom.current().nextInt(Device.MAX_REGISTRATION_ID);
final int pniRegistrationId = ThreadLocalRandom.current().nextInt(Device.MAX_REGISTRATION_ID);
final byte[] deviceName = RandomStringUtils.randomAlphabetic(16).getBytes(StandardCharsets.UTF_8);
final String deviceName = RandomStringUtils.randomAlphabetic(16);
final String registrationLockSecret = RandomStringUtils.randomAlphanumeric(16);

final Device.DeviceCapabilities deviceCapabilities = new Device.DeviceCapabilities(
Expand Down Expand Up @@ -381,7 +379,7 @@ private void assertExpectedStoredAccount(final Account account,
final DeliveryChannels deliveryChannels,
final int registrationId,
final int pniRegistrationId,
final byte[] deviceName,
final String deviceName,
final boolean discoverableByPhoneNumber,
final Device.DeviceCapabilities deviceCapabilities,
final List<AccountBadge> badges,
Expand All @@ -400,7 +398,7 @@ private void assertExpectedStoredAccount(final Account account,
assertEquals(deliveryChannels.fetchesMessages(), primaryDevice.getFetchesMessages());
assertEquals(registrationId, primaryDevice.getRegistrationId());
assertEquals(pniRegistrationId, primaryDevice.getPhoneNumberIdentityRegistrationId().orElseThrow());
assertArrayEquals(deviceName, primaryDevice.getName());
assertEquals(deviceName, primaryDevice.getName());
assertEquals(discoverableByPhoneNumber, account.isDiscoverableByPhoneNumber());
assertEquals(deviceCapabilities, primaryDevice.getCapabilities());
assertEquals(badges, account.getBadges());
Expand Down
Loading

0 comments on commit 45848e7

Please sign in to comment.