Allow primary devices to change names of linked devices

This commit is contained in:
Jon Chambers 2024-10-29 09:52:38 -04:00 committed by GitHub
parent 712f3affd9
commit f3b22e04e8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 231 additions and 41 deletions

View File

@ -7,6 +7,7 @@ package org.whispersystems.textsecuregcm.controllers;
import io.dropwizard.auth.Auth;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import java.util.Base64;
import java.util.Objects;
@ -19,6 +20,7 @@ import javax.validation.constraints.NotNull;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
import javax.ws.rs.ForbiddenException;
import javax.ws.rs.GET;
import javax.ws.rs.HEAD;
import javax.ws.rs.HeaderParam;
@ -27,6 +29,7 @@ import javax.ws.rs.PUT;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
@ -192,10 +195,39 @@ public class AccountController {
@PUT
@Path("/name/")
public void setName(@Mutable @Auth AuthenticatedDevice auth, @NotNull @Valid DeviceName deviceName) {
Account account = auth.getAccount();
Device device = auth.getAuthenticatedDevice();
accounts.updateDevice(account, device.getId(), d -> d.setName(deviceName.getDeviceName()));
@Operation(summary = "Set a device's encrypted name",
description = """
Sets the encrypted name for the specified device. Primary devices may change the name of any device associated
with their account, but linked devices may only change their own name. If no device ID is specified, then the
authenticated device's ID will be used.
""")
@ApiResponse(responseCode = "204", description = "Device name changed successfully")
@ApiResponse(responseCode = "404", description = "No device found with the given ID")
@ApiResponse(responseCode = "403", description = "Not authorized to change the name of the device with the given ID")
public void setName(@Mutable @Auth final AuthenticatedDevice auth,
@NotNull @Valid final DeviceName deviceName,
@Nullable
@QueryParam("deviceId")
@Schema(description = "The ID of the device for which to set a name; if omitted, the authenticated device will be targeted for a name change",
requiredMode = Schema.RequiredMode.NOT_REQUIRED)
final Byte deviceId) {
final Account account = auth.getAccount();
final byte targetDeviceId = deviceId == null ? auth.getAuthenticatedDevice().getId() : deviceId;
if (account.getDevice(targetDeviceId).isEmpty()) {
throw new NotFoundException();
}
final boolean mayChangeName = auth.getAuthenticatedDevice().isPrimary() ||
auth.getAuthenticatedDevice().getId() == targetDeviceId;
if (!mayChangeName) {
throw new ForbiddenException();
}
accounts.updateDevice(account, targetDeviceId, d -> d.setName(deviceName.deviceName()));
}
@PUT

View File

@ -5,25 +5,15 @@
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;
public DeviceName() {}
public byte[] getDeviceName() {
return deviceName;
}
public record DeviceName(@JsonSerialize(using = ByteArrayAdapter.Serializing.class)
@JsonDeserialize(using = ByteArrayAdapter.Deserializing.class)
@NotEmpty
@Size(max = 225)
byte[] deviceName) {
}

View File

@ -6,13 +6,15 @@
package org.whispersystems.textsecuregcm.grpc;
import io.grpc.Status;
import org.whispersystems.textsecuregcm.storage.Device;
public class DeviceIdUtil {
static byte validate(int deviceId) {
if (deviceId > Byte.MAX_VALUE) {
if (deviceId < Device.PRIMARY_ID || deviceId > Byte.MAX_VALUE) {
throw Status.INVALID_ARGUMENT.withDescription("Device ID is out of range").asRuntimeException();
}
return (byte) deviceId;
}
}

View File

@ -89,6 +89,17 @@ public class DevicesGrpcService extends ReactorDevicesGrpc.DevicesImplBase {
public Mono<SetDeviceNameResponse> setDeviceName(final SetDeviceNameRequest request) {
final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice();
final byte deviceId = DeviceIdUtil.validate(request.getId());
final boolean mayChangeName = authenticatedDevice.deviceId() == Device.PRIMARY_ID ||
authenticatedDevice.deviceId() == deviceId;
if (!mayChangeName) {
throw Status.PERMISSION_DENIED
.withDescription("Authenticated device is not authorized to change target device name")
.asRuntimeException();
}
if (request.getName().isEmpty()) {
throw Status.INVALID_ARGUMENT.withDescription("Must specify a device name").asRuntimeException();
}
@ -100,7 +111,12 @@ 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(),
.doOnNext(account -> {
if (account.getDevice(deviceId).isEmpty()) {
throw Status.NOT_FOUND.withDescription("No device found with given ID").asRuntimeException();
}
})
.flatMap(account -> Mono.fromFuture(() -> accountsManager.updateDeviceAsync(account, deviceId,
device -> device.setName(request.getName().toByteArray()))))
.thenReturn(SetDeviceNameResponse.newBuilder().build());
}

View File

@ -47,7 +47,9 @@ public class KeysAnonymousGrpcService extends ReactorKeysAnonymousGrpc.KeysAnony
final ServiceIdentifier serviceIdentifier =
ServiceIdentifierUtil.fromGrpcServiceIdentifier(request.getRequest().getTargetIdentifier());
final byte deviceId = DeviceIdUtil.validate(request.getRequest().getDeviceId());
final byte deviceId = request.getRequest().hasDeviceId()
? DeviceIdUtil.validate(request.getRequest().getDeviceId())
: KeysGrpcHelper.ALL_DEVICES;
return switch (request.getAuthorizationCase()) {
case GROUP_SEND_TOKEN ->

View File

@ -5,7 +5,6 @@
package org.whispersystems.textsecuregcm.grpc;
import com.google.common.annotations.VisibleForTesting;
import com.google.protobuf.ByteString;
import io.grpc.Status;
import org.signal.chat.common.EcPreKey;
@ -26,7 +25,6 @@ import reactor.util.function.Tuples;
class KeysGrpcHelper {
@VisibleForTesting
static final byte ALL_DEVICES = 0;
static Mono<GetPreKeysResponse> getPreKeys(final Account targetAccount,

View File

@ -122,7 +122,9 @@ public class KeysGrpcService extends ReactorKeysGrpc.KeysImplBase {
final ServiceIdentifier targetIdentifier =
ServiceIdentifierUtil.fromGrpcServiceIdentifier(request.getTargetIdentifier());
final byte deviceId = DeviceIdUtil.validate(request.getDeviceId());
final byte deviceId = request.hasDeviceId()
? DeviceIdUtil.validate(request.getDeviceId())
: KeysGrpcHelper.ALL_DEVICES;
final String rateLimitKey = authenticatedDevice.accountIdentifier() + "." +
authenticatedDevice.deviceId() + "__" +

View File

@ -27,6 +27,14 @@ service Devices {
*/
rpc RemoveDevice(RemoveDeviceRequest) returns (RemoveDeviceResponse) {}
/**
* Sets the encrypted human-readable name for a specific devices. Primary
* devices may change the name of any device associated with their account,
* but linked devices may only change their own name. This call will fail with
* a status of `NOT_FOUND` if no device was found with the given identifier.
* It will also fail with a status of `PERMISSION_DENIED` if a linked device
* tries to change the name of any device other than itself.
*/
rpc SetDeviceName(SetDeviceNameRequest) returns (SetDeviceNameResponse) {}
/**
@ -95,6 +103,11 @@ message SetDeviceNameRequest {
* device.
*/
bytes name = 1;
/**
* The identifier for the device for which to set a name.
*/
uint32 id = 2;
}
message SetDeviceNameResponse {}

View File

@ -154,7 +154,7 @@ message GetPreKeysRequest {
* retrieve pre-keys. If not set, pre-keys are returned for all devices
* associated with the targeted account.
*/
uint32 device_id = 2;
optional uint32 device_id = 2;
}
message GetPreKeysAnonymousRequest {

View File

@ -18,6 +18,7 @@ import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@ -63,6 +64,7 @@ import org.whispersystems.textsecuregcm.entities.AccountIdentifierResponse;
import org.whispersystems.textsecuregcm.entities.AccountIdentityResponse;
import org.whispersystems.textsecuregcm.entities.ApnRegistrationId;
import org.whispersystems.textsecuregcm.entities.ConfirmUsernameHashRequest;
import org.whispersystems.textsecuregcm.entities.DeviceName;
import org.whispersystems.textsecuregcm.entities.EncryptedUsername;
import org.whispersystems.textsecuregcm.entities.GcmRegistrationId;
import org.whispersystems.textsecuregcm.entities.RegistrationLock;
@ -81,6 +83,7 @@ import org.whispersystems.textsecuregcm.mappers.NonNormalizedPhoneNumberExceptio
import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper;
import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.AccountsManager;
import org.whispersystems.textsecuregcm.storage.Device;
import org.whispersystems.textsecuregcm.storage.RegistrationRecoveryPasswordsManager;
import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableException;
import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundException;
@ -999,4 +1002,72 @@ class AccountControllerTest {
verify(AuthHelper.VALID_ACCOUNT).setUsernameLinkDetails(eq(newHandle.usernameLinkHandle()), eq(encryptedUsername));
}
@Test
void testSetDeviceName() {
try (final Response response = resources.getJerseyTest()
.target("/v1/accounts/name/")
.request()
.header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_3, Device.PRIMARY_ID, AuthHelper.VALID_PASSWORD_3_PRIMARY))
.put(Entity.json(new DeviceName(TestRandomUtil.nextBytes(64))))) {
assertThat(response.getStatus()).isEqualTo(204);
verify(accountsManager).updateDevice(eq(AuthHelper.VALID_ACCOUNT_3), eq(Device.PRIMARY_ID), any());
}
}
@Test
void testSetLinkedDeviceNameFromPrimary() {
try (final Response response = resources.getJerseyTest()
.target("/v1/accounts/name/")
.queryParam("deviceId", AuthHelper.VALID_DEVICE_3_LINKED_ID)
.request()
.header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_3, Device.PRIMARY_ID, AuthHelper.VALID_PASSWORD_3_PRIMARY))
.put(Entity.json(new DeviceName(TestRandomUtil.nextBytes(64))))) {
assertThat(response.getStatus()).isEqualTo(204);
verify(accountsManager).updateDevice(eq(AuthHelper.VALID_ACCOUNT_3), eq(AuthHelper.VALID_DEVICE_3_LINKED_ID), any());
}
}
@Test
void testSetLinkedDeviceNameFromLinked() {
try (final Response response = resources.getJerseyTest()
.target("/v1/accounts/name/")
.queryParam("deviceId", AuthHelper.VALID_DEVICE_3_LINKED_ID)
.request()
.header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_3, AuthHelper.VALID_DEVICE_3_LINKED_ID, AuthHelper.VALID_PASSWORD_3_LINKED))
.put(Entity.json(new DeviceName(TestRandomUtil.nextBytes(64))))) {
assertThat(response.getStatus()).isEqualTo(204);
verify(accountsManager).updateDevice(eq(AuthHelper.VALID_ACCOUNT_3), eq(AuthHelper.VALID_DEVICE_3_LINKED_ID), any());
}
}
@Test
void testSetDeviceNameDeviceNotFound() {
try (final Response response = resources.getJerseyTest()
.target("/v1/accounts/name/")
.queryParam("deviceId", Device.MAXIMUM_DEVICE_ID)
.request()
.header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_3, AuthHelper.VALID_PASSWORD_3_PRIMARY))
.put(Entity.json(new DeviceName(TestRandomUtil.nextBytes(64))))) {
assertThat(response.getStatus()).isEqualTo(404);
verify(accountsManager, never()).updateDevice(any(), anyByte(), any());
}
}
@Test
void testSetPrimaryDeviceNameFromLinked() {
try (final Response response = resources.getJerseyTest()
.target("/v1/accounts/name/")
.queryParam("deviceId", Device.PRIMARY_ID)
.request()
.header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_3, AuthHelper.VALID_DEVICE_3_LINKED_ID, AuthHelper.VALID_PASSWORD_3_LINKED))
.put(Entity.json(new DeviceName(TestRandomUtil.nextBytes(64))))) {
assertThat(response.getStatus()).isEqualTo(403);
verify(accountsManager, never()).updateDevice(any(), anyByte(), any());
}
}
}

View File

@ -26,7 +26,6 @@ import java.util.concurrent.CompletableFuture;
import java.util.function.Consumer;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import org.apache.commons.lang3.RandomStringUtils;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
@ -187,12 +186,66 @@ class DevicesGrpcServiceTest extends SimpleBaseGrpcTest<DevicesGrpcService, Devi
final byte[] deviceName = TestRandomUtil.nextBytes(128);
final SetDeviceNameResponse ignored = authenticatedServiceStub().setDeviceName(SetDeviceNameRequest.newBuilder()
.setId(deviceId)
.setName(ByteString.copyFrom(deviceName))
.build());
verify(device).setName(deviceName);
}
@Test
void setLinkedDeviceNameFromPrimary() {
mockAuthenticationInterceptor().setAuthenticatedDevice(AUTHENTICATED_ACI, Device.PRIMARY_ID);
final byte deviceId = Device.PRIMARY_ID + 1;
final Device device = mock(Device.class);
when(authenticatedAccount.getDevice(deviceId)).thenReturn(Optional.of(device));
final byte[] deviceName = TestRandomUtil.nextBytes(128);
final SetDeviceNameResponse ignored = authenticatedServiceStub().setDeviceName(SetDeviceNameRequest.newBuilder()
.setId(deviceId)
.setName(ByteString.copyFrom(deviceName))
.build());
verify(device).setName(deviceName);
}
@Test
void setPrimaryDeviceNameFromLinkedDevice() {
mockAuthenticationInterceptor().setAuthenticatedDevice(AUTHENTICATED_ACI, (byte) (Device.PRIMARY_ID + 1));
final byte deviceId = Device.PRIMARY_ID;
final Device device = mock(Device.class);
when(authenticatedAccount.getDevice(deviceId)).thenReturn(Optional.of(device));
final byte[] deviceName = TestRandomUtil.nextBytes(128);
assertStatusException(Status.PERMISSION_DENIED,
() -> authenticatedServiceStub().setDeviceName(SetDeviceNameRequest.newBuilder()
.setId(deviceId)
.setName(ByteString.copyFrom(deviceName))
.build()));
verify(device, never()).setName(deviceName);
}
@Test
void setDeviceNameNotFound() {
mockAuthenticationInterceptor().setAuthenticatedDevice(AUTHENTICATED_ACI, Device.PRIMARY_ID);
when(authenticatedAccount.getDevice(anyByte())).thenReturn(Optional.empty());
final byte[] deviceName = TestRandomUtil.nextBytes(128);
assertStatusException(Status.NOT_FOUND,
() -> authenticatedServiceStub().setDeviceName(SetDeviceNameRequest.newBuilder()
.setId(Device.PRIMARY_ID + 1)
.setName(ByteString.copyFrom(deviceName))
.build()));
}
@ParameterizedTest
@MethodSource
void setDeviceNameIllegalArgument(final SetDeviceNameRequest request) {
@ -203,11 +256,25 @@ class DevicesGrpcServiceTest extends SimpleBaseGrpcTest<DevicesGrpcService, Devi
private static Stream<Arguments> setDeviceNameIllegalArgument() {
return Stream.of(
// No device name
Arguments.of(SetDeviceNameRequest.newBuilder().build()),
Arguments.of(SetDeviceNameRequest.newBuilder()
.setId(Device.PRIMARY_ID)
.build()),
// Excessively-long device name
Arguments.of(SetDeviceNameRequest.newBuilder()
.setName(ByteString.copyFrom(RandomStringUtils.randomAlphanumeric(1024).getBytes(StandardCharsets.UTF_8)))
.setId(Device.PRIMARY_ID)
.setName(ByteString.copyFrom(TestRandomUtil.nextBytes(1024)))
.build()),
// No device ID
Arguments.of(SetDeviceNameRequest.newBuilder()
.setName(ByteString.copyFrom(TestRandomUtil.nextBytes(32)))
.build()),
// Out-of-bounds device ID
Arguments.of(SetDeviceNameRequest.newBuilder()
.setId(Device.MAXIMUM_DEVICE_ID + 1)
.setName(ByteString.copyFrom(TestRandomUtil.nextBytes(32)))
.build())
);
}

View File

@ -27,8 +27,6 @@ import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mock;
import org.signal.chat.common.EcPreKey;
import org.signal.chat.common.EcSignedPreKey;
@ -306,9 +304,8 @@ class KeysAnonymousGrpcServiceTest extends SimpleBaseGrpcTest<KeysAnonymousGrpcS
verifyNoInteractions(keysManager);
}
@ParameterizedTest
@ValueSource(bytes = {KeysGrpcHelper.ALL_DEVICES, 1})
void getPreKeysDeviceNotFound(final byte deviceId) {
@Test
void getPreKeysDeviceNotFound() {
final UUID accountIdentifier = UUID.randomUUID();
final byte[] unidentifiedAccessKey = TestRandomUtil.nextBytes(UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH);
@ -329,7 +326,7 @@ class KeysAnonymousGrpcServiceTest extends SimpleBaseGrpcTest<KeysAnonymousGrpcS
.setTargetIdentifier(ServiceIdentifier.newBuilder()
.setIdentityType(org.signal.chat.common.IdentityType.IDENTITY_TYPE_ACI)
.setUuid(UUIDUtil.toByteString(accountIdentifier)))
.setDeviceId(deviceId))
.setDeviceId(Device.PRIMARY_ID))
.build());
}

View File

@ -37,7 +37,6 @@ import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.EnumSource;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mock;
import org.signal.chat.common.EcPreKey;
import org.signal.chat.common.EcSignedPreKey;
@ -592,9 +591,8 @@ class KeysGrpcServiceTest extends SimpleBaseGrpcTest<KeysGrpcService, KeysGrpc.K
.build()));
}
@ParameterizedTest
@ValueSource(bytes = {KeysGrpcHelper.ALL_DEVICES, 1})
void getPreKeysDeviceNotFound(final byte deviceId) {
@Test
void getPreKeysDeviceNotFound() {
final UUID accountIdentifier = UUID.randomUUID();
final Account targetAccount = mock(Account.class);
@ -611,7 +609,7 @@ class KeysGrpcServiceTest extends SimpleBaseGrpcTest<KeysGrpcService, KeysGrpc.K
.setIdentityType(org.signal.chat.common.IdentityType.IDENTITY_TYPE_ACI)
.setUuid(UUIDUtil.toByteString(accountIdentifier))
.build())
.setDeviceId(deviceId)
.setDeviceId(Device.PRIMARY_ID)
.build()));
}

View File

@ -105,6 +105,8 @@ public class AuthHelper {
public static Device VALID_DEVICE_3_PRIMARY = mock(Device.class);
public static Device VALID_DEVICE_3_LINKED = mock(Device.class);
public static final byte VALID_DEVICE_3_LINKED_ID = Device.PRIMARY_ID + 1;
private static SaltedTokenHash VALID_CREDENTIALS = mock(SaltedTokenHash.class);
private static SaltedTokenHash VALID_CREDENTIALS_TWO = mock(SaltedTokenHash.class);
private static SaltedTokenHash VALID_CREDENTIALS_3_PRIMARY = mock(SaltedTokenHash.class);
@ -136,7 +138,7 @@ public class AuthHelper {
when(VALID_DEVICE_TWO.getId()).thenReturn(Device.PRIMARY_ID);
when(UNDISCOVERABLE_DEVICE.getId()).thenReturn(Device.PRIMARY_ID);
when(VALID_DEVICE_3_PRIMARY.getId()).thenReturn(Device.PRIMARY_ID);
when(VALID_DEVICE_3_LINKED.getId()).thenReturn((byte) 2);
when(VALID_DEVICE_3_LINKED.getId()).thenReturn(VALID_DEVICE_3_LINKED_ID);
when(UNDISCOVERABLE_DEVICE.isPrimary()).thenReturn(true);