From ce026e7ad09218034520f1559706ced7cfe1235d Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Tue, 25 Aug 2020 12:40:51 -0400 Subject: [PATCH] Don't send contacts to CDS if they've opted out of discoverability. (SERVER-130) --- .../controllers/AccountController.java | 23 +++---- .../controllers/DeviceController.java | 6 +- .../controllers/KeysController.java | 4 +- .../textsecuregcm/sqs/DirectoryQueue.java | 12 ++-- .../textsecuregcm/storage/AccountCleaner.java | 3 +- .../storage/DirectoryReconciler.java | 5 +- .../storage/PushFeedbackProcessor.java | 5 +- .../workers/DeleteUserCommand.java | 2 +- .../textsecuregcm/sqs/DirectoryQueueTest.java | 61 +++++++++++++++++++ .../controllers/AccountControllerTest.java | 36 ++++++++--- .../tests/storage/AccountCleanerTest.java | 23 +++---- .../storage/DirectoryReconcilerTest.java | 41 ++++++++----- .../storage/PushFeedbackProcessorTest.java | 31 ++++++++-- .../textsecuregcm/tests/util/AuthHelper.java | 52 ++++++++++++---- 14 files changed, 215 insertions(+), 89 deletions(-) create mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/sqs/DirectoryQueueTest.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index 2cd4b9065..7d855e696 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -345,7 +345,7 @@ public class AccountController { accounts.update(account); if (!wasAccountEnabled && account.isEnabled()) { - directoryQueue.addRegisteredUser(account.getUuid(), account.getNumber()); + directoryQueue.refreshRegisteredUser(account); } } @@ -359,10 +359,7 @@ public class AccountController { device.setFetchesMessages(false); accounts.update(account); - - if (!account.isEnabled()) { - directoryQueue.deleteRegisteredUser(account.getUuid(), account.getNumber()); - } + directoryQueue.refreshRegisteredUser(account); } @Timed @@ -381,7 +378,7 @@ public class AccountController { accounts.update(account); if (!wasAccountEnabled && account.isEnabled()) { - directoryQueue.addRegisteredUser(account.getUuid(), account.getNumber()); + directoryQueue.refreshRegisteredUser(account); } } @@ -395,10 +392,7 @@ public class AccountController { device.setFetchesMessages(false); accounts.update(account); - - if (!account.isEnabled()) { - directoryQueue.deleteRegisteredUser(account.getUuid(), account.getNumber()); - } + directoryQueue.refreshRegisteredUser(account); } @Timed @@ -485,7 +479,9 @@ public class AccountController { account.setDiscoverableByPhoneNumber(attributes.isDiscoverableByPhoneNumber()); accounts.update(account); + directoryQueue.refreshRegisteredUser(account); } + @GET @Path("/whoami") @Produces(MediaType.APPLICATION_JSON) @@ -636,12 +632,7 @@ public class AccountController { newUserMeter.mark(); } - if (account.isEnabled()) { - directoryQueue.addRegisteredUser(account.getUuid(), number); - } else { - directoryQueue.deleteRegisteredUser(account.getUuid(), number); - } - + directoryQueue.refreshRegisteredUser(account); messagesManager.clear(number, maybeExistingAccount.map(Account::getUuid).orElse(null)); pendingAccounts.remove(number); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java index b5fc31475..32bfc1e3d 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java @@ -112,11 +112,7 @@ public class DeviceController { account.removeDevice(deviceId); accounts.update(account); - - if (!account.isEnabled()) { - directoryQueue.deleteRegisteredUser(account.getUuid(), account.getNumber()); - } - + directoryQueue.refreshRegisteredUser(account); messages.clear(account.getNumber(), account.getUuid(), deviceId); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java index 17c4de735..e88d7633b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java @@ -106,7 +106,7 @@ public class KeysController { accounts.update(account); if (!wasAccountEnabled && account.isEnabled()) { - directoryQueue.addRegisteredUser(account.getUuid(), account.getNumber()); + directoryQueue.refreshRegisteredUser(account); } } @@ -172,7 +172,7 @@ public class KeysController { accounts.update(account); if (!wasAccountEnabled && account.isEnabled()) { - directoryQueue.addRegisteredUser(account.getUuid(), account.getNumber()); + directoryQueue.refreshRegisteredUser(account); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/sqs/DirectoryQueue.java b/service/src/main/java/org/whispersystems/textsecuregcm/sqs/DirectoryQueue.java index af8b89a08..4e0275ccb 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/sqs/DirectoryQueue.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/sqs/DirectoryQueue.java @@ -28,9 +28,11 @@ import com.amazonaws.services.sqs.model.SendMessageRequest; import com.codahale.metrics.Meter; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.SharedMetricRegistries; +import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.configuration.SqsConfiguration; +import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.util.Constants; import java.util.HashMap; @@ -58,12 +60,14 @@ public class DirectoryQueue { this.sqs = AmazonSQSClientBuilder.standard().withRegion(sqsConfig.getRegion()).withCredentials(credentialsProvider).build(); } - public void addRegisteredUser(UUID uuid, String number) { - sendMessage("add", uuid, number); + @VisibleForTesting + DirectoryQueue(final String queueUrl, final AmazonSQS sqs) { + this.queueUrl = queueUrl; + this.sqs = sqs; } - public void deleteRegisteredUser(UUID uuid, String number) { - sendMessage("delete", uuid, number); + public void refreshRegisteredUser(final Account account) { + sendMessage(account.isEnabled() && account.isDiscoverableByPhoneNumber() ? "add" : "delete", account.getUuid(), account.getNumber()); } private void sendMessage(String action, UUID uuid, String number) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java index 45ecd89c5..fef78b39a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java @@ -71,8 +71,7 @@ public class AccountCleaner extends AccountDatabaseCrawlerListener { accountUpdateCount++; accountsManager.update(account); - - directoryQueue.deleteRegisteredUser(account.getUuid(), account.getNumber()); + directoryQueue.refreshRegisteredUser(account); } } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/DirectoryReconciler.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/DirectoryReconciler.java index 9037f74db..8a36335ab 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/DirectoryReconciler.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/DirectoryReconciler.java @@ -36,7 +36,6 @@ import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.UUID; -import java.util.stream.Collectors; import static com.codahale.metrics.MetricRegistry.name; @@ -83,7 +82,7 @@ public class DirectoryReconciler extends AccountDatabaseCrawlerListener { try { for (Account account : accounts) { - if (account.isEnabled()) { + if (account.isEnabled() && account.isDiscoverableByPhoneNumber()) { byte[] token = Util.getContactToken(account.getNumber()); ClientContact clientContact = new ClientContact(token, null, true, true); directoryManager.add(batchOperation, clientContact); @@ -100,7 +99,7 @@ public class DirectoryReconciler extends AccountDatabaseCrawlerListener { private DirectoryReconciliationRequest createChunkRequest(Optional fromUuid, List accounts) { List users = new ArrayList<>(accounts.size()); for (Account account : accounts) { - if (account.isEnabled()) { + if (account.isEnabled() && account.isDiscoverableByPhoneNumber()) { users.add(new DirectoryReconciliationRequest.User(account.getUuid(), account.getNumber())); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PushFeedbackProcessor.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PushFeedbackProcessor.java index cb4677ac5..821dd48e0 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PushFeedbackProcessor.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PushFeedbackProcessor.java @@ -60,10 +60,7 @@ public class PushFeedbackProcessor extends AccountDatabaseCrawlerListener { if (update) { accountsManager.update(account); - - if (!account.isEnabled()) { - directoryQueue.deleteRegisteredUser(account.getUuid(), account.getNumber()); - } + directoryQueue.refreshRegisteredUser(account); } } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java index 0138b5cdc..91cdcd8e7 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java @@ -89,7 +89,7 @@ public class DeleteUserCommand extends EnvironmentCommand requestCaptor = ArgumentCaptor.forClass(SendMessageRequest.class); + verify(sqs).sendMessage(requestCaptor.capture()); + + final Map messageAttributes = requestCaptor.getValue().getMessageAttributes(); + assertEquals(new MessageAttributeValue().withDataType("String").withStringValue(expectedAction), messageAttributes.get("action")); + } + + @SuppressWarnings("unused") + private Object argumentsForTestRefreshRegisteredUser() { + return new Object[] { + new Object[] { true, true, "add" }, + new Object[] { true, false, "delete" }, + new Object[] { false, true, "delete" }, + new Object[] { false, false, "delete" } + }; + } +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java index cbd99f81e..c24a43ac6 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -60,13 +60,17 @@ import java.util.UUID; import java.util.concurrent.TimeUnit; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.isA; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.notNull; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -148,6 +152,8 @@ public class AccountControllerTest { @Before public void setup() throws Exception { + clearInvocations(AuthHelper.VALID_ACCOUNT, AuthHelper.UNDISCOVERABLE_ACCOUNT); + new SecureRandom().nextBytes(registration_lock_key); AuthenticationCredentials registrationLockCredentials = new AuthenticationCredentials(Hex.toStringCondensed(registration_lock_key)); @@ -510,11 +516,9 @@ public class AccountControllerTest { final ArgumentCaptor accountArgumentCaptor = ArgumentCaptor.forClass(Account.class); verify(accountsManager, times(1)).create(accountArgumentCaptor.capture()); - verify(directoryQueue, times(1)).deleteRegisteredUser(notNull(), eq(SENDER)); + verify(directoryQueue, times(1)).refreshRegisteredUser(argThat(account -> SENDER.equals(account.getNumber()))); - final Account createdAccount = accountArgumentCaptor.getValue(); - - assertThat(createdAccount.isDiscoverableByPhoneNumber()).isTrue(); + assertThat(accountArgumentCaptor.getValue().isDiscoverableByPhoneNumber()).isTrue(); } @Test @@ -533,11 +537,9 @@ public class AccountControllerTest { final ArgumentCaptor accountArgumentCaptor = ArgumentCaptor.forClass(Account.class); verify(accountsManager, times(1)).create(accountArgumentCaptor.capture()); - verify(directoryQueue, times(1)).deleteRegisteredUser(notNull(), eq(SENDER)); + verify(directoryQueue, times(1)).refreshRegisteredUser(argThat(account -> SENDER.equals(account.getNumber()))); - final Account createdAccount = accountArgumentCaptor.getValue(); - - assertThat(createdAccount.isDiscoverableByPhoneNumber()).isFalse(); + assertThat(accountArgumentCaptor.getValue().isDiscoverableByPhoneNumber()).isFalse(); } @Test @@ -554,7 +556,7 @@ public class AccountControllerTest { assertThat(result.isStorageCapable()).isTrue(); verify(accountsManager, times(1)).create(isA(Account.class)); - verify(directoryQueue, times(1)).deleteRegisteredUser(notNull(), eq(SENDER_HAS_STORAGE)); + verify(directoryQueue, times(1)).refreshRegisteredUser(argThat(account -> SENDER_HAS_STORAGE.equals(account.getNumber()))); } @Test @@ -1007,6 +1009,7 @@ public class AccountControllerTest { verify(AuthHelper.DISABLED_DEVICE, times(1)).setGcmId(eq("c00lz0rz")); verify(accountsManager, times(1)).update(eq(AuthHelper.DISABLED_ACCOUNT)); + verify(directoryQueue, never()).refreshRegisteredUser(any(Account.class)); } @Test @@ -1022,6 +1025,7 @@ public class AccountControllerTest { verify(AuthHelper.DISABLED_DEVICE, times(1)).setGcmId(eq("z000")); verify(accountsManager, times(1)).update(eq(AuthHelper.DISABLED_ACCOUNT)); + verify(directoryQueue, never()).refreshRegisteredUser(any(Account.class)); } @Test @@ -1038,6 +1042,7 @@ public class AccountControllerTest { verify(AuthHelper.DISABLED_DEVICE, times(1)).setApnId(eq("first")); verify(AuthHelper.DISABLED_DEVICE, times(1)).setVoipApnId(eq("second")); verify(accountsManager, times(1)).update(eq(AuthHelper.DISABLED_ACCOUNT)); + verify(directoryQueue, never()).refreshRegisteredUser(any(Account.class)); } @Test @@ -1054,6 +1059,7 @@ public class AccountControllerTest { verify(AuthHelper.DISABLED_DEVICE, times(1)).setApnId(eq("third")); verify(AuthHelper.DISABLED_DEVICE, times(1)).setVoipApnId(eq("fourth")); verify(accountsManager, times(1)).update(eq(AuthHelper.DISABLED_ACCOUNT)); + verify(directoryQueue, never()).refreshRegisteredUser(any(Account.class)); } @Test @@ -1153,4 +1159,16 @@ public class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(401); } + @Test + public void testSetAccountAttributes() { + Response response = + resources.getJerseyTest() + .target("/v1/accounts/attributes/") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) + .put(Entity.json(new AccountAttributes("keykeykeykey", false, 2222, null, null, null, null, true))); + + assertThat(response.getStatus()).isEqualTo(204); + verify(directoryQueue, times(1)).refreshRegisteredUser(AuthHelper.VALID_ACCOUNT); + } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountCleanerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountCleanerTest.java index d8692360c..63cb27948 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountCleanerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountCleanerTest.java @@ -24,22 +24,24 @@ import org.whispersystems.textsecuregcm.storage.AccountCleaner; import org.whispersystems.textsecuregcm.storage.AccountDatabaseCrawlerRestartException; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; -import org.whispersystems.textsecuregcm.tests.util.AuthHelper; -import java.util.ArrayList; import java.util.Arrays; -import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Optional; import java.util.UUID; import java.util.concurrent.TimeUnit; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyBoolean; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +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 { @@ -98,8 +100,7 @@ public class AccountCleanerTest { verify(deletedDisabledDevice, never()).setFetchesMessages(anyBoolean()); verify(accountsManager, never()).update(eq(deletedDisabledAccount)); - verify(directoryQueue, never()).deleteRegisteredUser(notNull(), eq("+14151231234")); - + verify(directoryQueue, never()).refreshRegisteredUser(deletedDisabledAccount); verify(undeletedDisabledDevice, times(1)).setGcmId(isNull()); verify(undeletedDisabledDevice, times(1)).setApnId(isNull()); @@ -107,7 +108,7 @@ public class AccountCleanerTest { verify(undeletedDisabledDevice, times(1)).setFetchesMessages(eq(false)); verify(accountsManager, times(1)).update(eq(undeletedDisabledAccount)); - verify(directoryQueue, times(1)).deleteRegisteredUser(notNull(), eq("+14152222222")); + verify(directoryQueue, times(1)).refreshRegisteredUser(undeletedDisabledAccount); verify(undeletedEnabledDevice, never()).setGcmId(any()); verify(undeletedEnabledDevice, never()).setApnId(any()); @@ -115,7 +116,7 @@ public class AccountCleanerTest { verify(undeletedEnabledDevice, never()).setFetchesMessages(anyBoolean()); verify(accountsManager, never()).update(eq(undeletedEnabledAccount)); - verify(directoryQueue, never()).deleteRegisteredUser(notNull(), eq("+14153333333")); + verify(directoryQueue, never()).refreshRegisteredUser(undeletedEnabledAccount); verifyNoMoreInteractions(accountsManager); verifyNoMoreInteractions(directoryQueue); @@ -144,7 +145,7 @@ public class AccountCleanerTest { verify(undeletedDisabledDevice, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).setFetchesMessages(eq(false)); verify(accountsManager, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).update(eq(undeletedDisabledAccount)); - verify(directoryQueue, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).deleteRegisteredUser(notNull(), eq("+14152222222")); + verify(directoryQueue, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).refreshRegisteredUser(undeletedDisabledAccount); verify(deletedDisabledDevice, never()).setGcmId(any()); verify(deletedDisabledDevice, never()).setApnId(any()); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/DirectoryReconcilerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/DirectoryReconcilerTest.java index 28aca0ccb..5160aac46 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/DirectoryReconcilerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/DirectoryReconcilerTest.java @@ -41,55 +41,68 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; public class DirectoryReconcilerTest { - private static final UUID VALID_UUID = UUID.randomUUID(); - private static final String VALID_NUMBERRR = "+14152222222"; - private static final UUID INACTIVE_UUID = UUID.randomUUID(); - private static final String INACTIVE_NUMBERRR = "+14151111111"; + private static final UUID VALID_UUID = UUID.randomUUID(); + private static final String VALID_NUMBERRR = "+14152222222"; + private static final UUID INACTIVE_UUID = UUID.randomUUID(); + private static final String INACTIVE_NUMBERRR = "+14151111111"; + private static final UUID UNDISCOVERABLE_UUID = UUID.randomUUID(); + private static final String UNDISCOVERABLE_NUMBER = "+14153333333"; - private final Account activeAccount = mock(Account.class); - private final Account inactiveAccount = mock(Account.class); - private final BatchOperationHandle batchOperationHandle = mock(BatchOperationHandle.class); - private final DirectoryManager directoryManager = mock(DirectoryManager.class); - private final DirectoryReconciliationClient reconciliationClient = mock(DirectoryReconciliationClient.class); - private final DirectoryReconciler directoryReconciler = new DirectoryReconciler(reconciliationClient, directoryManager); + private final Account activeAccount = mock(Account.class); + private final Account inactiveAccount = mock(Account.class); + private final Account undiscoverableAccount = mock(Account.class); + private final BatchOperationHandle batchOperationHandle = mock(BatchOperationHandle.class); + private final DirectoryManager directoryManager = mock(DirectoryManager.class); + private final DirectoryReconciliationClient reconciliationClient = mock(DirectoryReconciliationClient.class); + private final DirectoryReconciler directoryReconciler = new DirectoryReconciler(reconciliationClient, directoryManager); private final DirectoryReconciliationResponse successResponse = new DirectoryReconciliationResponse(DirectoryReconciliationResponse.Status.OK); - private final DirectoryReconciliationResponse missingResponse = new DirectoryReconciliationResponse(DirectoryReconciliationResponse.Status.MISSING); @Before public void setup() { when(activeAccount.getUuid()).thenReturn(VALID_UUID); when(activeAccount.isEnabled()).thenReturn(true); when(activeAccount.getNumber()).thenReturn(VALID_NUMBERRR); + when(activeAccount.isDiscoverableByPhoneNumber()).thenReturn(true); when(inactiveAccount.getUuid()).thenReturn(INACTIVE_UUID); when(inactiveAccount.getNumber()).thenReturn(INACTIVE_NUMBERRR); when(inactiveAccount.isEnabled()).thenReturn(false); + when(inactiveAccount.isDiscoverableByPhoneNumber()).thenReturn(true); + when(undiscoverableAccount.getUuid()).thenReturn(UNDISCOVERABLE_UUID); + when(undiscoverableAccount.getNumber()).thenReturn(UNDISCOVERABLE_NUMBER); + when(undiscoverableAccount.isEnabled()).thenReturn(true); + when(undiscoverableAccount.isDiscoverableByPhoneNumber()).thenReturn(false); when(directoryManager.startBatchOperation()).thenReturn(batchOperationHandle); } @Test public void testCrawlChunkValid() throws AccountDatabaseCrawlerRestartException { when(reconciliationClient.sendChunk(any())).thenReturn(successResponse); - directoryReconciler.timeAndProcessCrawlChunk(Optional.of(VALID_UUID), Arrays.asList(activeAccount, inactiveAccount)); + directoryReconciler.timeAndProcessCrawlChunk(Optional.of(VALID_UUID), Arrays.asList(activeAccount, inactiveAccount, undiscoverableAccount)); verify(activeAccount, atLeastOnce()).getUuid(); verify(activeAccount, atLeastOnce()).getNumber(); verify(activeAccount, atLeastOnce()).isEnabled(); - verify(inactiveAccount, atLeastOnce()).getUuid(); + verify(activeAccount, atLeastOnce()).isDiscoverableByPhoneNumber(); verify(inactiveAccount, atLeastOnce()).getNumber(); verify(inactiveAccount, atLeastOnce()).isEnabled(); + verify(undiscoverableAccount, atLeastOnce()).getUuid(); + verify(undiscoverableAccount, atLeastOnce()).getNumber(); + verify(undiscoverableAccount, atLeastOnce()).isEnabled(); + verify(undiscoverableAccount, atLeastOnce()).isDiscoverableByPhoneNumber(); ArgumentCaptor request = ArgumentCaptor.forClass(DirectoryReconciliationRequest.class); verify(reconciliationClient, times(1)).sendChunk(request.capture()); assertThat(request.getValue().getFromUuid()).isEqualTo(VALID_UUID); - assertThat(request.getValue().getToUuid()).isEqualTo(INACTIVE_UUID); + assertThat(request.getValue().getToUuid()).isEqualTo(UNDISCOVERABLE_UUID); assertThat(request.getValue().getUsers()).isEqualTo(Arrays.asList(new DirectoryReconciliationRequest.User(VALID_UUID, VALID_NUMBERRR))); ArgumentCaptor addedContact = ArgumentCaptor.forClass(ClientContact.class); verify(directoryManager, times(1)).startBatchOperation(); verify(directoryManager, times(1)).add(eq(batchOperationHandle), addedContact.capture()); verify(directoryManager, times(1)).remove(eq(batchOperationHandle), eq(INACTIVE_NUMBERRR)); + verify(directoryManager, times(1)).remove(eq(batchOperationHandle), eq(UNDISCOVERABLE_NUMBER)); verify(directoryManager, times(1)).stopBatchOperation(eq(batchOperationHandle)); assertThat(addedContact.getValue().getToken()).isEqualTo(Util.getContactToken(VALID_NUMBERRR)); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PushFeedbackProcessorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PushFeedbackProcessorTest.java index 2a542fd4c..1d2bd1c37 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PushFeedbackProcessorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PushFeedbackProcessorTest.java @@ -24,11 +24,12 @@ public class PushFeedbackProcessorTest { private AccountsManager accountsManager = mock(AccountsManager.class); private DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - private Account uninstalledAccount = mock(Account.class); - private Account mixedAccount = mock(Account.class); - private Account freshAccount = mock(Account.class); - private Account cleanAccount = mock(Account.class); - private Account stillActiveAccount = mock(Account.class); + private Account uninstalledAccount = mock(Account.class); + private Account mixedAccount = mock(Account.class); + private Account freshAccount = mock(Account.class); + private Account cleanAccount = mock(Account.class); + private Account stillActiveAccount = mock(Account.class); + private Account undiscoverableAccount = mock(Account.class); private Device uninstalledDevice = mock(Device.class); private Device uninstalledDeviceTwo = mock(Device.class); @@ -36,6 +37,7 @@ public class PushFeedbackProcessorTest { private Device installedDeviceTwo = mock(Device.class); private Device recentUninstalledDevice = mock(Device.class); private Device stillActiveDevice = mock(Device.class); + private Device undiscoverableDevice = mock(Device.class); @Before public void setup() { @@ -53,11 +55,25 @@ public class PushFeedbackProcessorTest { when(stillActiveDevice.getUninstalledFeedbackTimestamp()).thenReturn(Util.todayInMillis() - TimeUnit.DAYS.toMillis(2)); when(stillActiveDevice.getLastSeen()).thenReturn(Util.todayInMillis()); + when(undiscoverableDevice.getUninstalledFeedbackTimestamp()).thenReturn(Util.todayInMillis() - TimeUnit.DAYS.toMillis(2)); + when(undiscoverableDevice.getLastSeen()).thenReturn(Util.todayInMillis() - TimeUnit.DAYS.toMillis(2)); + when(uninstalledAccount.getDevices()).thenReturn(Set.of(uninstalledDevice)); when(mixedAccount.getDevices()).thenReturn(Set.of(installedDevice, uninstalledDeviceTwo)); when(freshAccount.getDevices()).thenReturn(Set.of(recentUninstalledDevice)); when(cleanAccount.getDevices()).thenReturn(Set.of(installedDeviceTwo)); when(stillActiveAccount.getDevices()).thenReturn(Set.of(stillActiveDevice)); + when(undiscoverableAccount.getDevices()).thenReturn(Set.of(undiscoverableDevice)); + + when(uninstalledAccount.isEnabled()).thenReturn(true); + when(uninstalledAccount.isDiscoverableByPhoneNumber()).thenReturn(true); + when(uninstalledAccount.getUuid()).thenReturn(UUID.randomUUID()); + when(uninstalledAccount.getNumber()).thenReturn("+18005551234"); + + when(undiscoverableAccount.isEnabled()).thenReturn(true); + when(undiscoverableAccount.isDiscoverableByPhoneNumber()).thenReturn(false); + when(undiscoverableAccount.getUuid()).thenReturn(UUID.randomUUID()); + when(undiscoverableAccount.getNumber()).thenReturn("+18005559876"); } @@ -73,7 +89,7 @@ public class PushFeedbackProcessorTest { @Test public void testUpdate() throws AccountDatabaseCrawlerRestartException { PushFeedbackProcessor processor = new PushFeedbackProcessor(accountsManager, directoryQueue); - processor.timeAndProcessCrawlChunk(Optional.of(UUID.randomUUID()), List.of(uninstalledAccount, mixedAccount, stillActiveAccount, freshAccount, cleanAccount)); + processor.timeAndProcessCrawlChunk(Optional.of(UUID.randomUUID()), List.of(uninstalledAccount, mixedAccount, stillActiveAccount, freshAccount, cleanAccount, undiscoverableAccount)); verify(uninstalledDevice).setApnId(isNull()); verify(uninstalledDevice).setGcmId(isNull()); @@ -109,6 +125,9 @@ public class PushFeedbackProcessorTest { verify(stillActiveDevice, never()).setFetchesMessages(anyBoolean()); verify(accountsManager).update(eq(stillActiveAccount)); + + verify(directoryQueue).refreshRegisteredUser(undiscoverableAccount); + verify(directoryQueue).refreshRegisteredUser(uninstalledAccount); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java index 19ebfe951..bbcc81e0b 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java @@ -48,45 +48,58 @@ public class AuthHelper { public static final UUID DISABLED_UUID = UUID.randomUUID(); public static final String DISABLED_PASSWORD = "poof"; + public static final String UNDISCOVERABLE_NUMBER = "+18005551234"; + public static final UUID UNDISCOVERABLE_UUID = UUID.randomUUID(); + public static final String UNDISCOVERABLE_PASSWORD = "IT'S A SECRET TO EVERYBODY."; + public static final String VALID_IDENTITY = "BcxxDU9FGMda70E7+Uvm7pnQcEdXQ64aJCpPUeRSfcFo"; - public static AccountsManager ACCOUNTS_MANAGER = mock(AccountsManager.class); - public static Account VALID_ACCOUNT = mock(Account.class ); - public static Account VALID_ACCOUNT_TWO = mock(Account.class ); - public static Account DISABLED_ACCOUNT = mock(Account.class ); + public static AccountsManager ACCOUNTS_MANAGER = mock(AccountsManager.class); + public static Account VALID_ACCOUNT = mock(Account.class ); + public static Account VALID_ACCOUNT_TWO = mock(Account.class ); + public static Account DISABLED_ACCOUNT = mock(Account.class ); + public static Account UNDISCOVERABLE_ACCOUNT = mock(Account.class ); - public static Device VALID_DEVICE = mock(Device.class); - public static Device VALID_DEVICE_TWO = mock(Device.class); - public static Device DISABLED_DEVICE = mock(Device.class); + public static Device VALID_DEVICE = mock(Device.class); + public static Device VALID_DEVICE_TWO = mock(Device.class); + public static Device DISABLED_DEVICE = mock(Device.class); + public static Device UNDISCOVERABLE_DEVICE = mock(Device.class); - private static AuthenticationCredentials VALID_CREDENTIALS = mock(AuthenticationCredentials.class); - private static AuthenticationCredentials VALID_CREDENTIALS_TWO = mock(AuthenticationCredentials.class); - private static AuthenticationCredentials DISABLED_CREDENTIALS = mock(AuthenticationCredentials.class); + private static AuthenticationCredentials VALID_CREDENTIALS = mock(AuthenticationCredentials.class); + private static AuthenticationCredentials VALID_CREDENTIALS_TWO = mock(AuthenticationCredentials.class); + private static AuthenticationCredentials DISABLED_CREDENTIALS = mock(AuthenticationCredentials.class); + private static AuthenticationCredentials UNDISCOVERABLE_CREDENTIALS = mock(AuthenticationCredentials.class); public static PolymorphicAuthDynamicFeature getAuthFilter() { when(VALID_CREDENTIALS.verify("foo")).thenReturn(true); when(VALID_CREDENTIALS_TWO.verify("baz")).thenReturn(true); when(DISABLED_CREDENTIALS.verify(DISABLED_PASSWORD)).thenReturn(true); + when(UNDISCOVERABLE_CREDENTIALS.verify(UNDISCOVERABLE_PASSWORD)).thenReturn(true); when(VALID_DEVICE.getAuthenticationCredentials()).thenReturn(VALID_CREDENTIALS); when(VALID_DEVICE_TWO.getAuthenticationCredentials()).thenReturn(VALID_CREDENTIALS_TWO); when(DISABLED_DEVICE.getAuthenticationCredentials()).thenReturn(DISABLED_CREDENTIALS); + when(UNDISCOVERABLE_DEVICE.getAuthenticationCredentials()).thenReturn(UNDISCOVERABLE_CREDENTIALS); when(VALID_DEVICE.isMaster()).thenReturn(true); when(VALID_DEVICE_TWO.isMaster()).thenReturn(true); when(DISABLED_DEVICE.isMaster()).thenReturn(true); + when(UNDISCOVERABLE_DEVICE.isMaster()).thenReturn(true); when(VALID_DEVICE.getId()).thenReturn(1L); when(VALID_DEVICE_TWO.getId()).thenReturn(1L); when(DISABLED_DEVICE.getId()).thenReturn(1L); + when(UNDISCOVERABLE_DEVICE.getId()).thenReturn(1L); when(VALID_DEVICE.isEnabled()).thenReturn(true); when(VALID_DEVICE_TWO.isEnabled()).thenReturn(true); when(DISABLED_DEVICE.isEnabled()).thenReturn(false); + when(UNDISCOVERABLE_DEVICE.isMaster()).thenReturn(true); when(VALID_ACCOUNT.getDevice(1L)).thenReturn(Optional.of(VALID_DEVICE)); when(VALID_ACCOUNT_TWO.getDevice(eq(1L))).thenReturn(Optional.of(VALID_DEVICE_TWO)); when(DISABLED_ACCOUNT.getDevice(eq(1L))).thenReturn(Optional.of(DISABLED_DEVICE)); + when(UNDISCOVERABLE_ACCOUNT.getDevice(eq(1L))).thenReturn(Optional.of(UNDISCOVERABLE_DEVICE)); when(VALID_ACCOUNT_TWO.getEnabledDeviceCount()).thenReturn(6); @@ -96,17 +109,27 @@ public class AuthHelper { when(VALID_ACCOUNT_TWO.getUuid()).thenReturn(VALID_UUID_TWO); when(DISABLED_ACCOUNT.getNumber()).thenReturn(DISABLED_NUMBER); when(DISABLED_ACCOUNT.getUuid()).thenReturn(DISABLED_UUID); + when(UNDISCOVERABLE_ACCOUNT.getNumber()).thenReturn(UNDISCOVERABLE_NUMBER); + when(UNDISCOVERABLE_ACCOUNT.getUuid()).thenReturn(UNDISCOVERABLE_UUID); when(VALID_ACCOUNT.getAuthenticatedDevice()).thenReturn(Optional.of(VALID_DEVICE)); when(VALID_ACCOUNT_TWO.getAuthenticatedDevice()).thenReturn(Optional.of(VALID_DEVICE_TWO)); when(DISABLED_ACCOUNT.getAuthenticatedDevice()).thenReturn(Optional.of(DISABLED_DEVICE)); + when(UNDISCOVERABLE_ACCOUNT.getAuthenticatedDevice()).thenReturn(Optional.of(UNDISCOVERABLE_DEVICE)); - when(VALID_ACCOUNT.getRelay()).thenReturn(Optional.empty()); - when(VALID_ACCOUNT_TWO.getRelay()).thenReturn(Optional.empty()); + when(VALID_ACCOUNT.getRelay()).thenReturn(Optional.empty()); + when(VALID_ACCOUNT_TWO.getRelay()).thenReturn(Optional.empty()); + when(UNDISCOVERABLE_ACCOUNT.getRelay()).thenReturn(Optional.empty()); when(VALID_ACCOUNT.isEnabled()).thenReturn(true); when(VALID_ACCOUNT_TWO.isEnabled()).thenReturn(true); when(DISABLED_ACCOUNT.isEnabled()).thenReturn(false); + when(UNDISCOVERABLE_ACCOUNT.isEnabled()).thenReturn(true); + + when(VALID_ACCOUNT.isDiscoverableByPhoneNumber()).thenReturn(true); + when(VALID_ACCOUNT_TWO.isDiscoverableByPhoneNumber()).thenReturn(true); + when(DISABLED_ACCOUNT.isDiscoverableByPhoneNumber()).thenReturn(true); + when(UNDISCOVERABLE_ACCOUNT.isDiscoverableByPhoneNumber()).thenReturn(false); when(VALID_ACCOUNT.getIdentityKey()).thenReturn(VALID_IDENTITY); @@ -125,6 +148,11 @@ public class AuthHelper { when(ACCOUNTS_MANAGER.get(argThat((ArgumentMatcher) identifier -> identifier != null && identifier.hasNumber() && identifier.getNumber().equals(DISABLED_NUMBER)))).thenReturn(Optional.of(DISABLED_ACCOUNT)); when(ACCOUNTS_MANAGER.get(argThat((ArgumentMatcher) identifier -> identifier != null && identifier.hasUuid() && identifier.getUuid().equals(DISABLED_UUID)))).thenReturn(Optional.of(DISABLED_ACCOUNT)); + when(ACCOUNTS_MANAGER.get(UNDISCOVERABLE_NUMBER)).thenReturn(Optional.of(UNDISCOVERABLE_ACCOUNT)); + when(ACCOUNTS_MANAGER.get(UNDISCOVERABLE_UUID)).thenReturn(Optional.of(UNDISCOVERABLE_ACCOUNT)); + when(ACCOUNTS_MANAGER.get(argThat((ArgumentMatcher) identifier -> identifier != null && identifier.hasNumber() && identifier.getNumber().equals(UNDISCOVERABLE_NUMBER)))).thenReturn(Optional.of(UNDISCOVERABLE_ACCOUNT)); + when(ACCOUNTS_MANAGER.get(argThat((ArgumentMatcher) identifier -> identifier != null && identifier.hasUuid() && identifier.getUuid().equals(UNDISCOVERABLE_UUID)))).thenReturn(Optional.of(UNDISCOVERABLE_ACCOUNT)); + for (TestAccount testAccount : TEST_ACCOUNTS) { testAccount.setup(ACCOUNTS_MANAGER); }