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

Reenable account cleaner

This commit is contained in:
Moxie Marlinspike 2019-05-07 18:14:32 -07:00
parent e0b85131bd
commit a029768d24
2 changed files with 110 additions and 173 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java
View File

@ -22,6 +22,7 @@ import com.codahale.metrics.SharedMetricRegistries;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue;
import org.whispersystems.textsecuregcm.util.Constants; import org.whispersystems.textsecuregcm.util.Constants;
import org.whispersystems.textsecuregcm.util.Util;
import java.util.List; import java.util.List;
import java.util.Optional; import java.util.Optional;
@ -30,6 +31,7 @@ import java.util.concurrent.TimeUnit;
import static com.codahale.metrics.MetricRegistry.name; import static com.codahale.metrics.MetricRegistry.name;
public class AccountCleaner implements AccountDatabaseCrawlerListener { public class AccountCleaner implements AccountDatabaseCrawlerListener {
private static final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); private static final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME);
private static final Meter expiredAccountsMeter = metricRegistry.meter(name(AccountCleaner.class, "expiredAccounts")); private static final Meter expiredAccountsMeter = metricRegistry.meter(name(AccountCleaner.class, "expiredAccounts"));
@ -50,25 +52,23 @@ public class AccountCleaner implements AccountDatabaseCrawlerListener {
@Override @Override
public void onCrawlChunk(Optional<String> fromNumber, List<Account> chunkAccounts) { public void onCrawlChunk(Optional<String> fromNumber, List<Account> chunkAccounts) {
long nowMs = System.currentTimeMillis();
int accountUpdateCount = 0; int accountUpdateCount = 0;
for (Account account : chunkAccounts) { for (Account account : chunkAccounts) {
if (account.getMasterDevice().isPresent() && if (needsExplicitRemoval(account)) {
account.getMasterDevice().get().isEnabled() &&
isAccountExpired(account, nowMs))
{
expiredAccountsMeter.mark(); expiredAccountsMeter.mark();
// Device masterDevice = account.getMasterDevice().get(); if (accountUpdateCount < MAX_ACCOUNT_UPDATES_PER_CHUNK) {
// masterDevice.setFetchesMessages(false); Device masterDevice = account.getMasterDevice().get();
// masterDevice.setApnId(null); masterDevice.setFetchesMessages(false);
// masterDevice.setGcmId(null); masterDevice.setApnId(null);
// masterDevice.setVoipApnId(null);
// if (accountUpdateCount < MAX_ACCOUNT_UPDATES_PER_CHUNK) { masterDevice.setGcmId(null);
// accountUpdateCount++;
// accountsManager.update(account); accountUpdateCount++;
// } accountsManager.update(account);
// directoryQueue.deleteRegisteredUser(account.getNumber());
directoryQueue.deleteRegisteredUser(account.getNumber());
}
} }
} }
} }
@ -77,9 +77,18 @@ public class AccountCleaner implements AccountDatabaseCrawlerListener {
public void onCrawlEnd(Optional<String> fromNumber) { public void onCrawlEnd(Optional<String> fromNumber) {
} }
@VisibleForTesting private boolean needsExplicitRemoval(Account account) {
public static boolean isAccountExpired(Account account, long nowMs) { return account.getMasterDevice().isPresent() &&
return account.getLastSeen() < (nowMs - TimeUnit.DAYS.toMillis(365)); hasPushToken(account.getMasterDevice().get()) &&
isExpired(account);
}
private boolean hasPushToken(Device device) {
return !Util.isEmpty(device.getGcmId()) || !Util.isEmpty(device.getApnId()) || !Util.isEmpty(device.getVoipApnId()) || device.getFetchesMessages();
}
private boolean isExpired(Account account) {
return account.getLastSeen() + TimeUnit.DAYS.toMillis(365) < System.currentTimeMillis();
} }
} }

238
service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountCleanerTest.java
View File

@ -28,6 +28,8 @@ import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.HashSet; import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Optional; import java.util.Optional;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
@ -35,195 +37,121 @@ import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.*;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
public class AccountCleanerTest { public class AccountCleanerTest {
private final AccountsManager accountsManager = mock(AccountsManager.class); private final AccountsManager accountsManager = mock(AccountsManager.class);
private final DirectoryQueue directoryQueue = mock(DirectoryQueue.class); private final DirectoryQueue directoryQueue = mock(DirectoryQueue.class);
private final Account activeUnexpiredAccount = mock(Account.class); private final Account deletedDisabledAccount = mock(Account.class);
private final Device activeUnexpiredDevice = mock(Device.class); private final Account undeletedDisabledAccount = mock(Account.class);
private final Account activeExpiredAccount = mock(Account.class); private final Account undeletedEnabledAccount = mock(Account.class);
private final Device activeExpiredDevice = mock(Device.class);
private final Account inactiveUnexpiredAccount = mock(Account.class);
private final Device inactiveUnexpiredDevice = mock(Device.class);
private final Account inactiveExpiredAccount = mock(Account.class);
private final Device inactiveExpiredDevice = mock(Device.class);
private final Device oldMasterDevice = mock(Device.class); private final Device deletedDisabledDevice = mock(Device.class );
private final Device recentMasterDevice = mock(Device.class); private final Device undeletedDisabledDevice = mock(Device.class );
private final Device agingSecondaryDevice = mock(Device.class); private final Device undeletedEnabledDevice = mock(Device.class );
private final Device recentSecondaryDevice = mock(Device.class);
private final Device oldSecondaryDevice = mock(Device.class);
private long nowMs;
@Before @Before
public void setup() { public void setup() {
when(activeUnexpiredDevice.isEnabled()).thenReturn(true); when(deletedDisabledDevice.isEnabled()).thenReturn(false);
when(activeUnexpiredAccount.getLastSeen()).thenReturn(Long.MAX_VALUE); when(deletedDisabledDevice.getGcmId()).thenReturn(null);
when(activeUnexpiredAccount.getMasterDevice()).thenReturn(Optional.of(activeUnexpiredDevice)); when(deletedDisabledDevice.getApnId()).thenReturn(null);
when(deletedDisabledDevice.getVoipApnId()).thenReturn(null);
when(deletedDisabledDevice.getFetchesMessages()).thenReturn(false);
when(deletedDisabledAccount.isEnabled()).thenReturn(false);
when(deletedDisabledAccount.getLastSeen()).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(1000));
when(deletedDisabledAccount.getMasterDevice()).thenReturn(Optional.of(deletedDisabledDevice));
when(deletedDisabledAccount.getNumber()).thenReturn("+14151231234");
when(activeExpiredAccount.getNumber()).thenReturn(AuthHelper.VALID_NUMBER); when(undeletedDisabledDevice.isEnabled()).thenReturn(false);
when(activeExpiredDevice.isEnabled()).thenReturn(true); when(undeletedDisabledDevice.getGcmId()).thenReturn("foo");
when(activeExpiredAccount.getLastSeen()).thenReturn(0L); when(undeletedDisabledAccount.isEnabled()).thenReturn(false);
when(activeExpiredAccount.getMasterDevice()).thenReturn(Optional.of(activeExpiredDevice)); when(undeletedDisabledAccount.getLastSeen()).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(366));
when(undeletedDisabledAccount.getMasterDevice()).thenReturn(Optional.of(undeletedDisabledDevice));
when(undeletedDisabledAccount.getNumber()).thenReturn("+14152222222");
when(inactiveUnexpiredDevice.isEnabled()).thenReturn(false); when(undeletedEnabledDevice.isEnabled()).thenReturn(true);
when(inactiveUnexpiredAccount.getLastSeen()).thenReturn(Long.MAX_VALUE); when(undeletedEnabledDevice.getApnId()).thenReturn("bar");
when(inactiveUnexpiredAccount.getMasterDevice()).thenReturn(Optional.of(inactiveUnexpiredDevice)); when(undeletedEnabledAccount.isEnabled()).thenReturn(true);
when(undeletedEnabledAccount.getMasterDevice()).thenReturn(Optional.of(undeletedEnabledDevice));
when(inactiveExpiredDevice.isEnabled()).thenReturn(false); when(undeletedEnabledAccount.getNumber()).thenReturn("+14153333333");
when(inactiveExpiredAccount.getLastSeen()).thenReturn(0L); when(undeletedEnabledAccount.getLastSeen()).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(364));
when(inactiveExpiredAccount.getMasterDevice()).thenReturn(Optional.of(inactiveExpiredDevice));
this.nowMs = System.currentTimeMillis();
when(oldMasterDevice.getLastSeen()).thenReturn(nowMs - TimeUnit.DAYS.toMillis(366));
when(oldMasterDevice.isEnabled()).thenReturn(true);
when(oldMasterDevice.getId()).thenReturn(Device.MASTER_ID);
when(recentMasterDevice.getLastSeen()).thenReturn(nowMs - TimeUnit.DAYS.toMillis(1));
when(recentMasterDevice.isEnabled()).thenReturn(true);
when(recentMasterDevice.getId()).thenReturn(Device.MASTER_ID);
when(agingSecondaryDevice.getLastSeen()).thenReturn(nowMs - TimeUnit.DAYS.toMillis(31));
when(agingSecondaryDevice.isEnabled()).thenReturn(false);
when(agingSecondaryDevice.getId()).thenReturn(2L);
when(recentSecondaryDevice.getLastSeen()).thenReturn(nowMs - TimeUnit.DAYS.toMillis(1));
when(recentSecondaryDevice.isEnabled()).thenReturn(true);
when(recentSecondaryDevice.getId()).thenReturn(2L);
when(oldSecondaryDevice.getLastSeen()).thenReturn(nowMs - TimeUnit.DAYS.toMillis(366));
when(oldSecondaryDevice.isEnabled()).thenReturn(false);
when(oldSecondaryDevice.getId()).thenReturn(2L);
} }
@Test @Test
public void testUnexpiredAccounts() { public void testAccounts() {
AccountCleaner accountCleaner = new AccountCleaner(accountsManager, directoryQueue); AccountCleaner accountCleaner = new AccountCleaner(accountsManager, directoryQueue);
accountCleaner.onCrawlStart(); accountCleaner.onCrawlStart();
accountCleaner.onCrawlChunk(Optional.empty(), Arrays.asList(activeUnexpiredAccount, inactiveUnexpiredAccount, inactiveExpiredAccount)); accountCleaner.onCrawlChunk(Optional.empty(), Arrays.asList(deletedDisabledAccount, undeletedDisabledAccount, undeletedEnabledAccount));
accountCleaner.onCrawlEnd(Optional.empty()); accountCleaner.onCrawlEnd(Optional.empty());
verify(activeUnexpiredDevice, atLeastOnce()).isEnabled(); verify(deletedDisabledDevice, never()).setGcmId(any());
verify(inactiveUnexpiredDevice, atLeastOnce()).isEnabled(); verify(deletedDisabledDevice, never()).setApnId(any());
verify(inactiveExpiredDevice, atLeastOnce()).isEnabled(); verify(deletedDisabledDevice, never()).setVoipApnId(any());
verify(deletedDisabledDevice, never()).setFetchesMessages(anyBoolean());
verifyNoMoreInteractions(activeUnexpiredDevice); verify(accountsManager, never()).update(eq(deletedDisabledAccount));
verifyNoMoreInteractions(activeExpiredDevice); verify(directoryQueue, never()).deleteRegisteredUser(eq("+14151231234"));
verifyNoMoreInteractions(inactiveUnexpiredDevice);
verifyNoMoreInteractions(inactiveExpiredDevice);
verify(undeletedDisabledDevice, times(1)).setGcmId(isNull());
verify(undeletedDisabledDevice, times(1)).setApnId(isNull());
verify(undeletedDisabledDevice, times(1)).setVoipApnId(isNull());
verify(undeletedDisabledDevice, times(1)).setFetchesMessages(eq(false));
verify(accountsManager, times(1)).update(eq(undeletedDisabledAccount));
verify(directoryQueue, times(1)).deleteRegisteredUser(eq("+14152222222"));
verify(undeletedEnabledDevice, never()).setGcmId(any());
verify(undeletedEnabledDevice, never()).setApnId(any());
verify(undeletedEnabledDevice, never()).setVoipApnId(any());
verify(undeletedEnabledDevice, never()).setFetchesMessages(anyBoolean());
verify(accountsManager, never()).update(eq(undeletedEnabledAccount));
verify(directoryQueue, never()).deleteRegisteredUser(eq("+14153333333"));
verifyNoMoreInteractions(accountsManager); verifyNoMoreInteractions(accountsManager);
verifyNoMoreInteractions(directoryQueue); verifyNoMoreInteractions(directoryQueue);
} }
// @Test
// public void testExpiredAccounts() {
// AccountCleaner accountCleaner = new AccountCleaner(accountsManager, directoryQueue);
// accountCleaner.onCrawlStart();
// accountCleaner.onCrawlChunk(Optional.empty(), Arrays.asList(activeUnexpiredAccount, activeExpiredAccount, inactiveUnexpiredAccount, inactiveExpiredAccount));
// accountCleaner.onCrawlEnd(Optional.empty());
//
// verify(activeExpiredDevice).setGcmId(isNull());
// verify(activeExpiredDevice).setApnId(isNull());
// verify(activeExpiredDevice).setFetchesMessages(eq(false));
//
// verify(accountsManager).update(eq(activeExpiredAccount));
// verify(directoryQueue).deleteRegisteredUser(eq(AuthHelper.VALID_NUMBER));
//
// verify(activeUnexpiredDevice, atLeastOnce()).isActive();
// verify(activeExpiredDevice, atLeastOnce()).isActive();
// verify(inactiveUnexpiredDevice, atLeastOnce()).isActive();
// verify(inactiveExpiredDevice, atLeastOnce()).isActive();
//
// verifyNoMoreInteractions(activeUnexpiredDevice);
// verifyNoMoreInteractions(activeExpiredDevice);
// verifyNoMoreInteractions(inactiveUnexpiredDevice);
// verifyNoMoreInteractions(inactiveExpiredDevice);
//
// verifyNoMoreInteractions(accountsManager);
// verifyNoMoreInteractions(directoryQueue);
// }
// @Test
// public void testMaxAccountUpdates() {
// ArrayList<Account> accounts = new ArrayList<>();
// accounts.add(activeUnexpiredAccount);
//
// int activeExpiredAccountCount = AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK + 1;
// for (int addedAccountCount = 0; addedAccountCount < activeExpiredAccountCount; addedAccountCount++) {
// accounts.add(activeExpiredAccount);
// }
//
// accounts.add(inactiveUnexpiredAccount);
// accounts.add(inactiveExpiredAccount);
//
// AccountCleaner accountCleaner = new AccountCleaner(accountsManager, directoryQueue);
// accountCleaner.onCrawlStart();
// accountCleaner.onCrawlChunk(Optional.empty(), accounts);
// accountCleaner.onCrawlEnd(Optional.empty());
//
// verify(activeExpiredDevice, atLeast(0)).setGcmId(isNull());
// verify(activeExpiredDevice, atLeast(0)).setApnId(isNull());
// verify(activeExpiredDevice, atLeast(0)).setFetchesMessages(eq(false));
//
// verify(accountsManager, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).update(eq(activeExpiredAccount));
// verify(directoryQueue, times(activeExpiredAccountCount)).deleteRegisteredUser(eq(AuthHelper.VALID_NUMBER));
//
// verify(activeUnexpiredDevice, atLeastOnce()).isActive();
// verify(activeExpiredDevice, atLeastOnce()).isActive();
// verify(inactiveUnexpiredDevice, atLeastOnce()).isActive();
// verify(inactiveExpiredDevice, atLeastOnce()).isActive();
//
// verifyNoMoreInteractions(activeUnexpiredDevice);
// verifyNoMoreInteractions(activeExpiredDevice);
// verifyNoMoreInteractions(inactiveUnexpiredDevice);
// verifyNoMoreInteractions(inactiveExpiredDevice);
//
// verifyNoMoreInteractions(accountsManager);
// verifyNoMoreInteractions(directoryQueue);
// }
@Test @Test
public void testIsAccountExpired() { public void testMaxAccountUpdates() {
Account recentAccount = new Account("+14152222222", new HashSet<Device>() {{ List<Account> accounts = new LinkedList<>();
add(recentMasterDevice); accounts.add(undeletedEnabledAccount);
add(recentSecondaryDevice);
}}, "1234".getBytes());
assertFalse(AccountCleaner.isAccountExpired(recentAccount, nowMs)); int activeExpiredAccountCount = AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK + 1;
for (int addedAccountCount = 0; addedAccountCount < activeExpiredAccountCount; addedAccountCount++) {
accounts.add(undeletedDisabledAccount);
}
Account oldSecondaryAccount = new Account("+14152222222", new HashSet<Device>() {{ accounts.add(deletedDisabledAccount);
add(recentMasterDevice);
add(agingSecondaryDevice);
}}, "1234".getBytes());
assertFalse(AccountCleaner.isAccountExpired(oldSecondaryAccount, nowMs)); AccountCleaner accountCleaner = new AccountCleaner(accountsManager, directoryQueue);
accountCleaner.onCrawlStart();
accountCleaner.onCrawlChunk(Optional.empty(), accounts);
accountCleaner.onCrawlEnd(Optional.empty());
Account agingPrimaryAccount = new Account("+14152222222", new HashSet<Device>() {{ verify(undeletedDisabledDevice, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).setGcmId(isNull());
add(oldMasterDevice); verify(undeletedDisabledDevice, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).setApnId(isNull());
add(agingSecondaryDevice); verify(undeletedDisabledDevice, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).setVoipApnId(isNull());
}}, "1234".getBytes()); verify(undeletedDisabledDevice, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).setFetchesMessages(eq(false));
assertFalse(AccountCleaner.isAccountExpired(agingPrimaryAccount, nowMs)); verify(accountsManager, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).update(eq(undeletedDisabledAccount));
verify(directoryQueue, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).deleteRegisteredUser(eq("+14152222222"));
Account oldPrimaryAccount = new Account("+14152222222", new HashSet<Device>() {{ verify(deletedDisabledDevice, never()).setGcmId(any());
add(oldMasterDevice); verify(deletedDisabledDevice, never()).setApnId(any());
add(oldSecondaryDevice); verify(deletedDisabledDevice, never()).setFetchesMessages(anyBoolean());
}}, "1234".getBytes());
assertTrue(AccountCleaner.isAccountExpired(oldPrimaryAccount, nowMs)); verify(undeletedEnabledDevice, never()).setGcmId(any());
verify(undeletedEnabledDevice, never()).setApnId(any());
verify(undeletedEnabledDevice, never()).setVoipApnId(any());
verify(undeletedEnabledDevice, never()).setFetchesMessages(anyBoolean());
verifyNoMoreInteractions(accountsManager);
verifyNoMoreInteractions(directoryQueue);
} }
} }