diff --git a/abusive-message-filter b/abusive-message-filter index d20873c7d..6a74e85e4 160000 --- a/abusive-message-filter +++ b/abusive-message-filter @@ -1 +1 @@ -Subproject commit d20873c7d78eb0a33cb27d103ba6ee6807b09a88 +Subproject commit 6a74e85e41d706e48865f45cfcb41208c28c7e44 diff --git a/service/config/sample.yml b/service/config/sample.yml index 7e46f4a0f..73e4dcd6c 100644 --- a/service/config/sample.yml +++ b/service/config/sample.yml @@ -137,6 +137,7 @@ accountsDynamoDb: # DynamoDB table configuration tableName: Example_Accounts phoneNumberTableName: Example_Accounts_PhoneNumbers phoneNumberIdentifierTableName: Example_Accounts_PhoneNumberIdentifiers + usernamesTableName: Example_Accounts_Usernames deletedAccountsDynamoDb: # DynamoDb table configuration region: us-west-2 diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index d7b678b56..2bce4bf89 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -202,8 +202,6 @@ import org.whispersystems.textsecuregcm.storage.ReportMessageManager; import org.whispersystems.textsecuregcm.storage.ReservedUsernames; import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; import org.whispersystems.textsecuregcm.storage.SubscriptionManager; -import org.whispersystems.textsecuregcm.storage.Usernames; -import org.whispersystems.textsecuregcm.storage.UsernamesManager; import org.whispersystems.textsecuregcm.storage.VerificationCodeStore; import org.whispersystems.textsecuregcm.stripe.StripeManager; import org.whispersystems.textsecuregcm.util.Constants; @@ -384,10 +382,10 @@ public class WhisperServerService extends Application dynamicConfigurationManager; private final ProfileBadgeConverter profileBadgeConverter; private final Map badgeConfigurationMap; @@ -112,7 +110,6 @@ public class ProfileController { RateLimiters rateLimiters, AccountsManager accountsManager, ProfilesManager profilesManager, - UsernamesManager usernamesManager, DynamicConfigurationManager dynamicConfigurationManager, ProfileBadgeConverter profileBadgeConverter, BadgesConfiguration badgesConfiguration, @@ -125,7 +122,6 @@ public class ProfileController { this.rateLimiters = rateLimiters; this.accountsManager = accountsManager; this.profilesManager = profilesManager; - this.usernamesManager = usernamesManager; this.dynamicConfigurationManager = dynamicConfigurationManager; this.profileBadgeConverter = profileBadgeConverter; this.badgeConfigurationMap = badgesConfiguration.getBadges().stream().collect(Collectors.toMap( @@ -263,7 +259,7 @@ public class ProfileController { assert(accountProfile.isPresent()); - Optional username = usernamesManager.get(accountProfile.get().getUuid()); + Optional username = accountProfile.flatMap(Account::getUsername); Optional profile = profilesManager.get(uuid, version); String name = profile.map(VersionedProfile::getName).orElse(accountProfile.get().getProfileName()); @@ -315,35 +311,26 @@ public class ProfileController { username = username.toLowerCase(); - Optional uuid = usernamesManager.get(username); + final Account accountProfile = accountsManager.getByUsername(username) + .orElseThrow(() -> new WebApplicationException(Response.status(Response.Status.NOT_FOUND).build())); - if (uuid.isEmpty()) { - throw new WebApplicationException(Response.status(Response.Status.NOT_FOUND).build()); - } - - final boolean isSelf = auth.getAccount().getUuid().equals(uuid.get()); - - Optional accountProfile = accountsManager.getByAccountIdentifier(uuid.get()); - - if (accountProfile.isEmpty()) { - throw new WebApplicationException(Response.status(Response.Status.NOT_FOUND).build()); - } + final boolean isSelf = auth.getAccount().getUuid().equals(accountProfile.getUuid()); return new Profile( - accountProfile.get().getProfileName(), + accountProfile.getProfileName(), null, null, - accountProfile.get().getAvatar(), + accountProfile.getAvatar(), null, - accountProfile.get().getIdentityKey(), - UnidentifiedAccessChecksum.generateFor(accountProfile.get().getUnidentifiedAccessKey()), - accountProfile.get().isUnrestrictedUnidentifiedAccess(), - UserCapabilities.createForAccount(accountProfile.get()), + accountProfile.getIdentityKey(), + UnidentifiedAccessChecksum.generateFor(accountProfile.getUnidentifiedAccessKey()), + accountProfile.isUnrestrictedUnidentifiedAccess(), + UserCapabilities.createForAccount(accountProfile), username, - accountProfile.get().getUuid(), + accountProfile.getUuid(), profileBadgeConverter.convert( getAcceptableLanguagesForRequest(containerRequestContext), - accountProfile.get().getBadges(), + accountProfile.getBadges(), isSelf), null); } @@ -410,7 +397,7 @@ public class ProfileController { Optional accountProfile = accountsManager.getByAccountIdentifier(identifier); OptionalAccess.verify(auth.map(AuthenticatedAccount::getAccount), accessKey, accountProfile); - Optional username = usernamesManager.get(accountProfile.get().getUuid()); + Optional username = accountProfile.flatMap(Account::getUsername); return new Profile( accountProfile.get().getProfileName(), 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 b4612ba39..3f5ad4354 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java @@ -23,6 +23,7 @@ import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials; import org.whispersystems.textsecuregcm.auth.StoredRegistrationLock; import org.whispersystems.textsecuregcm.entities.AccountAttributes; import org.whispersystems.textsecuregcm.util.Util; +import javax.annotation.Nullable; public class Account { @@ -38,6 +39,10 @@ public class Account { @JsonProperty private String number; + @JsonProperty + @Nullable + private String username; + @JsonProperty private Set devices = new HashSet<>(); @@ -134,6 +139,18 @@ public class Account { this.phoneNumberIdentifier = phoneNumberIdentifier; } + public Optional getUsername() { + requireNotStale(); + + return Optional.ofNullable(username); + } + + public void setUsername(final String username) { + requireNotStale(); + + this.username = username; + } + public void addDevice(Device device) { requireNotStale(); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java index b25f42d35..7db4a0835 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java @@ -19,6 +19,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.UUID; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -57,19 +58,25 @@ public class Accounts extends AbstractDynamoDbStore { static final String ATTR_VERSION = "V"; // canonically discoverable static final String ATTR_CANONICALLY_DISCOVERABLE = "C"; + // username; string + static final String ATTR_USERNAME = "N"; private final DynamoDbClient client; private final String phoneNumberConstraintTableName; private final String phoneNumberIdentifierConstraintTableName; + private final String usernamesConstraintTableName; private final String accountsTableName; private final int scanPageSize; private static final Timer CREATE_TIMER = Metrics.timer(name(Accounts.class, "create")); private static final Timer CHANGE_NUMBER_TIMER = Metrics.timer(name(Accounts.class, "changeNumber")); + private static final Timer SET_USERNAME_TIMER = Metrics.timer(name(Accounts.class, "setUsername")); + private static final Timer CLEAR_USERNAME_TIMER = Metrics.timer(name(Accounts.class, "clearUsername")); private static final Timer UPDATE_TIMER = Metrics.timer(name(Accounts.class, "update")); private static final Timer GET_BY_NUMBER_TIMER = Metrics.timer(name(Accounts.class, "getByNumber")); + private static final Timer GET_BY_USERNAME_TIMER = Metrics.timer(name(Accounts.class, "getByUsername")); private static final Timer GET_BY_PNI_TIMER = Metrics.timer(name(Accounts.class, "getByPni")); private static final Timer GET_BY_UUID_TIMER = Metrics.timer(name(Accounts.class, "getByUuid")); private static final Timer GET_ALL_FROM_START_TIMER = Metrics.timer(name(Accounts.class, "getAllFrom")); @@ -79,7 +86,8 @@ public class Accounts extends AbstractDynamoDbStore { private static final Logger log = LoggerFactory.getLogger(Accounts.class); public Accounts(DynamoDbClient client, String accountsTableName, String phoneNumberConstraintTableName, - String phoneNumberIdentifierConstraintTableName, final int scanPageSize) { + String phoneNumberIdentifierConstraintTableName, final String usernamesConstraintTableName, + final int scanPageSize) { super(client); @@ -87,6 +95,7 @@ public class Accounts extends AbstractDynamoDbStore { this.phoneNumberConstraintTableName = phoneNumberConstraintTableName; this.phoneNumberIdentifierConstraintTableName = phoneNumberIdentifierConstraintTableName; this.accountsTableName = accountsTableName; + this.usernamesConstraintTableName = usernamesConstraintTableName; this.scanPageSize = scanPageSize; } @@ -304,6 +313,141 @@ public class Accounts extends AbstractDynamoDbStore { }); } + public void setUsername(final Account account, final String username) + throws ContestedOptimisticLockException, UsernameNotAvailableException { + final long startNanos = System.nanoTime(); + + final Optional maybeOriginalUsername = account.getUsername(); + account.setUsername(username); + + boolean succeeded = false; + + try { + final List writeItems = new ArrayList<>(); + + writeItems.add(TransactWriteItem.builder() + .put(Put.builder() + .tableName(usernamesConstraintTableName) + .item(Map.of( + KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()), + ATTR_USERNAME, AttributeValues.fromString(username))) + .conditionExpression("attribute_not_exists(#username)") + .expressionAttributeNames(Map.of("#username", ATTR_USERNAME)) + .returnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD) + .build()) + .build()); + + writeItems.add( + TransactWriteItem.builder() + .update(Update.builder() + .tableName(accountsTableName) + .key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()))) + .updateExpression("SET #data = :data, #username = :username ADD #version :version_increment") + .conditionExpression("#version = :version") + .expressionAttributeNames(Map.of("#data", ATTR_ACCOUNT_DATA, + "#username", ATTR_USERNAME, + "#version", ATTR_VERSION)) + .expressionAttributeValues(Map.of( + ":data", AttributeValues.fromByteArray(SystemMapper.getMapper().writeValueAsBytes(account)), + ":username", AttributeValues.fromString(username), + ":version", AttributeValues.fromInt(account.getVersion()), + ":version_increment", AttributeValues.fromInt(1))) + .build()) + .build()); + + maybeOriginalUsername.ifPresent(originalUsername -> writeItems.add(TransactWriteItem.builder() + .delete(Delete.builder() + .tableName(usernamesConstraintTableName) + .key(Map.of(ATTR_USERNAME, AttributeValues.fromString(originalUsername))) + .build()) + .build())); + + final TransactWriteItemsRequest request = TransactWriteItemsRequest.builder() + .transactItems(writeItems) + .build(); + + client.transactWriteItems(request); + + account.setVersion(account.getVersion() + 1); + succeeded = true; + } catch (final JsonProcessingException e) { + throw new IllegalArgumentException(e); + } catch (final TransactionCanceledException e) { + if ("ConditionalCheckFailed".equals(e.cancellationReasons().get(0).code())) { + throw new UsernameNotAvailableException(); + } else if ("ConditionalCheckFailed".equals(e.cancellationReasons().get(1).code())) { + throw new ContestedOptimisticLockException(); + } + + throw e; + } finally { + if (!succeeded) { + account.setUsername(maybeOriginalUsername.orElse(null)); + } + + SET_USERNAME_TIMER.record(System.nanoTime() - startNanos, TimeUnit.NANOSECONDS); + } + } + + public void clearUsername(Account account) { + account.getUsername().ifPresent(username -> { + CLEAR_USERNAME_TIMER.record(() -> { + account.setUsername(null); + + boolean succeeded = false; + + try { + final List writeItems = new ArrayList<>(); + + writeItems.add( + TransactWriteItem.builder() + .update(Update.builder() + .tableName(accountsTableName) + .key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()))) + .updateExpression("SET #data = :data REMOVE #username ADD #version :version_increment") + .conditionExpression("#version = :version") + .expressionAttributeNames(Map.of("#data", ATTR_ACCOUNT_DATA, + "#username", ATTR_USERNAME, + "#version", ATTR_VERSION)) + .expressionAttributeValues(Map.of( + ":data", AttributeValues.fromByteArray(SystemMapper.getMapper().writeValueAsBytes(account)), + ":version", AttributeValues.fromInt(account.getVersion()), + ":version_increment", AttributeValues.fromInt(1))) + .build()) + .build()); + + writeItems.add(TransactWriteItem.builder() + .delete(Delete.builder() + .tableName(usernamesConstraintTableName) + .key(Map.of(ATTR_USERNAME, AttributeValues.fromString(username))) + .build()) + .build()); + + final TransactWriteItemsRequest request = TransactWriteItemsRequest.builder() + .transactItems(writeItems) + .build(); + + client.transactWriteItems(request); + + account.setVersion(account.getVersion() + 1); + succeeded = true; + } catch (final JsonProcessingException e) { + throw new IllegalArgumentException(e); + } catch (final TransactionCanceledException e) { + if ("ConditionalCheckFailed".equals(e.cancellationReasons().get(0).code())) { + throw new ContestedOptimisticLockException(); + } + + throw e; + } finally { + if (!succeeded) { + account.setUsername(username); + } + } + }); + }); + } + public void update(Account account) throws ContestedOptimisticLockException { UPDATE_TIMER.record(() -> { final UpdateItemRequest updateItemRequest; @@ -358,6 +502,21 @@ public class Accounts extends AbstractDynamoDbStore { }); } + public Optional getByUsername(final String username) { + return GET_BY_USERNAME_TIMER.record(() -> { + + final GetItemResponse response = client.getItem(GetItemRequest.builder() + .tableName(usernamesConstraintTableName) + .key(Map.of(ATTR_USERNAME, AttributeValues.fromString(username))) + .build()); + + return Optional.ofNullable(response.item()) + .map(item -> item.get(KEY_ACCOUNT_UUID)) + .map(this::accountByUuid) + .map(Accounts::fromItem); + }); + } + public Optional getByPhoneNumberIdentifier(final UUID phoneNumberIdentifier) { return GET_BY_PNI_TIMER.record(() -> { @@ -416,6 +575,13 @@ public class Accounts extends AbstractDynamoDbStore { .build()) .build()); + account.getUsername().ifPresent(username -> transactWriteItems.add(TransactWriteItem.builder() + .delete(Delete.builder() + .tableName(usernamesConstraintTableName) + .key(Map.of(ATTR_USERNAME, AttributeValues.fromString(username))) + .build()) + .build())); + TransactWriteItemsRequest request = TransactWriteItemsRequest.builder() .transactItems(transactWriteItems).build(); @@ -480,6 +646,7 @@ public class Accounts extends AbstractDynamoDbStore { account.setNumber(item.get(ATTR_ACCOUNT_E164).s(), phoneNumberIdentifierFromAttribute); account.setUuid(accountIdentifier); + account.setUsername(AttributeValues.getString(item, ATTR_USERNAME, null)); account.setVersion(Integer.parseInt(item.get(ATTR_VERSION).n())); account.setCanonicallyDiscoverable(Optional.ofNullable(item.get(ATTR_CANONICALLY_DISCOVERABLE)).map(av -> av.bool()).orElse(false)); 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 92c306491..ad56e1fa6 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -44,18 +44,20 @@ import org.whispersystems.textsecuregcm.util.Util; public class AccountsManager { - private static final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); - private static final Timer createTimer = metricRegistry.timer(name(AccountsManager.class, "create" )); - private static final Timer updateTimer = metricRegistry.timer(name(AccountsManager.class, "update" )); - private static final Timer getByNumberTimer = metricRegistry.timer(name(AccountsManager.class, "getByNumber")); - private static final Timer getByUuidTimer = metricRegistry.timer(name(AccountsManager.class, "getByUuid" )); - private static final Timer deleteTimer = metricRegistry.timer(name(AccountsManager.class, "delete")); + private static final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); + private static final Timer createTimer = metricRegistry.timer(name(AccountsManager.class, "create")); + private static final Timer updateTimer = metricRegistry.timer(name(AccountsManager.class, "update")); + private static final Timer getByNumberTimer = metricRegistry.timer(name(AccountsManager.class, "getByNumber")); + private static final Timer getByUsernameTimer = metricRegistry.timer(name(AccountsManager.class, "getByUsername")); + private static final Timer getByUuidTimer = metricRegistry.timer(name(AccountsManager.class, "getByUuid")); + private static final Timer deleteTimer = metricRegistry.timer(name(AccountsManager.class, "delete")); - private static final Timer redisSetTimer = metricRegistry.timer(name(AccountsManager.class, "redisSet" )); + 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 redisPniGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisPniGet")); - 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 Timer redisUsernameGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisUsernameGet")); + private static final Timer redisPniGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisPniGet")); + 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"); @@ -71,7 +73,7 @@ public class AccountsManager { private final DirectoryQueue directoryQueue; private final Keys keys; private final MessagesManager messagesManager; - private final UsernamesManager usernamesManager; + private final ReservedUsernames reservedUsernames; private final ProfilesManager profilesManager; private final StoredVerificationCodeManager pendingAccounts; private final SecureStorageClient secureStorageClient; @@ -86,6 +88,11 @@ public class AccountsManager { // the owner. private static final long CACHE_TTL_SECONDS = Duration.ofDays(2).toSeconds(); + @FunctionalInterface + private interface AccountPersister { + void persistAccount(Account account) throws UsernameNotAvailableException; + } + public enum DeletionReason { ADMIN_DELETED("admin"), EXPIRED ("expired"), @@ -105,7 +112,7 @@ public class AccountsManager { final DirectoryQueue directoryQueue, final Keys keys, final MessagesManager messagesManager, - final UsernamesManager usernamesManager, + final ReservedUsernames reservedUsernames, final ProfilesManager profilesManager, final StoredVerificationCodeManager pendingAccounts, final SecureStorageClient secureStorageClient, @@ -119,12 +126,12 @@ public class AccountsManager { this.directoryQueue = directoryQueue; this.keys = keys; this.messagesManager = messagesManager; - this.usernamesManager = usernamesManager; this.profilesManager = profilesManager; this.pendingAccounts = pendingAccounts; this.secureStorageClient = secureStorageClient; this.secureBackupClient = secureBackupClient; this.clientPresenceManager = clientPresenceManager; + this.reservedUsernames = reservedUsernames; this.mapper = SystemMapper.getMapper(); this.clock = Objects.requireNonNull(clock); } @@ -236,11 +243,18 @@ public class AccountsManager { final UUID uuid = account.getUuid(); final UUID phoneNumberIdentifier = phoneNumberIdentifiers.getPhoneNumberIdentifier(number); - final Account numberChangedAccount = updateWithRetries( - account, - a -> true, - a -> accounts.changeNumber(a, number, phoneNumberIdentifier), - () -> accounts.getByAccountIdentifier(uuid).orElseThrow()); + final Account numberChangedAccount; + + try { + numberChangedAccount = updateWithRetries( + account, + a -> true, + a -> accounts.changeNumber(a, number, phoneNumberIdentifier), + () -> accounts.getByAccountIdentifier(uuid).orElseThrow()); + } catch (UsernameNotAvailableException e) { + // This should never happen when changing numbers + throw new RuntimeException(e); + } updatedAccount.set(numberChangedAccount); directoryQueue.changePhoneNumber(numberChangedAccount, originalNumber, number); @@ -251,13 +265,51 @@ public class AccountsManager { return updatedAccount.get(); } + public Account setUsername(final Account account, final String username) throws UsernameNotAvailableException { + if (account.getUsername().map(username::equals).orElse(false)) { + return account; + } + + if (reservedUsernames.isReserved(username, account.getUuid())) { + throw new UsernameNotAvailableException(); + } + + redisDelete(account); + + return updateWithRetries( + account, + a -> true, + a -> accounts.setUsername(a, username), + () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow()); + } + + public Account clearUsername(final Account account) { + redisDelete(account); + + try { + return updateWithRetries( + account, + a -> true, + accounts::clearUsername, + () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow()); + } catch (UsernameNotAvailableException e) { + // This should never happen + throw new RuntimeException(e); + } + } + public Account update(Account account, Consumer updater) { - return update(account, a -> { - updater.accept(a); - // assume that all updaters passed to the public method actually modify the account - return true; - }); + try { + return update(account, a -> { + updater.accept(a); + // assume that all updaters passed to the public method actually modify the account + return true; + }); + } catch (UsernameNotAvailableException e) { + // This should never happen for general-purpose, public account updates + throw new RuntimeException(e); + } } /** @@ -266,28 +318,33 @@ public class AccountsManager { */ public Account updateDeviceLastSeen(Account account, Device device, final long lastSeen) { - return update(account, a -> { + try { + return update(account, a -> { - final Optional maybeDevice = a.getDevice(device.getId()); + final Optional maybeDevice = a.getDevice(device.getId()); - return maybeDevice.map(d -> { - if (d.getLastSeen() >= lastSeen) { - return false; - } + return maybeDevice.map(d -> { + if (d.getLastSeen() >= lastSeen) { + return false; + } - d.setLastSeen(lastSeen); + d.setLastSeen(lastSeen); - return true; + return true; - }).orElse(false); - }); + }).orElse(false); + }); + } catch (UsernameNotAvailableException e) { + // This should never happen when updating last-seen timestamps + throw new RuntimeException(e); + } } /** * @param account account to update * @param updater must return {@code true} if the account was actually updated */ - private Account update(Account account, Function updater) { + private Account update(Account account, Function updater) throws UsernameNotAvailableException { final boolean wasVisibleBeforeUpdate = account.shouldBeVisibleInDirectory(); @@ -300,6 +357,7 @@ public class AccountsManager { final UUID uuid = account.getUuid(); final String originalNumber = account.getNumber(); final UUID originalPhoneNumberIdentifier = account.getPhoneNumberIdentifier(); + final Optional originalUsername = account.getUsername(); updatedAccount = updateWithRetries(account, updater, @@ -320,6 +378,13 @@ public class AccountsManager { new RuntimeException()); } + assert updatedAccount.getUsername().equals(originalUsername); + + if (!updatedAccount.getUsername().equals(originalUsername)) { + logger.error("Username changed via \"normal\" update; usernames must be changed via setUsername method", + new RuntimeException()); + } + redisSet(updatedAccount); } @@ -332,8 +397,8 @@ public class AccountsManager { return updatedAccount; } - private Account updateWithRetries(Account account, Function updater, Consumer persister, - Supplier retriever) { + private Account updateWithRetries(Account account, Function updater, AccountPersister persister, + Supplier retriever) throws UsernameNotAvailableException { if (!updater.apply(account)) { return account; @@ -345,7 +410,7 @@ public class AccountsManager { while (tries < maxTries) { try { - persister.accept(account); + persister.persistAccount(account); final Account updatedAccount; try { @@ -373,11 +438,16 @@ public class AccountsManager { } public Account updateDevice(Account account, long deviceId, Consumer deviceUpdater) { - return update(account, a -> { - a.getDevice(deviceId).ifPresent(deviceUpdater); - // assume that all updaters passed to the public method actually modify the device - return true; - }); + try { + return update(account, a -> { + a.getDevice(deviceId).ifPresent(deviceUpdater); + // assume that all updaters passed to the public method actually modify the device + return true; + }); + } catch (UsernameNotAvailableException e) { + // This should never happen when updating devices + throw new RuntimeException(e); + } } public Optional getByE164(String number) { @@ -406,6 +476,19 @@ public class AccountsManager { } } + public Optional getByUsername(final String username) { + try (final Timer.Context ignored = getByUsernameTimer.time()) { + Optional account = redisGetByUsername(username); + + if (account.isEmpty()) { + account = accounts.getByUsername(username); + account.ifPresent(this::redisSet); + } + + return account; + } + } + public Optional getByAccountIdentifier(UUID uuid) { try (Timer.Context ignored = getByUuidTimer.time()) { Optional account = redisGetByAccountIdentifier(uuid); @@ -450,7 +533,6 @@ public class AccountsManager { final CompletableFuture deleteStorageServiceDataFuture = secureStorageClient.deleteStoredData(account.getUuid()); final CompletableFuture deleteBackupServiceDataFuture = secureBackupClient.deleteBackups(account.getUuid()); - usernamesManager.delete(account.getUuid()); profilesManager.deleteAll(account.getUuid()); keys.delete(account.getUuid()); keys.delete(account.getPhoneNumberIdentifier()); @@ -486,6 +568,9 @@ public class AccountsManager { commands.setex(getAccountMapKey(account.getPhoneNumberIdentifier().toString()), CACHE_TTL_SECONDS, account.getUuid().toString()); commands.setex(getAccountMapKey(account.getNumber()), CACHE_TTL_SECONDS, account.getUuid().toString()); commands.setex(getAccountEntityKey(account.getUuid()), CACHE_TTL_SECONDS, accountJson); + + account.getUsername().ifPresent(username -> + commands.setex(getAccountMapKey(username), CACHE_TTL_SECONDS, account.getUuid().toString())); }); } catch (JsonProcessingException e) { throw new IllegalStateException(e); @@ -500,6 +585,10 @@ public class AccountsManager { return redisGetBySecondaryKey(e164, redisNumberGetTimer); } + private Optional redisGetByUsername(String username) { + return redisGetBySecondaryKey(username, redisUsernameGetTimer); + } + private Optional redisGetBySecondaryKey(String secondaryKey, Timer timer) { try (Timer.Context ignored = timer.time()) { final String uuid = cacheCluster.withCluster(connection -> connection.sync().get(getAccountMapKey(secondaryKey))); @@ -542,10 +631,14 @@ public class AccountsManager { private void redisDelete(final Account account) { try (final Timer.Context ignored = redisDeleteTimer.time()) { - cacheCluster.useCluster(connection -> connection.sync().del( - getAccountMapKey(account.getNumber()), - getAccountMapKey(account.getPhoneNumberIdentifier().toString()), - getAccountEntityKey(account.getUuid()))); + cacheCluster.useCluster(connection -> { + connection.sync().del( + getAccountMapKey(account.getNumber()), + getAccountMapKey(account.getPhoneNumberIdentifier().toString()), + getAccountEntityKey(account.getUuid())); + + account.getUsername().ifPresent(username -> connection.sync().del(getAccountMapKey(username))); + }); } } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameNotAvailableException.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameNotAvailableException.java new file mode 100644 index 000000000..a2d25f49f --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameNotAvailableException.java @@ -0,0 +1,9 @@ +/* + * Copyright 2013-2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.storage; + +public class UsernameNotAvailableException extends Exception { +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Usernames.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Usernames.java deleted file mode 100644 index 2ebd00117..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Usernames.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Copyright 2013-2020 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.storage; - -import static com.codahale.metrics.MetricRegistry.name; - -import com.codahale.metrics.MetricRegistry; -import com.codahale.metrics.SharedMetricRegistries; -import com.codahale.metrics.Timer; -import java.sql.SQLException; -import java.util.Optional; -import java.util.UUID; -import org.jdbi.v3.core.JdbiException; -import org.whispersystems.textsecuregcm.util.Constants; - -public class Usernames { - - public static final String ID = "id"; - public static final String UID = "uuid"; - public static final String USERNAME = "username"; - - private final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); - private final Timer createTimer = metricRegistry.timer(name(Usernames.class, "create" )); - private final Timer deleteTimer = metricRegistry.timer(name(Usernames.class, "delete" )); - private final Timer getByUsernameTimer = metricRegistry.timer(name(Usernames.class, "getByUsername")); - private final Timer getByUuidTimer = metricRegistry.timer(name(Usernames.class, "getByUuid" )); - - private final FaultTolerantDatabase database; - - public Usernames(FaultTolerantDatabase database) { - this.database = database; - } - - public boolean put(UUID uuid, String username) { - return database.with(jdbi -> jdbi.withHandle(handle -> { - try (Timer.Context ignored = createTimer.time()) { - int modified = handle.createUpdate("INSERT INTO usernames (" + UID + ", " + USERNAME + ") VALUES (:uuid, :username) ON CONFLICT (" + UID + ") DO UPDATE SET " + USERNAME + " = EXCLUDED.username") - .bind("uuid", uuid) - .bind("username", username) - .execute(); - - return modified > 0; - } catch (JdbiException e) { - if (e.getCause() instanceof SQLException) { - if (((SQLException)e.getCause()).getSQLState().equals("23505")) { - return false; - } - } - - throw e; - } - })); - } - - public void delete(UUID uuid) { - database.use(jdbi -> jdbi.useHandle(handle -> { - try (Timer.Context ignored = deleteTimer.time()) { - handle.createUpdate("DELETE FROM usernames WHERE " + UID + " = :uuid") - .bind("uuid", uuid) - .execute(); - } - })); - } - - public Optional get(String username) { - return database.with(jdbi -> jdbi.withHandle(handle -> { - try (Timer.Context ignored = getByUsernameTimer.time()) { - return handle.createQuery("SELECT " + UID + " FROM usernames WHERE " + USERNAME + " = :username") - .bind("username", username) - .mapTo(UUID.class) - .findFirst(); - } - })); - } - - public Optional get(UUID uuid) { - return database.with(jdbi -> jdbi.withHandle(handle -> { - try (Timer.Context ignored = getByUuidTimer.time()) { - return handle.createQuery("SELECT " + USERNAME + " FROM usernames WHERE " + UID + " = :uuid") - .bind("uuid", uuid) - .mapTo(String.class) - .findFirst(); - } - })); - } - -} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernamesManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernamesManager.java deleted file mode 100644 index 85850a5a2..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernamesManager.java +++ /dev/null @@ -1,181 +0,0 @@ -/* - * Copyright 2013-2020 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.storage; - -import com.codahale.metrics.MetricRegistry; -import com.codahale.metrics.SharedMetricRegistries; -import com.codahale.metrics.Timer; -import io.lettuce.core.RedisException; -import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; -import org.whispersystems.textsecuregcm.util.Constants; - -import java.util.Optional; -import java.util.UUID; - -import static com.codahale.metrics.MetricRegistry.name; - -public class UsernamesManager { - - private static final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); - private static final Timer createTimer = metricRegistry.timer(name(UsernamesManager.class, "create" )); - private static final Timer deleteTimer = metricRegistry.timer(name(UsernamesManager.class, "delete" )); - private static final Timer getByUuidTimer = metricRegistry.timer(name(UsernamesManager.class, "getByUuid" )); - private static final Timer getByUsernameTimer = metricRegistry.timer(name(UsernamesManager.class, "getByUsername" )); - - private static final Timer redisSetTimer = metricRegistry.timer(name(UsernamesManager.class, "redisSet" )); - private static final Timer redisUuidGetTimer = metricRegistry.timer(name(UsernamesManager.class, "redisUuidGet" )); - private static final Timer redisUsernameGetTimer = metricRegistry.timer(name(UsernamesManager.class, "redisUsernameGet")); - - private final Logger logger = LoggerFactory.getLogger(UsernamesManager.class); - - private final Usernames usernames; - private final ReservedUsernames reservedUsernames; - private final FaultTolerantRedisCluster cacheCluster; - - public UsernamesManager(Usernames usernames, ReservedUsernames reservedUsernames, FaultTolerantRedisCluster cacheCluster) { - this.usernames = usernames; - this.reservedUsernames = reservedUsernames; - this.cacheCluster = cacheCluster; - } - - public boolean put(UUID uuid, String username) { - try (Timer.Context ignored = createTimer.time()) { - if (reservedUsernames.isReserved(username, uuid)) { - return false; - } - - if (databasePut(uuid, username)) { - redisSet(uuid, username, true); - - return true; - } - - return false; - } - } - - public Optional get(String username) { - try (Timer.Context ignored = getByUsernameTimer.time()) { - Optional uuid = redisGet(username); - - if (uuid.isPresent()) { - return uuid; - } - - Optional retrieved = databaseGet(username); - retrieved.ifPresent(retrievedUuid -> redisSet(retrievedUuid, username, false)); - - return retrieved; - } - } - - public Optional get(UUID uuid) { - try (Timer.Context ignored = getByUuidTimer.time()) { - Optional username = redisGet(uuid); - - if (username.isPresent()) { - return username; - } - - Optional retrieved = databaseGet(uuid); - retrieved.ifPresent(retrievedUsername -> redisSet(uuid, retrievedUsername, false)); - - return retrieved; - } - } - - public void delete(UUID uuid) { - try (Timer.Context ignored = deleteTimer.time()) { - redisDelete(uuid); - databaseDelete(uuid); - } - } - - private boolean databasePut(UUID uuid, String username) { - return usernames.put(uuid, username); - } - - private Optional databaseGet(String username) { - return usernames.get(username); - } - - private void databaseDelete(UUID uuid) { - usernames.delete(uuid); - } - - private Optional databaseGet(UUID uuid) { - return usernames.get(uuid); - } - - private void redisSet(UUID uuid, String username, boolean required) { - final String uuidMapKey = getUuidMapKey(uuid); - final String usernameMapKey = getUsernameMapKey(username); - - try (Timer.Context ignored = redisSetTimer.time()) { - cacheCluster.useCluster(connection -> { - final RedisAdvancedClusterCommands commands = connection.sync(); - - final Optional maybeOldUsername = Optional.ofNullable(commands.get(uuidMapKey)); - - maybeOldUsername.ifPresent(oldUsername -> commands.del(getUsernameMapKey(oldUsername))); - commands.set(uuidMapKey, username); - commands.set(usernameMapKey, uuid.toString()); - }); - } catch (RedisException e) { - if (required) throw e; - else logger.warn("Ignoring Redis failure", e); - } - } - - private Optional redisGet(String username) { - try (Timer.Context ignored = redisUsernameGetTimer.time()) { - final String result = cacheCluster.withCluster(connection -> connection.sync().get(getUsernameMapKey(username))); - - if (result == null) return Optional.empty(); - else return Optional.of(UUID.fromString(result)); - } catch (RedisException e) { - logger.warn("Redis get failure", e); - return Optional.empty(); - } - } - - private Optional redisGet(UUID uuid) { - try (Timer.Context ignored = redisUuidGetTimer.time()) { - final String result = cacheCluster.withCluster(connection -> connection.sync().get(getUuidMapKey(uuid))); - - return Optional.ofNullable(result); - } catch (RedisException e) { - logger.warn("Redis get failure", e); - return Optional.empty(); - } - } - - private void redisDelete(UUID uuid) { - try (Timer.Context ignored = redisUuidGetTimer.time()) { - cacheCluster.useCluster(connection -> { - final RedisAdvancedClusterCommands commands = connection.sync(); - - commands.del(getUuidMapKey(uuid)); - - redisGet(uuid).ifPresent(username -> { - commands.del(getUsernameMapKey(username)); - }); - }); - } - } - - private String getUuidMapKey(UUID uuid) { - return "UsernameByUuid::" + uuid.toString(); - } - - private String getUsernameMapKey(String username) { - return "UsernameByUsername::" + username; - } - -} 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 8d5055d3d..8f86a6e48 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,6 @@ 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; @@ -175,10 +173,10 @@ public class DeleteUserCommand extends EnvironmentCommand maybeAccount; diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java index a2a7d977a..4b4a7ea57 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java @@ -51,6 +51,7 @@ class AccountsManagerChangeNumberIntegrationTest { private static final String ACCOUNTS_TABLE_NAME = "accounts_test"; private static final String NUMBERS_TABLE_NAME = "numbers_test"; private static final String PNI_ASSIGNMENT_TABLE_NAME = "pni_assignment_test"; + private static final String USERNAMES_TABLE_NAME = "usernames_test"; private static final String PNI_TABLE_NAME = "pni_test"; private static final String NEEDS_RECONCILIATION_INDEX_NAME = "needs_reconciliation_test"; private static final String DELETED_ACCOUNTS_LOCK_TABLE_NAME = "deleted_accounts_lock_test"; @@ -155,6 +156,7 @@ class AccountsManagerChangeNumberIntegrationTest { ACCOUNTS_DYNAMO_EXTENSION.getTableName(), NUMBERS_TABLE_NAME, PNI_ASSIGNMENT_TABLE_NAME, + USERNAMES_TABLE_NAME, SCAN_PAGE_SIZE); { @@ -191,7 +193,7 @@ class AccountsManagerChangeNumberIntegrationTest { mock(DirectoryQueue.class), mock(Keys.class), mock(MessagesManager.class), - mock(UsernamesManager.class), + mock(ReservedUsernames.class), mock(ProfilesManager.class), mock(StoredVerificationCodeManager.class), secureStorageClient, 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 db6ca8897..fed2c6e35 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java @@ -59,6 +59,7 @@ class AccountsManagerConcurrentModificationIntegrationTest { private static final String ACCOUNTS_TABLE_NAME = "accounts_test"; private static final String NUMBERS_TABLE_NAME = "numbers_test"; private static final String PNI_TABLE_NAME = "pni_test"; + private static final String USERNAMES_TABLE_NAME = "usernames_test"; private static final int SCAN_PAGE_SIZE = 1; @@ -122,6 +123,7 @@ class AccountsManagerConcurrentModificationIntegrationTest { dynamoDbExtension.getTableName(), NUMBERS_TABLE_NAME, PNI_TABLE_NAME, + USERNAMES_TABLE_NAME, SCAN_PAGE_SIZE); { @@ -148,7 +150,7 @@ class AccountsManagerConcurrentModificationIntegrationTest { mock(DirectoryQueue.class), mock(Keys.class), mock(MessagesManager.class), - mock(UsernamesManager.class), + mock(ReservedUsernames.class), mock(ProfilesManager.class), mock(StoredVerificationCodeManager.class), mock(SecureStorageClient.class), diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java similarity index 80% rename from service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java rename to service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java index 99d932d2a..c13136d88 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -3,8 +3,9 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -package org.whispersystems.textsecuregcm.tests.storage; +package org.whispersystems.textsecuregcm.storage; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -44,25 +45,14 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import org.mockito.stubbing.Answer; +import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.entities.AccountAttributes; import org.whispersystems.textsecuregcm.entities.SignedPreKey; import org.whispersystems.textsecuregcm.push.ClientPresenceManager; import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; -import org.whispersystems.textsecuregcm.storage.Account; -import org.whispersystems.textsecuregcm.storage.Accounts; -import org.whispersystems.textsecuregcm.storage.AccountsManager; -import org.whispersystems.textsecuregcm.storage.ContestedOptimisticLockException; -import org.whispersystems.textsecuregcm.storage.DeletedAccountsManager; -import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.storage.Device.DeviceCapabilities; -import org.whispersystems.textsecuregcm.storage.Keys; -import org.whispersystems.textsecuregcm.storage.MessagesManager; -import org.whispersystems.textsecuregcm.storage.PhoneNumberIdentifiers; -import org.whispersystems.textsecuregcm.storage.ProfilesManager; -import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; -import org.whispersystems.textsecuregcm.storage.UsernamesManager; import org.whispersystems.textsecuregcm.tests.util.RedisClusterHelper; class AccountsManagerTest { @@ -73,10 +63,13 @@ class AccountsManagerTest { private Keys keys; private MessagesManager messagesManager; private ProfilesManager profilesManager; + private ReservedUsernames reservedUsernames; private RedisAdvancedClusterCommands commands; private AccountsManager accountsManager; + private static final UUID RESERVED_FOR_UUID = UUID.randomUUID(); + private static final Answer ACCOUNT_UPDATE_ANSWER = (answer) -> { // it is implicit in the update() contract is that a successful call will // result in an incremented version @@ -93,6 +86,7 @@ class AccountsManagerTest { keys = mock(Keys.class); messagesManager = mock(MessagesManager.class); profilesManager = mock(ProfilesManager.class); + reservedUsernames = mock(ReservedUsernames.class); //noinspection unchecked commands = mock(RedisAdvancedClusterCommands.class); @@ -127,6 +121,13 @@ class AccountsManagerTest { return phoneNumberIdentifiersByE164.computeIfAbsent(number, n -> UUID.randomUUID()); }); + @SuppressWarnings("unchecked") final DynamicConfigurationManager dynamicConfigurationManager = + mock(DynamicConfigurationManager.class); + + final DynamicConfiguration dynamicConfiguration = mock(DynamicConfiguration.class); + + when(dynamicConfigurationManager.getConfiguration()).thenReturn(dynamicConfiguration); + accountsManager = new AccountsManager( accounts, phoneNumberIdentifiers, @@ -135,7 +136,7 @@ class AccountsManagerTest { directoryQueue, keys, messagesManager, - mock(UsernamesManager.class), + reservedUsernames, profilesManager, mock(StoredVerificationCodeManager.class), storageClient, @@ -207,6 +208,29 @@ class AccountsManagerTest { verifyNoInteractions(accounts); } + @Test + void testGetByUsernameInCache() { + UUID uuid = UUID.randomUUID(); + String username = "test"; + + when(commands.get(eq("AccountMap::" + username))).thenReturn(uuid.toString()); + when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"name\": \"test\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\", \"username\": \"test\"}"); + + Optional account = accountsManager.getByUsername(username); + + assertTrue(account.isPresent()); + assertEquals(account.get().getNumber(), "+14152222222"); + assertEquals(account.get().getProfileName(), "test"); + assertEquals(UUID.fromString("de24dc73-fbd8-41be-a7d5-764c70d9da7e"), account.get().getPhoneNumberIdentifier()); + assertEquals(Optional.of(username), account.get().getUsername()); + + verify(commands).get(eq("AccountMap::" + username)); + verify(commands).get(eq("Account3::" + uuid)); + verifyNoMoreInteractions(commands); + + verifyNoInteractions(accounts); + } + @Test void testGetAccountByNumberNotInCache() { UUID uuid = UUID.randomUUID(); @@ -280,6 +304,33 @@ class AccountsManagerTest { verifyNoMoreInteractions(accounts); } + @Test + void testGetAccountByUsernameNotInCache() { + UUID uuid = UUID.randomUUID(); + String username = "test"; + + Account account = new Account("+14152222222", uuid, UUID.randomUUID(), new HashSet<>(), new byte[16]); + account.setUsername(username); + + when(commands.get(eq("AccountMap::" + username))).thenReturn(null); + when(accounts.getByUsername(username)).thenReturn(Optional.of(account)); + + Optional retrieved = accountsManager.getByUsername(username); + + assertTrue(retrieved.isPresent()); + assertSame(retrieved.get(), account); + + verify(commands).get(eq("AccountMap::" + username)); + verify(commands).setex(eq("AccountMap::" + username), anyLong(), eq(uuid.toString())); + verify(commands).setex(eq("AccountMap::" + account.getPhoneNumberIdentifier()), anyLong(), eq(uuid.toString())); + verify(commands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); + verify(commands).setex(eq("Account3::" + uuid), anyLong(), anyString()); + verifyNoMoreInteractions(commands); + + verify(accounts).getByUsername(username); + verifyNoMoreInteractions(accounts); + } + @Test void testGetAccountByNumberBrokenCache() { UUID uuid = UUID.randomUUID(); @@ -353,6 +404,33 @@ class AccountsManagerTest { verifyNoMoreInteractions(accounts); } + @Test + void testGetAccountByUsernameBrokenCache() { + UUID uuid = UUID.randomUUID(); + String username = "test"; + + Account account = new Account("+14152222222", uuid, UUID.randomUUID(), new HashSet<>(), new byte[16]); + account.setUsername(username); + + when(commands.get(eq("AccountMap::" + username))).thenThrow(new RedisException("OH NO")); + when(accounts.getByUsername(username)).thenReturn(Optional.of(account)); + + Optional retrieved = accountsManager.getByUsername(username); + + assertTrue(retrieved.isPresent()); + assertSame(retrieved.get(), account); + + verify(commands).get(eq("AccountMap::" + username)); + verify(commands).setex(eq("AccountMap::" + username), anyLong(), eq(uuid.toString())); + verify(commands).setex(eq("AccountMap::" + account.getPhoneNumberIdentifier()), anyLong(), eq(uuid.toString())); + verify(commands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); + verify(commands).setex(eq("Account3::" + uuid), anyLong(), anyString()); + verifyNoMoreInteractions(commands); + + verify(accounts).getByUsername(username); + verifyNoMoreInteractions(accounts); + } + @Test void testUpdate_optimisticLockingFailure() { UUID uuid = UUID.randomUUID(); @@ -627,4 +705,54 @@ class AccountsManagerTest { assertThrows(AssertionError.class, () -> accountsManager.update(account, a -> a.setNumber(targetNumber, UUID.randomUUID()))); } + + @Test + void testSetUsername() throws UsernameNotAvailableException { + final Account account = new Account("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new HashSet<>(), new byte[16]); + final String username = "test"; + + assertDoesNotThrow(() -> accountsManager.setUsername(account, username)); + verify(accounts).setUsername(account, username); + } + + @Test + void testSetUsernameSameUsername() throws UsernameNotAvailableException { + final Account account = new Account("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new HashSet<>(), new byte[16]); + final String username = "test"; + account.setUsername(username); + + assertDoesNotThrow(() -> accountsManager.setUsername(account, username)); + verify(accounts, never()).setUsername(eq(account), any()); + } + + @Test + void testSetUsernameNotAvailable() throws UsernameNotAvailableException { + final Account account = new Account("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new HashSet<>(), new byte[16]); + final String username = "test"; + + doThrow(new UsernameNotAvailableException()).when(accounts).setUsername(account, username); + + assertThrows(UsernameNotAvailableException.class, () -> accountsManager.setUsername(account, username)); + verify(accounts).setUsername(account, username); + + assertTrue(account.getUsername().isEmpty()); + } + + @Test + void testSetUsernameReserved() { + final String username = "reserved"; + when(reservedUsernames.isReserved(eq(username), any())).thenReturn(true); + + final Account account = new Account("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new HashSet<>(), new byte[16]); + + assertThrows(UsernameNotAvailableException.class, () -> accountsManager.setUsername(account, username)); + assertTrue(account.getUsername().isEmpty()); + } + + @Test + void testSetUsernameViaUpdate() { + final Account account = new Account("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new HashSet<>(), new byte[16]); + + assertThrows(AssertionError.class, () -> accountsManager.update(account, a -> a.setUsername("test"))); + } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java index 96faa029c..cd7ba22e3 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java @@ -6,6 +6,8 @@ package org.whispersystems.textsecuregcm.storage; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; @@ -19,7 +21,6 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -38,8 +39,6 @@ import org.whispersystems.textsecuregcm.util.AttributeValues; import org.whispersystems.textsecuregcm.util.SystemMapper; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; import software.amazon.awssdk.services.dynamodb.model.AttributeDefinition; -import software.amazon.awssdk.services.dynamodb.model.AttributeValue; -import software.amazon.awssdk.services.dynamodb.model.CancellationReason; import software.amazon.awssdk.services.dynamodb.model.ConditionalCheckFailedException; import software.amazon.awssdk.services.dynamodb.model.CreateTableRequest; import software.amazon.awssdk.services.dynamodb.model.GetItemRequest; @@ -61,6 +60,7 @@ class AccountsTest { private static final String ACCOUNTS_TABLE_NAME = "accounts_test"; private static final String NUMBER_CONSTRAINT_TABLE_NAME = "numbers_test"; private static final String PNI_CONSTRAINT_TABLE_NAME = "pni_test"; + private static final String USERNAME_CONSTRAINT_TABLE_NAME = "username_test"; private static final int SCAN_PAGE_SIZE = 1; @@ -108,11 +108,27 @@ class AccountsTest { dynamoDbExtension.getDynamoDbClient().createTable(createPhoneNumberIdentifierTableRequest); + CreateTableRequest createUsernamesTableRequest = CreateTableRequest.builder() + .tableName(USERNAME_CONSTRAINT_TABLE_NAME) + .keySchema(KeySchemaElement.builder() + .attributeName(Accounts.ATTR_USERNAME) + .keyType(KeyType.HASH) + .build()) + .attributeDefinitions(AttributeDefinition.builder() + .attributeName(Accounts.ATTR_USERNAME) + .attributeType(ScalarAttributeType.S) + .build()) + .provisionedThroughput(DynamoDbExtension.DEFAULT_PROVISIONED_THROUGHPUT) + .build(); + + dynamoDbExtension.getDynamoDbClient().createTable(createUsernamesTableRequest); + this.accounts = new Accounts( dynamoDbExtension.getDynamoDbClient(), dynamoDbExtension.getTableName(), NUMBER_CONSTRAINT_TABLE_NAME, PNI_CONSTRAINT_TABLE_NAME, + USERNAME_CONSTRAINT_TABLE_NAME, SCAN_PAGE_SIZE); } @@ -357,7 +373,7 @@ class AccountsTest { final DynamoDbClient dynamoDbClient = mock(DynamoDbClient.class); accounts = new Accounts(dynamoDbClient, - dynamoDbExtension.getTableName(), NUMBER_CONSTRAINT_TABLE_NAME, PNI_CONSTRAINT_TABLE_NAME, SCAN_PAGE_SIZE); + dynamoDbExtension.getTableName(), NUMBER_CONSTRAINT_TABLE_NAME, PNI_CONSTRAINT_TABLE_NAME, USERNAME_CONSTRAINT_TABLE_NAME, SCAN_PAGE_SIZE); when(dynamoDbClient.updateItem(any(UpdateItemRequest.class))) .thenThrow(TransactionConflictException.class); @@ -495,7 +511,7 @@ class AccountsTest { .thenThrow(RuntimeException.class); Accounts accounts = new Accounts(client, ACCOUNTS_TABLE_NAME, NUMBER_CONSTRAINT_TABLE_NAME, - PNI_CONSTRAINT_TABLE_NAME, SCAN_PAGE_SIZE); + PNI_CONSTRAINT_TABLE_NAME, USERNAME_CONSTRAINT_TABLE_NAME, SCAN_PAGE_SIZE); Account account = generateAccount("+14151112222", UUID.randomUUID(), UUID.randomUUID()); try { @@ -646,6 +662,118 @@ class AccountsTest { assertThrows(TransactionCanceledException.class, () -> accounts.changeNumber(account, targetNumber, existingPhoneNumberIdentifier)); } + @Test + void testSetUsername() throws UsernameNotAvailableException { + final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account); + + final String username = "test"; + + assertThat(accounts.getByUsername(username)).isEmpty(); + + accounts.setUsername(account, username); + + { + final Optional maybeAccount = accounts.getByUsername(username); + + assertThat(maybeAccount).hasValueSatisfying(retrievedAccount -> + assertThat(retrievedAccount.getUsername()).hasValueSatisfying(retrievedUsername -> + assertThat(retrievedUsername).isEqualTo(username))); + + verifyStoredState(account.getNumber(), account.getUuid(), account.getPhoneNumberIdentifier(), maybeAccount.orElseThrow(), account); + } + + final String secondUsername = username + "2"; + + accounts.setUsername(account, secondUsername); + + assertThat(accounts.getByUsername(username)).isEmpty(); + + { + final Optional maybeAccount = accounts.getByUsername(secondUsername); + + assertThat(maybeAccount).isPresent(); + verifyStoredState(account.getNumber(), account.getUuid(), account.getPhoneNumberIdentifier(), + maybeAccount.get(), account); + } + } + + @Test + void testSetUsernameConflict() { + final Account firstAccount = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); + final Account secondAccount = generateAccount("+18005559876", UUID.randomUUID(), UUID.randomUUID()); + + accounts.create(firstAccount); + accounts.create(secondAccount); + + final String username = "test"; + + assertThatNoException().isThrownBy(() -> accounts.setUsername(firstAccount, username)); + + final Optional maybeAccount = accounts.getByUsername(username); + + assertThat(maybeAccount).isPresent(); + verifyStoredState(firstAccount.getNumber(), firstAccount.getUuid(), firstAccount.getPhoneNumberIdentifier(), maybeAccount.get(), firstAccount); + + assertThatExceptionOfType(UsernameNotAvailableException.class) + .isThrownBy(() -> accounts.setUsername(secondAccount, username)); + + assertThat(secondAccount.getUsername()).isEmpty(); + } + + @Test + void testSetUsernameVersionMismatch() { + final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account); + account.setVersion(account.getVersion() + 77); + + assertThatExceptionOfType(ContestedOptimisticLockException.class) + .isThrownBy(() -> accounts.setUsername(account, "test")); + + assertThat(account.getUsername()).isEmpty(); + } + + @Test + void testClearUsername() throws UsernameNotAvailableException { + final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account); + + final String username = "test"; + + accounts.setUsername(account, username); + assertThat(accounts.getByUsername(username)).isPresent(); + + accounts.clearUsername(account); + + assertThat(accounts.getByUsername(username)).isEmpty(); + assertThat(accounts.getByAccountIdentifier(account.getUuid())) + .hasValueSatisfying(clearedAccount -> assertThat(clearedAccount.getUsername()).isEmpty()); + } + + @Test + void testClearUsernameNoUsername() { + final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account); + + assertThatNoException().isThrownBy(() -> accounts.clearUsername(account)); + } + + @Test + void testClearUsernameVersionMismatch() throws UsernameNotAvailableException { + final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account); + + final String username = "test"; + + accounts.setUsername(account, username); + + account.setVersion(account.getVersion() + 12); + + assertThatExceptionOfType(ContestedOptimisticLockException.class).isThrownBy(() -> accounts.clearUsername(account)); + + assertThat(account.getUsername()).hasValueSatisfying(u -> assertThat(u).isEqualTo(username)); + } + private Device generateDevice(long id) { Random random = new Random(System.currentTimeMillis()); SignedPreKey signedPreKey = new SignedPreKey(random.nextInt(), "testPublicKey-" + random.nextInt(), "testSignature-" + random.nextInt()); 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 a3ac8812f..e1d2ef41f 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 @@ -90,7 +90,7 @@ import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; -import org.whispersystems.textsecuregcm.storage.UsernamesManager; +import org.whispersystems.textsecuregcm.storage.UsernameNotAvailableException; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; import org.whispersystems.textsecuregcm.tests.util.AuthHelper; import org.whispersystems.textsecuregcm.util.Hex; @@ -141,7 +141,6 @@ class AccountControllerTest { private static RecaptchaClient recaptchaClient = mock(RecaptchaClient.class); private static GCMSender gcmSender = mock(GCMSender.class); private static APNSender apnSender = mock(APNSender.class); - private static UsernamesManager usernamesManager = mock(UsernamesManager.class); private static DynamicConfigurationManager dynamicConfigurationManager = mock(DynamicConfigurationManager.class); @@ -164,7 +163,6 @@ class AccountControllerTest { .setTestContainerFactory(new GrizzlyWebTestContainerFactory()) .addResource(new AccountController(pendingAccountsManager, accountsManager, - usernamesManager, abusiveHostRules, rateLimiters, smsSender, @@ -242,8 +240,8 @@ class AccountControllerTest { return account; }); - when(usernamesManager.put(eq(AuthHelper.VALID_UUID), eq("n00bkiller"))).thenReturn(true); - when(usernamesManager.put(eq(AuthHelper.VALID_UUID), eq("takenusername"))).thenReturn(false); + when(accountsManager.setUsername(AuthHelper.VALID_ACCOUNT, "takenusername")) + .thenThrow(new UsernameNotAvailableException()); { DynamicConfiguration dynamicConfiguration = mock(DynamicConfiguration.class); @@ -293,7 +291,6 @@ class AccountControllerTest { recaptchaClient, gcmSender, apnSender, - usernamesManager, verifyExperimentEnrollmentManager); clearInvocations(AuthHelper.DISABLED_DEVICE); @@ -1622,7 +1619,7 @@ class AccountControllerTest { .delete(); assertThat(response.getStatus()).isEqualTo(204); - verify(usernamesManager, times(1)).delete(eq(AuthHelper.VALID_UUID)); + verify(accountsManager).clearUsername(AuthHelper.VALID_ACCOUNT); } @Test diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/ProfileControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/ProfileControllerTest.java index 89fe0902d..116c7fa05 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/ProfileControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/ProfileControllerTest.java @@ -66,7 +66,6 @@ import org.whispersystems.textsecuregcm.storage.AccountBadge; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.storage.ProfilesManager; -import org.whispersystems.textsecuregcm.storage.UsernamesManager; import org.whispersystems.textsecuregcm.storage.VersionedProfile; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; import org.whispersystems.textsecuregcm.tests.util.AuthHelper; @@ -80,7 +79,6 @@ class ProfileControllerTest { private static final Clock clock = mock(Clock.class); private static final AccountsManager accountsManager = mock(AccountsManager.class); private static final ProfilesManager profilesManager = mock(ProfilesManager.class); - private static final UsernamesManager usernamesManager = mock(UsernamesManager.class); private static final RateLimiters rateLimiters = mock(RateLimiters.class); private static final RateLimiter rateLimiter = mock(RateLimiter.class); private static final RateLimiter usernameRateLimiter = mock(RateLimiter.class); @@ -107,7 +105,6 @@ class ProfileControllerTest { rateLimiters, accountsManager, profilesManager, - usernamesManager, dynamicConfigurationManager, (acceptableLanguages, accountBadges, isSelf) -> List.of(new Badge("TEST", "other", "Test Badge", "This badge is in unit tests.", List.of("l", "m", "h", "x", "xx", "xxx"), "SVG", List.of(new BadgeSvg("sl", "sd"), new BadgeSvg("ml", "md"), new BadgeSvg("ll", "ld"))) @@ -156,6 +153,7 @@ class ProfileControllerTest { when(profileAccount.isAnnouncementGroupSupported()).thenReturn(false); when(profileAccount.isChangeNumberSupported()).thenReturn(false); when(profileAccount.getCurrentProfileVersion()).thenReturn(Optional.empty()); + when(profileAccount.getUsername()).thenReturn(Optional.of("n00bkiller")); Account capabilitiesAccount = mock(Account.class); @@ -171,8 +169,7 @@ class ProfileControllerTest { when(accountsManager.getByE164(AuthHelper.VALID_NUMBER_TWO)).thenReturn(Optional.of(profileAccount)); when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID_TWO)).thenReturn(Optional.of(profileAccount)); - when(usernamesManager.get(AuthHelper.VALID_UUID_TWO)).thenReturn(Optional.of("n00bkiller")); - when(usernamesManager.get("n00bkiller")).thenReturn(Optional.of(AuthHelper.VALID_UUID_TWO)); + when(accountsManager.getByUsername("n00bkiller")).thenReturn(Optional.of(profileAccount)); when(accountsManager.getByE164(AuthHelper.VALID_NUMBER)).thenReturn(Optional.of(capabilitiesAccount)); when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(capabilitiesAccount)); @@ -183,7 +180,6 @@ class ProfileControllerTest { clearInvocations(rateLimiter); clearInvocations(accountsManager); - clearInvocations(usernamesManager); clearInvocations(usernameRateLimiter); clearInvocations(profilesManager); } @@ -209,7 +205,6 @@ class ProfileControllerTest { badge -> "Test Badge".equals(badge.getName()), "has badge with expected name")); verify(accountsManager).getByAccountIdentifier(AuthHelper.VALID_UUID_TWO); - verify(usernamesManager, times(1)).get(eq(AuthHelper.VALID_UUID_TWO)); verify(rateLimiter, times(1)).validate(AuthHelper.VALID_UUID); } @@ -229,8 +224,7 @@ class ProfileControllerTest { assertThat(profile.getBadges()).hasSize(1).element(0).has(new Condition<>( badge -> "Test Badge".equals(badge.getName()), "has badge with expected name")); - verify(accountsManager, times(1)).getByAccountIdentifier(eq(AuthHelper.VALID_UUID_TWO)); - verify(usernamesManager, times(1)).get(eq("n00bkiller")); + verify(accountsManager).getByUsername("n00bkiller"); verify(usernameRateLimiter, times(1)).validate(eq(AuthHelper.VALID_UUID)); } @@ -265,8 +259,8 @@ class ProfileControllerTest { assertThat(response.getStatus()).isEqualTo(404); - verify(usernamesManager, times(1)).get(eq("n00bkillerzzzzz")); - verify(usernameRateLimiter, times(1)).validate(eq(AuthHelper.VALID_UUID)); + verify(accountsManager).getByUsername("n00bkillerzzzzz"); + verify(usernameRateLimiter).validate(eq(AuthHelper.VALID_UUID)); } @@ -594,7 +588,6 @@ class ProfileControllerTest { badge -> "Test Badge".equals(badge.getName()), "has badge with expected name")); verify(accountsManager, times(1)).getByAccountIdentifier(eq(AuthHelper.VALID_UUID_TWO)); - verify(usernamesManager, times(1)).get(eq(AuthHelper.VALID_UUID_TWO)); verify(profilesManager, times(1)).get(eq(AuthHelper.VALID_UUID_TWO), eq("validversion")); verify(rateLimiter, times(1)).validate(AuthHelper.VALID_UUID); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/UsernamesManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/UsernamesManagerTest.java deleted file mode 100644 index b068774e1..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/UsernamesManagerTest.java +++ /dev/null @@ -1,185 +0,0 @@ -/* - * Copyright 2013-2020 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.tests.storage; - -import io.lettuce.core.RedisException; -import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; -import org.junit.Test; -import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; -import org.whispersystems.textsecuregcm.storage.ReservedUsernames; -import org.whispersystems.textsecuregcm.storage.Usernames; -import org.whispersystems.textsecuregcm.storage.UsernamesManager; -import org.whispersystems.textsecuregcm.tests.util.RedisClusterHelper; - -import java.util.Optional; -import java.util.UUID; - -import static junit.framework.TestCase.assertSame; -import static junit.framework.TestCase.assertTrue; -import static org.junit.Assert.assertEquals; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; - -public class UsernamesManagerTest { - - @Test - public void testGetByUsernameInCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Usernames usernames = mock(Usernames.class); - ReservedUsernames reserved = mock(ReservedUsernames.class); - - UUID uuid = UUID.randomUUID(); - - when(commands.get(eq("UsernameByUsername::n00bkiller"))).thenReturn(uuid.toString()); - - UsernamesManager usernamesManager = new UsernamesManager(usernames, reserved, cacheCluster); - Optional retrieved = usernamesManager.get("n00bkiller"); - - assertTrue(retrieved.isPresent()); - assertEquals(retrieved.get(), uuid); - - verify(commands, times(1)).get(eq("UsernameByUsername::n00bkiller")); - verifyNoMoreInteractions(commands); - verifyNoMoreInteractions(usernames); - } - - @Test - public void testGetByUuidInCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Usernames usernames = mock(Usernames.class); - ReservedUsernames reserved = mock(ReservedUsernames.class); - - UUID uuid = UUID.randomUUID(); - - when(commands.get(eq("UsernameByUuid::" + uuid.toString()))).thenReturn("n00bkiller"); - - UsernamesManager usernamesManager = new UsernamesManager(usernames, reserved, cacheCluster); - Optional retrieved = usernamesManager.get(uuid); - - assertTrue(retrieved.isPresent()); - assertEquals(retrieved.get(), "n00bkiller"); - - verify(commands, times(1)).get(eq("UsernameByUuid::" + uuid.toString())); - verifyNoMoreInteractions(commands); - verifyNoMoreInteractions(usernames); - } - - - @Test - public void testGetByUsernameNotInCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Usernames usernames = mock(Usernames.class); - ReservedUsernames reserved = mock(ReservedUsernames.class); - - UUID uuid = UUID.randomUUID(); - - when(commands.get(eq("UsernameByUsername::n00bkiller"))).thenReturn(null); - when(usernames.get(eq("n00bkiller"))).thenReturn(Optional.of(uuid)); - - UsernamesManager usernamesManager = new UsernamesManager(usernames, reserved, cacheCluster); - Optional retrieved = usernamesManager.get("n00bkiller"); - - assertTrue(retrieved.isPresent()); - assertSame(retrieved.get(), uuid); - - verify(commands, times(1)).get(eq("UsernameByUsername::n00bkiller")); - verify(commands, times(1)).set(eq("UsernameByUsername::n00bkiller"), eq(uuid.toString())); - verify(commands, times(1)).set(eq("UsernameByUuid::" + uuid.toString()), eq("n00bkiller")); - verify(commands, times(1)).get(eq("UsernameByUuid::" + uuid.toString())); - verifyNoMoreInteractions(commands); - - verify(usernames, times(1)).get(eq("n00bkiller")); - verifyNoMoreInteractions(usernames); - } - - @Test - public void testGetByUuidNotInCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Usernames usernames = mock(Usernames.class); - ReservedUsernames reserved = mock(ReservedUsernames.class); - - UUID uuid = UUID.randomUUID(); - - when(commands.get(eq("UsernameByUuid::" + uuid.toString()))).thenReturn(null); - when(usernames.get(eq(uuid))).thenReturn(Optional.of("n00bkiller")); - - UsernamesManager usernamesManager = new UsernamesManager(usernames, reserved, cacheCluster); - Optional retrieved = usernamesManager.get(uuid); - - assertTrue(retrieved.isPresent()); - assertEquals(retrieved.get(), "n00bkiller"); - - verify(commands, times(2)).get(eq("UsernameByUuid::" + uuid)); - verify(commands, times(1)).set(eq("UsernameByUuid::" + uuid), eq("n00bkiller")); - verify(commands, times(1)).set(eq("UsernameByUsername::n00bkiller"), eq(uuid.toString())); - verifyNoMoreInteractions(commands); - - verify(usernames, times(1)).get(eq(uuid)); - verifyNoMoreInteractions(usernames); - } - - @Test - public void testGetByUsernameBrokenCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Usernames usernames = mock(Usernames.class); - ReservedUsernames reserved = mock(ReservedUsernames.class); - - UUID uuid = UUID.randomUUID(); - - when(commands.get(eq("UsernameByUsername::n00bkiller"))).thenThrow(new RedisException("Connection lost!")); - when(usernames.get(eq("n00bkiller"))).thenReturn(Optional.of(uuid)); - - UsernamesManager usernamesManager = new UsernamesManager(usernames, reserved, cacheCluster); - Optional retrieved = usernamesManager.get("n00bkiller"); - - assertTrue(retrieved.isPresent()); - assertEquals(retrieved.get(), uuid); - - verify(commands, times(1)).get(eq("UsernameByUsername::n00bkiller")); - verify(commands, times(1)).set(eq("UsernameByUsername::n00bkiller"), eq(uuid.toString())); - verify(commands, times(1)).set(eq("UsernameByUuid::" + uuid.toString()), eq("n00bkiller")); - verify(commands, times(1)).get(eq("UsernameByUuid::" + uuid.toString())); - verifyNoMoreInteractions(commands); - - verify(usernames, times(1)).get(eq("n00bkiller")); - verifyNoMoreInteractions(usernames); - } - - @Test - public void testGetAccountByUuidBrokenCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Usernames usernames = mock(Usernames.class); - ReservedUsernames reserved = mock(ReservedUsernames.class); - - UUID uuid = UUID.randomUUID(); - - when(commands.get(eq("UsernameByUuid::" + uuid))).thenThrow(new RedisException("Connection lost!")); - when(usernames.get(eq(uuid))).thenReturn(Optional.of("n00bkiller")); - - UsernamesManager usernamesManager = new UsernamesManager(usernames, reserved, cacheCluster); - Optional retrieved = usernamesManager.get(uuid); - - assertTrue(retrieved.isPresent()); - assertEquals(retrieved.get(), "n00bkiller"); - - verify(commands, times(2)).get(eq("UsernameByUuid::" + uuid)); - verifyNoMoreInteractions(commands); - - verify(usernames, times(1)).get(eq(uuid)); - verifyNoMoreInteractions(usernames); - } - -} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/UsernamesTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/UsernamesTest.java deleted file mode 100644 index 876259988..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/UsernamesTest.java +++ /dev/null @@ -1,169 +0,0 @@ -/* - * Copyright 2013-2020 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.tests.storage; - -import com.opentable.db.postgres.embedded.LiquibasePreparer; -import com.opentable.db.postgres.junit.EmbeddedPostgresRules; -import com.opentable.db.postgres.junit.PreparedDbRule; -import org.jdbi.v3.core.Jdbi; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration; -import org.whispersystems.textsecuregcm.storage.FaultTolerantDatabase; -import org.whispersystems.textsecuregcm.storage.Usernames; - -import java.io.IOException; -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.util.Optional; -import java.util.UUID; - -import static junit.framework.TestCase.assertTrue; -import static org.assertj.core.api.AssertionsForClassTypes.assertThat; -import static org.junit.Assert.assertFalse; - -public class UsernamesTest { - - @Rule - public PreparedDbRule db = EmbeddedPostgresRules.preparedDatabase(LiquibasePreparer.forClasspathLocation("accountsdb.xml")); - - private Usernames usernames; - - @Before - public void setupAccountsDao() { - FaultTolerantDatabase faultTolerantDatabase = new FaultTolerantDatabase("usernamesTest", - Jdbi.create(db.getTestDatabase()), - new CircuitBreakerConfiguration()); - - this.usernames = new Usernames(faultTolerantDatabase); - } - - @Test - public void testPut() throws SQLException, IOException { - UUID uuid = UUID.randomUUID(); - String username = "myusername"; - - assertTrue(usernames.put(uuid, username)); - - PreparedStatement statement = db.getTestDatabase().getConnection().prepareStatement("SELECT * FROM usernames WHERE uuid = ?"); - verifyStoredState(statement, uuid, username); - } - - @Test - public void testPutChange() throws SQLException, IOException { - UUID uuid = UUID.randomUUID(); - String firstUsername = "myfirstusername"; - String secondUsername = "mysecondusername"; - - assertTrue(usernames.put(uuid, firstUsername)); - - PreparedStatement statement = db.getTestDatabase().getConnection().prepareStatement("SELECT * FROM usernames WHERE uuid = ?"); - verifyStoredState(statement, uuid, firstUsername); - - assertTrue(usernames.put(uuid, secondUsername)); - - verifyStoredState(statement, uuid, secondUsername); - } - - @Test - public void testPutConflict() throws SQLException { - UUID firstUuid = UUID.randomUUID(); - UUID secondUuid = UUID.randomUUID(); - - String username = "myfirstusername"; - - assertTrue(usernames.put(firstUuid, username)); - assertFalse(usernames.put(secondUuid, username)); - - PreparedStatement statement = db.getTestDatabase().getConnection().prepareStatement("SELECT * FROM usernames WHERE username = ?"); - statement.setString(1, username); - - ResultSet resultSet = statement.executeQuery(); - - assertTrue(resultSet.next()); - assertThat(resultSet.getString("uuid")).isEqualTo(firstUuid.toString()); - assertThat(resultSet.next()).isFalse(); - } - - @Test - public void testGetByUuid() { - UUID uuid = UUID.randomUUID(); - String username = "myusername"; - - assertTrue(usernames.put(uuid, username)); - - Optional retrieved = usernames.get(uuid); - - assertTrue(retrieved.isPresent()); - assertThat(retrieved.get()).isEqualTo(username); - } - - @Test - public void testGetByUuidMissing() { - Optional retrieved = usernames.get(UUID.randomUUID()); - assertFalse(retrieved.isPresent()); - } - - @Test - public void testGetByUsername() { - UUID uuid = UUID.randomUUID(); - String username = "myusername"; - - assertTrue(usernames.put(uuid, username)); - - Optional retrieved = usernames.get(username); - - assertTrue(retrieved.isPresent()); - assertThat(retrieved.get()).isEqualTo(uuid); - } - - @Test - public void testGetByUsernameMissing() { - Optional retrieved = usernames.get("myusername"); - - assertFalse(retrieved.isPresent()); - } - - - @Test - public void testDelete() { - UUID uuid = UUID.randomUUID(); - String username = "myusername"; - - assertTrue(usernames.put(uuid, username)); - - Optional retrieved = usernames.get(username); - - assertTrue(retrieved.isPresent()); - assertThat(retrieved.get()).isEqualTo(uuid); - - usernames.delete(uuid); - - assertThat(usernames.get(uuid).isPresent()).isFalse(); - } - - private void verifyStoredState(PreparedStatement statement, UUID uuid, String expectedUsername) - throws SQLException, IOException - { - statement.setObject(1, uuid); - - ResultSet resultSet = statement.executeQuery(); - - if (resultSet.next()) { - String data = resultSet.getString("username"); - assertThat(data).isNotEmpty(); - assertThat(data).isEqualTo(expectedUsername); - } else { - throw new AssertionError("No data"); - } - - assertThat(resultSet.next()).isFalse(); - } - - -} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java index 2f659acc7..6c0702f23 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java @@ -103,6 +103,10 @@ public class AccountsHelper { when(updatedAccount.getNumber()).thenAnswer(stubbing); break; } + case "getUsername": { + when(updatedAccount.getUsername()).thenAnswer(stubbing); + break; + } case "getDevices": { when(updatedAccount.getDevices()) .thenAnswer(stubbing);