MirrorMirror
/
Signal-Server
mirror of https://github.com/signalapp/Signal-Server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Delete FCM tokens immediately if FCM reports that they're no longer active

This commit is contained in:
Jon Chambers 2024-06-21 16:35:44 -04:00 committed by Jon Chambers
parent 90e622b307
commit 1cd16eaa08
6 changed files with 21 additions and 472 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
View File

@ -253,7 +253,6 @@ import org.whispersystems.textsecuregcm.workers.CertificateCommand;
import org.whispersystems.textsecuregcm.workers.CheckDynamicConfigurationCommand;
import org.whispersystems.textsecuregcm.workers.DeleteUserCommand;
import org.whispersystems.textsecuregcm.workers.MessagePersisterServiceCommand;
import org.whispersystems.textsecuregcm.workers.ProcessPushNotificationFeedbackCommand;
import org.whispersystems.textsecuregcm.workers.RemoveExpiredAccountsCommand;
import org.whispersystems.textsecuregcm.workers.RemoveExpiredBackupsCommand;
import org.whispersystems.textsecuregcm.workers.RemoveExpiredLinkedDevicesCommand;
@ -314,7 +313,6 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
bootstrap.addCommand(new RemoveExpiredUsernameHoldsCommand(Clock.systemUTC()));
bootstrap.addCommand(new RemoveExpiredBackupsCommand(Clock.systemUTC()));
bootstrap.addCommand(new BackupMetricsCommand(Clock.systemUTC()));
bootstrap.addCommand(new ProcessPushNotificationFeedbackCommand(Clock.systemUTC()));
bootstrap.addCommand(new RemoveExpiredLinkedDevicesCommand());
}

34
service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java
View File

@ -20,7 +20,6 @@ import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.AccountsManager;
import org.whispersystems.textsecuregcm.storage.Device;
import org.whispersystems.textsecuregcm.util.Pair;
import org.whispersystems.textsecuregcm.util.Util;
public class PushNotificationManager {
@ -167,23 +166,24 @@ public class PushNotificationManager {
private void handleDeviceUnregistered(final Account account, final Device device) {
if (StringUtils.isNotBlank(device.getGcmId())) {
if (device.getUninstalledFeedbackTimestamp() == 0) {
// Reread the account to avoid marking the caller's account as stale. The consumers of this class tend to
// promise not to modify accounts. There's no need to force the caller to be considered mutable just for
// updating an uninstalled feedback timestamp though.
final Optional<Account> rereadAccount = accountsManager.getByAccountIdentifier(account.getUuid());
if (rereadAccount.isEmpty()) {
// don't bother adding the uninstalled timestamp, the account is gone
return;
}
final Optional<Device> rereadDevice = rereadAccount.get().getDevice(device.getId());
if (rereadDevice.map(Device::getUninstalledFeedbackTimestamp).orElse(-1L) != 0) {
// don't bother adding the uninstalled timestamp, the device is gone or already updated
return;
}
accountsManager.updateDevice(rereadAccount.get(), device.getId(), d ->
d.setUninstalledFeedbackTimestamp(Util.todayInMillis()));
final String originalFcmId = device.getGcmId();
// Reread the account to avoid marking the caller's account as stale. The consumers of this class tend to
// promise not to modify accounts. There's no need to force the caller to be considered mutable just for
// updating an uninstalled feedback timestamp though.
final Optional<Account> rereadAccount = accountsManager.getByAccountIdentifier(account.getUuid());
if (rereadAccount.isEmpty()) {
// Don't bother removing the token; the account is gone
return;
}
rereadAccount.get().getDevice(device.getId()).ifPresent(rereadDevice ->
accountsManager.updateDevice(rereadAccount.get(), device.getId(), d -> {
// Don't clear the token if it's already changed
if (originalFcmId.equals(d.getGcmId())) {
d.setGcmId(null);
}
}));
} else {
apnPushNotificationScheduler.cancelScheduledNotifications(account, device).whenComplete(logErrors());
}

11
service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java
View File

@ -56,9 +56,6 @@ public class Device {
@JsonProperty
private long pushTimestamp;
@JsonProperty
private long uninstalledFeedback;
@JsonProperty
private boolean fetchesMessages;
@ -101,14 +98,6 @@ public class Device {
this.voipApnId = voipApnId;
}
public void setUninstalledFeedbackTimestamp(long uninstalledFeedback) {
this.uninstalledFeedback = uninstalledFeedback;
}
public long getUninstalledFeedbackTimestamp() {
return uninstalledFeedback;
}
public void setLastSeen(long lastSeen) {
this.lastSeen = lastSeen;
}

160
service/src/main/java/org/whispersystems/textsecuregcm/workers/ProcessPushNotificationFeedbackCommand.java
View File

@ -1,160 +0,0 @@
/*
* Copyright 2023 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.whispersystems.textsecuregcm.workers;
import com.google.common.annotations.VisibleForTesting;
import io.micrometer.core.instrument.Metrics;
import io.micrometer.core.instrument.Tags;
import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
import java.util.Optional;
import net.sourceforge.argparse4j.inf.Subparser;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.whispersystems.textsecuregcm.metrics.MetricsUtil;
import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.Device;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
public class ProcessPushNotificationFeedbackCommand extends AbstractSinglePassCrawlAccountsCommand {
private final Clock clock;
@VisibleForTesting
static final Duration MAX_TOKEN_REFRESH_DELAY = Duration.ofDays(14);
@VisibleForTesting
static final String DRY_RUN_ARGUMENT = "dry-run";
private static final int MAX_CONCURRENCY = 16;
private static final String EXPIRED_DEVICE_COUNTER_NAME =
MetricsUtil.name(ProcessPushNotificationFeedbackCommand.class, "expiredDevice");
private static final String RECOVERED_DEVICE_COUNTER_NAME =
MetricsUtil.name(ProcessPushNotificationFeedbackCommand.class, "recoveredDevice");
private static final Logger log = LoggerFactory.getLogger(ProcessPushNotificationFeedbackCommand.class);
public ProcessPushNotificationFeedbackCommand(final Clock clock) {
super("process-push-notification-feedback", "");
this.clock = clock;
}
@Override
public void configure(final Subparser subparser) {
super.configure(subparser);
subparser.addArgument("--dry-run")
.type(Boolean.class)
.dest(DRY_RUN_ARGUMENT)
.required(false)
.setDefault(true)
.help("If true, don't actually modify accounts with stale devices");
}
@Override
protected void crawlAccounts(final Flux<Account> accounts) {
final boolean isDryRun = getNamespace().getBoolean(DRY_RUN_ARGUMENT);
accounts
.filter(account -> account.getDevices().stream().anyMatch(this::deviceNeedsUpdate))
.flatMap(account -> {
account.getDevices().stream()
.filter(this::deviceNeedsUpdate)
.forEach(device -> {
final Tags tags = Tags.of(
UserAgentTagUtil.PLATFORM_TAG, getPlatform(device),
"dryRun", String.valueOf(isDryRun));
if (deviceExpired(device)) {
Metrics.counter(EXPIRED_DEVICE_COUNTER_NAME, tags).increment();
} else {
Metrics.counter(RECOVERED_DEVICE_COUNTER_NAME, tags).increment();
}
});
if (isDryRun) {
return Mono.just(account);
} else {
return Mono.fromFuture(() -> getCommandDependencies().accountsManager().updateAsync(account,
a -> a.getDevices().stream()
.filter(this::deviceNeedsUpdate)
.forEach(device -> {
if (deviceExpired(device)) {
getUserAgent(device).ifPresent(device::setUserAgent);
device.setGcmId(null);
device.setApnId(null);
device.setVoipApnId(null);
device.setFetchesMessages(false);
} else {
device.setUninstalledFeedbackTimestamp(0);
}
})))
.onErrorResume(throwable -> {
log.warn("Failed to process push notification feedback for account {}", account.getUuid(), throwable);
return Mono.empty();
});
}
}, MAX_CONCURRENCY)
.then()
.block();
}
@VisibleForTesting
boolean pushFeedbackIntervalElapsed(final Device device) {
// After we get an indication that a device may have uninstalled the Signal app (`uninstalledFeedbackTimestamp` is
// non-zero), check back in after a few days to see what ultimately happened.
return device.getUninstalledFeedbackTimestamp() != 0 &&
Instant.ofEpochMilli(device.getUninstalledFeedbackTimestamp()).plus(MAX_TOKEN_REFRESH_DELAY)
.isBefore(clock.instant());
}
@VisibleForTesting
boolean deviceExpired(final Device device) {
// If we received an indication that a device may have uninstalled the Signal app and we haven't seen that device in
// a few days since that event, we consider the device "expired" and should clean up its push tokens. If we have
// seen the device recently, though, we assume that the "uninstalled" hint was either incorrect or the device has
// since reinstalled the app and provided new push tokens.
return Instant.ofEpochMilli(device.getLastSeen()).plus(MAX_TOKEN_REFRESH_DELAY).isBefore(clock.instant());
}
@VisibleForTesting
boolean deviceNeedsUpdate(final Device device) {
return pushFeedbackIntervalElapsed(device) && (device.hasMessageDeliveryChannel() || device.getLastSeen() > device.getUninstalledFeedbackTimestamp());
}
@VisibleForTesting
static Optional<String> getUserAgent(final Device device) {
if (StringUtils.isNotBlank(device.getApnId())) {
if (device.isPrimary()) {
return Optional.of("OWI");
} else {
return Optional.of("OWP");
}
} else if (StringUtils.isNotBlank(device.getGcmId())) {
return Optional.of("OWA");
}
return Optional.empty();
}
private static String getPlatform(final Device device) {
if (StringUtils.isNotBlank(device.getApnId())) {
return "ios";
} else if (StringUtils.isNotBlank(device.getGcmId())) {
return "android";
}
return "unrecognized";
}
}

9
service/src/test/java/org/whispersystems/textsecuregcm/push/PushNotificationManagerTest.java
View File

@ -26,7 +26,6 @@ import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.AccountsManager;
import org.whispersystems.textsecuregcm.storage.Device;
import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
import org.whispersystems.textsecuregcm.util.Util;
class PushNotificationManagerTest {
@ -153,7 +152,7 @@ class PushNotificationManagerTest {
verify(fcmSender).sendNotification(pushNotification);
verifyNoInteractions(apnSender);
verify(accountsManager, never()).updateDevice(eq(account), eq(Device.PRIMARY_ID), any());
verify(device, never()).setUninstalledFeedbackTimestamp(Util.todayInMillis());
verify(device, never()).setGcmId(any());
verifyNoInteractions(apnPushNotificationScheduler);
}
@ -211,7 +210,7 @@ class PushNotificationManagerTest {
verifyNoInteractions(fcmSender);
verify(accountsManager, never()).updateDevice(eq(account), eq(Device.PRIMARY_ID), any());
verify(device, never()).setUninstalledFeedbackTimestamp(Util.todayInMillis());
verify(device, never()).setGcmId(any());
verify(apnPushNotificationScheduler).scheduleRecurringVoipNotification(account, device);
verify(apnPushNotificationScheduler, never()).scheduleBackgroundNotification(any(), any());
}
@ -236,7 +235,7 @@ class PushNotificationManagerTest {
pushNotificationManager.sendNotification(pushNotification);
verify(accountsManager).updateDevice(eq(account), eq(Device.PRIMARY_ID), any());
verify(device).setUninstalledFeedbackTimestamp(Util.todayInMillis());
verify(device).setGcmId(null);
verifyNoInteractions(apnSender);
verifyNoInteractions(apnPushNotificationScheduler);
}
@ -262,7 +261,7 @@ class PushNotificationManagerTest {
verifyNoInteractions(fcmSender);
verify(accountsManager, never()).updateDevice(eq(account), eq(Device.PRIMARY_ID), any());
verify(device, never()).setUninstalledFeedbackTimestamp(Util.todayInMillis());
verify(device, never()).setGcmId(any());
verify(apnPushNotificationScheduler).cancelScheduledNotifications(account, device);
}

277
service/src/test/java/org/whispersystems/textsecuregcm/workers/ProcessPushNotificationFeedbackCommandTest.java
View File

@ -1,277 +0,0 @@
/*
* Copyright 2023 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.whispersystems.textsecuregcm.workers;
import net.sourceforge.argparse4j.inf.Namespace;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.AccountsManager;
import org.whispersystems.textsecuregcm.storage.Device;
import reactor.core.publisher.Flux;
import java.time.Clock;
import java.time.Instant;
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.function.Consumer;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
class ProcessPushNotificationFeedbackCommandTest {
private AccountsManager accountsManager;
private Clock clock;
private ProcessPushNotificationFeedbackCommand processPushNotificationFeedbackCommand;
private static final Instant CURRENT_TIME = Instant.now();
private static class TestProcessPushNotificationFeedbackCommand extends ProcessPushNotificationFeedbackCommand {
private final CommandDependencies commandDependencies;
private final Namespace namespace;
public TestProcessPushNotificationFeedbackCommand(final Clock clock, final AccountsManager accountsManager, final boolean isDryRun) {
super(clock);
commandDependencies = mock(CommandDependencies.class);
when(commandDependencies.accountsManager()).thenReturn(accountsManager);
namespace = mock(Namespace.class);
when(namespace.getBoolean(RemoveExpiredAccountsCommand.DRY_RUN_ARGUMENT)).thenReturn(isDryRun);
}
@Override
protected CommandDependencies getCommandDependencies() {
return commandDependencies;
}
@Override
protected Namespace getNamespace() {
return namespace;
}
}
@BeforeEach
void setUpBeforeEach() {
accountsManager = mock(AccountsManager.class);
when(accountsManager.updateAsync(any(), any()))
.thenAnswer(invocation -> {
final Account account = invocation.getArgument(0);
final Consumer<Account> accountUpdater = invocation.getArgument(1);
accountUpdater.accept(account);
return CompletableFuture.completedFuture(account);
});
clock = Clock.fixed(CURRENT_TIME, ZoneId.systemDefault());
processPushNotificationFeedbackCommand =
new TestProcessPushNotificationFeedbackCommand(clock, accountsManager, true);
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
void crawlAccounts(final boolean isDryRun) {
processPushNotificationFeedbackCommand =
new TestProcessPushNotificationFeedbackCommand(clock, accountsManager, isDryRun);
final Account accountWithActiveDevice = mock(Account.class);
{
final Device device = mock(Device.class);
when(device.hasMessageDeliveryChannel()).thenReturn(true);
when(accountWithActiveDevice.getDevices()).thenReturn(List.of(device));
}
final Account accountWithUninstalledDevice = mock(Account.class);
{
final Device uninstalledDevice = mock(Device.class);
when(uninstalledDevice.hasMessageDeliveryChannel()).thenReturn(true);
when(uninstalledDevice.getUninstalledFeedbackTimestamp())
.thenReturn(clock.instant().minus(ProcessPushNotificationFeedbackCommand.MAX_TOKEN_REFRESH_DELAY.multipliedBy(2)).toEpochMilli());
when(accountWithUninstalledDevice.getDevices()).thenReturn(List.of(uninstalledDevice));
}
final Account accountWithAlreadyDisabledUninstalledDevice = mock(Account.class);
{
final Device previouslyDisabledUninstalledDevice = mock(Device.class);
when(previouslyDisabledUninstalledDevice.hasMessageDeliveryChannel()).thenReturn(false);
when(previouslyDisabledUninstalledDevice.getUninstalledFeedbackTimestamp())
.thenReturn(clock.instant().minus(ProcessPushNotificationFeedbackCommand.MAX_TOKEN_REFRESH_DELAY.multipliedBy(2)).toEpochMilli());
when(accountWithAlreadyDisabledUninstalledDevice.getDevices()).thenReturn(List.of(previouslyDisabledUninstalledDevice));
}
processPushNotificationFeedbackCommand.crawlAccounts(
Flux.just(accountWithActiveDevice, accountWithUninstalledDevice, accountWithAlreadyDisabledUninstalledDevice));
if (isDryRun) {
verify(accountsManager, never()).updateAsync(any(), any());
} else {
verify(accountsManager, never()).updateAsync(eq(accountWithActiveDevice), any());
verify(accountsManager).updateAsync(eq(accountWithUninstalledDevice), any());
verify(accountsManager, never()).updateAsync(eq(accountWithAlreadyDisabledUninstalledDevice), any());
}
}
@Test
void pushFeedbackIntervalElapsed() {
{
final Device deviceWithMaturePushNotificationFeedback = mock(Device.class);
when(deviceWithMaturePushNotificationFeedback.getUninstalledFeedbackTimestamp())
.thenReturn(clock.instant().minus(ProcessPushNotificationFeedbackCommand.MAX_TOKEN_REFRESH_DELAY.multipliedBy(2)).toEpochMilli());
assertTrue(processPushNotificationFeedbackCommand.pushFeedbackIntervalElapsed(deviceWithMaturePushNotificationFeedback));
}
{
final Device deviceWithRecentPushNotificationFeedback = mock(Device.class);
when(deviceWithRecentPushNotificationFeedback.getUninstalledFeedbackTimestamp())
.thenReturn(clock.instant().minus(ProcessPushNotificationFeedbackCommand.MAX_TOKEN_REFRESH_DELAY.dividedBy(2)).toEpochMilli());
assertFalse(processPushNotificationFeedbackCommand.pushFeedbackIntervalElapsed(deviceWithRecentPushNotificationFeedback));
}
{
final Device deviceWithoutPushNotificationFeedback = mock(Device.class);
assertFalse(processPushNotificationFeedbackCommand.pushFeedbackIntervalElapsed(deviceWithoutPushNotificationFeedback));
}
}
@Test
void deviceExpired() {
{
final Device expiredDevice = mock(Device.class);
when(expiredDevice.getLastSeen())
.thenReturn(
clock.instant().minus(ProcessPushNotificationFeedbackCommand.MAX_TOKEN_REFRESH_DELAY.multipliedBy(2))
.toEpochMilli());
assertTrue(processPushNotificationFeedbackCommand.deviceExpired(expiredDevice));
}
{
final Device activeDevice = mock(Device.class);
when(activeDevice.getLastSeen()).thenReturn(clock.instant().toEpochMilli());
assertFalse(processPushNotificationFeedbackCommand.deviceExpired(activeDevice));
}
}
@ParameterizedTest
@MethodSource
void deviceNeedsUpdate(final Device device, final boolean expectNeedsUpdate) {
assertEquals(expectNeedsUpdate, processPushNotificationFeedbackCommand.deviceNeedsUpdate(device));
}
private static List<Arguments> deviceNeedsUpdate() {
final long maturePushFeedbackTimestamp =
CURRENT_TIME.minus(ProcessPushNotificationFeedbackCommand.MAX_TOKEN_REFRESH_DELAY.multipliedBy(2))
.toEpochMilli();
final List<Arguments> arguments = new ArrayList<>();
{
// Device is active, enabled, and has no push feedback
final Device device = mock(Device.class);
when(device.hasMessageDeliveryChannel()).thenReturn(true);
when(device.getUninstalledFeedbackTimestamp()).thenReturn(0L);
when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli());
arguments.add(Arguments.of(device, false));
}
{
// Device is active, but not enabled, and has no push feedback
final Device device = mock(Device.class);
when(device.hasMessageDeliveryChannel()).thenReturn(false);
when(device.getUninstalledFeedbackTimestamp()).thenReturn(0L);
when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli());
arguments.add(Arguments.of(device, false));
}
{
// Device is active, enabled, and has "mature" push feedback
final Device device = mock(Device.class);
when(device.hasMessageDeliveryChannel()).thenReturn(true);
when(device.getUninstalledFeedbackTimestamp()).thenReturn(maturePushFeedbackTimestamp);
when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli());
arguments.add(Arguments.of(device, true));
}
{
// Device is active, but not enabled, and has "mature" push feedback
final Device device = mock(Device.class);
when(device.hasMessageDeliveryChannel()).thenReturn(false);
when(device.getUninstalledFeedbackTimestamp()).thenReturn(maturePushFeedbackTimestamp);
when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli());
arguments.add(Arguments.of(device, true));
}
{
// Device is inactive, not enabled, and has "mature" push feedback
final Device device = mock(Device.class);
when(device.hasMessageDeliveryChannel()).thenReturn(false);
when(device.getUninstalledFeedbackTimestamp()).thenReturn(maturePushFeedbackTimestamp);
when(device.getLastSeen()).thenReturn(maturePushFeedbackTimestamp);
arguments.add(Arguments.of(device, false));
}
return arguments;
}
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
@ParameterizedTest
@MethodSource
void getUserAgent(final Device device, final Optional<String> expectedUserAgentString) {
assertEquals(expectedUserAgentString, ProcessPushNotificationFeedbackCommand.getUserAgent(device));
}
private static List<Arguments> getUserAgent() {
final Device iosPrimaryDevice = mock(Device.class);
when(iosPrimaryDevice.isPrimary()).thenReturn(true);
when(iosPrimaryDevice.getApnId()).thenReturn("apns-token");
final Device iosLinkedDevice = mock(Device.class);
when(iosLinkedDevice.isPrimary()).thenReturn(false);
when(iosLinkedDevice.getApnId()).thenReturn("apns-token");
final Device androidDevice = mock(Device.class);
when(androidDevice.getGcmId()).thenReturn("gcm-id");
final Device deviceWithoutTokens = mock(Device.class);
return List.of(
Arguments.of(iosPrimaryDevice, Optional.of("OWI")),
Arguments.of(iosLinkedDevice, Optional.of("OWP")),
Arguments.of(androidDevice, Optional.of("OWA")),
Arguments.of(deviceWithoutTokens, Optional.empty())
);
}
}