diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java index 445127b27..acbba7285 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java @@ -475,7 +475,10 @@ public class MessageController { Deliver a common-payload message to multiple recipients. An unidentifed-access key for all recipients must be provided, unless the message is a story. """) - @ApiResponse(responseCode="200", description="Message was successfully sent to all recipients", useReturnTypeSchema=true) + @ApiResponse( + responseCode="200", + description="Message was successfully sent", + content = @Content(schema = @Schema(implementation = SendMultiRecipientMessageResponse.class))) @ApiResponse(responseCode="400", description="The envelope specified delivery to the same recipient device multiple times") @ApiResponse( responseCode="401", @@ -578,7 +581,7 @@ public class MessageController { return Mono.fromFuture(() -> accountsManager.getByServiceIdentifierAsync(serviceIdentifier)) .flatMap(Mono::justOrEmpty) - .switchIfEmpty(isStory ? Mono.empty() : Mono.error(NotFoundException::new)) + .switchIfEmpty(isStory || groupSendToken != null ? Mono.empty() : Mono.error(NotFoundException::new)) .map(account -> Tuples.of(serviceIdAndRecipient.getValue(), account)); }, MAX_FETCH_ACCOUNT_CONCURRENCY) .collectMap(Tuple2::getT1, Tuple2::getT2) @@ -677,12 +680,20 @@ public class MessageController { } try { - messageSender.sendMultiRecipientMessage(multiRecipientMessage, resolvedRecipients, timestamp, isStory, online, isUrgent).get(); + if (!resolvedRecipients.isEmpty()) { + messageSender.sendMultiRecipientMessage(multiRecipientMessage, resolvedRecipients, timestamp, isStory, online, isUrgent).get(); + } + + final List unresolvedRecipientServiceIds = authType == AUTH_TYPE_GROUP_SEND_TOKEN ? new ArrayList<>() : List.of(); multiRecipientMessage.getRecipients().forEach((serviceId, recipient) -> { if (!resolvedRecipients.containsKey(recipient)) { - // We skipped sending to this recipient because we're sending a story and couldn't resolve the recipient to - // an existing account; don't increment the counter for this recipient. + // We skipped sending to this recipient because we couldn't resolve the recipient to an + // existing account; don't increment the counter for this recipient. If the client was + // using a GSE, track the missing recipients to include in the response. + if (authType == AUTH_TYPE_GROUP_SEND_TOKEN) { + unresolvedRecipientServiceIds.add(ServiceIdentifier.fromLibsignal(serviceId)); + } return; } @@ -701,6 +712,8 @@ public class MessageController { Tag.of(IDENTITY_TYPE_TAG_NAME, identityType))) .increment(recipient.getDevices().length); }); + + return Response.ok(new SendMultiRecipientMessageResponse(unresolvedRecipientServiceIds)).build(); } catch (InterruptedException e) { logger.error("interrupted while delivering multi-recipient messages", e); throw new InternalServerErrorException("interrupted during delivery"); @@ -711,7 +724,6 @@ public class MessageController { logger.error("partial failure while delivering multi-recipient messages", e.getCause()); throw new InternalServerErrorException("failure during delivery"); } - return Response.ok(new SendMultiRecipientMessageResponse(Collections.emptyList())).build(); } private void checkGroupSendToken( diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/SendMultiRecipientMessageResponse.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/SendMultiRecipientMessageResponse.java index cc3967df9..6f3b7dd8f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/SendMultiRecipientMessageResponse.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/SendMultiRecipientMessageResponse.java @@ -5,16 +5,18 @@ 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 com.google.common.annotations.VisibleForTesting; + +import io.swagger.v3.oas.annotations.media.Schema; + +import java.util.List; import org.whispersystems.textsecuregcm.identity.ServiceIdentifier; import org.whispersystems.textsecuregcm.util.ServiceIdentifierAdapter; -import java.util.List; -import java.util.UUID; -public record SendMultiRecipientMessageResponse(@JsonSerialize(contentUsing = ServiceIdentifierAdapter.ServiceIdentifierSerializer.class) - @JsonDeserialize(contentUsing = ServiceIdentifierAdapter.ServiceIdentifierDeserializer.class) - List uuids404) { +public record SendMultiRecipientMessageResponse( + @Schema(description = "a list of the service identifiers in the request that do not correspond to registered Signal users; will only be present if a group send endorsement was supplied for the request") + @JsonSerialize(contentUsing = ServiceIdentifierAdapter.ServiceIdentifierSerializer.class) + @JsonDeserialize(contentUsing = ServiceIdentifierAdapter.ServiceIdentifierDeserializer.class) + List uuids404) { } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java index 1ccdf2334..80577aa55 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java @@ -9,6 +9,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInAnyOrder; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -47,6 +48,7 @@ import java.time.ZoneOffset; import java.time.temporal.ChronoUnit; import java.util.Arrays; import java.util.Base64; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -82,6 +84,7 @@ import org.whispersystems.textsecuregcm.entities.MessageProtos.Envelope; import org.whispersystems.textsecuregcm.entities.MismatchedDevices; import org.whispersystems.textsecuregcm.entities.OutgoingMessageEntity; import org.whispersystems.textsecuregcm.entities.OutgoingMessageEntityList; +import org.whispersystems.textsecuregcm.entities.SendMultiRecipientMessageResponse; import org.whispersystems.textsecuregcm.entities.SpamReport; import org.whispersystems.textsecuregcm.entities.StaleDevices; import org.whispersystems.textsecuregcm.identity.AciServiceIdentifier; @@ -1162,7 +1165,8 @@ class MessageControllerTest { final Optional maybeAccessKey, final Optional maybeGroupSendToken, final int expectedStatus, - final Set expectedResolvedAccounts) { + final Set expectedResolvedAccounts, + final Set expectedUuids404) { clock.pin(START_OF_DAY); @@ -1204,6 +1208,11 @@ class MessageControllerTest { assertThat(response.getStatus(), is(equalTo(expectedStatus))); + if (expectedStatus == 200) { + final SendMultiRecipientMessageResponse entity = response.readEntity(SendMultiRecipientMessageResponse.class); + assertThat(Set.copyOf(entity.uuids404()), equalTo(expectedUuids404)); + } + if (expectedStatus == 200 && !expectedResolvedAccounts.isEmpty()) { verify(messageSender).sendMultiRecipientMessage(any(), argThat(resolvedRecipients -> @@ -1283,7 +1292,8 @@ class MessageControllerTest { Optional.empty(), Optional.empty(), 200, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Multi-recipient message with combined UAKs", accountsByServiceIdentifier, @@ -1294,7 +1304,8 @@ class MessageControllerTest { Optional.of(Base64.getEncoder().encodeToString(UnidentifiedAccessUtil.getCombinedUnidentifiedAccessKey(List.of(singleDeviceAccount, multiDeviceAccount)))), Optional.empty(), 200, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Multi-recipient message with group send endorsement", accountsByServiceIdentifier, @@ -1305,7 +1316,8 @@ class MessageControllerTest { Optional.empty(), Optional.of(groupSendEndorsement), 200, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Incorrect combined UAK", accountsByServiceIdentifier, @@ -1316,7 +1328,8 @@ class MessageControllerTest { Optional.of(Base64.getEncoder().encodeToString(TestRandomUtil.nextBytes(UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH))), Optional.empty(), 401, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Incorrect group send endorsement", accountsByServiceIdentifier, @@ -1329,7 +1342,8 @@ class MessageControllerTest { List.of(new AciServiceIdentifier(UUID.randomUUID())), START_OF_DAY.plus(Duration.ofDays(1)))), 401, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), // Stories don't require credentials of any kind, but for historical reasons, we don't reject a combined UAK if // provided @@ -1342,7 +1356,8 @@ class MessageControllerTest { Optional.of(Base64.getEncoder().encodeToString(UnidentifiedAccessUtil.getCombinedUnidentifiedAccessKey(List.of(singleDeviceAccount, multiDeviceAccount)))), Optional.empty(), 200, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Story with group send endorsement", accountsByServiceIdentifier, @@ -1353,7 +1368,8 @@ class MessageControllerTest { Optional.empty(), Optional.of(groupSendEndorsement), 400, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Conflicting credentials", accountsByServiceIdentifier, @@ -1364,7 +1380,8 @@ class MessageControllerTest { Optional.of(Base64.getEncoder().encodeToString(UnidentifiedAccessUtil.getCombinedUnidentifiedAccessKey(List.of(singleDeviceAccount, multiDeviceAccount)))), Optional.of(groupSendEndorsement), 400, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("No credentials", accountsByServiceIdentifier, @@ -1375,7 +1392,8 @@ class MessageControllerTest { Optional.empty(), Optional.empty(), 401, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Oversized payload", accountsByServiceIdentifier, @@ -1390,7 +1408,8 @@ class MessageControllerTest { Optional.empty(), Optional.of(groupSendEndorsement), 413, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Negative timestamp", accountsByServiceIdentifier, @@ -1401,7 +1420,8 @@ class MessageControllerTest { Optional.empty(), Optional.of(groupSendEndorsement), 400, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Excessive timestamp", accountsByServiceIdentifier, @@ -1412,7 +1432,8 @@ class MessageControllerTest { Optional.empty(), Optional.of(groupSendEndorsement), 400, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Empty recipient list", accountsByServiceIdentifier, @@ -1425,7 +1446,8 @@ class MessageControllerTest { List.of(), START_OF_DAY.plus(Duration.ofDays(1)))), 400, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Story with empty recipient list", accountsByServiceIdentifier, @@ -1436,7 +1458,8 @@ class MessageControllerTest { Optional.empty(), Optional.empty(), 400, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Duplicate recipient", accountsByServiceIdentifier, @@ -1449,7 +1472,8 @@ class MessageControllerTest { Optional.empty(), Optional.of(groupSendEndorsement), 400, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Missing account", Map.of(), @@ -1459,8 +1483,21 @@ class MessageControllerTest { false, Optional.empty(), Optional.of(groupSendEndorsement), - 404, - Collections.emptySet()), + 200, + Collections.emptySet(), + Set.of(new AciServiceIdentifier(singleDeviceAccountAci), new AciServiceIdentifier(multiDeviceAccountAci))), + + Arguments.argumentSet("One missing and one existing account", + Map.of(new AciServiceIdentifier(singleDeviceAccountAci), singleDeviceAccount), + aciMessage, + clock.instant().toEpochMilli(), + false, + false, + Optional.empty(), + Optional.of(groupSendEndorsement), + 200, + Set.of(singleDeviceAccount), + Set.of(new AciServiceIdentifier(multiDeviceAccountAci))), Arguments.argumentSet("Missing account for story", Map.of(), @@ -1471,7 +1508,20 @@ class MessageControllerTest { Optional.empty(), Optional.empty(), 200, - Collections.emptySet()), + Collections.emptySet(), + Set.of()), + + Arguments.argumentSet("One missing and one existing account for story", + Map.of(new AciServiceIdentifier(singleDeviceAccountAci), singleDeviceAccount), + aciMessage, + clock.instant().toEpochMilli(), + true, + false, + Optional.empty(), + Optional.empty(), + 200, + Set.of(singleDeviceAccount), + Set.of()), Arguments.argumentSet("Missing device", accountsByServiceIdentifier, @@ -1484,7 +1534,8 @@ class MessageControllerTest { Optional.empty(), Optional.of(groupSendEndorsement), 409, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Extra device", accountsByServiceIdentifier, @@ -1499,7 +1550,8 @@ class MessageControllerTest { Optional.empty(), Optional.of(groupSendEndorsement), 409, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Stale registration ID", accountsByServiceIdentifier, @@ -1513,7 +1565,8 @@ class MessageControllerTest { Optional.empty(), Optional.of(groupSendEndorsement), 410, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Rate-limited story", accountsByServiceIdentifier, @@ -1524,7 +1577,8 @@ class MessageControllerTest { Optional.empty(), Optional.empty(), 429, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Story to PNI recipients", accountsByServiceIdentifier, @@ -1538,7 +1592,8 @@ class MessageControllerTest { Optional.empty(), Optional.empty(), 200, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Multi-recipient message to PNI recipients with UAK", accountsByServiceIdentifier, @@ -1552,7 +1607,8 @@ class MessageControllerTest { Optional.of(Base64.getEncoder().encodeToString(UnidentifiedAccessUtil.getCombinedUnidentifiedAccessKey(List.of(singleDeviceAccount, multiDeviceAccount)))), Optional.empty(), 401, - Set.of(singleDeviceAccount, multiDeviceAccount)), + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()), Arguments.argumentSet("Multi-recipient message to PNI recipients with group send endorsement", accountsByServiceIdentifier, @@ -1568,7 +1624,8 @@ class MessageControllerTest { List.of(new PniServiceIdentifier(singleDeviceAccountPni), new PniServiceIdentifier(multiDeviceAccountPni)), START_OF_DAY.plus(Duration.ofDays(1)))), 200, - Set.of(singleDeviceAccount, multiDeviceAccount)) + Set.of(singleDeviceAccount, multiDeviceAccount), + Set.of()) ); }