mirror of
https://github.com/signalapp/Signal-Server
synced 2026-04-20 20:58:00 +01:00
Allow, but do not require, message delivery to devices without active delivery channels
This commit is contained in:
@@ -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()}.
|
||||
* <p>
|
||||
* 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<Pair<UUID, Byte>> 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<Byte, Boolean> initialDevicesEnabled =
|
||||
|
||||
@@ -18,6 +18,8 @@ import java.util.Optional;
|
||||
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
|
||||
public class OptionalAccess {
|
||||
|
||||
public static String ALL_DEVICES_SELECTOR = "*";
|
||||
|
||||
public static void verify(Optional<Account> requestAccount,
|
||||
Optional<Anonymous> accessKey,
|
||||
Optional<Account> 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<Device> 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<Account> requestAccount,
|
||||
Optional<Anonymous> accessKey,
|
||||
Optional<Account> targetAccount)
|
||||
{
|
||||
Optional<Account> 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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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<Device> 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());
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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())),
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
@@ -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<String, AttributeValue> 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()
|
||||
|
||||
@@ -490,7 +490,6 @@ public class AccountsManager {
|
||||
|
||||
account.getDevices()
|
||||
.stream()
|
||||
.filter(Device::hasMessageDeliveryChannel)
|
||||
.forEach(device -> device.setPhoneNumberIdentityRegistrationId(pniRegistrationIds.get(device.getId())));
|
||||
|
||||
account.setPhoneNumberIdentityKey(pniIdentityKey);
|
||||
|
||||
@@ -89,7 +89,6 @@ public class DestinationDeviceValidator {
|
||||
final Set<Byte> excludedDeviceIds) throws MismatchedDevicesException {
|
||||
|
||||
final Set<Byte> 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<Byte> 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<Byte> extraDeviceIds = new HashSet<>(messageDeviceIds);
|
||||
extraDeviceIds.removeAll(accountDeviceIds);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user