Use UUIDs instead of e164s to associate accounts with push notifications.

This commit is contained in:
Jon Chambers 2021-06-30 16:57:47 -04:00 committed by Jon Chambers
parent ce5edbb7fc
commit 73c368ea86
11 changed files with 69 additions and 60 deletions

View File

@ -179,9 +179,9 @@ public class AccountController {
pendingAccounts.store(number, storedVerificationCode);
if ("fcm".equals(pushType)) {
gcmSender.sendMessage(new GcmMessage(pushToken, number, 0, GcmMessage.Type.CHALLENGE, Optional.of(storedVerificationCode.getPushCode())));
gcmSender.sendMessage(new GcmMessage(pushToken, null, 0, GcmMessage.Type.CHALLENGE, Optional.of(storedVerificationCode.getPushCode())));
} else if ("apn".equals(pushType)) {
apnSender.sendMessage(new ApnMessage(pushToken, number, 0, useVoip.orElse(true), ApnMessage.Type.CHALLENGE, Optional.of(storedVerificationCode.getPushCode())));
apnSender.sendMessage(new ApnMessage(pushToken, null, 0, useVoip.orElse(true), ApnMessage.Type.CHALLENGE, Optional.of(storedVerificationCode.getPushCode())));
} else {
throw new AssertionError();
}

View File

@ -69,10 +69,10 @@ public class PushChallengeManager {
sent = true;
if (StringUtils.isNotBlank(masterDevice.getGcmId())) {
gcmSender.sendMessage(new GcmMessage(masterDevice.getGcmId(), account.getNumber(), 0, GcmMessage.Type.RATE_LIMIT_CHALLENGE, Optional.of(tokenHex)));
gcmSender.sendMessage(new GcmMessage(masterDevice.getGcmId(), account.getUuid(), 0, GcmMessage.Type.RATE_LIMIT_CHALLENGE, Optional.of(tokenHex)));
platform = ClientPlatform.ANDROID.name().toLowerCase();
} else if (StringUtils.isNotBlank(masterDevice.getApnId())) {
apnSender.sendMessage(new ApnMessage(masterDevice.getApnId(), account.getNumber(), 0, false, Type.RATE_LIMIT_CHALLENGE, Optional.of(tokenHex)));
apnSender.sendMessage(new ApnMessage(masterDevice.getApnId(), account.getUuid(), 0, false, Type.RATE_LIMIT_CHALLENGE, Optional.of(tokenHex)));
platform = ClientPlatform.IOS.name().toLowerCase();
} else {
throw new AssertionError();

View File

@ -27,8 +27,8 @@ import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.time.Instant;
import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import static com.codahale.metrics.MetricRegistry.name;
@ -84,17 +84,19 @@ public class APNSender implements Managed {
Instant.ofEpochMilli(message.getExpirationTime()),
message.isVoip());
Futures.addCallback(future, new FutureCallback<ApnResult>() {
Futures.addCallback(future, new FutureCallback<>() {
@Override
public void onSuccess(@Nullable ApnResult result) {
if (message.getChallengeData().isPresent()) return;
if (message.getChallengeData().isPresent()) {
return;
}
if (result == null) {
logger.warn("*** RECEIVED NULL APN RESULT ***");
} else if (result.getStatus() == ApnResult.Status.NO_SUCH_USER) {
handleUnregisteredUser(message.getApnId(), message.getNumber(), message.getDeviceId());
message.getUuid().ifPresent(uuid -> handleUnregisteredUser(message.getApnId(), uuid, message.getDeviceId()));
} else if (result.getStatus() == ApnResult.Status.GENERIC_FAILURE) {
logger.warn("*** Got APN generic failure: " + result.getReason() + ", " + message.getNumber());
logger.warn("*** Got APN generic failure: " + result.getReason() + ", " + message.getUuid());
}
}
@ -120,21 +122,21 @@ public class APNSender implements Managed {
this.fallbackManager = fallbackManager;
}
private void handleUnregisteredUser(String registrationId, String number, long deviceId) {
private void handleUnregisteredUser(String registrationId, UUID uuid, long deviceId) {
// logger.info("Got APN Unregistered: " + number + "," + deviceId);
Optional<Account> account = accountsManager.get(number);
Optional<Account> account = accountsManager.get(uuid);
if (!account.isPresent()) {
logger.info("No account found: " + number);
if (account.isEmpty()) {
logger.info("No account found: {}", uuid);
unregisteredEventStale.mark();
return;
}
Optional<Device> device = account.get().getDevice(deviceId);
if (!device.isPresent()) {
logger.info("No device found: " + number);
if (device.isEmpty()) {
logger.info("No device found: {}", uuid);
unregisteredEventStale.mark();
return;
}
@ -157,7 +159,7 @@ public class APNSender implements Managed {
if (tokenTimestamp != 0 && System.currentTimeMillis() < tokenTimestamp + TimeUnit.SECONDS.toMillis(10))
{
logger.info("APN Unregister push timestamp is more recent: " + tokenTimestamp + ", " + number);
logger.info("APN Unregister push timestamp is more recent: {}, {}", tokenTimestamp, uuid);
unregisteredEventStale.mark();
return;
}

View File

@ -193,7 +193,7 @@ public class ApnFallbackManager implements Managed {
return;
}
apnSender.sendMessage(new ApnMessage(apnId, account.getNumber(), device.getId(), true, Type.NOTIFICATION, Optional.empty()));
apnSender.sendMessage(new ApnMessage(apnId, account.getUuid(), device.getId(), true, Type.NOTIFICATION, Optional.empty()));
retry.mark();
}

View File

@ -7,7 +7,9 @@ package org.whispersystems.textsecuregcm.push;
import com.google.common.annotations.VisibleForTesting;
import javax.annotation.Nullable;
import java.util.Optional;
import java.util.UUID;
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
public class ApnMessage {
@ -23,15 +25,17 @@ public class ApnMessage {
public static final long MAX_EXPIRATION = Integer.MAX_VALUE * 1000L;
private final String apnId;
private final String number;
private final long deviceId;
private final boolean isVoip;
private final Type type;
private final Optional<String> challengeData;
public ApnMessage(String apnId, String number, long deviceId, boolean isVoip, Type type, Optional<String> challengeData) {
@Nullable
private final UUID uuid;
public ApnMessage(String apnId, @Nullable UUID uuid, long deviceId, boolean isVoip, Type type, Optional<String> challengeData) {
this.apnId = apnId;
this.number = number;
this.uuid = uuid;
this.deviceId = deviceId;
this.isVoip = isVoip;
this.type = type;
@ -71,8 +75,8 @@ public class ApnMessage {
return MAX_EXPIRATION;
}
public String getNumber() {
return number;
public Optional<UUID> getUuid() {
return Optional.ofNullable(uuid);
}
public long getDeviceId() {

View File

@ -119,8 +119,8 @@ public class GCMSender {
}
private void handleCanonicalRegistrationId(GcmMessage message, Result result) {
logger.warn(String.format("Actually received 'CanonicalRegistrationId' ::: (canonical=%s), (original=%s)",
result.getCanonicalRegistrationId(), message.getGcmId()));
logger.warn("Actually received 'CanonicalRegistrationId' ::: (canonical={}}), (original={}})",
result.getCanonicalRegistrationId(), message.getGcmId());
getAccountForEvent(message).ifPresent(account ->
accountsManager.updateDevice(
@ -132,15 +132,14 @@ public class GCMSender {
}
private void handleGenericError(GcmMessage message, Result result) {
logger.warn(String.format("Unrecoverable Error ::: (error=%s), (gcm_id=%s), " +
"(destination=%s), (device_id=%d)",
result.getError(), message.getGcmId(), message.getNumber(),
message.getDeviceId()));
logger.warn("Unrecoverable Error ::: (error={}}), (gcm_id={}}), (destination={}}), (device_id={}})",
result.getError(), message.getGcmId(), message.getUuid(), message.getDeviceId());
failure.mark();
}
private Optional<Account> getAccountForEvent(GcmMessage message) {
Optional<Account> account = accountsManager.get(message.getNumber());
Optional<Account> account = message.getUuid().flatMap(accountsManager::get);
if (account.isPresent()) {
Optional<Device> device = account.get().getDevice(message.getDeviceId());

View File

@ -6,7 +6,9 @@
package org.whispersystems.textsecuregcm.push;
import javax.annotation.Nullable;
import java.util.Optional;
import java.util.UUID;
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
public class GcmMessage {
@ -16,14 +18,16 @@ public class GcmMessage {
}
private final String gcmId;
private final String number;
private final int deviceId;
private final Type type;
private final Optional<String> data;
public GcmMessage(String gcmId, String number, int deviceId, Type type, Optional<String> data) {
@Nullable
private final UUID uuid;
public GcmMessage(String gcmId, @Nullable UUID uuid, int deviceId, Type type, Optional<String> data) {
this.gcmId = gcmId;
this.number = number;
this.uuid = uuid;
this.deviceId = deviceId;
this.type = type;
this.data = data;
@ -33,8 +37,8 @@ public class GcmMessage {
return gcmId;
}
public String getNumber() {
return number;
public Optional<UUID> getUuid() {
return Optional.ofNullable(uuid);
}
public Type getType() {

View File

@ -120,7 +120,7 @@ public class MessageSender implements Managed {
}
private void sendGcmNotification(Account account, Device device) {
GcmMessage gcmMessage = new GcmMessage(device.getGcmId(), account.getNumber(),
GcmMessage gcmMessage = new GcmMessage(device.getGcmId(), account.getUuid(),
(int)device.getId(), GcmMessage.Type.NOTIFICATION, Optional.empty());
gcmSender.sendMessage(gcmMessage);
@ -132,10 +132,10 @@ public class MessageSender implements Managed {
ApnMessage apnMessage;
if (!Util.isEmpty(device.getVoipApnId())) {
apnMessage = new ApnMessage(device.getVoipApnId(), account.getNumber(), device.getId(), true, Type.NOTIFICATION, Optional.empty());
apnMessage = new ApnMessage(device.getVoipApnId(), account.getUuid(), device.getId(), true, Type.NOTIFICATION, Optional.empty());
RedisOperation.unchecked(() -> apnFallbackManager.schedule(account, device));
} else {
apnMessage = new ApnMessage(device.getApnId(), account.getNumber(), device.getId(), false, Type.NOTIFICATION, Optional.empty());
apnMessage = new ApnMessage(device.getApnId(), account.getUuid(), device.getId(), false, Type.NOTIFICATION, Optional.empty());
}
apnSender.sendMessage(apnMessage);

View File

@ -103,7 +103,7 @@ public class ApnFallbackManagerTest extends AbstractRedisClusterTest {
final ApnMessage message = messageCaptor.getValue();
assertEquals(VOIP_APN_ID, message.getApnId());
assertEquals(ACCOUNT_NUMBER, message.getNumber());
assertEquals(Optional.of(ACCOUNT_UUID), message.getUuid());
assertEquals(DEVICE_ID, message.getDeviceId());
assertEquals(0, worker.processNextSlot());

View File

@ -29,17 +29,16 @@ import org.whispersystems.textsecuregcm.tests.util.SynchronousExecutorService;
import java.time.Instant;
import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import io.netty.util.concurrent.DefaultEventExecutor;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.*;
public class APNSenderTest {
private static final String DESTINATION_NUMBER = "+14151231234";
private static final UUID DESTINATION_UUID = UUID.randomUUID();
private static final String DESTINATION_APN_ID = "foo";
private final AccountsManager accountsManager = mock(AccountsManager.class);
@ -52,7 +51,7 @@ public class APNSenderTest {
public void setup() {
when(destinationAccount.getDevice(1)).thenReturn(Optional.of(destinationDevice));
when(destinationDevice.getApnId()).thenReturn(DESTINATION_APN_ID);
when(accountsManager.get(DESTINATION_NUMBER)).thenReturn(Optional.of(destinationAccount));
when(accountsManager.get(DESTINATION_UUID)).thenReturn(Optional.of(destinationAccount));
}
@Test
@ -66,7 +65,7 @@ public class APNSenderTest {
.thenAnswer((Answer) invocationOnMock -> new MockPushNotificationFuture<>(invocationOnMock.getArgument(0), response));
RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient);
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, true, Type.NOTIFICATION, Optional.empty());
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_UUID, 1, true, Type.NOTIFICATION, Optional.empty());
APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false);
apnSender.setApnFallbackManager(fallbackManager);
@ -100,7 +99,7 @@ public class APNSenderTest {
.thenAnswer((Answer) invocationOnMock -> new MockPushNotificationFuture<>(invocationOnMock.getArgument(0), response));
RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient);
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, false, Type.NOTIFICATION, Optional.empty());
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_UUID, 1, false, Type.NOTIFICATION, Optional.empty());
APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false);
apnSender.setApnFallbackManager(fallbackManager);
@ -136,7 +135,7 @@ public class APNSenderTest {
RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient);
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, true, Type.NOTIFICATION, Optional.empty());
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_UUID, 1, true, Type.NOTIFICATION, Optional.empty());
APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false);
apnSender.setApnFallbackManager(fallbackManager);
@ -159,7 +158,7 @@ public class APNSenderTest {
assertThat(apnResult.getStatus()).isEqualTo(ApnResult.Status.NO_SUCH_USER);
verifyNoMoreInteractions(apnsClient);
verify(accountsManager, times(1)).get(eq(DESTINATION_NUMBER));
verify(accountsManager, times(1)).get(eq(DESTINATION_UUID));
verify(destinationAccount, times(1)).getDevice(1);
verify(destinationDevice, times(1)).getApnId();
verify(destinationDevice, times(1)).getPushTimestamp();
@ -239,7 +238,7 @@ public class APNSenderTest {
.thenAnswer((Answer) invocationOnMock -> new MockPushNotificationFuture<>(invocationOnMock.getArgument(0), response));
RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient);
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, true, Type.NOTIFICATION, Optional.empty());
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_UUID, 1, true, Type.NOTIFICATION, Optional.empty());
APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false);
apnSender.setApnFallbackManager(fallbackManager);
@ -262,7 +261,7 @@ public class APNSenderTest {
assertThat(apnResult.getStatus()).isEqualTo(ApnResult.Status.NO_SUCH_USER);
verifyNoMoreInteractions(apnsClient);
verify(accountsManager, times(1)).get(eq(DESTINATION_NUMBER));
verify(accountsManager, times(1)).get(eq(DESTINATION_UUID));
verify(destinationAccount, times(1)).getDevice(1);
verify(destinationDevice, times(1)).getApnId();
verify(destinationDevice, times(1)).getPushTimestamp();
@ -334,7 +333,7 @@ public class APNSenderTest {
.thenAnswer((Answer) invocationOnMock -> new MockPushNotificationFuture<>(invocationOnMock.getArgument(0), response));
RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient);
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, true, Type.NOTIFICATION, Optional.empty());
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_UUID, 1, true, Type.NOTIFICATION, Optional.empty());
APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false);
apnSender.setApnFallbackManager(fallbackManager);
@ -367,7 +366,7 @@ public class APNSenderTest {
.thenAnswer((Answer) invocationOnMock -> new MockPushNotificationFuture<>(invocationOnMock.getArgument(0), new Exception("lost connection")));
RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient);
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, true, Type.NOTIFICATION, Optional.empty());
ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_UUID, 1, true, Type.NOTIFICATION, Optional.empty());
APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false);
apnSender.setApnFallbackManager(fallbackManager);

View File

@ -13,6 +13,7 @@ import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import org.junit.Test;
import org.whispersystems.gcm.server.Message;
@ -43,7 +44,7 @@ public class GCMSenderTest {
AccountsHelper.setupMockUpdate(accountsManager);
GcmMessage message = new GcmMessage("foo", "+12223334444", 1, GcmMessage.Type.NOTIFICATION, Optional.empty());
GcmMessage message = new GcmMessage("foo", UUID.randomUUID(), 1, GcmMessage.Type.NOTIFICATION, Optional.empty());
GCMSender gcmSender = new GCMSender(executorService, accountsManager, sender);
CompletableFuture<Result> successFuture = CompletableFuture.completedFuture(successResult);
@ -57,8 +58,8 @@ public class GCMSenderTest {
@Test
public void testSendUninstalled() {
String destinationNumber = "+12223334444";
String gcmId = "foo";
UUID destinationUuid = UUID.randomUUID();
String gcmId = "foo";
AccountsManager accountsManager = mock(AccountsManager.class);
Sender sender = mock(Sender.class );
@ -71,7 +72,7 @@ public class GCMSenderTest {
AccountsHelper.setupMockUpdate(accountsManager);
when(destinationAccount.getDevice(1)).thenReturn(Optional.of(destinationDevice));
when(accountsManager.get(destinationNumber)).thenReturn(Optional.of(destinationAccount));
when(accountsManager.get(destinationUuid)).thenReturn(Optional.of(destinationAccount));
when(destinationDevice.getGcmId()).thenReturn(gcmId);
when(invalidResult.isInvalidRegistrationId()).thenReturn(true);
@ -79,7 +80,7 @@ public class GCMSenderTest {
when(invalidResult.hasCanonicalRegistrationId()).thenReturn(false);
when(invalidResult.isSuccess()).thenReturn(true);
GcmMessage message = new GcmMessage(gcmId, destinationNumber, 1, GcmMessage.Type.NOTIFICATION, Optional.empty());
GcmMessage message = new GcmMessage(gcmId, destinationUuid, 1, GcmMessage.Type.NOTIFICATION, Optional.empty());
GCMSender gcmSender = new GCMSender(executorService, accountsManager, sender);
CompletableFuture<Result> invalidFuture = CompletableFuture.completedFuture(invalidResult);
@ -89,14 +90,14 @@ public class GCMSenderTest {
gcmSender.sendMessage(message);
verify(sender, times(1)).send(any(Message.class));
verify(accountsManager, times(1)).get(eq(destinationNumber));
verify(accountsManager, times(1)).get(eq(destinationUuid));
verify(accountsManager, times(1)).updateDevice(eq(destinationAccount), eq(1L), any());
verify(destinationDevice, times(1)).setUninstalledFeedbackTimestamp(eq(Util.todayInMillis()));
}
@Test
public void testCanonicalId() {
String destinationNumber = "+12223334444";
UUID destinationUuid = UUID.randomUUID();
String gcmId = "foo";
String canonicalId = "bar";
@ -109,7 +110,7 @@ public class GCMSenderTest {
Device destinationDevice = mock(Device.class );
when(destinationAccount.getDevice(1)).thenReturn(Optional.of(destinationDevice));
when(accountsManager.get(destinationNumber)).thenReturn(Optional.of(destinationAccount));
when(accountsManager.get(destinationUuid)).thenReturn(Optional.of(destinationAccount));
when(destinationDevice.getGcmId()).thenReturn(gcmId);
AccountsHelper.setupMockUpdate(accountsManager);
@ -120,7 +121,7 @@ public class GCMSenderTest {
when(canonicalResult.isSuccess()).thenReturn(false);
when(canonicalResult.getCanonicalRegistrationId()).thenReturn(canonicalId);
GcmMessage message = new GcmMessage(gcmId, destinationNumber, 1, GcmMessage.Type.NOTIFICATION, Optional.empty());
GcmMessage message = new GcmMessage(gcmId, destinationUuid, 1, GcmMessage.Type.NOTIFICATION, Optional.empty());
GCMSender gcmSender = new GCMSender(executorService, accountsManager, sender);
CompletableFuture<Result> invalidFuture = CompletableFuture.completedFuture(canonicalResult);
@ -130,7 +131,7 @@ public class GCMSenderTest {
gcmSender.sendMessage(message);
verify(sender, times(1)).send(any(Message.class));
verify(accountsManager, times(1)).get(eq(destinationNumber));
verify(accountsManager, times(1)).get(eq(destinationUuid));
verify(accountsManager, times(1)).updateDevice(eq(destinationAccount), eq(1L), any());
verify(destinationDevice, times(1)).setGcmId(eq(canonicalId));
}