From c2ba8ab5622885f3b30a2d654d538d5457fcd2ee Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Fri, 3 Sep 2021 10:28:10 -0400 Subject: [PATCH] Identify receipt destinations by UUID instead of e164 --- .../controllers/MessageController.java | 4 +-- .../controllers/NoSuchUserException.java | 19 ++---------- .../textsecuregcm/push/ReceiptSender.java | 31 ++++++------------- .../websocket/WebSocketConnection.java | 8 +++-- .../controllers/MessageControllerTest.java | 4 +-- .../websocket/WebSocketConnectionTest.java | 8 +++-- 6 files changed, 26 insertions(+), 48 deletions(-) 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 e08575d68..38ad7bca4 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java @@ -498,7 +498,7 @@ public class MessageController { try { receiptSender.sendReceipt( new AuthenticatedAccount(() -> new Pair<>(destination, destination.getMasterDevice().get())), - source.getNumber(), timestamp); + source.getUuid(), timestamp); } catch (final NoSuchUserException ignored) { } }, receiptDelay.toMillis(), TimeUnit.MILLISECONDS); @@ -583,7 +583,7 @@ public class MessageController { WebSocketConnection.recordMessageDeliveryDuration(message.get().getTimestamp(), auth.getAuthenticatedDevice()); if (!Util.isEmpty(message.get().getSource()) && message.get().getType() != Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE) { - receiptSender.sendReceipt(auth, message.get().getSource(), message.get().getTimestamp()); + receiptSender.sendReceipt(auth, message.get().getSourceUuid(), message.get().getTimestamp()); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/NoSuchUserException.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/NoSuchUserException.java index 1f8500f3b..68c3c58b0 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/NoSuchUserException.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/NoSuchUserException.java @@ -4,28 +4,15 @@ */ package org.whispersystems.textsecuregcm.controllers; -import java.util.LinkedList; -import java.util.List; +import java.util.UUID; public class NoSuchUserException extends Exception { - private List missing; - - public NoSuchUserException(String user) { - super(user); - missing = new LinkedList<>(); - missing.add(user); - } - - public NoSuchUserException(List missing) { - this.missing = missing; + public NoSuchUserException(final UUID uuid) { + super(uuid.toString()); } public NoSuchUserException(Exception e) { super(e); } - - public List getMissing() { - return missing; - } } 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 739a5bcdc..c3f839bbf 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/ReceiptSender.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/ReceiptSender.java @@ -5,7 +5,7 @@ package org.whispersystems.textsecuregcm.push; -import java.util.Optional; +import java.util.UUID; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount; @@ -22,21 +22,21 @@ public class ReceiptSender { private static final Logger logger = LoggerFactory.getLogger(ReceiptSender.class); - public ReceiptSender(AccountsManager accountManager, - MessageSender messageSender) { + public ReceiptSender(final AccountsManager accountManager, final MessageSender messageSender) { this.accountManager = accountManager; this.messageSender = messageSender; } - public void sendReceipt(AuthenticatedAccount source, String destination, long messageId) - throws NoSuchUserException { + public void sendReceipt(AuthenticatedAccount source, UUID destinationUuid, long messageId) throws NoSuchUserException { final Account sourceAccount = source.getAccount(); - if (sourceAccount.getNumber().equals(destination)) { + if (sourceAccount.getUuid().equals(destinationUuid)) { return; } - Account destinationAccount = getDestinationAccount(destination); - Envelope.Builder message = Envelope.newBuilder() + final Account destinationAccount = accountManager.get(destinationUuid) + .orElseThrow(() -> new NoSuchUserException(destinationUuid)); + + final Envelope.Builder message = Envelope.newBuilder() .setServerTimestamp(System.currentTimeMillis()) .setSource(sourceAccount.getNumber()) .setSourceUuid(sourceAccount.getUuid().toString()) @@ -51,22 +51,9 @@ public class ReceiptSender { for (final Device destinationDevice : destinationAccount.getDevices()) { try { messageSender.sendMessage(destinationAccount, destinationDevice, message.build(), false); - } catch (NotPushRegisteredException e) { + } catch (final NotPushRegisteredException e) { logger.info("User no longer push registered for delivery receipt: " + e.getMessage()); } } } - - private Account getDestinationAccount(String destination) - throws NoSuchUserException - { - Optional account = accountManager.get(destination); - - if (account.isEmpty()) { - throw new NoSuchUserException(destination); - } - - return account.get(); - } - } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java b/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java index 8b9cc40ae..ac3b006fb 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java @@ -207,11 +207,13 @@ public class WebSocketConnection implements MessageAvailabilityListener, Displac if (!message.hasSource()) return; try { - receiptSender.sendReceipt(auth, message.getSource(), message.getTimestamp()); + receiptSender.sendReceipt(auth, UUID.fromString(message.getSourceUuid()), message.getTimestamp()); } catch (NoSuchUserException e) { - logger.info("No longer registered " + e.getMessage()); + logger.info("No longer registered: {}", e.getMessage()); } catch (WebApplicationException e) { - logger.warn("Bad federated response for receipt: " + e.getResponse().getStatus()); + logger.warn("Bad federated response for receipt: {}", e.getResponse().getStatus()); + } catch (IllegalArgumentException e) { + logger.error("Could not parse UUID: {}", message.getSourceUuid()); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java index 015a23bb6..bf871d51a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java @@ -291,7 +291,7 @@ class MessageControllerTest { assertThat("Good Response", response.getStatus(), is(equalTo(200))); verify(messageSender, never()).sendMessage(any(), any(), any(), anyBoolean()); - verify(receiptSender).sendReceipt(any(), eq(AuthHelper.VALID_NUMBER), anyLong()); + verify(receiptSender).sendReceipt(any(), eq(AuthHelper.VALID_UUID), anyLong()); } @ParameterizedTest @@ -575,7 +575,7 @@ class MessageControllerTest { .delete(); assertThat("Good Response Code", response.getStatus(), is(equalTo(204))); - verify(receiptSender).sendReceipt(any(AuthenticatedAccount.class), eq("+14152222222"), eq(timestamp)); + verify(receiptSender).sendReceipt(any(AuthenticatedAccount.class), eq(sourceUuid), eq(timestamp)); response = resources.getJerseyTest() .target(String.format("/v1/messages/uuid/%s", uuid2)) diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnectionTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnectionTest.java index 0261cbc3c..5d0b76a02 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnectionTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnectionTest.java @@ -210,7 +210,7 @@ public class WebSocketConnectionTest { futures.get(2).completeExceptionally(new IOException()); verify(storedMessages, times(1)).delete(eq(accountUuid), eq(2L), eq(outgoingMessages.get(1).getGuid())); - verify(receiptSender, times(1)).sendReceipt(eq(auth), eq("sender1"), eq(2222L)); + verify(receiptSender, times(1)).sendReceipt(eq(auth), eq(senderOneUuid), eq(2222L)); connection.stop(); verify(client).close(anyInt(), anyString()); @@ -279,6 +279,8 @@ public class WebSocketConnectionTest { public void testPendingSend() throws Exception { MessagesManager storedMessages = mock(MessagesManager.class); + final UUID senderTwoUuid = UUID.randomUUID(); + final Envelope firstMessage = Envelope.newBuilder() .setLegacyMessage(ByteString.copyFrom("first".getBytes())) .setSource("sender1") @@ -291,7 +293,7 @@ public class WebSocketConnectionTest { final Envelope secondMessage = Envelope.newBuilder() .setLegacyMessage(ByteString.copyFrom("second".getBytes())) .setSource("sender2") - .setSourceUuid(UUID.randomUUID().toString()) + .setSourceUuid(senderTwoUuid.toString()) .setTimestamp(System.currentTimeMillis()) .setSourceDevice(2) .setType(Envelope.Type.CIPHERTEXT) @@ -361,7 +363,7 @@ public class WebSocketConnectionTest { futures.get(1).complete(response); futures.get(0).completeExceptionally(new IOException()); - verify(receiptSender, times(1)).sendReceipt(eq(auth), eq("sender2"), eq(secondMessage.getTimestamp())); + verify(receiptSender, times(1)).sendReceipt(eq(auth), eq(senderTwoUuid), eq(secondMessage.getTimestamp())); connection.stop(); verify(client).close(anyInt(), anyString());