From 8579190cdf5c24d13dca1399385becb78079d260 Mon Sep 17 00:00:00 2001 From: Jon Chambers <63609320+jon-signal@users.noreply.github.com> Date: Tue, 27 Jul 2021 10:27:47 -0400 Subject: [PATCH] Consolidate account creation/directory updates into `AccountsManager` --- .../textsecuregcm/WhisperServerService.java | 11 +- .../controllers/AccountController.java | 97 +----- .../controllers/DeviceController.java | 4 - .../controllers/KeysController.java | 18 +- .../textsecuregcm/sqs/DirectoryQueue.java | 10 +- .../textsecuregcm/storage/Account.java | 15 + .../storage/AccountsManager.java | 76 ++++- .../storage/PushFeedbackProcessor.java | 13 +- .../workers/DeleteUserCommand.java | 8 +- .../textsecuregcm/sqs/DirectoryQueueTest.java | 6 +- ...ConcurrentModificationIntegrationTest.java | 49 +-- .../controllers/AccountControllerTest.java | 308 +++++++----------- .../controllers/DeviceControllerTest.java | 7 +- .../tests/controllers/KeysControllerTest.java | 5 +- .../tests/storage/AccountsManagerTest.java | 307 ++++++----------- .../storage/PushFeedbackProcessorTest.java | 23 +- 16 files changed, 368 insertions(+), 589 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index a2e8257f4..f24454af8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -442,7 +442,7 @@ public class WhisperServerService extends Application deletedAccountsDirectoryReconcilers = new ArrayList<>(); final List accountDatabaseCrawlerListeners = new ArrayList<>(); - accountDatabaseCrawlerListeners.add(new PushFeedbackProcessor(accountsManager, directoryQueue)); + accountDatabaseCrawlerListeners.add(new PushFeedbackProcessor(accountsManager)); accountDatabaseCrawlerListeners.add(new ActiveUserCounter(config.getMetricsFactory(), cacheCluster)); for (DirectoryServerConfiguration directoryServerConfiguration : config.getDirectoryConfiguration().getDirectoryServerConfiguration()) { final DirectoryReconciliationClient directoryReconciliationClient = new DirectoryReconciliationClient(directoryServerConfiguration); @@ -555,8 +555,8 @@ public class WhisperServerService extends Application commonControllers = List.of( new AttachmentControllerV1(rateLimiters, config.getAwsAttachmentsConfiguration().getAccessKey(), config.getAwsAttachmentsConfiguration().getAccessSecret(), config.getAwsAttachmentsConfiguration().getBucket()), @@ -564,7 +564,7 @@ public class WhisperServerService extends Application testDevices; @@ -138,9 +130,6 @@ public class AccountController { AbusiveHostRules abusiveHostRules, RateLimiters rateLimiters, SmsSender smsSenderFactory, - DirectoryQueue directoryQueue, - MessagesManager messagesManager, - KeysDynamoDb keys, DynamicConfigurationManager dynamicConfigurationManager, TurnTokenGenerator turnTokenGenerator, Map testDevices, @@ -156,9 +145,6 @@ public class AccountController { this.abusiveHostRules = abusiveHostRules; this.rateLimiters = rateLimiters; this.smsSender = smsSenderFactory; - this.directoryQueue = directoryQueue; - this.messagesManager = messagesManager; - this.keys = keys; this.dynamicConfigurationManager = dynamicConfigurationManager; this.testDevices = testDevices; this.turnTokenGenerator = turnTokenGenerator; @@ -318,6 +304,7 @@ public class AccountController { }); }); + // TODO Remove this meter when external dependencies have been resolved metricRegistry.meter(name(AccountController.class, "create", Util.getCountryCode(number))).mark(); { @@ -392,7 +379,7 @@ public class AccountController { throw new WebApplicationException(Response.status(409).build()); } - Account account = createAccount(number, password, signalAgent, accountAttributes); + Account account = accounts.create(number, password, signalAgent, accountAttributes); { metricRegistry.meter(name(AccountController.class, "verify", Util.getCountryCode(number))).mark(); @@ -431,7 +418,6 @@ public class AccountController { public void setGcmRegistrationId(@Auth DisabledPermittedAccount disabledPermittedAccount, @Valid GcmRegistrationId registrationId) { Account account = disabledPermittedAccount.getAccount(); Device device = account.getAuthenticatedDevice().get(); - boolean wasAccountEnabled = account.isEnabled(); if (device.getGcmId() != null && device.getGcmId().equals(registrationId.getGcmRegistrationId())) @@ -439,16 +425,12 @@ public class AccountController { return; } - account = accounts.updateDevice(account, device.getId(), d -> { + accounts.updateDevice(account, device.getId(), d -> { d.setApnId(null); d.setVoipApnId(null); d.setGcmId(registrationId.getGcmRegistrationId()); d.setFetchesMessages(false); }); - - if (!wasAccountEnabled && account.isEnabled()) { - directoryQueue.refreshRegisteredUser(account); - } } @Timed @@ -458,12 +440,11 @@ public class AccountController { Account account = disabledPermittedAccount.getAccount(); Device device = account.getAuthenticatedDevice().get(); - account = accounts.updateDevice(account, device.getId(), d -> { + accounts.updateDevice(account, device.getId(), d -> { d.setGcmId(null); d.setFetchesMessages(false); d.setUserAgent("OWA"); }); - directoryQueue.refreshRegisteredUser(account); } @Timed @@ -473,18 +454,13 @@ public class AccountController { public void setApnRegistrationId(@Auth DisabledPermittedAccount disabledPermittedAccount, @Valid ApnRegistrationId registrationId) { Account account = disabledPermittedAccount.getAccount(); Device device = account.getAuthenticatedDevice().get(); - boolean wasAccountEnabled = account.isEnabled(); - account = accounts.updateDevice(account, device.getId(), d -> { + accounts.updateDevice(account, device.getId(), d -> { d.setApnId(registrationId.getApnRegistrationId()); d.setVoipApnId(registrationId.getVoipRegistrationId()); d.setGcmId(null); d.setFetchesMessages(false); }); - - if (!wasAccountEnabled && account.isEnabled()) { - directoryQueue.refreshRegisteredUser(account); - } } @Timed @@ -503,7 +479,6 @@ public class AccountController { d.setUserAgent("OWP"); } }); - directoryQueue.refreshRegisteredUser(account); } @Timed @@ -577,7 +552,7 @@ public class AccountController { Account account = disabledPermittedAccount.getAccount(); long deviceId = account.getAuthenticatedDevice().get().getId(); - account = accounts.update(account, a-> { + accounts.update(account, a-> { a.getDevice(deviceId).ifPresent(d -> { d.setFetchesMessages(attributes.getFetchesMessages()); @@ -588,19 +563,12 @@ public class AccountController { d.setUserAgent(userAgent); }); - setAccountRegistrationLockFromAttributes(a, attributes); + a.setRegistrationLockFromAttributes(attributes); a.setUnidentifiedAccessKey(attributes.getUnidentifiedAccessKey()); a.setUnrestrictedUnidentifiedAccess(attributes.isUnrestrictedUnidentifiedAccess()); a.setDiscoverableByPhoneNumber(attributes.isDiscoverableByPhoneNumber()); - }); - - final boolean hasDiscoverabilityChange = (account.isDiscoverableByPhoneNumber() != attributes.isDiscoverableByPhoneNumber()); - - if (hasDiscoverabilityChange) { - directoryQueue.refreshRegisteredUser(account); - } } @GET @@ -752,57 +720,6 @@ public class AccountController { return false; } - private Account createAccount(String number, String password, String signalAgent, AccountAttributes accountAttributes) { - Optional maybeExistingAccount = accounts.get(number); - - Device device = new Device(); - device.setId(Device.MASTER_ID); - device.setAuthenticationCredentials(new AuthenticationCredentials(password)); - device.setFetchesMessages(accountAttributes.getFetchesMessages()); - device.setRegistrationId(accountAttributes.getRegistrationId()); - device.setName(accountAttributes.getName()); - device.setCapabilities(accountAttributes.getCapabilities()); - device.setCreated(System.currentTimeMillis()); - device.setLastSeen(Util.todayInMillis()); - device.setUserAgent(signalAgent); - - Account account = new Account(); - account.setNumber(number); - account.setUuid(UUID.randomUUID()); - account.addDevice(device); - setAccountRegistrationLockFromAttributes(account, accountAttributes); - account.setUnidentifiedAccessKey(accountAttributes.getUnidentifiedAccessKey()); - account.setUnrestrictedUnidentifiedAccess(accountAttributes.isUnrestrictedUnidentifiedAccess()); - account.setDiscoverableByPhoneNumber(accountAttributes.isDiscoverableByPhoneNumber()); - - if (accounts.create(account)) { - newUserMeter.mark(); - } - - directoryQueue.refreshRegisteredUser(account); - - maybeExistingAccount.ifPresent(definitelyExistingAccount -> { - messagesManager.clear(definitelyExistingAccount.getUuid()); - keys.delete(definitelyExistingAccount); - }); - - pendingAccounts.remove(number); - - return account; - } - - private void setAccountRegistrationLockFromAttributes(Account account, @Valid AccountAttributes attributes) { - if (!Util.isEmpty(attributes.getPin())) { - account.setPin(attributes.getPin()); - } else if (!Util.isEmpty(attributes.getRegistrationLock())) { - AuthenticationCredentials credentials = new AuthenticationCredentials(attributes.getRegistrationLock()); - account.setRegistrationLock(credentials.getHashedAuthenticationToken(), credentials.getSalt()); - } else { - account.setPin(null); - account.setRegistrationLock(null, null); - } - } - @VisibleForTesting protected VerificationCode generateVerificationCode(String number) { if (testDevices.containsKey(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 8b3b7e39d..d1f22f988 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java @@ -62,13 +62,11 @@ public class DeviceController { private final KeysDynamoDb keys; private final RateLimiters rateLimiters; private final Map maxDeviceConfiguration; - private final DirectoryQueue directoryQueue; public DeviceController(StoredVerificationCodeManager pendingDevices, AccountsManager accounts, MessagesManager messages, KeysDynamoDb keys, - DirectoryQueue directoryQueue, RateLimiters rateLimiters, Map maxDeviceConfiguration) { @@ -76,7 +74,6 @@ public class DeviceController { this.accounts = accounts; this.messages = messages; this.keys = keys; - this.directoryQueue = directoryQueue; this.rateLimiters = rateLimiters; this.maxDeviceConfiguration = maxDeviceConfiguration; } @@ -105,7 +102,6 @@ public class DeviceController { messages.clear(account.getUuid(), deviceId); account = accounts.update(account, a -> a.removeDevice(deviceId)); - directoryQueue.refreshRegisteredUser(account); keys.delete(account, deviceId); // ensure any messages that came in after the first clear() are also removed messages.clear(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 04030c2a0..972fe0ea8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java @@ -55,7 +55,6 @@ public class KeysController { private final RateLimiters rateLimiters; private final KeysDynamoDb keysDynamoDb; private final AccountsManager accounts; - private final DirectoryQueue directoryQueue; private final PreKeyRateLimiter preKeyRateLimiter; private final DynamicConfigurationManager dynamicConfigurationManager; @@ -69,13 +68,12 @@ public class KeysController { private static final String PREKEY_TARGET_IDENTIFIER_TAG_NAME = "identifierType"; public KeysController(RateLimiters rateLimiters, KeysDynamoDb keysDynamoDb, AccountsManager accounts, - DirectoryQueue directoryQueue, PreKeyRateLimiter preKeyRateLimiter, + PreKeyRateLimiter preKeyRateLimiter, DynamicConfigurationManager dynamicConfigurationManager, RateLimitChallengeManager rateLimitChallengeManager) { this.rateLimiters = rateLimiters; this.keysDynamoDb = keysDynamoDb; this.accounts = accounts; - this.directoryQueue = directoryQueue; this.preKeyRateLimiter = preKeyRateLimiter; this.dynamicConfigurationManager = dynamicConfigurationManager; @@ -100,7 +98,6 @@ public class KeysController { public void setKeys(@Auth DisabledPermittedAccount disabledPermittedAccount, @Valid PreKeyState preKeys) { Account account = disabledPermittedAccount.getAccount(); Device device = account.getAuthenticatedDevice().get(); - boolean wasAccountEnabled = account.isEnabled(); boolean updateAccount = false; if (!preKeys.getSignedPreKey().equals(device.getSignedPreKey())) { @@ -116,10 +113,6 @@ public class KeysController { a.getDevice(device.getId()).ifPresent(d -> d.setSignedPreKey(preKeys.getSignedPreKey())); a.setIdentityKey(preKeys.getIdentityKey()); }); - - if (!wasAccountEnabled && account.isEnabled()) { - directoryQueue.refreshRegisteredUser(account); - } } keysDynamoDb.store(account, device.getId(), preKeys.getPreKeys()); @@ -198,14 +191,9 @@ public class KeysController { @Path("/signed") @Consumes(MediaType.APPLICATION_JSON) public void setSignedKey(@Auth Account account, @Valid SignedPreKey signedPreKey) { - Device device = account.getAuthenticatedDevice().get(); - boolean wasAccountEnabled = account.isEnabled(); + Device device = account.getAuthenticatedDevice().get(); - account = accounts.updateDevice(account, device.getId(), d -> d.setSignedPreKey(signedPreKey)); - - if (!wasAccountEnabled && account.isEnabled()) { - directoryQueue.refreshRegisteredUser(account); - } + accounts.updateDevice(account, device.getId(), d -> d.setSignedPreKey(signedPreKey)); } @Timed 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 122d08d42..e26265c9e 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/sqs/DirectoryQueue.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/sqs/DirectoryQueue.java @@ -61,11 +61,15 @@ public class DirectoryQueue { this.sqs = sqs; } - public void refreshRegisteredUser(final Account account) { - refreshRegisteredUsers(List.of(account)); + public boolean isDiscoverable(final Account account) { + return account.isEnabled() && account.isDiscoverableByPhoneNumber(); } - public void refreshRegisteredUsers(final List accounts) { + public void refreshAccount(final Account account) { + refreshAccounts(List.of(account)); + } + + public void refreshAccounts(final List accounts) { final List> accountsAndActions = accounts.stream() .map(account -> new Pair<>(account, account.isEnabled() && account.isDiscoverableByPhoneNumber() ? "add" : "delete")) .collect(Collectors.toList()); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java index a437fe5e5..b7e94b7c4 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java @@ -17,7 +17,10 @@ import javax.security.auth.Subject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.auth.AmbiguousIdentifier; +import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials; import org.whispersystems.textsecuregcm.auth.StoredRegistrationLock; +import org.whispersystems.textsecuregcm.entities.AccountAttributes; +import org.whispersystems.textsecuregcm.util.Util; public class Account implements Principal { @@ -312,6 +315,18 @@ public class Account implements Principal { this.pin = pin; } + public void setRegistrationLockFromAttributes(final AccountAttributes attributes) { + if (!Util.isEmpty(attributes.getPin())) { + setPin(attributes.getPin()); + } else if (!Util.isEmpty(attributes.getRegistrationLock())) { + AuthenticationCredentials credentials = new AuthenticationCredentials(attributes.getRegistrationLock()); + setRegistrationLock(credentials.getHashedAuthenticationToken(), credentials.getSalt()); + } else { + setPin(null); + setRegistrationLock(null, null); + } + } + public void setRegistrationLock(String registrationLock, String registrationLockSalt) { requireNotStale(); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java index b62d73401..0338d0de0 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -7,6 +7,7 @@ package org.whispersystems.textsecuregcm.storage; import static com.codahale.metrics.MetricRegistry.name; +import com.codahale.metrics.Meter; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.SharedMetricRegistries; import com.codahale.metrics.Timer; @@ -17,6 +18,7 @@ import io.lettuce.core.RedisException; import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; import io.micrometer.core.instrument.Counter; import io.micrometer.core.instrument.Metrics; +import io.micrometer.core.instrument.Tags; import java.io.IOException; import java.util.Arrays; import java.util.Map; @@ -34,6 +36,9 @@ import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.auth.AmbiguousIdentifier; +import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials; +import org.whispersystems.textsecuregcm.controllers.AccountController; +import org.whispersystems.textsecuregcm.entities.AccountAttributes; import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; @@ -52,11 +57,16 @@ public class AccountsManager { private static final Timer getByUuidTimer = metricRegistry.timer(name(AccountsManager.class, "getByUuid" )); private static final Timer deleteTimer = metricRegistry.timer(name(AccountsManager.class, "delete")); + // TODO Remove this meter when external dependencies have been resolved + // Note that this is deliberately namespaced to `AccountController` for metric continuity. + private static final Meter newUserMeter = metricRegistry.meter(name(AccountController.class, "brand_new_user")); + private static final Timer redisSetTimer = metricRegistry.timer(name(AccountsManager.class, "redisSet" )); private static final Timer redisNumberGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisNumberGet")); private static final Timer redisUuidGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisUuidGet" )); private static final Timer redisDeleteTimer = metricRegistry.timer(name(AccountsManager.class, "redisDelete" )); + private static final String CREATE_COUNTER_NAME = name(AccountsManager.class, "createCounter"); private static final String DELETE_COUNTER_NAME = name(AccountsManager.class, "deleteCounter"); private static final String DELETE_ERROR_COUNTER_NAME = name(AccountsManager.class, "deleteError"); private static final String COUNTRY_CODE_TAG_NAME = "country"; @@ -77,6 +87,7 @@ public class AccountsManager { private final MessagesManager messagesManager; private final UsernamesManager usernamesManager; private final ProfilesManager profilesManager; + private final StoredVerificationCodeManager pendingAccounts; private final SecureStorageClient secureStorageClient; private final SecureBackupClient secureBackupClient; private final ObjectMapper mapper; @@ -102,7 +113,9 @@ public class AccountsManager { final DeletedAccountsManager deletedAccountsManager, final DirectoryQueue directoryQueue, final KeysDynamoDb keysDynamoDb, final MessagesManager messagesManager, final UsernamesManager usernamesManager, - final ProfilesManager profilesManager, final SecureStorageClient secureStorageClient, + final ProfilesManager profilesManager, + final StoredVerificationCodeManager pendingAccounts, + final SecureStorageClient secureStorageClient, final SecureBackupClient secureBackupClient, final ExperimentEnrollmentManager experimentEnrollmentManager, final DynamicConfigurationManager dynamicConfigurationManager) { @@ -115,6 +128,7 @@ public class AccountsManager { this.messagesManager = messagesManager; this.usernamesManager = usernamesManager; this.profilesManager = profilesManager; + this.pendingAccounts = pendingAccounts; this.secureStorageClient = secureStorageClient; this.secureBackupClient = secureBackupClient; this.mapper = SystemMapper.getMapper(); @@ -126,8 +140,34 @@ public class AccountsManager { this.experimentEnrollmentManager = experimentEnrollmentManager; } - public boolean create(Account account) { + public Account create(final String number, + final String password, + final String signalAgent, + final AccountAttributes accountAttributes) { + try (Timer.Context ignored = createTimer.time()) { + Optional maybeExistingAccount = get(number); + + Device device = new Device(); + device.setId(Device.MASTER_ID); + device.setAuthenticationCredentials(new AuthenticationCredentials(password)); + device.setFetchesMessages(accountAttributes.getFetchesMessages()); + device.setRegistrationId(accountAttributes.getRegistrationId()); + device.setName(accountAttributes.getName()); + device.setCapabilities(accountAttributes.getCapabilities()); + device.setCreated(System.currentTimeMillis()); + device.setLastSeen(Util.todayInMillis()); + device.setUserAgent(signalAgent); + + Account account = new Account(); + account.setNumber(number); + account.setUuid(UUID.randomUUID()); + account.addDevice(device); + account.setRegistrationLockFromAttributes(accountAttributes); + account.setUnidentifiedAccessKey(accountAttributes.getUnidentifiedAccessKey()); + account.setUnrestrictedUnidentifiedAccess(accountAttributes.isUnrestrictedUnidentifiedAccess()); + account.setDiscoverableByPhoneNumber(accountAttributes.isDiscoverableByPhoneNumber()); + final UUID originalUuid = account.getUuid(); boolean freshUser = databaseCreate(account); @@ -165,12 +205,36 @@ public class AccountsManager { redisSet(account); - return freshUser; + final Tags tags; + + if (freshUser) { + tags = Tags.of("type", "new"); + } else { + tags = Tags.of("type", "reregister"); + } + + Metrics.counter(CREATE_COUNTER_NAME, tags).increment(); + + if (!account.isDiscoverableByPhoneNumber()) { + // The newly-created account has explicitly opted out of discoverability + directoryQueue.deleteAccount(account); + } + + maybeExistingAccount.ifPresent(definitelyExistingAccount -> { + messagesManager.clear(definitelyExistingAccount.getUuid()); + keysDynamoDb.delete(definitelyExistingAccount); + }); + + pendingAccounts.remove(number); + + return account; } } public Account update(Account account, Consumer updater) { + final boolean wasDiscoverableBeforeUpdate = directoryQueue.isDiscoverable(account); + final Account updatedAccount; try (Timer.Context ignored = updateTimer.time()) { @@ -212,6 +276,12 @@ public class AccountsManager { redisSet(updatedAccount); } + final boolean isDiscoverableAfterUpdate = directoryQueue.isDiscoverable(updatedAccount); + + if (wasDiscoverableBeforeUpdate != isDiscoverableAfterUpdate) { + directoryQueue.refreshAccount(updatedAccount); + } + return updatedAccount; } 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 0565c5623..1ddea5c8b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PushFeedbackProcessor.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PushFeedbackProcessor.java @@ -27,11 +27,9 @@ public class PushFeedbackProcessor extends AccountDatabaseCrawlerListener { private final Meter recovered = metricRegistry.meter(name(getClass(), "unregistered", "recovered")); private final AccountsManager accountsManager; - private final DirectoryQueue directoryQueue; - public PushFeedbackProcessor(AccountsManager accountsManager, DirectoryQueue directoryQueue) { + public PushFeedbackProcessor(AccountsManager accountsManager) { this.accountsManager = accountsManager; - this.directoryQueue = directoryQueue; } @Override @@ -42,8 +40,6 @@ public class PushFeedbackProcessor extends AccountDatabaseCrawlerListener { @Override protected void onCrawlChunk(Optional fromUuid, List chunkAccounts) { - final List directoryUpdateAccounts = new ArrayList<>(); - for (Account account : chunkAccounts) { boolean update = false; @@ -62,7 +58,7 @@ public class PushFeedbackProcessor extends AccountDatabaseCrawlerListener { if (update) { // fetch a new version, since the chunk is shared and implicitly read-only accountsManager.get(account.getUuid()).ifPresent(accountToUpdate -> { - Account updatedAccount = accountsManager.update(accountToUpdate, a -> { + accountsManager.update(accountToUpdate, a -> { for (Device device : a.getDevices()) { if (deviceNeedsUpdate(device)) { if (deviceExpired(device)) { @@ -85,14 +81,9 @@ public class PushFeedbackProcessor extends AccountDatabaseCrawlerListener { } } }); - directoryUpdateAccounts.add(updatedAccount); }); } } - - if (!directoryUpdateAccounts.isEmpty()) { - directoryQueue.refreshRegisteredUsers(directoryUpdateAccounts); - } } private boolean deviceNeedsUpdate(final Device device) { 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 a2c21a4d5..7138dfb07 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java @@ -56,8 +56,10 @@ import org.whispersystems.textsecuregcm.storage.ProfilesManager; import org.whispersystems.textsecuregcm.storage.ReportMessageDynamoDb; import org.whispersystems.textsecuregcm.storage.ReportMessageManager; import org.whispersystems.textsecuregcm.storage.ReservedUsernames; +import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; import org.whispersystems.textsecuregcm.storage.Usernames; import org.whispersystems.textsecuregcm.storage.UsernamesManager; +import org.whispersystems.textsecuregcm.storage.VerificationCodeStore; import org.whispersystems.textsecuregcm.util.DynamoDbFromConfig; import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; @@ -137,6 +139,8 @@ public class DeleteUserCommand extends EnvironmentCommand account = accountsManager.get(user); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/sqs/DirectoryQueueTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/sqs/DirectoryQueueTest.java index 921751719..9cab34286 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/sqs/DirectoryQueueTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/sqs/DirectoryQueueTest.java @@ -40,7 +40,7 @@ public class DirectoryQueueTest { when(account.isEnabled()).thenReturn(accountEnabled); when(account.isDiscoverableByPhoneNumber()).thenReturn(accountDiscoverableByPhoneNumber); - directoryQueue.refreshRegisteredUser(account); + directoryQueue.refreshAccount(account); final ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(SendMessageBatchRequest.class); verify(sqs).sendMessageBatch(requestCaptor.capture()); @@ -68,7 +68,7 @@ public class DirectoryQueueTest { when(undiscoverableAccount.isEnabled()).thenReturn(true); when(undiscoverableAccount.isDiscoverableByPhoneNumber()).thenReturn(false); - directoryQueue.refreshRegisteredUsers(List.of(discoverableAccount, undiscoverableAccount)); + directoryQueue.refreshAccounts(List.of(discoverableAccount, undiscoverableAccount)); final ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(SendMessageBatchRequest.class); verify(sqs).sendMessageBatch(requestCaptor.capture()); @@ -97,7 +97,7 @@ public class DirectoryQueueTest { when(account.isEnabled()).thenReturn(true); when(account.isDiscoverableByPhoneNumber()).thenReturn(true); - directoryQueue.refreshRegisteredUser(account); + directoryQueue.refreshAccount(account); final ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(SendMessageBatchRequest.class); verify(sqs, times(2)).sendMessageBatch(requestCaptor.capture()); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java index 2ff5ef98e..0e86f0471 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java @@ -44,6 +44,7 @@ import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials; import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicAccountsDynamoDbMigrationConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; +import org.whispersystems.textsecuregcm.entities.AccountAttributes; import org.whispersystems.textsecuregcm.entities.SignedPreKey; import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; @@ -154,6 +155,7 @@ class AccountsManagerConcurrentModificationIntegrationTest { mock(MessagesManager.class), mock(UsernamesManager.class), mock(ProfilesManager.class), + mock(StoredVerificationCodeManager.class), mock(SecureStorageClient.class), mock(SecureBackupClient.class), experimentEnrollmentManager, @@ -164,9 +166,30 @@ class AccountsManagerConcurrentModificationIntegrationTest { @Test void testConcurrentUpdate() throws IOException { - final UUID uuid = UUID.randomUUID(); + final UUID uuid; + { + final Account account = accountsManager.update( + accountsManager.create("+14155551212", "password", null, new AccountAttributes()), + a -> { + a.setUnidentifiedAccessKey(new byte[16]); - accountsManager.create(generateAccount("+14155551212", uuid)); + final Random random = new Random(); + final SignedPreKey signedPreKey = new SignedPreKey(random.nextInt(), "testPublicKey-" + random.nextInt(), + "testSignature-" + random.nextInt()); + + a.removeDevice(1); + a.addDevice(new Device(1, "testName-" + random.nextInt(), "testAuthToken-" + random.nextInt(), + "testSalt-" + random.nextInt(), + "testGcmId-" + random.nextInt(), "testApnId-" + random.nextInt(), "testVoipApnId-" + random.nextInt(), + random.nextBoolean(), random.nextInt(), signedPreKey, random.nextInt(), random.nextInt(), + "testUserAgent-" + random.nextInt(), 0, + new Device.DeviceCapabilities(random.nextBoolean(), random.nextBoolean(), random.nextBoolean(), + random.nextBoolean(), random.nextBoolean(), random.nextBoolean(), + random.nextBoolean(), random.nextBoolean()))); + }); + + uuid = account.getUuid(); + } final String profileName = "name"; final String avatar = "avatar"; @@ -249,26 +272,4 @@ class AccountsManagerConcurrentModificationIntegrationTest { accountsManager.updateDevice(account, deviceId, deviceMutation); }, mutationExecutor); } - - private Account generateAccount(String number, UUID uuid) { - Device device = generateDevice(1); - return generateAccount(number, uuid, Collections.singleton(device)); - } - - private Account generateAccount(String number, UUID uuid, Set devices) { - byte[] unidentifiedAccessKey = new byte[16]; - Random random = new Random(System.currentTimeMillis()); - Arrays.fill(unidentifiedAccessKey, (byte)random.nextInt(255)); - - return new Account(number, uuid, devices, unidentifiedAccessKey); - } - - private Device generateDevice(long id) { - Random random = new Random(System.currentTimeMillis()); - SignedPreKey signedPreKey = new SignedPreKey(random.nextInt(), "testPublicKey-" + random.nextInt(), "testSignature-" + random.nextInt()); - return new Device(id, "testName-" + random.nextInt(), "testAuthToken-" + random.nextInt(), "testSalt-" + random.nextInt(), - "testGcmId-" + random.nextInt(), "testApnId-" + random.nextInt(), "testVoipApnId-" + random.nextInt(), random.nextBoolean(), random.nextInt(), signedPreKey, random.nextInt(), random.nextInt(), "testUserAgent-" + random.nextInt() , 0, new Device.DeviceCapabilities(random.nextBoolean(), random.nextBoolean(), random.nextBoolean(), random.nextBoolean(), random.nextBoolean(), random.nextBoolean(), - random.nextBoolean(), random.nextBoolean())); - } - } 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 f95314cac..f2dcba8b0 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 @@ -10,8 +10,17 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.Mockito.*; -import static org.whispersystems.textsecuregcm.tests.util.AccountsHelper.eqUuid; +import static org.mockito.Mockito.anyLong; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableSet; import io.dropwizard.auth.PolymorphicAuthValueFactoryProvider; @@ -42,7 +51,7 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; -import org.mockito.ArgumentMatcher; +import org.mockito.stubbing.Answer; import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials; import org.whispersystems.textsecuregcm.auth.DisabledPermittedAccount; import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentialGenerator; @@ -70,14 +79,11 @@ import org.whispersystems.textsecuregcm.push.GcmMessage; import org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient; import org.whispersystems.textsecuregcm.sms.SmsSender; import org.whispersystems.textsecuregcm.sms.TwilioVerifyExperimentEnrollmentManager; -import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.AbusiveHostRule; import org.whispersystems.textsecuregcm.storage.AbusiveHostRules; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; -import org.whispersystems.textsecuregcm.storage.KeysDynamoDb; -import org.whispersystems.textsecuregcm.storage.MessagesManager; import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; import org.whispersystems.textsecuregcm.storage.UsernamesManager; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; @@ -121,9 +127,6 @@ class AccountControllerTest { private static RateLimiter autoBlockLimiter = mock(RateLimiter.class); private static RateLimiter usernameSetLimiter = mock(RateLimiter.class); private static SmsSender smsSender = mock(SmsSender.class); - private static DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - private static MessagesManager storedMessages = mock(MessagesManager.class); - private static KeysDynamoDb keys = mock(KeysDynamoDb.class); private static TurnTokenGenerator turnTokenGenerator = mock(TurnTokenGenerator.class); private static Account senderPinAccount = mock(Account.class); private static Account senderRegLockAccount = mock(Account.class); @@ -155,9 +158,6 @@ class AccountControllerTest { abusiveHostRules, rateLimiters, smsSender, - directoryQueue, - storedMessages, - keys, dynamicConfigurationManager, turnTokenGenerator, new HashMap<>(), @@ -218,6 +218,14 @@ class AccountControllerTest { when(accountsManager.get(eq(SENDER_HAS_STORAGE))).thenReturn(Optional.of(senderHasStorage)); when(accountsManager.get(eq(SENDER_TRANSFER))).thenReturn(Optional.of(senderTransfer)); + when(accountsManager.create(any(), any(), any(), any())).thenAnswer((Answer) invocation -> { + final Account account = mock(Account.class); + when(account.getUuid()).thenReturn(UUID.randomUUID()); + when(account.getNumber()).thenReturn(invocation.getArgument(0, String.class)); + + return account; + }); + when(usernamesManager.put(eq(AuthHelper.VALID_UUID), eq("n00bkiller"))).thenReturn(true); when(usernamesManager.put(eq(AuthHelper.VALID_UUID), eq("takenusername"))).thenReturn(false); @@ -261,9 +269,6 @@ class AccountControllerTest { autoBlockLimiter, usernameSetLimiter, smsSender, - directoryQueue, - storedMessages, - keys, turnTokenGenerator, senderPinAccount, senderRegLockAccount, @@ -922,23 +927,13 @@ class AccountControllerTest { Optional.of(new StoredVerificationCode("1234", System.currentTimeMillis(), "1234-push", "VerificationSid")));; } - AccountCreationResult result = - resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "1234")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER, "bar")) - .put(Entity.entity(new AccountAttributes(false, 2222, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); + resources.getJerseyTest() + .target(String.format("/v1/accounts/code/%s", "1234")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER, "bar")) + .put(Entity.entity(new AccountAttributes(), MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); - assertThat(result.getUuid()).isNotNull(); - assertThat(result.isStorageCapable()).isFalse(); - - final ArgumentCaptor accountArgumentCaptor = ArgumentCaptor.forClass(Account.class); - - verify(accountsManager, times(1)).create(accountArgumentCaptor.capture()); - verify(directoryQueue, times(1)).refreshRegisteredUser(argThat(account -> SENDER.equals(account.getNumber()))); - - assertThat(accountArgumentCaptor.getValue().isDiscoverableByPhoneNumber()).isTrue(); + verify(accountsManager).create(eq(SENDER), eq("bar"), any(), any()); if (enrolledInVerifyExperiment) { verify(smsSender).reportVerificationSucceeded("VerificationSid"); @@ -946,52 +941,14 @@ class AccountControllerTest { } @Test - void testVerifyCodeUndiscoverable() throws Exception { - AccountCreationResult result = - resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "1234")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER, "bar")) - .put(Entity.entity(new AccountAttributes(false, 2222, null, null, null, false, null), - MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); - - assertThat(result.getUuid()).isNotNull(); - assertThat(result.isStorageCapable()).isFalse(); - - final ArgumentCaptor accountArgumentCaptor = ArgumentCaptor.forClass(Account.class); - - verify(accountsManager, times(1)).create(accountArgumentCaptor.capture()); - verify(directoryQueue, times(1)).refreshRegisteredUser(argThat(account -> SENDER.equals(account.getNumber()))); - - assertThat(accountArgumentCaptor.getValue().isDiscoverableByPhoneNumber()).isFalse(); - } - - @Test - void testVerifySupportsStorage() throws Exception { - AccountCreationResult result = - resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "666666")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_HAS_STORAGE, "bar")) - .put(Entity.entity(new AccountAttributes(false, 2222, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); - - assertThat(result.getUuid()).isNotNull(); - assertThat(result.isStorageCapable()).isTrue(); - - verify(accountsManager, times(1)).create(isA(Account.class)); - verify(directoryQueue, times(1)).refreshRegisteredUser(argThat(account -> SENDER_HAS_STORAGE.equals(account.getNumber()))); - } - - @Test - void testVerifyCodeOld() throws Exception { + void testVerifyCodeOld() { Response response = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "1234")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_OLD, "bar")) - .put(Entity.entity(new AccountAttributes(false, 2222, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE)); + .target(String.format("/v1/accounts/code/%s", "1234")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_OLD, "bar")) + .put(Entity.entity(new AccountAttributes(false, 2222, null, null, null, true, null), + MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatus()).isEqualTo(403); @@ -999,14 +956,14 @@ class AccountControllerTest { } @Test - void testVerifyBadCode() throws Exception { + void testVerifyBadCode() { Response response = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "1111")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER, "bar")) - .put(Entity.entity(new AccountAttributes(false, 3333, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE)); + .target(String.format("/v1/accounts/code/%s", "1111")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER, "bar")) + .put(Entity.entity(new AccountAttributes(false, 3333, null, null, null, true, null), + MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatus()).isEqualTo(403); @@ -1017,11 +974,11 @@ class AccountControllerTest { void testVerifyPin() throws Exception { AccountCreationResult result = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "333333")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_PIN, "bar")) - .put(Entity.entity(new AccountAttributes(false, 3333, null, "31337", null, true, null), - MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); + .target(String.format("/v1/accounts/code/%s", "333333")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_PIN, "bar")) + .put(Entity.entity(new AccountAttributes(false, 3333, null, "31337", null, true, null), + MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); assertThat(result.getUuid()).isNotNull(); @@ -1032,11 +989,11 @@ class AccountControllerTest { void testVerifyRegistrationLock() throws Exception { AccountCreationResult result = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "666666")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) - .put(Entity.entity(new AccountAttributes(false, 3333, null, null, Hex.toStringCondensed(registration_lock_key), true, null), - MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); + .target(String.format("/v1/accounts/code/%s", "666666")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) + .put(Entity.entity(new AccountAttributes(false, 3333, null, null, Hex.toStringCondensed(registration_lock_key), true, null), + MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); assertThat(result.getUuid()).isNotNull(); @@ -1045,36 +1002,24 @@ class AccountControllerTest { @Test void testVerifyRegistrationLockSetsRegistrationLockOnNewAccount() throws Exception { - AccountCreationResult result = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "666666")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) - .put(Entity.entity(new AccountAttributes(false, 3333, null, null, Hex.toStringCondensed(registration_lock_key), true, null), - MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); + .target(String.format("/v1/accounts/code/%s", "666666")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) + .put(Entity.entity(new AccountAttributes(false, 3333, null, null, Hex.toStringCondensed(registration_lock_key), true, null), + MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); assertThat(result.getUuid()).isNotNull(); verify(pinLimiter).validate(eq(SENDER_REG_LOCK)); - verify(accountsManager).create(argThat(new ArgumentMatcher<>() { - @Override - public boolean matches(final Account account) { - final StoredRegistrationLock regLock = account.getRegistrationLock(); - return regLock.requiresClientRegistrationLock() && regLock.verify(Hex.toStringCondensed(registration_lock_key), null); - } - - @Override - public String toString() { - return "Account that has registration lock set"; - } - })); - + verify(accountsManager).create(eq(SENDER_REG_LOCK), eq("bar"), any(), argThat( + attributes -> Hex.toStringCondensed(registration_lock_key).equals(attributes.getRegistrationLock()))); } @Test - void testVerifyRegistrationLockOld() throws Exception { + void testVerifyRegistrationLockOld() { StoredRegistrationLock lock = senderRegLockAccount.getRegistrationLock(); try { @@ -1082,11 +1027,11 @@ class AccountControllerTest { AccountCreationResult result = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "666666")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) - .put(Entity.entity(new AccountAttributes(false, 3333, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); + .target(String.format("/v1/accounts/code/%s", "666666")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) + .put(Entity.entity(new AccountAttributes(false, 3333, null, null, null, true, null), + MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); assertThat(result.getUuid()).isNotNull(); @@ -1100,11 +1045,11 @@ class AccountControllerTest { void testVerifyWrongPin() throws Exception { Response response = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "333333")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_PIN, "bar")) - .put(Entity.entity(new AccountAttributes(false, 3333, null, "31338", null, true, null), - MediaType.APPLICATION_JSON_TYPE)); + .target(String.format("/v1/accounts/code/%s", "333333")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_PIN, "bar")) + .put(Entity.entity(new AccountAttributes(false, 3333, null, "31338", null, true, null), + MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatus()).isEqualTo(423); @@ -1115,12 +1060,12 @@ class AccountControllerTest { void testVerifyWrongRegistrationLock() throws Exception { Response response = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "666666")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) - .put(Entity.entity(new AccountAttributes(false, 3333, null, - Hex.toStringCondensed(new byte[32]), null, true, null), - MediaType.APPLICATION_JSON_TYPE)); + .target(String.format("/v1/accounts/code/%s", "666666")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) + .put(Entity.entity(new AccountAttributes(false, 3333, null, + Hex.toStringCondensed(new byte[32]), null, true, null), + MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatus()).isEqualTo(423); @@ -1131,11 +1076,11 @@ class AccountControllerTest { void testVerifyNoPin() throws Exception { Response response = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "333333")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_PIN, "bar")) - .put(Entity.entity(new AccountAttributes(false, 3333, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE)); + .target(String.format("/v1/accounts/code/%s", "333333")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_PIN, "bar")) + .put(Entity.entity(new AccountAttributes(false, 3333, null, null, null, true, null), + MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatus()).isEqualTo(423); @@ -1149,11 +1094,11 @@ class AccountControllerTest { void testVerifyNoRegistrationLock() throws Exception { Response response = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "666666")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) - .put(Entity.entity(new AccountAttributes(false, 3333, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE)); + .target(String.format("/v1/accounts/code/%s", "666666")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) + .put(Entity.entity(new AccountAttributes(false, 3333, null, null, null, true, null), + MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatus()).isEqualTo(423); @@ -1172,11 +1117,11 @@ class AccountControllerTest { void testVerifyLimitPin() throws Exception { Response response = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "444444")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_OVER_PIN, "bar")) - .put(Entity.entity(new AccountAttributes(false, 3333, null, "31337", null, true, null), - MediaType.APPLICATION_JSON_TYPE)); + .target(String.format("/v1/accounts/code/%s", "444444")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_OVER_PIN, "bar")) + .put(Entity.entity(new AccountAttributes(false, 3333, null, "31337", null, true, null), + MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatus()).isEqualTo(413); @@ -1190,11 +1135,11 @@ class AccountControllerTest { AccountCreationResult result = resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "444444")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_OVER_PIN, "bar")) - .put(Entity.entity(new AccountAttributes(false, 3333, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); + .target(String.format("/v1/accounts/code/%s", "444444")) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_OVER_PIN, "bar")) + .put(Entity.entity(new AccountAttributes(false, 3333, null, null, null, true, null), + MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); assertThat(result.getUuid()).isNotNull(); @@ -1208,13 +1153,13 @@ class AccountControllerTest { when(senderTransfer.isTransferSupported()).thenReturn(true); final Response response = - resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "1234")) - .queryParam("transfer", true) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_TRANSFER, "bar")) - .put(Entity.entity(new AccountAttributes(false, 2222, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE)); + resources.getJerseyTest() + .target(String.format("/v1/accounts/code/%s", "1234")) + .queryParam("transfer", true) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_TRANSFER, "bar")) + .put(Entity.entity(new AccountAttributes(false, 2222, null, null, null, true, null), + MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatus()).isEqualTo(409); } @@ -1224,13 +1169,13 @@ class AccountControllerTest { when(senderTransfer.isTransferSupported()).thenReturn(false); final Response response = - resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "1234")) - .queryParam("transfer", true) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_TRANSFER, "bar")) - .put(Entity.entity(new AccountAttributes(false, 2222, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE)); + resources.getJerseyTest() + .target(String.format("/v1/accounts/code/%s", "1234")) + .queryParam("transfer", true) + .request() + .header("Authorization", AuthHelper.getAuthHeader(SENDER_TRANSFER, "bar")) + .put(Entity.entity(new AccountAttributes(false, 2222, null, null, null, true, null), + MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatus()).isEqualTo(200); } @@ -1240,41 +1185,14 @@ class AccountControllerTest { when(senderTransfer.isTransferSupported()).thenReturn(true); final Response response = - resources.getJerseyTest() - .target(String.format("/v1/accounts/code/%s", "1234")) - .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER_TRANSFER, "bar")) - .put(Entity.entity(new AccountAttributes(false, 2222, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE)); - - assertThat(response.getStatus()).isEqualTo(200); - } - - @Test - void testVerifyReregistration() throws InterruptedException { - final UUID existingUuid = UUID.randomUUID(); - final Account existingAccount = mock(Account.class); - - when(existingAccount.getUuid()).thenReturn(existingUuid); - when(accountsManager.get(SENDER)).thenReturn(Optional.of(existingAccount)); - - AccountCreationResult result = resources.getJerseyTest() .target(String.format("/v1/accounts/code/%s", "1234")) .request() - .header("Authorization", AuthHelper.getAuthHeader(SENDER, "bar")) + .header("Authorization", AuthHelper.getAuthHeader(SENDER_TRANSFER, "bar")) .put(Entity.entity(new AccountAttributes(false, 2222, null, null, null, true, null), - MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); + MediaType.APPLICATION_JSON_TYPE)); - final ArgumentCaptor accountArgumentCaptor = ArgumentCaptor.forClass(Account.class); - - verify(accountsManager, times(1)).create(accountArgumentCaptor.capture()); - verify(directoryQueue, times(1)).refreshRegisteredUser(argThat(account -> SENDER.equals(account.getNumber()))); - - verify(storedMessages).clear(existingUuid); - verify(keys).delete(existingAccount); - - assertThat(accountArgumentCaptor.getValue().isDiscoverableByPhoneNumber()).isTrue(); + assertThat(response.getStatus()).isEqualTo(200); } @Test @@ -1388,7 +1306,6 @@ class AccountControllerTest { verify(AuthHelper.DISABLED_DEVICE, times(1)).setGcmId(eq("c00lz0rz")); verify(accountsManager, times(1)).updateDevice(eq(AuthHelper.DISABLED_ACCOUNT), anyLong(), any()); - verify(directoryQueue, never()).refreshRegisteredUser(any(Account.class)); } @Test @@ -1404,7 +1321,6 @@ class AccountControllerTest { verify(AuthHelper.DISABLED_DEVICE, times(1)).setGcmId(eq("z000")); verify(accountsManager, times(1)).updateDevice(eq(AuthHelper.DISABLED_ACCOUNT), anyLong(), any()); - verify(directoryQueue, never()).refreshRegisteredUser(any(Account.class)); } @Test @@ -1421,7 +1337,6 @@ class AccountControllerTest { verify(AuthHelper.DISABLED_DEVICE, times(1)).setApnId(eq("first")); verify(AuthHelper.DISABLED_DEVICE, times(1)).setVoipApnId(eq("second")); verify(accountsManager, times(1)).updateDevice(eq(AuthHelper.DISABLED_ACCOUNT), anyLong(), any()); - verify(directoryQueue, never()).refreshRegisteredUser(any(Account.class)); } @Test @@ -1438,7 +1353,6 @@ class AccountControllerTest { verify(AuthHelper.DISABLED_DEVICE, times(1)).setApnId(eq("first")); verify(AuthHelper.DISABLED_DEVICE, times(1)).setVoipApnId(null); verify(accountsManager, times(1)).updateDevice(eq(AuthHelper.DISABLED_ACCOUNT), anyLong(), any()); - verify(directoryQueue, never()).refreshRegisteredUser(any(Account.class)); } @Test @@ -1455,7 +1369,6 @@ class AccountControllerTest { verify(AuthHelper.DISABLED_DEVICE, times(1)).setApnId(eq("third")); verify(AuthHelper.DISABLED_DEVICE, times(1)).setVoipApnId(eq("fourth")); verify(accountsManager, times(1)).updateDevice(eq(AuthHelper.DISABLED_ACCOUNT), anyLong(), any()); - verify(directoryQueue, never()).refreshRegisteredUser(any(Account.class)); } @ParameterizedTest @@ -1566,7 +1479,6 @@ class AccountControllerTest { .put(Entity.json(new AccountAttributes(false, 2222, null, null, null, true, null))); assertThat(response.getStatus()).isEqualTo(204); - verify(directoryQueue, never()).refreshRegisteredUser(any()); } @Test @@ -1579,7 +1491,6 @@ class AccountControllerTest { .put(Entity.json(new AccountAttributes(false, 2222, null, null, null, true, null))); assertThat(response.getStatus()).isEqualTo(204); - verify(directoryQueue, times(1)).refreshRegisteredUser(eqUuid(AuthHelper.UNDISCOVERABLE_ACCOUNT)); } @Test @@ -1592,7 +1503,6 @@ class AccountControllerTest { .put(Entity.json(new AccountAttributes(false, 2222, null, null, null, false, null))); assertThat(response.getStatus()).isEqualTo(204); - verify(directoryQueue, times(1)).refreshRegisteredUser(eqUuid(AuthHelper.VALID_ACCOUNT)); } @Test diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java index 533fe128a..e8bc69cc7 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java @@ -45,7 +45,6 @@ import org.whispersystems.textsecuregcm.entities.DeviceResponse; import org.whispersystems.textsecuregcm.limits.RateLimiter; import org.whispersystems.textsecuregcm.limits.RateLimiters; import org.whispersystems.textsecuregcm.mappers.DeviceLimitExceededExceptionMapper; -import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; @@ -65,11 +64,10 @@ class DeviceControllerTest { AccountsManager accounts, MessagesManager messages, KeysDynamoDb keys, - DirectoryQueue cdsSender, RateLimiters rateLimiters, Map deviceConfiguration) { - super(pendingDevices, accounts, messages, keys, cdsSender, rateLimiters, deviceConfiguration); + super(pendingDevices, accounts, messages, keys, rateLimiters, deviceConfiguration); } @Override @@ -82,7 +80,6 @@ class DeviceControllerTest { private static AccountsManager accountsManager = mock(AccountsManager.class ); private static MessagesManager messagesManager = mock(MessagesManager.class); private static KeysDynamoDb keys = mock(KeysDynamoDb.class); - private static DirectoryQueue directoryQueue = mock(DirectoryQueue.class); private static RateLimiters rateLimiters = mock(RateLimiters.class ); private static RateLimiter rateLimiter = mock(RateLimiter.class ); private static Account account = mock(Account.class ); @@ -100,7 +97,6 @@ class DeviceControllerTest { accountsManager, messagesManager, keys, - directoryQueue, rateLimiters, deviceConfiguration)) .build(); @@ -142,7 +138,6 @@ class DeviceControllerTest { accountsManager, messagesManager, keys, - directoryQueue, rateLimiters, rateLimiter, account, diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/KeysControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/KeysControllerTest.java index 09c540b42..1eca5981c 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/KeysControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/KeysControllerTest.java @@ -57,7 +57,6 @@ import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager; import org.whispersystems.textsecuregcm.limits.RateLimiter; import org.whispersystems.textsecuregcm.limits.RateLimiters; import org.whispersystems.textsecuregcm.mappers.RateLimitChallengeExceptionMapper; -import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; @@ -92,7 +91,6 @@ class KeysControllerTest { private final static KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class ); private final static AccountsManager accounts = mock(AccountsManager.class ); - private final static DirectoryQueue directoryQueue = mock(DirectoryQueue.class ); private final static PreKeyRateLimiter preKeyRateLimiter = mock(PreKeyRateLimiter.class ); private final static RateLimitChallengeManager rateLimitChallengeManager = mock(RateLimitChallengeManager.class ); private final static Account existsAccount = mock(Account.class ); @@ -107,7 +105,7 @@ class KeysControllerTest { .addProvider(new PolymorphicAuthValueFactoryProvider.Binder<>(ImmutableSet.of(Account.class, DisabledPermittedAccount.class))) .setTestContainerFactory(new GrizzlyWebTestContainerFactory()) .addResource(new RateLimitChallengeExceptionMapper(rateLimitChallengeManager)) - .addResource(new KeysController(rateLimiters, keysDynamoDb, accounts, directoryQueue, preKeyRateLimiter, dynamicConfigurationManager, rateLimitChallengeManager)) + .addResource(new KeysController(rateLimiters, keysDynamoDb, accounts, preKeyRateLimiter, dynamicConfigurationManager, rateLimitChallengeManager)) .build(); @BeforeEach @@ -184,7 +182,6 @@ class KeysControllerTest { reset( keysDynamoDb, accounts, - directoryQueue, preKeyRateLimiter, existsAccount, rateLimiters, diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java index 2be2d2295..e6218a182 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java @@ -19,8 +19,8 @@ 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.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import io.lettuce.core.RedisException; @@ -30,17 +30,20 @@ import java.util.HashSet; import java.util.Optional; import java.util.UUID; import java.util.function.Consumer; +import java.util.stream.Stream; 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.mockito.ArgumentCaptor; import org.mockito.stubbing.Answer; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicAccountsDynamoDbMigrationConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; +import org.whispersystems.textsecuregcm.entities.AccountAttributes; import org.whispersystems.textsecuregcm.entities.SignedPreKey; import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; -import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; @@ -49,21 +52,28 @@ import org.whispersystems.textsecuregcm.storage.Accounts; import org.whispersystems.textsecuregcm.storage.AccountsDynamoDb; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.ContestedOptimisticLockException; -import org.whispersystems.textsecuregcm.storage.DeletedAccounts; import org.whispersystems.textsecuregcm.storage.DeletedAccountsManager; import org.whispersystems.textsecuregcm.storage.Device; +import org.whispersystems.textsecuregcm.storage.Device.DeviceCapabilities; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.storage.KeysDynamoDb; import org.whispersystems.textsecuregcm.storage.MessagesManager; import org.whispersystems.textsecuregcm.storage.ProfilesManager; +import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; import org.whispersystems.textsecuregcm.storage.UsernamesManager; import org.whispersystems.textsecuregcm.tests.util.JsonHelpers; import org.whispersystems.textsecuregcm.tests.util.RedisClusterHelper; class AccountsManagerTest { - private DynamicConfigurationManager dynamicConfigurationManager = mock(DynamicConfigurationManager.class); - private ExperimentEnrollmentManager experimentEnrollmentManager = mock(ExperimentEnrollmentManager.class); + private Accounts accounts; + private AccountsDynamoDb accountsDynamoDb; + private DirectoryQueue directoryQueue; + private DynamicConfigurationManager dynamicConfigurationManager; + private ExperimentEnrollmentManager experimentEnrollmentManager; + + private RedisAdvancedClusterCommands commands; + private AccountsManager accountsManager; private static final Answer ACCOUNT_UPDATE_ANSWER = (answer) -> { // it is implicit in the update() contract is that a successful call will @@ -75,49 +85,57 @@ class AccountsManagerTest { @BeforeEach void setup() { + accounts = mock(Accounts.class); + accountsDynamoDb = mock(AccountsDynamoDb.class); + directoryQueue = mock(DirectoryQueue.class); + dynamicConfigurationManager = mock(DynamicConfigurationManager.class); + experimentEnrollmentManager = mock(ExperimentEnrollmentManager.class); - DynamicConfiguration dynamicConfiguration = new DynamicConfiguration(); + //noinspection unchecked + commands = mock(RedisAdvancedClusterCommands.class); + final DynamicConfiguration dynamicConfiguration = new DynamicConfiguration(); when(dynamicConfigurationManager.getConfiguration()).thenReturn(dynamicConfiguration); + + accountsManager = new AccountsManager( + accounts, + accountsDynamoDb, + RedisClusterHelper.buildMockRedisCluster(commands), + mock(DeletedAccountsManager.class), + directoryQueue, + mock(KeysDynamoDb.class), + mock(MessagesManager.class), + mock(UsernamesManager.class), + mock(ProfilesManager.class), + mock(StoredVerificationCodeManager.class), + mock(SecureStorageClient.class), + mock(SecureBackupClient.class), + experimentEnrollmentManager, + dynamicConfigurationManager); } @ParameterizedTest @ValueSource(booleans = {true, false}) void testGetAccountByNumberInCache(final boolean dynamoEnabled) { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccounts = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); - UUID uuid = UUID.randomUUID(); enableDynamo(dynamoEnabled); when(commands.get(eq("AccountMap::+14152222222"))).thenReturn(uuid.toString()); - when(commands.get(eq("Account3::" + uuid.toString()))).thenReturn("{\"number\": \"+14152222222\", \"name\": \"test\"}"); + when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"name\": \"test\"}"); - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccounts, - directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); - Optional account = accountsManager.get("+14152222222"); + Optional account = accountsManager.get("+14152222222"); assertTrue(account.isPresent()); assertEquals(account.get().getNumber(), "+14152222222"); assertEquals(account.get().getProfileName(), "test"); verify(commands, times(1)).get(eq("AccountMap::+14152222222")); - verify(commands, times(1)).get(eq("Account3::" + uuid.toString())); + verify(commands, times(1)).get(eq("Account3::" + uuid)); verifyNoMoreInteractions(commands); verifyNoMoreInteractions(accounts); - verifyZeroInteractions(accountsDynamoDb); + verifyNoInteractions(accountsDynamoDb); } private void enableDynamo(boolean dynamoEnabled) { @@ -136,75 +154,46 @@ class AccountsManagerTest { @ParameterizedTest @ValueSource(booleans = {true, false}) void testGetAccountByUuidInCache(boolean dynamoEnabled) { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccounts = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); - UUID uuid = UUID.randomUUID(); enableDynamo(dynamoEnabled); - when(commands.get(eq("Account3::" + uuid.toString()))).thenReturn("{\"number\": \"+14152222222\", \"name\": \"test\"}"); + when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"name\": \"test\"}"); - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccounts, - directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); - Optional account = accountsManager.get(uuid); + Optional account = accountsManager.get(uuid); assertTrue(account.isPresent()); assertEquals(account.get().getNumber(), "+14152222222"); assertEquals(account.get().getUuid(), uuid); assertEquals(account.get().getProfileName(), "test"); - verify(commands, times(1)).get(eq("Account3::" + uuid.toString())); + verify(commands, times(1)).get(eq("Account3::" + uuid)); verifyNoMoreInteractions(commands); verifyNoMoreInteractions(accounts); - verifyZeroInteractions(accountsDynamoDb); + verifyNoInteractions(accountsDynamoDb); } @ParameterizedTest @ValueSource(booleans = {true, false}) void testGetAccountByNumberNotInCache(boolean dynamoEnabled) { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccounts = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); - UUID uuid = UUID.randomUUID(); - Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); + UUID uuid = UUID.randomUUID(); + Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); enableDynamo(dynamoEnabled); when(commands.get(eq("AccountMap::+14152222222"))).thenReturn(null); when(accounts.get(eq("+14152222222"))).thenReturn(Optional.of(account)); - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccounts, - directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); - Optional retrieved = accountsManager.get("+14152222222"); + Optional retrieved = accountsManager.get("+14152222222"); assertTrue(retrieved.isPresent()); assertSame(retrieved.get(), account); verify(commands, times(1)).get(eq("AccountMap::+14152222222")); verify(commands, times(1)).set(eq("AccountMap::+14152222222"), eq(uuid.toString())); - verify(commands, times(1)).set(eq("Account3::" + uuid.toString()), anyString()); + verify(commands, times(1)).set(eq("Account3::" + uuid), anyString()); verifyNoMoreInteractions(commands); verify(accounts, times(1)).get(eq("+14152222222")); @@ -218,36 +207,22 @@ class AccountsManagerTest { @ParameterizedTest @ValueSource(booleans = {true, false}) void testGetAccountByUuidNotInCache(boolean dynamoEnabled) { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccounts = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); - UUID uuid = UUID.randomUUID(); - Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); + UUID uuid = UUID.randomUUID(); + Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); enableDynamo(dynamoEnabled); when(commands.get(eq("Account3::" + uuid))).thenReturn(null); when(accounts.get(eq(uuid))).thenReturn(Optional.of(account)); - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccounts, - directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); - Optional retrieved = accountsManager.get(uuid); + Optional retrieved = accountsManager.get(uuid); assertTrue(retrieved.isPresent()); assertSame(retrieved.get(), account); verify(commands, times(1)).get(eq("Account3::" + uuid)); verify(commands, times(1)).set(eq("AccountMap::+14152222222"), eq(uuid.toString())); - verify(commands, times(1)).set(eq("Account3::" + uuid.toString()), anyString()); + verify(commands, times(1)).set(eq("Account3::" + uuid), anyString()); verifyNoMoreInteractions(commands); verify(accounts, times(1)).get(eq(uuid)); @@ -260,18 +235,6 @@ class AccountsManagerTest { @ParameterizedTest @ValueSource(booleans = {true, false}) void testGetAccountByNumberBrokenCache(boolean dynamoEnabled) { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccounts = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); UUID uuid = UUID.randomUUID(); Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); @@ -280,8 +243,6 @@ class AccountsManagerTest { when(commands.get(eq("AccountMap::+14152222222"))).thenThrow(new RedisException("Connection lost!")); when(accounts.get(eq("+14152222222"))).thenReturn(Optional.of(account)); - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccounts, - directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); Optional retrieved = accountsManager.get("+14152222222"); assertTrue(retrieved.isPresent()); @@ -289,7 +250,7 @@ class AccountsManagerTest { verify(commands, times(1)).get(eq("AccountMap::+14152222222")); verify(commands, times(1)).set(eq("AccountMap::+14152222222"), eq(uuid.toString())); - verify(commands, times(1)).set(eq("Account3::" + uuid.toString()), anyString()); + verify(commands, times(1)).set(eq("Account3::" + uuid), anyString()); verifyNoMoreInteractions(commands); verify(accounts, times(1)).get(eq("+14152222222")); @@ -302,18 +263,6 @@ class AccountsManagerTest { @ParameterizedTest @ValueSource(booleans = {true, false}) void testGetAccountByUuidBrokenCache(boolean dynamoEnabled) { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccounts = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); UUID uuid = UUID.randomUUID(); Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); @@ -322,8 +271,6 @@ class AccountsManagerTest { when(commands.get(eq("Account3::" + uuid))).thenThrow(new RedisException("Connection lost!")); when(accounts.get(eq(uuid))).thenReturn(Optional.of(account)); - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccounts, - directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); Optional retrieved = accountsManager.get(uuid); assertTrue(retrieved.isPresent()); @@ -331,7 +278,7 @@ class AccountsManagerTest { verify(commands, times(1)).get(eq("Account3::" + uuid)); verify(commands, times(1)).set(eq("AccountMap::+14152222222"), eq(uuid.toString())); - verify(commands, times(1)).set(eq("Account3::" + uuid.toString()), anyString()); + verify(commands, times(1)).set(eq("Account3::" + uuid), anyString()); verifyNoMoreInteractions(commands); verify(accounts, times(1)).get(eq(uuid)); @@ -344,18 +291,6 @@ class AccountsManagerTest { @ParameterizedTest @ValueSource(booleans = {true, false}) void testUpdate_dynamoDbMigration(boolean dynamoEnabled) throws IOException { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccountsManager = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); UUID uuid = UUID.randomUUID(); Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); @@ -367,9 +302,6 @@ class AccountsManagerTest { when(accountsDynamoDb.get(uuid)).thenReturn(Optional.of(new Account("+14152222222", uuid, new HashSet<>(), new byte[16]))); doAnswer(ACCOUNT_UPDATE_ANSWER).when(accounts).update(any(Account.class)); - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccountsManager, - directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); - Account updatedAccount = accountsManager.update(account, a -> a.setProfileName("name")); assertThrows(AssertionError.class, account::getProfileName, "Account passed to update() should be stale"); @@ -405,18 +337,6 @@ class AccountsManagerTest { @Test void testUpdate_dynamoMissing() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccounts = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); UUID uuid = UUID.randomUUID(); Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); @@ -427,9 +347,6 @@ class AccountsManagerTest { doAnswer(ACCOUNT_UPDATE_ANSWER).when(accounts).update(any()); doAnswer(ACCOUNT_UPDATE_ANSWER).when(accountsDynamoDb).update(any()); - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccounts, - directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); - Account updatedAccount = accountsManager.update(account, a -> {}); verify(accounts, times(1)).update(account); @@ -444,18 +361,6 @@ class AccountsManagerTest { @Test void testUpdate_optimisticLockingFailure() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccounts = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); UUID uuid = UUID.randomUUID(); Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); @@ -473,8 +378,6 @@ class AccountsManagerTest { .doAnswer(ACCOUNT_UPDATE_ANSWER) .when(accountsDynamoDb).update(any()); - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccounts, directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); - account = accountsManager.update(account, a -> a.setProfileName("name")); assertEquals(1, account.getVersion()); @@ -492,18 +395,6 @@ class AccountsManagerTest { @Test void testUpdate_dynamoOptimisticLockingFailureDuringCreate() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccountsManager = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); UUID uuid = UUID.randomUUID(); Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); @@ -514,8 +405,6 @@ class AccountsManagerTest { .thenReturn(Optional.of(account)); when(accountsDynamoDb.create(any())).thenThrow(ContestedOptimisticLockException.class); - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccountsManager, directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); - accountsManager.update(account, a -> {}); verify(accounts, times(1)).update(account); @@ -526,22 +415,7 @@ class AccountsManagerTest { } @Test - void testUpdateDevice() throws Exception { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccountsManager = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); - - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccountsManager, directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); - + void testUpdateDevice() { assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.empty(), Optional.empty())); final UUID uuid = UUID.randomUUID(); @@ -565,7 +439,7 @@ class AccountsManagerTest { account = accountsManager.updateDevice(account, deviceId, deviceUpdater); account = accountsManager.updateDevice(account, deviceId, d -> d.setName("deviceName")); - assertEquals("deviceName", account.getDevice(deviceId).get().getName()); + assertEquals("deviceName", account.getDevice(deviceId).orElseThrow().getName()); verify(deviceUpdater, times(1)).accept(any(Device.class)); @@ -574,25 +448,8 @@ class AccountsManagerTest { verify(unknownDeviceUpdater, never()).accept(any(Device.class)); } - @Test void testCompareAccounts() throws Exception { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - AccountsDynamoDb accountsDynamoDb = mock(AccountsDynamoDb.class); - DeletedAccountsManager deletedAccounts = mock(DeletedAccountsManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - KeysDynamoDb keysDynamoDb = mock(KeysDynamoDb.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - SecureBackupClient secureBackupClient = mock(SecureBackupClient.class); - SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); - - AccountsManager accountsManager = new AccountsManager(accounts, accountsDynamoDb, cacheCluster, deletedAccounts, - directoryQueue, keysDynamoDb, messagesManager, usernamesManager, profilesManager, secureStorageClient, secureBackupClient, experimentEnrollmentManager, dynamicConfigurationManager); - assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.empty(), Optional.empty())); final UUID uuidA = UUID.randomUUID(); @@ -663,4 +520,50 @@ class AccountsManagerTest { assertEquals(Optional.of("profileName"), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2))); } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testCreateWithDiscoverability(final boolean discoverable) { + final AccountAttributes attributes = new AccountAttributes(false, 0, null, null, null, discoverable, null); + final Account account = accountsManager.create("+18005550123", "password", null, attributes); + + assertEquals(discoverable, account.isDiscoverableByPhoneNumber()); + + if (!discoverable) { + verify(directoryQueue).deleteAccount(account); + } + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testCreateWithStorageCapability(final boolean hasStorage) { + final AccountAttributes attributes = new AccountAttributes(false, 0, null, null, null, true, + new DeviceCapabilities(false, false, false, hasStorage, false, false, false, false)); + + final Account account = accountsManager.create("+18005550123", "password", null, attributes); + + assertEquals(hasStorage, account.isStorageSupported()); + } + + @ParameterizedTest + @MethodSource + void testUpdateDirectoryQueue(final boolean discoverableBeforeUpdate, final boolean discoverableAfterUpdate, final boolean expectRefresh) { + final Account account = new Account("+14152222222", UUID.randomUUID(), new HashSet<>(), new byte[16]); + + when(directoryQueue.isDiscoverable(any())) + .thenReturn(discoverableBeforeUpdate) + .thenReturn(discoverableAfterUpdate); + + final Account updatedAccount = accountsManager.update(account, a -> a.setProfileName("Hello I am a unit test")); + + verify(directoryQueue, times(expectRefresh ? 1 : 0)).refreshAccount(updatedAccount); + } + + private static Stream testUpdateDirectoryQueue() { + return Stream.of( + Arguments.of(false, false, false), + Arguments.of(true, true, false), + Arguments.of(false, true, true), + Arguments.of(true, false, true)); + } } 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 bd4a8b65b..8cd29897f 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 @@ -5,7 +5,6 @@ package org.whispersystems.textsecuregcm.tests.storage; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.eq; @@ -13,7 +12,7 @@ import static org.mockito.Mockito.isNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.whispersystems.textsecuregcm.tests.util.AccountsHelper.eqUuid; @@ -23,11 +22,8 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; -import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountDatabaseCrawlerRestartException; import org.whispersystems.textsecuregcm.storage.AccountsManager; @@ -39,7 +35,6 @@ import org.whispersystems.textsecuregcm.util.Util; 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); @@ -105,16 +100,15 @@ class PushFeedbackProcessorTest { @Test void testEmpty() throws AccountDatabaseCrawlerRestartException { - PushFeedbackProcessor processor = new PushFeedbackProcessor(accountsManager, directoryQueue); + PushFeedbackProcessor processor = new PushFeedbackProcessor(accountsManager); processor.timeAndProcessCrawlChunk(Optional.of(UUID.randomUUID()), Collections.emptyList()); - verifyZeroInteractions(accountsManager); - verifyZeroInteractions(directoryQueue); + verifyNoInteractions(accountsManager); } @Test void testUpdate() throws AccountDatabaseCrawlerRestartException { - PushFeedbackProcessor processor = new PushFeedbackProcessor(accountsManager, directoryQueue); + PushFeedbackProcessor processor = new PushFeedbackProcessor(accountsManager); processor.timeAndProcessCrawlChunk(Optional.of(UUID.randomUUID()), List.of(uninstalledAccount, mixedAccount, stillActiveAccount, freshAccount, cleanAccount, undiscoverableAccount)); verify(uninstalledDevice).setApnId(isNull()); @@ -151,15 +145,6 @@ class PushFeedbackProcessorTest { verify(stillActiveDevice, never()).setFetchesMessages(anyBoolean()); verify(accountsManager).update(eqUuid(stillActiveAccount), any()); - - final ArgumentCaptor> refreshedAccountArgumentCaptor = ArgumentCaptor.forClass(List.class); - verify(directoryQueue).refreshRegisteredUsers(refreshedAccountArgumentCaptor.capture()); - - final List refreshedUuids = refreshedAccountArgumentCaptor.getValue().stream() - .map(Account::getUuid) - .collect(Collectors.toList()); - - assertTrue(refreshedUuids.containsAll(List.of(undiscoverableAccount.getUuid(), uninstalledAccount.getUuid()))); } }