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 091f7fd0e..854ae7adc 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java @@ -112,7 +112,6 @@ import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil; import org.whispersystems.textsecuregcm.providers.MultiRecipientMessageProvider; import org.whispersystems.textsecuregcm.push.MessageSender; -import org.whispersystems.textsecuregcm.push.NotPushRegisteredException; import org.whispersystems.textsecuregcm.push.PushNotificationManager; import org.whispersystems.textsecuregcm.push.ReceiptSender; import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider; @@ -180,7 +179,6 @@ private record MultiRecipientDeliveryData( private static final String RATE_LIMITED_MESSAGE_COUNTER_NAME = name(MessageController.class, "rateLimitedMessage"); private static final String REJECT_INVALID_ENVELOPE_TYPE = name(MessageController.class, "rejectInvalidEnvelopeType"); - private static final String UNEXPECTED_MISSING_USER_COUNTER_NAME = name(MessageController.class, "unexpectedMissingDestinationForMultiRecipientMessage"); private static final Timer SEND_MESSAGE_LATENCY_TIMER = Timer.builder(MetricsUtil.name(MessageController.class, "sendMessageLatency")) .publishPercentileHistogram(true) @@ -434,8 +432,6 @@ public Response sendMessage(@ReadOnly @Auth Optional sourc } return Response.ok(new SendMessageResponse(needsSync)).build(); - } catch (NoSuchUserException e) { - throw new WebApplicationException(Response.status(404).build()); } catch (MismatchedDevicesException e) { throw new WebApplicationException(Response.status(409) .type(MediaType.APPLICATION_JSON_TYPE) @@ -627,8 +623,6 @@ public Response sendMultiRecipientMessage( .build(); } - List uuids404 = Collections.synchronizedList(new ArrayList<>()); - try { CompletableFuture.allOf( recipients.values().stream() @@ -649,19 +643,12 @@ public Response sendMultiRecipientMessage( // we asserted this must exist in validateCompleteDeviceList final Device destinationDevice = destinationAccount.getDevice(deviceId).orElseThrow(); - try { - sentMessageCounter.increment(); - sendCommonPayloadMessage( - destinationAccount, destinationDevice, recipientData.serviceIdentifier(), timestamp, - online, - isStory, isUrgent, payload); - } catch (NoSuchUserException e) { - // this should never happen, because we already asserted the device is present and enabled - Metrics.counter( - UNEXPECTED_MISSING_USER_COUNTER_NAME, - Tags.of("isPrimary", String.valueOf(destinationDevice.isPrimary()))).increment(); - uuids404.add(recipientData.serviceIdentifier()); - } + + sentMessageCounter.increment(); + sendCommonPayloadMessage( + destinationAccount, destinationDevice, recipientData.serviceIdentifier(), timestamp, + online, + isStory, isUrgent, payload); }, multiRecipientMessageExecutor)); }) @@ -677,7 +664,7 @@ public Response sendMultiRecipientMessage( logger.error("partial failure while delivering multi-recipient messages", e.getCause()); return Response.serverError().entity("failure during delivery").build(); } - return Response.ok(new SendMultiRecipientMessageResponse(uuids404)).build(); + return Response.ok(new SendMultiRecipientMessageResponse(Collections.emptyList())).build(); } private void checkGroupSendToken( @@ -858,32 +845,27 @@ private void sendIndividualMessage( boolean urgent, IncomingMessage incomingMessage, String userAgentString, - Optional spamReportToken) - throws NoSuchUserException { - try { - final Envelope envelope; + Optional spamReportToken) { - try { - Account sourceAccount = source.map(AuthenticatedAccount::getAccount).orElse(null); - Byte sourceDeviceId = source.map(account -> account.getAuthenticatedDevice().getId()).orElse(null); - envelope = incomingMessage.toEnvelope( - destinationIdentifier, - sourceAccount, - sourceDeviceId, - timestamp == 0 ? System.currentTimeMillis() : timestamp, - story, - urgent, - spamReportToken.orElse(null)); - } catch (final IllegalArgumentException e) { - logger.warn("Received bad envelope type {} from {}", incomingMessage.type(), userAgentString); - throw new BadRequestException(e); - } + final Envelope envelope; - messageSender.sendMessage(destinationAccount, destinationDevice, envelope, online); - } catch (NotPushRegisteredException e) { - if (destinationDevice.isPrimary()) throw new NoSuchUserException(e); - else logger.debug("Not registered", e); + try { + final Account sourceAccount = source.map(AuthenticatedAccount::getAccount).orElse(null); + final Byte sourceDeviceId = source.map(account -> account.getAuthenticatedDevice().getId()).orElse(null); + envelope = incomingMessage.toEnvelope( + destinationIdentifier, + sourceAccount, + sourceDeviceId, + timestamp == 0 ? System.currentTimeMillis() : timestamp, + story, + urgent, + spamReportToken.orElse(null)); + } catch (final IllegalArgumentException e) { + logger.warn("Received bad envelope type {} from {}", incomingMessage.type(), userAgentString); + throw new BadRequestException(e); } + + messageSender.sendMessage(destinationAccount, destinationDevice, envelope, online); } private void sendCommonPayloadMessage(Account destinationAccount, @@ -893,28 +875,21 @@ private void sendCommonPayloadMessage(Account destinationAccount, boolean online, boolean story, boolean urgent, - byte[] payload) throws NoSuchUserException { - try { - Envelope.Builder messageBuilder = Envelope.newBuilder(); - long serverTimestamp = System.currentTimeMillis(); - - messageBuilder - .setType(Type.UNIDENTIFIED_SENDER) - .setTimestamp(timestamp == 0 ? serverTimestamp : timestamp) - .setServerTimestamp(serverTimestamp) - .setContent(ByteString.copyFrom(payload)) - .setStory(story) - .setUrgent(urgent) - .setDestinationUuid(serviceIdentifier.toServiceIdentifierString()); - - messageSender.sendMessage(destinationAccount, destinationDevice, messageBuilder.build(), online); - } catch (NotPushRegisteredException e) { - if (destinationDevice.isPrimary()) { - throw new NoSuchUserException(e); - } else { - logger.debug("Not registered", e); - } - } + byte[] payload) { + + final Envelope.Builder messageBuilder = Envelope.newBuilder(); + final long serverTimestamp = System.currentTimeMillis(); + + messageBuilder + .setType(Type.UNIDENTIFIED_SENDER) + .setTimestamp(timestamp == 0 ? serverTimestamp : timestamp) + .setServerTimestamp(serverTimestamp) + .setContent(ByteString.copyFrom(payload)) + .setStory(story) + .setUrgent(urgent) + .setDestinationUuid(serviceIdentifier.toServiceIdentifierString()); + + messageSender.sendMessage(destinationAccount, destinationDevice, messageBuilder.build(), online); } private void checkMessageRateLimit(AuthenticatedAccount source, Account destination, String userAgent) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/NoSuchUserException.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/NoSuchUserException.java deleted file mode 100644 index 68c3c58b0..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/NoSuchUserException.java +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Copyright 2013-2020 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ -package org.whispersystems.textsecuregcm.controllers; - -import java.util.UUID; - -public class NoSuchUserException extends Exception { - - public NoSuchUserException(final UUID uuid) { - super(uuid.toString()); - } - - public NoSuchUserException(Exception e) { - super(e); - } -} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java index 27c8e4c52..035306715 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java @@ -52,8 +52,7 @@ public MessageSender(ClientPresenceManager clientPresenceManager, this.pushLatencyManager = pushLatencyManager; } - public void sendMessage(final Account account, final Device device, final Envelope message, final boolean online) - throws NotPushRegisteredException { + public void sendMessage(final Account account, final Device device, final Envelope message, final boolean online) { final String channel; @@ -64,7 +63,7 @@ public void sendMessage(final Account account, final Device device, final Envelo } else if (device.getFetchesMessages()) { channel = "websocket"; } else { - throw new AssertionError(); + channel = "none"; } final boolean clientPresent; @@ -89,10 +88,7 @@ public void sendMessage(final Account account, final Device device, final Envelo final boolean useVoip = StringUtils.isNotBlank(device.getVoipApnId()); RedisOperation.unchecked(() -> pushLatencyManager.recordPushSent(account.getUuid(), device.getId(), useVoip, message.getUrgent())); - } catch (final NotPushRegisteredException e) { - if (!device.getFetchesMessages()) { - throw e; - } + } catch (final NotPushRegisteredException ignored) { } } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/ReceiptSender.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/ReceiptSender.java index f6605fa56..fff044b84 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/ReceiptSender.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/ReceiptSender.java @@ -55,8 +55,6 @@ public void sendReceipt(ServiceIdentifier sourceIdentifier, byte sourceDeviceId, for (final Device destinationDevice : destinationAccount.getDevices()) { try { messageSender.sendMessage(destinationAccount, destinationDevice, message.build(), false); - } catch (final NotPushRegisteredException e) { - logger.debug("User no longer push registered for delivery receipt: {}", e.getMessage()); } catch (final Exception e) { logger.warn("Could not send delivery receipt", e); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java index 2c8dcc4f7..927f537be 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java @@ -130,23 +130,20 @@ void sendMessageToSelf( logger.debug("destination device not present"); return; } - try { - long serverTimestamp = System.currentTimeMillis(); - Envelope envelope = Envelope.newBuilder() - .setType(Envelope.Type.forNumber(message.type())) - .setTimestamp(serverTimestamp) - .setServerTimestamp(serverTimestamp) - .setDestinationUuid(new AciServiceIdentifier(sourceAndDestinationAccount.getUuid()).toServiceIdentifierString()) - .setContent(ByteString.copyFrom(contents.get())) - .setSourceUuid(new AciServiceIdentifier(sourceAndDestinationAccount.getUuid()).toServiceIdentifierString()) - .setSourceDevice((int) Device.PRIMARY_ID) - .setUpdatedPni(sourceAndDestinationAccount.getPhoneNumberIdentifier().toString()) - .setUrgent(true) - .build(); - - messageSender.sendMessage(sourceAndDestinationAccount, destinationDevice.get(), envelope, false); - } catch (NotPushRegisteredException e) { - logger.debug("Not registered", e); - } + + final long serverTimestamp = System.currentTimeMillis(); + final Envelope envelope = Envelope.newBuilder() + .setType(Envelope.Type.forNumber(message.type())) + .setTimestamp(serverTimestamp) + .setServerTimestamp(serverTimestamp) + .setDestinationUuid(new AciServiceIdentifier(sourceAndDestinationAccount.getUuid()).toServiceIdentifierString()) + .setContent(ByteString.copyFrom(contents.get())) + .setSourceUuid(new AciServiceIdentifier(sourceAndDestinationAccount.getUuid()).toServiceIdentifierString()) + .setSourceDevice((int) Device.PRIMARY_ID) + .setUpdatedPni(sourceAndDestinationAccount.getPhoneNumberIdentifier().toString()) + .setUrgent(true) + .build(); + + messageSender.sendMessage(sourceAndDestinationAccount, destinationDevice.get(), envelope, false); } } 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 7a80fafd0..e09e94a08 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java @@ -20,7 +20,6 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.anyString; -import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; @@ -109,7 +108,6 @@ import org.whispersystems.textsecuregcm.metrics.MessageMetrics; import org.whispersystems.textsecuregcm.providers.MultiRecipientMessageProvider; import org.whispersystems.textsecuregcm.push.MessageSender; -import org.whispersystems.textsecuregcm.push.NotPushRegisteredException; import org.whispersystems.textsecuregcm.push.PushNotificationManager; import org.whispersystems.textsecuregcm.push.ReceiptSender; import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider; @@ -1681,50 +1679,6 @@ private static Stream sendMultiRecipientMessageStaleDevices() { Arguments.of(MULTI_DEVICE_PNI_ID)); } - @ParameterizedTest - @MethodSource - void sendMultiRecipientMessage404(final ServiceIdentifier serviceIdentifier, final int regId1, final int regId2) - throws NotPushRegisteredException { - - final List recipients = List.of( - new Recipient(serviceIdentifier, MULTI_DEVICE_ID1, regId1, new byte[48]), - new Recipient(serviceIdentifier, MULTI_DEVICE_ID2, regId2, new byte[48])); - - // initialize our binary payload and create an input stream - byte[] buffer = new byte[2048]; - // InputStream stream = initializeMultiPayload(recipientUUID, buffer); - InputStream stream = initializeMultiPayload(recipients, buffer, true); - - // set up the entity to use in our PUT request - Entity entity = Entity.entity(stream, MultiRecipientMessageProvider.MEDIA_TYPE); - - // start building the request - final Invocation.Builder invocationBuilder = resources - .getJerseyTest() - .target("/v1/messages/multi_recipient") - .queryParam("online", false) - .queryParam("ts", System.currentTimeMillis()) - .queryParam("story", true) - .queryParam("urgent", true) - .request() - .header(HttpHeaders.USER_AGENT, "FIXME") - .header(HeaderUtils.UNIDENTIFIED_ACCESS_KEY, Base64.getEncoder().encodeToString(UNIDENTIFIED_ACCESS_BYTES)); - - doThrow(NotPushRegisteredException.class) - .when(messageSender).sendMessage(any(), any(), any(), anyBoolean()); - - // make the PUT request - final SendMultiRecipientMessageResponse response = invocationBuilder.put(entity, SendMultiRecipientMessageResponse.class); - - assertEquals(List.of(serviceIdentifier), response.uuids404()); - } - - private static Stream sendMultiRecipientMessage404() { - return Stream.of( - Arguments.of(MULTI_DEVICE_ACI_ID, MULTI_DEVICE_REG_ID1, MULTI_DEVICE_REG_ID2), - Arguments.of(MULTI_DEVICE_PNI_ID, MULTI_DEVICE_PNI_REG_ID1, MULTI_DEVICE_PNI_REG_ID2)); - } - @Test void sendMultiRecipientMessageStoryRateLimited() { final List recipients = List.of(new Recipient(SINGLE_DEVICE_ACI_ID, SINGLE_DEVICE_ID1, SINGLE_DEVICE_REG_ID1, new byte[48])); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java index cb7ca6d3b..cee9abcae 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java @@ -142,6 +142,16 @@ void testSendMessageFetchClientNotPresent() throws Exception { verify(messagesManager).insert(ACCOUNT_UUID, DEVICE_ID, message); } + @Test + void testSendMessageNoChannel() { + when(device.getGcmId()).thenReturn(null); + when(device.getApnId()).thenReturn(null); + when(device.getFetchesMessages()).thenReturn(false); + + assertDoesNotThrow(() -> messageSender.sendMessage(account, device, message, false)); + verify(messagesManager).insert(ACCOUNT_UUID, DEVICE_ID, message); + } + private MessageProtos.Envelope generateRandomMessage() { return MessageProtos.Envelope.newBuilder() .setTimestamp(System.currentTimeMillis())