Identify receipt destinations by UUID instead of e164

This commit is contained in:
Jon Chambers 2021-09-03 10:28:10 -04:00 committed by Jon Chambers
parent cd49ea43c0
commit c2ba8ab562
6 changed files with 26 additions and 48 deletions

View File

@ -498,7 +498,7 @@ public class MessageController {
try { try {
receiptSender.sendReceipt( receiptSender.sendReceipt(
new AuthenticatedAccount(() -> new Pair<>(destination, destination.getMasterDevice().get())), new AuthenticatedAccount(() -> new Pair<>(destination, destination.getMasterDevice().get())),
source.getNumber(), timestamp); source.getUuid(), timestamp);
} catch (final NoSuchUserException ignored) { } catch (final NoSuchUserException ignored) {
} }
}, receiptDelay.toMillis(), TimeUnit.MILLISECONDS); }, receiptDelay.toMillis(), TimeUnit.MILLISECONDS);
@ -583,7 +583,7 @@ public class MessageController {
WebSocketConnection.recordMessageDeliveryDuration(message.get().getTimestamp(), auth.getAuthenticatedDevice()); WebSocketConnection.recordMessageDeliveryDuration(message.get().getTimestamp(), auth.getAuthenticatedDevice());
if (!Util.isEmpty(message.get().getSource()) if (!Util.isEmpty(message.get().getSource())
&& message.get().getType() != Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE) { && 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());
} }
} }

View File

@ -4,28 +4,15 @@
*/ */
package org.whispersystems.textsecuregcm.controllers; package org.whispersystems.textsecuregcm.controllers;
import java.util.LinkedList; import java.util.UUID;
import java.util.List;
public class NoSuchUserException extends Exception { public class NoSuchUserException extends Exception {
private List<String> missing; public NoSuchUserException(final UUID uuid) {
super(uuid.toString());
public NoSuchUserException(String user) {
super(user);
missing = new LinkedList<>();
missing.add(user);
}
public NoSuchUserException(List<String> missing) {
this.missing = missing;
} }
public NoSuchUserException(Exception e) { public NoSuchUserException(Exception e) {
super(e); super(e);
} }
public List<String> getMissing() {
return missing;
}
} }

View File

@ -5,7 +5,7 @@
package org.whispersystems.textsecuregcm.push; package org.whispersystems.textsecuregcm.push;
import java.util.Optional; import java.util.UUID;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount; import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount;
@ -22,21 +22,21 @@ public class ReceiptSender {
private static final Logger logger = LoggerFactory.getLogger(ReceiptSender.class); private static final Logger logger = LoggerFactory.getLogger(ReceiptSender.class);
public ReceiptSender(AccountsManager accountManager, public ReceiptSender(final AccountsManager accountManager, final MessageSender messageSender) {
MessageSender messageSender) {
this.accountManager = accountManager; this.accountManager = accountManager;
this.messageSender = messageSender; this.messageSender = messageSender;
} }
public void sendReceipt(AuthenticatedAccount source, String destination, long messageId) public void sendReceipt(AuthenticatedAccount source, UUID destinationUuid, long messageId) throws NoSuchUserException {
throws NoSuchUserException {
final Account sourceAccount = source.getAccount(); final Account sourceAccount = source.getAccount();
if (sourceAccount.getNumber().equals(destination)) { if (sourceAccount.getUuid().equals(destinationUuid)) {
return; return;
} }
Account destinationAccount = getDestinationAccount(destination); final Account destinationAccount = accountManager.get(destinationUuid)
Envelope.Builder message = Envelope.newBuilder() .orElseThrow(() -> new NoSuchUserException(destinationUuid));
final Envelope.Builder message = Envelope.newBuilder()
.setServerTimestamp(System.currentTimeMillis()) .setServerTimestamp(System.currentTimeMillis())
.setSource(sourceAccount.getNumber()) .setSource(sourceAccount.getNumber())
.setSourceUuid(sourceAccount.getUuid().toString()) .setSourceUuid(sourceAccount.getUuid().toString())
@ -51,22 +51,9 @@ 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 (NotPushRegisteredException e) { } catch (final NotPushRegisteredException e) {
logger.info("User no longer push registered for delivery receipt: " + e.getMessage()); logger.info("User no longer push registered for delivery receipt: " + e.getMessage());
} }
} }
} }
private Account getDestinationAccount(String destination)
throws NoSuchUserException
{
Optional<Account> account = accountManager.get(destination);
if (account.isEmpty()) {
throw new NoSuchUserException(destination);
}
return account.get();
}
} }

View File

@ -207,11 +207,13 @@ public class WebSocketConnection implements MessageAvailabilityListener, Displac
if (!message.hasSource()) return; if (!message.hasSource()) return;
try { try {
receiptSender.sendReceipt(auth, message.getSource(), message.getTimestamp()); receiptSender.sendReceipt(auth, UUID.fromString(message.getSourceUuid()), message.getTimestamp());
} catch (NoSuchUserException e) { } catch (NoSuchUserException e) {
logger.info("No longer registered " + e.getMessage()); logger.info("No longer registered: {}", e.getMessage());
} catch (WebApplicationException e) { } 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());
} }
} }

View File

@ -291,7 +291,7 @@ class MessageControllerTest {
assertThat("Good Response", response.getStatus(), is(equalTo(200))); assertThat("Good Response", response.getStatus(), is(equalTo(200)));
verify(messageSender, never()).sendMessage(any(), any(), any(), anyBoolean()); 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 @ParameterizedTest
@ -575,7 +575,7 @@ class MessageControllerTest {
.delete(); .delete();
assertThat("Good Response Code", response.getStatus(), is(equalTo(204))); 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() response = resources.getJerseyTest()
.target(String.format("/v1/messages/uuid/%s", uuid2)) .target(String.format("/v1/messages/uuid/%s", uuid2))

View File

@ -210,7 +210,7 @@ public class WebSocketConnectionTest {
futures.get(2).completeExceptionally(new IOException()); futures.get(2).completeExceptionally(new IOException());
verify(storedMessages, times(1)).delete(eq(accountUuid), eq(2L), eq(outgoingMessages.get(1).getGuid())); 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(); connection.stop();
verify(client).close(anyInt(), anyString()); verify(client).close(anyInt(), anyString());
@ -279,6 +279,8 @@ public class WebSocketConnectionTest {
public void testPendingSend() throws Exception { public void testPendingSend() throws Exception {
MessagesManager storedMessages = mock(MessagesManager.class); MessagesManager storedMessages = mock(MessagesManager.class);
final UUID senderTwoUuid = UUID.randomUUID();
final Envelope firstMessage = Envelope.newBuilder() final Envelope firstMessage = Envelope.newBuilder()
.setLegacyMessage(ByteString.copyFrom("first".getBytes())) .setLegacyMessage(ByteString.copyFrom("first".getBytes()))
.setSource("sender1") .setSource("sender1")
@ -291,7 +293,7 @@ public class WebSocketConnectionTest {
final Envelope secondMessage = Envelope.newBuilder() final Envelope secondMessage = Envelope.newBuilder()
.setLegacyMessage(ByteString.copyFrom("second".getBytes())) .setLegacyMessage(ByteString.copyFrom("second".getBytes()))
.setSource("sender2") .setSource("sender2")
.setSourceUuid(UUID.randomUUID().toString()) .setSourceUuid(senderTwoUuid.toString())
.setTimestamp(System.currentTimeMillis()) .setTimestamp(System.currentTimeMillis())
.setSourceDevice(2) .setSourceDevice(2)
.setType(Envelope.Type.CIPHERTEXT) .setType(Envelope.Type.CIPHERTEXT)
@ -361,7 +363,7 @@ public class WebSocketConnectionTest {
futures.get(1).complete(response); futures.get(1).complete(response);
futures.get(0).completeExceptionally(new IOException()); 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(); connection.stop();
verify(client).close(anyInt(), anyString()); verify(client).close(anyInt(), anyString());