Gracefully handle `NotPushRegisteredException`

This commit is contained in:
Jon Chambers 2024-06-25 11:23:16 -04:00 committed by GitHub
parent 2619569549
commit cb5cd64c05
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 66 additions and 154 deletions

View File

@ -112,7 +112,6 @@ import org.whispersystems.textsecuregcm.metrics.MetricsUtil;
import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil; import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
import org.whispersystems.textsecuregcm.providers.MultiRecipientMessageProvider; import org.whispersystems.textsecuregcm.providers.MultiRecipientMessageProvider;
import org.whispersystems.textsecuregcm.push.MessageSender; import org.whispersystems.textsecuregcm.push.MessageSender;
import org.whispersystems.textsecuregcm.push.NotPushRegisteredException;
import org.whispersystems.textsecuregcm.push.PushNotificationManager; import org.whispersystems.textsecuregcm.push.PushNotificationManager;
import org.whispersystems.textsecuregcm.push.ReceiptSender; import org.whispersystems.textsecuregcm.push.ReceiptSender;
import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider; import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider;
@ -180,7 +179,6 @@ public class MessageController {
private static final String RATE_LIMITED_MESSAGE_COUNTER_NAME = name(MessageController.class, "rateLimitedMessage"); 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 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 = private static final Timer SEND_MESSAGE_LATENCY_TIMER =
Timer.builder(MetricsUtil.name(MessageController.class, "sendMessageLatency")) Timer.builder(MetricsUtil.name(MessageController.class, "sendMessageLatency"))
.publishPercentileHistogram(true) .publishPercentileHistogram(true)
@ -434,8 +432,6 @@ public class MessageController {
} }
return Response.ok(new SendMessageResponse(needsSync)).build(); return Response.ok(new SendMessageResponse(needsSync)).build();
} catch (NoSuchUserException e) {
throw new WebApplicationException(Response.status(404).build());
} catch (MismatchedDevicesException e) { } catch (MismatchedDevicesException e) {
throw new WebApplicationException(Response.status(409) throw new WebApplicationException(Response.status(409)
.type(MediaType.APPLICATION_JSON_TYPE) .type(MediaType.APPLICATION_JSON_TYPE)
@ -627,8 +623,6 @@ public class MessageController {
.build(); .build();
} }
List<ServiceIdentifier> uuids404 = Collections.synchronizedList(new ArrayList<>());
try { try {
CompletableFuture.allOf( CompletableFuture.allOf(
recipients.values().stream() recipients.values().stream()
@ -649,19 +643,12 @@ public class MessageController {
// we asserted this must exist in validateCompleteDeviceList // we asserted this must exist in validateCompleteDeviceList
final Device destinationDevice = destinationAccount.getDevice(deviceId).orElseThrow(); final Device destinationDevice = destinationAccount.getDevice(deviceId).orElseThrow();
try {
sentMessageCounter.increment(); sentMessageCounter.increment();
sendCommonPayloadMessage( sendCommonPayloadMessage(
destinationAccount, destinationDevice, recipientData.serviceIdentifier(), timestamp, destinationAccount, destinationDevice, recipientData.serviceIdentifier(), timestamp,
online, online,
isStory, isUrgent, payload); 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());
}
}, },
multiRecipientMessageExecutor)); multiRecipientMessageExecutor));
}) })
@ -677,7 +664,7 @@ public class MessageController {
logger.error("partial failure while delivering multi-recipient messages", e.getCause()); logger.error("partial failure while delivering multi-recipient messages", e.getCause());
return Response.serverError().entity("failure during delivery").build(); 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( private void checkGroupSendToken(
@ -858,32 +845,27 @@ public class MessageController {
boolean urgent, boolean urgent,
IncomingMessage incomingMessage, IncomingMessage incomingMessage,
String userAgentString, String userAgentString,
Optional<byte[]> spamReportToken) Optional<byte[]> spamReportToken) {
throws NoSuchUserException {
final Envelope envelope;
try { try {
final Envelope envelope; final Account sourceAccount = source.map(AuthenticatedAccount::getAccount).orElse(null);
final Byte sourceDeviceId = source.map(account -> account.getAuthenticatedDevice().getId()).orElse(null);
try { envelope = incomingMessage.toEnvelope(
Account sourceAccount = source.map(AuthenticatedAccount::getAccount).orElse(null); destinationIdentifier,
Byte sourceDeviceId = source.map(account -> account.getAuthenticatedDevice().getId()).orElse(null); sourceAccount,
envelope = incomingMessage.toEnvelope( sourceDeviceId,
destinationIdentifier, timestamp == 0 ? System.currentTimeMillis() : timestamp,
sourceAccount, story,
sourceDeviceId, urgent,
timestamp == 0 ? System.currentTimeMillis() : timestamp, spamReportToken.orElse(null));
story, } catch (final IllegalArgumentException e) {
urgent, logger.warn("Received bad envelope type {} from {}", incomingMessage.type(), userAgentString);
spamReportToken.orElse(null)); throw new BadRequestException(e);
} catch (final IllegalArgumentException e) {
logger.warn("Received bad envelope type {} from {}", incomingMessage.type(), userAgentString);
throw new BadRequestException(e);
}
messageSender.sendMessage(destinationAccount, destinationDevice, envelope, online);
} catch (NotPushRegisteredException e) {
if (destinationDevice.isPrimary()) throw new NoSuchUserException(e);
else logger.debug("Not registered", e);
} }
messageSender.sendMessage(destinationAccount, destinationDevice, envelope, online);
} }
private void sendCommonPayloadMessage(Account destinationAccount, private void sendCommonPayloadMessage(Account destinationAccount,
@ -893,28 +875,21 @@ public class MessageController {
boolean online, boolean online,
boolean story, boolean story,
boolean urgent, boolean urgent,
byte[] payload) throws NoSuchUserException { byte[] payload) {
try {
Envelope.Builder messageBuilder = Envelope.newBuilder();
long serverTimestamp = System.currentTimeMillis();
messageBuilder final Envelope.Builder messageBuilder = Envelope.newBuilder();
.setType(Type.UNIDENTIFIED_SENDER) final long serverTimestamp = System.currentTimeMillis();
.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); messageBuilder
} catch (NotPushRegisteredException e) { .setType(Type.UNIDENTIFIED_SENDER)
if (destinationDevice.isPrimary()) { .setTimestamp(timestamp == 0 ? serverTimestamp : timestamp)
throw new NoSuchUserException(e); .setServerTimestamp(serverTimestamp)
} else { .setContent(ByteString.copyFrom(payload))
logger.debug("Not registered", e); .setStory(story)
} .setUrgent(urgent)
} .setDestinationUuid(serviceIdentifier.toServiceIdentifierString());
messageSender.sendMessage(destinationAccount, destinationDevice, messageBuilder.build(), online);
} }
private void checkMessageRateLimit(AuthenticatedAccount source, Account destination, String userAgent) private void checkMessageRateLimit(AuthenticatedAccount source, Account destination, String userAgent)

View File

@ -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);
}
}

View File

@ -52,8 +52,7 @@ public class MessageSender {
this.pushLatencyManager = pushLatencyManager; this.pushLatencyManager = pushLatencyManager;
} }
public void sendMessage(final Account account, final Device device, final Envelope message, final boolean online) public void sendMessage(final Account account, final Device device, final Envelope message, final boolean online) {
throws NotPushRegisteredException {
final String channel; final String channel;
@ -64,7 +63,7 @@ public class MessageSender {
} else if (device.getFetchesMessages()) { } else if (device.getFetchesMessages()) {
channel = "websocket"; channel = "websocket";
} else { } else {
throw new AssertionError(); channel = "none";
} }
final boolean clientPresent; final boolean clientPresent;
@ -89,10 +88,7 @@ public class MessageSender {
final boolean useVoip = StringUtils.isNotBlank(device.getVoipApnId()); final boolean useVoip = StringUtils.isNotBlank(device.getVoipApnId());
RedisOperation.unchecked(() -> pushLatencyManager.recordPushSent(account.getUuid(), device.getId(), useVoip, message.getUrgent())); RedisOperation.unchecked(() -> pushLatencyManager.recordPushSent(account.getUuid(), device.getId(), useVoip, message.getUrgent()));
} catch (final NotPushRegisteredException e) { } catch (final NotPushRegisteredException ignored) {
if (!device.getFetchesMessages()) {
throw e;
}
} }
} }
} }

View File

@ -55,8 +55,6 @@ public class ReceiptSender {
for (final Device destinationDevice : destinationAccount.getDevices()) { for (final Device destinationDevice : destinationAccount.getDevices()) {
try { try {
messageSender.sendMessage(destinationAccount, destinationDevice, message.build(), false); 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) { } catch (final Exception e) {
logger.warn("Could not send delivery receipt", e); logger.warn("Could not send delivery receipt", e);
} }

View File

@ -130,23 +130,20 @@ public class ChangeNumberManager {
logger.debug("destination device not present"); logger.debug("destination device not present");
return; 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); final long serverTimestamp = System.currentTimeMillis();
} catch (NotPushRegisteredException e) { final Envelope envelope = Envelope.newBuilder()
logger.debug("Not registered", e); .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);
} }
} }

View File

@ -20,7 +20,6 @@ import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset; import static org.mockito.Mockito.reset;
@ -109,7 +108,6 @@ import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper
import org.whispersystems.textsecuregcm.metrics.MessageMetrics; import org.whispersystems.textsecuregcm.metrics.MessageMetrics;
import org.whispersystems.textsecuregcm.providers.MultiRecipientMessageProvider; import org.whispersystems.textsecuregcm.providers.MultiRecipientMessageProvider;
import org.whispersystems.textsecuregcm.push.MessageSender; import org.whispersystems.textsecuregcm.push.MessageSender;
import org.whispersystems.textsecuregcm.push.NotPushRegisteredException;
import org.whispersystems.textsecuregcm.push.PushNotificationManager; import org.whispersystems.textsecuregcm.push.PushNotificationManager;
import org.whispersystems.textsecuregcm.push.ReceiptSender; import org.whispersystems.textsecuregcm.push.ReceiptSender;
import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider; import org.whispersystems.textsecuregcm.spam.ReportSpamTokenProvider;
@ -1681,50 +1679,6 @@ class MessageControllerTest {
Arguments.of(MULTI_DEVICE_PNI_ID)); Arguments.of(MULTI_DEVICE_PNI_ID));
} }
@ParameterizedTest
@MethodSource
void sendMultiRecipientMessage404(final ServiceIdentifier serviceIdentifier, final int regId1, final int regId2)
throws NotPushRegisteredException {
final List<Recipient> 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<InputStream> 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<Arguments> 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 @Test
void sendMultiRecipientMessageStoryRateLimited() { void sendMultiRecipientMessageStoryRateLimited() {
final List<Recipient> recipients = List.of(new Recipient(SINGLE_DEVICE_ACI_ID, SINGLE_DEVICE_ID1, SINGLE_DEVICE_REG_ID1, new byte[48])); final List<Recipient> recipients = List.of(new Recipient(SINGLE_DEVICE_ACI_ID, SINGLE_DEVICE_ID1, SINGLE_DEVICE_REG_ID1, new byte[48]));

View File

@ -142,6 +142,16 @@ class MessageSenderTest {
verify(messagesManager).insert(ACCOUNT_UUID, DEVICE_ID, message); 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() { private MessageProtos.Envelope generateRandomMessage() {
return MessageProtos.Envelope.newBuilder() return MessageProtos.Envelope.newBuilder()
.setTimestamp(System.currentTimeMillis()) .setTimestamp(System.currentTimeMillis())