From 4032ddd4fd9c21b32d1c2469d2d93986362b7d5c Mon Sep 17 00:00:00 2001 From: Ravi Khadiwala Date: Wed, 31 Aug 2022 21:02:09 -0500 Subject: [PATCH] Add reserve/confirm for usernames --- .../textsecuregcm/WhisperServerService.java | 6 +- .../RateLimitsConfiguration.java | 7 + .../configuration/UsernameConfiguration.java | 8 + .../controllers/AccountController.java | 77 +++++-- .../entities/ConfirmUsernameRequest.java | 12 ++ .../entities/ReserveUsernameRequest.java | 12 ++ .../entities/ReserveUsernameResponse.java | 10 + .../textsecuregcm/limits/RateLimiters.java | 11 + .../textsecuregcm/storage/Account.java | 16 ++ .../textsecuregcm/storage/Accounts.java | 188 ++++++++++++++++-- .../storage/AccountsManager.java | 114 ++++++++++- ...sernames.java => ProhibitedUsernames.java} | 20 +- .../UsernameReservationNotFoundException.java | 10 + .../textsecuregcm/util/UsernameGenerator.java | 14 +- .../workers/AssignUsernameCommand.java | 6 +- .../workers/DeleteUserCommand.java | 6 +- .../workers/ReserveUsernameCommand.java | 6 +- .../SetUserDiscoverabilityCommand.java | 6 +- ...ntsManagerChangeNumberIntegrationTest.java | 2 +- ...ConcurrentModificationIntegrationTest.java | 2 +- .../storage/AccountsManagerTest.java | 107 +++++++++- ...ccountsManagerUsernameIntegrationTest.java | 147 +++++++++++++- .../textsecuregcm/storage/AccountsTest.java | 163 ++++++++++++++- ...Test.java => ProhibitedUsernamesTest.java} | 18 +- .../controllers/AccountControllerTest.java | 65 ++++++ .../tests/util/UsernameGeneratorTest.java | 19 +- 26 files changed, 956 insertions(+), 96 deletions(-) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameRequest.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameRequest.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameResponse.java rename service/src/main/java/org/whispersystems/textsecuregcm/storage/{ReservedUsernames.java => ProhibitedUsernames.java} (79%) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameReservationNotFoundException.java rename service/src/test/java/org/whispersystems/textsecuregcm/storage/{ReservedUsernamesTest.java => ProhibitedUsernamesTest.java} (79%) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index f7c671ada..a5793e884 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -195,7 +195,7 @@ import org.whispersystems.textsecuregcm.storage.RemoteConfigs; import org.whispersystems.textsecuregcm.storage.RemoteConfigsManager; import org.whispersystems.textsecuregcm.storage.ReportMessageDynamoDb; import org.whispersystems.textsecuregcm.storage.ReportMessageManager; -import org.whispersystems.textsecuregcm.storage.ReservedUsernames; +import org.whispersystems.textsecuregcm.storage.ProhibitedUsernames; import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; import org.whispersystems.textsecuregcm.storage.SubscriptionManager; import org.whispersystems.textsecuregcm.storage.VerificationCodeStore; @@ -337,7 +337,7 @@ public class WhisperServerService extends Application new IllegalStateException("Could not get username after setting")); + } catch (final UsernameReservationNotFoundException e) { + throw new WebApplicationException(Status.CONFLICT); + } catch (final UsernameNotAvailableException e) { + throw new WebApplicationException(Status.GONE); + } + } + @Timed @PUT @Path("/username") @@ -652,14 +702,7 @@ public class AccountController { @HeaderParam("X-Signal-Agent") String userAgent, @NotNull @Valid UsernameRequest usernameRequest) throws RateLimitExceededException { rateLimiters.getUsernameSetLimiter().validate(auth.getAccount().getUuid()); - - if (StringUtils.isNotBlank(usernameRequest.existingUsername()) && - !UsernameGenerator.isStandardFormat(usernameRequest.existingUsername())) { - // Technically, a username may not be in the nickname#discriminator format - // if created through some out-of-band mechanism, but it is atypical. - Metrics.counter(NONSTANDARD_USERNAME_COUNTER_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))) - .increment(); - } + checkUsername(usernameRequest.existingUsername(), userAgent); try { final Account account = accounts.setUsername(auth.getAccount(), usernameRequest.nickname(), @@ -688,15 +731,10 @@ public class AccountController { throw new BadRequestException(); } - if (!UsernameGenerator.isStandardFormat(username)) { - // Technically, a username may not be in the nickname#discriminator format - // if created through some out-of-band mechanism, but it is atypical. - Metrics.counter(NONSTANDARD_USERNAME_COUNTER_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))) - .increment(); - } - rateLimitByClientIp(rateLimiters.getUsernameLookupLimiter(), forwardedFor); + checkUsername(username, userAgent); + return accounts .getByUsername(username) .map(Account::getUuid) @@ -866,6 +904,15 @@ public class AccountController { accounts.delete(auth.getAccount(), AccountsManager.DeletionReason.USER_REQUEST); } + private void checkUsername(final String username, final String userAgent) { + if (StringUtils.isNotBlank(username) && !UsernameGenerator.isStandardFormat(username)) { + // Technically, a username may not be in the nickname#discriminator format + // if created through some out-of-band mechanism, but it is atypical. + Metrics.counter(NONSTANDARD_USERNAME_COUNTER_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))) + .increment(); + } + } + private boolean shouldAutoBlock(String sourceHost) { try { rateLimiters.getAutoBlockLimiter().validate(sourceHost); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameRequest.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameRequest.java new file mode 100644 index 000000000..49110012e --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameRequest.java @@ -0,0 +1,12 @@ +/* + * Copyright 2022 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.entities; + +import javax.validation.constraints.NotBlank; +import javax.validation.constraints.NotNull; +import java.util.UUID; + +public record ConfirmUsernameRequest(@NotBlank String usernameToConfirm, @NotNull UUID reservationToken) {} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameRequest.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameRequest.java new file mode 100644 index 000000000..4c1433860 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameRequest.java @@ -0,0 +1,12 @@ +/* + * Copyright 2022 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.entities; + +import org.whispersystems.textsecuregcm.util.Nickname; + +import javax.validation.Valid; + +public record ReserveUsernameRequest(@Valid @Nickname String nickname) {} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameResponse.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameResponse.java new file mode 100644 index 000000000..de16f488f --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameResponse.java @@ -0,0 +1,10 @@ +/* + * Copyright 2022 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.entities; + +import java.util.UUID; + +public record ReserveUsernameResponse(String username, UUID reservationToken) {} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimiters.java b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimiters.java index f39b8cefb..4dee74de5 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimiters.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimiters.java @@ -33,6 +33,8 @@ public class RateLimiters { private final RateLimiter usernameLookupLimiter; private final RateLimiter usernameSetLimiter; + private final RateLimiter usernameReserveLimiter; + private final RateLimiter checkAccountExistenceLimiter; public RateLimiters(RateLimitsConfiguration config, FaultTolerantRedisCluster cacheCluster) { @@ -108,6 +110,11 @@ public class RateLimiters { config.getUsernameSet().getBucketSize(), config.getUsernameSet().getLeakRatePerMinute()); + this.usernameReserveLimiter = new RateLimiter(cacheCluster, "usernameReserve", + config.getUsernameReserve().getBucketSize(), + config.getUsernameReserve().getLeakRatePerMinute()); + + this.checkAccountExistenceLimiter = new RateLimiter(cacheCluster, "checkAccountExistence", config.getCheckAccountExistence().getBucketSize(), config.getCheckAccountExistence().getLeakRatePerMinute()); @@ -185,6 +192,10 @@ public class RateLimiters { return usernameSetLimiter; } + public RateLimiter getUsernameReserveLimiter() { + return usernameReserveLimiter; + } + public RateLimiter getCheckAccountExistenceLimiter() { return checkAccountExistenceLimiter; } 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 18d02560b..7f90a4d86 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java @@ -42,6 +42,10 @@ public class Account { @Nullable private String username; + @JsonProperty + @Nullable + private byte[] reservedUsernameHash; + @JsonProperty private List devices = new ArrayList<>(); @@ -133,6 +137,18 @@ public class Account { this.username = username; } + public Optional getReservedUsernameHash() { + requireNotStale(); + + return Optional.ofNullable(reservedUsernameHash); + } + + public void setReservedUsernameHash(final byte[] reservedUsernameHash) { + requireNotStale(); + + this.reservedUsernameHash = reservedUsernameHash; + } + 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 c902f39d0..3e79bd41f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java @@ -13,6 +13,10 @@ import io.micrometer.core.instrument.Metrics; import io.micrometer.core.instrument.Timer; import java.io.IOException; import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -70,7 +74,10 @@ public class Accounts extends AbstractDynamoDbStore { static final String ATTR_USERNAME = "N"; // unidentified access key; byte[] or null static final String ATTR_UAK = "UAK"; + // time to live; number + static final String ATTR_TTL = "TTL"; + private final Clock clock; private final DynamoDbClient client; private final DynamoDbAsyncClient asyncClient; @@ -81,9 +88,12 @@ public class Accounts extends AbstractDynamoDbStore { private final int scanPageSize; + private static final byte RESERVED_USERNAME_HASH_VERSION = 1; + 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 RESERVE_USERNAME_TIMER = Metrics.timer(name(Accounts.class, "reserveUsername")); 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")); @@ -96,13 +106,16 @@ public class Accounts extends AbstractDynamoDbStore { private static final Logger log = LoggerFactory.getLogger(Accounts.class); - public Accounts(final DynamicConfigurationManager dynamicConfigurationManager, + @VisibleForTesting + public Accounts( + final Clock clock, + final DynamicConfigurationManager dynamicConfigurationManager, DynamoDbClient client, DynamoDbAsyncClient asyncClient, String accountsTableName, String phoneNumberConstraintTableName, String phoneNumberIdentifierConstraintTableName, final String usernamesConstraintTableName, final int scanPageSize) { - super(client); + this.clock = clock; this.client = client; this.asyncClient = asyncClient; this.phoneNumberConstraintTableName = phoneNumberConstraintTableName; @@ -112,6 +125,16 @@ public class Accounts extends AbstractDynamoDbStore { this.scanPageSize = scanPageSize; } + public Accounts(final DynamicConfigurationManager dynamicConfigurationManager, + DynamoDbClient client, DynamoDbAsyncClient asyncClient, + String accountsTableName, String phoneNumberConstraintTableName, + String phoneNumberIdentifierConstraintTableName, final String usernamesConstraintTableName, + final int scanPageSize) { + this(Clock.systemUTC(), dynamicConfigurationManager, client, asyncClient, accountsTableName, + phoneNumberConstraintTableName, phoneNumberIdentifierConstraintTableName, usernamesConstraintTableName, + scanPageSize); + } + public boolean create(Account account) { return CREATE_TIMER.record(() -> { @@ -331,33 +354,150 @@ public class Accounts extends AbstractDynamoDbStore { }); } + public static byte[] reservedUsernameHash(final UUID accountId, final String reservedUsername) { + final MessageDigest sha256; + try { + sha256 = MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + throw new AssertionError(e); + } + final ByteBuffer byteBuffer = ByteBuffer.allocate(32 + 1); + sha256.update(reservedUsername.getBytes(StandardCharsets.UTF_8)); + sha256.update(UUIDUtil.toBytes(accountId)); + byteBuffer.put(RESERVED_USERNAME_HASH_VERSION); + byteBuffer.put(sha256.digest()); + return byteBuffer.array(); + } + /** - * Set the account username + * Reserve a username under a token * - * @param account to update - * @param username believed to be available - * @throws ContestedOptimisticLockException if the account has been updated or the username taken by someone else + * @return a reservation token that must be provided when {@link #confirmUsername(Account, String, UUID)} is called */ - public void setUsername(final Account account, final String username) - throws ContestedOptimisticLockException { + public UUID reserveUsername( + final Account account, + final String reservedUsername, + final Duration ttl) { final long startNanos = System.nanoTime(); - final Optional maybeOriginalUsername = account.getUsername(); - account.setUsername(username); + // if there is an existing old reservation it will be cleaned up via ttl + final Optional maybeOriginalReservation = account.getReservedUsernameHash(); + account.setReservedUsernameHash(reservedUsernameHash(account.getUuid(), reservedUsername)); boolean succeeded = false; + long expirationTime = clock.instant().plus(ttl).getEpochSecond(); + + final UUID reservationToken = UUID.randomUUID(); try { final List writeItems = new ArrayList<>(); + writeItems.add(TransactWriteItem.builder() + .put(Put.builder() + .tableName(usernamesConstraintTableName) + .item(Map.of( + KEY_ACCOUNT_UUID, AttributeValues.fromUUID(reservationToken), + ATTR_USERNAME, AttributeValues.fromString(reservedUsername), + ATTR_TTL, AttributeValues.fromLong(expirationTime))) + .conditionExpression("attribute_not_exists(#username) OR (#ttl < :now)") + .expressionAttributeNames(Map.of("#username", ATTR_USERNAME, "#ttl", ATTR_TTL)) + .expressionAttributeValues(Map.of(":now", AttributeValues.fromLong(clock.instant().getEpochSecond()))) + .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 ADD #version :version_increment") + .conditionExpression("#version = :version") + .expressionAttributeNames(Map.of("#data", ATTR_ACCOUNT_DATA, "#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()); + + 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 (e.cancellationReasons().stream().map(CancellationReason::code).anyMatch("ConditionalCheckFailed"::equals)) { + throw new ContestedOptimisticLockException(); + } + throw e; + } finally { + if (!succeeded) { + account.setReservedUsernameHash(maybeOriginalReservation.orElse(null)); + } + RESERVE_USERNAME_TIMER.record(System.nanoTime() - startNanos, TimeUnit.NANOSECONDS); + } + return reservationToken; + } + + /** + * Confirm (set) a previously reserved username + * + * @param account to update + * @param username believed to be available + * @param reservationToken a token returned by the call to {@link #reserveUsername(Account, String, Duration)}, + * only required if setting a reserved username + * @throws ContestedOptimisticLockException if the account has been updated or the username taken by someone else + */ + public void confirmUsername(final Account account, final String username, final UUID reservationToken) + throws ContestedOptimisticLockException { + setUsername(account, username, Optional.of(reservationToken)); + } + + /** + * Set the account username + * + * @param account to update + * @param username believed to be available + * @throws ContestedOptimisticLockException if the account has been updated or the username taken by someone else + */ + public void setUsername(final Account account, final String username) throws ContestedOptimisticLockException { + setUsername(account, username, Optional.empty()); + } + + private void setUsername(final Account account, final String username, final Optional reservationToken) + throws ContestedOptimisticLockException { + final long startNanos = System.nanoTime(); + + final Optional maybeOriginalUsername = account.getUsername(); + final Optional maybeOriginalReservation = account.getReservedUsernameHash(); + + account.setUsername(username); + account.setReservedUsernameHash(null); + + boolean succeeded = false; + + try { + final List writeItems = new ArrayList<>(); + + // add the username to the constraint table, wiping out the ttl if we had already reserved the name 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)) + // it's not in the constraint table OR it's expired OR it was reserved by us + .conditionExpression("attribute_not_exists(#username) OR #ttl < :now OR #aci = :reservation ") + .expressionAttributeNames(Map.of("#username", ATTR_USERNAME, "#ttl", ATTR_TTL, "#aci", KEY_ACCOUNT_UUID)) + .expressionAttributeValues(Map.of( + ":now", AttributeValues.fromLong(clock.instant().getEpochSecond()), + ":reservation", AttributeValues.fromUUID(reservationToken.orElseGet(UUID::randomUUID)))) .returnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD) .build()) .build()); @@ -405,6 +545,7 @@ public class Accounts extends AbstractDynamoDbStore { } finally { if (!succeeded) { account.setUsername(maybeOriginalUsername.orElse(null)); + account.setReservedUsernameHash(maybeOriginalReservation.orElse(null)); } SET_USERNAME_TIMER.record(System.nanoTime() - startNanos, TimeUnit.NANOSECONDS); } @@ -553,11 +694,29 @@ public class Accounts extends AbstractDynamoDbStore { } public boolean usernameAvailable(final String username) { + return usernameAvailable(Optional.empty(), username); + } + + public boolean usernameAvailable(final Optional reservationToken, final String username) { final GetItemResponse response = client.getItem(GetItemRequest.builder() .tableName(usernamesConstraintTableName) .key(Map.of(ATTR_USERNAME, AttributeValues.fromString(username))) .build()); - return !response.hasItem(); + if (!response.hasItem()) { + // username is free + return true; + } + final Map item = response.item(); + + if (AttributeValues.getLong(item, ATTR_TTL, Long.MAX_VALUE) < clock.instant().getEpochSecond()) { + // username was reserved, but has expired + return true; + } + + // username is reserved by us + return reservationToken + .map(AttributeValues.getUUID(item, KEY_ACCOUNT_UUID, new UUID(0, 0))::equals) + .orElse(false); } public Optional getByE164(String number) { @@ -583,7 +742,10 @@ public class Accounts extends AbstractDynamoDbStore { .key(Map.of(ATTR_USERNAME, AttributeValues.fromString(username))) .build()); + return Optional.ofNullable(response.item()) + // ignore items with a ttl (reservations) + .filter(item -> !item.containsKey(ATTR_TTL)) .map(item -> item.get(KEY_ACCOUNT_UUID)) .map(this::accountByUuid) .map(Accounts::fromItem); 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 f50b8d67c..2bbc1868d 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -21,6 +21,7 @@ import io.micrometer.core.instrument.Tags; import java.io.IOException; import java.time.Clock; import java.time.Duration; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; @@ -86,7 +87,7 @@ public class AccountsManager { private final DirectoryQueue directoryQueue; private final Keys keys; private final MessagesManager messagesManager; - private final ReservedUsernames reservedUsernames; + private final ProhibitedUsernames prohibitedUsernames; private final ProfilesManager profilesManager; private final StoredVerificationCodeManager pendingAccounts; private final SecureStorageClient secureStorageClient; @@ -128,7 +129,7 @@ public class AccountsManager { final DirectoryQueue directoryQueue, final Keys keys, final MessagesManager messagesManager, - final ReservedUsernames reservedUsernames, + final ProhibitedUsernames prohibitedUsernames, final ProfilesManager profilesManager, final StoredVerificationCodeManager pendingAccounts, final SecureStorageClient secureStorageClient, @@ -149,7 +150,7 @@ public class AccountsManager { this.secureStorageClient = secureStorageClient; this.secureBackupClient = secureBackupClient; this.clientPresenceManager = clientPresenceManager; - this.reservedUsernames = reservedUsernames; + this.prohibitedUsernames = prohibitedUsernames; this.usernameGenerator = usernameGenerator; this.experimentEnrollmentManager = experimentEnrollmentManager; this.clock = Objects.requireNonNull(clock); @@ -322,12 +323,112 @@ public class AccountsManager { return updatedAccount.get(); } + public record UsernameReservation(Account account, String reservedUsername, UUID reservationToken){} + + /** + * Generate a username from a nickname, and reserve it so no other accounts may take it. + * + * The reserved username can later be set with {@link #confirmReservedUsername(Account, String, UUID)}. The reservation + * will eventually expire, after which point confirmReservedUsername may fail if another account has taken the + * username. + * + * @param account the account to update + * @param requestedNickname the nickname to reserve a username for + * @return the reserved username and an updated Account object + * @throws UsernameNotAvailableException no username is available for the requested nickname + */ + public UsernameReservation reserveUsername(final Account account, final String requestedNickname) throws UsernameNotAvailableException { + if (!experimentEnrollmentManager.isEnrolled(account.getUuid(), USERNAME_EXPERIMENT_NAME)) { + throw new UsernameNotAvailableException(); + } + + if (prohibitedUsernames.isProhibited(requestedNickname, account.getUuid())) { + throw new UsernameNotAvailableException(); + } + redisDelete(account); + + class Reserver implements AccountPersister { + UUID reservationToken; + String reservedUsername; + + @Override + public void persistAccount(final Account account) throws UsernameNotAvailableException { + // In the future, this may also check for any forbidden discriminators + reservedUsername = usernameGenerator.generateAvailableUsername(requestedNickname, accounts::usernameAvailable); + reservationToken = accounts.reserveUsername( + account, + reservedUsername, + usernameGenerator.getReservationTtl()); + } + } + final Reserver reserver = new Reserver(); + final Account updatedAccount = failableUpdateWithRetries( + account, + a -> true, + reserver, + () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), + AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); + return new UsernameReservation(updatedAccount, reserver.reservedUsername, reserver.reservationToken); + } + + /** + * Set a username previously reserved with {@link #reserveUsername(Account, String)} + * + * @param account the account to update + * @param reservedUsername the previously reserved username + * @param reservationToken the UUID returned from the reservation + * @return the updated account with the username field set + * @throws UsernameNotAvailableException if the reserved username has been taken (because the reservation expired) + * @throws UsernameReservationNotFoundException if `reservedUsername` was not reserved in the account + */ + public Account confirmReservedUsername(final Account account, final String reservedUsername, final UUID reservationToken) throws UsernameNotAvailableException, UsernameReservationNotFoundException { + if (!experimentEnrollmentManager.isEnrolled(account.getUuid(), USERNAME_EXPERIMENT_NAME)) { + throw new UsernameNotAvailableException(); + } + + if (account.getUsername().map(reservedUsername::equals).orElse(false)) { + // the client likely already succeeded and is retrying + return account; + } + + final byte[] newHash = Accounts.reservedUsernameHash(account.getUuid(), reservedUsername); + if (!account.getReservedUsernameHash().map(oldHash -> Arrays.equals(oldHash, newHash)).orElse(false)) { + // no such reservation existed, either there was no previous call to reserveUsername + // or the reservation changed + throw new UsernameReservationNotFoundException(); + } + + redisDelete(account); + + return failableUpdateWithRetries( + account, + a -> true, + a -> { + // though we know this username was reserved, the reservation could have lapsed + if (!accounts.usernameAvailable(Optional.of(reservationToken), reservedUsername)) { + throw new UsernameNotAvailableException(); + } + accounts.confirmUsername(a, reservedUsername, reservationToken); + }, + () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), + AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); + } + + /** + * Sets a username generated from `requestedNickname` with no prior reservation + * + * @param account the account to update + * @param requestedNickname the nickname to generate a username from + * @param expectedOldUsername the expected existing username of the account (for replay detection) + * @return the updated account with the username field set + * @throws UsernameNotAvailableException if no free username could be set for `requestedNickname` + */ public Account setUsername(final Account account, final String requestedNickname, final @Nullable String expectedOldUsername) throws UsernameNotAvailableException { if (!experimentEnrollmentManager.isEnrolled(account.getUuid(), USERNAME_EXPERIMENT_NAME)) { throw new UsernameNotAvailableException(); } - if (reservedUsernames.isReserved(requestedNickname, account.getUuid())) { + if (prohibitedUsernames.isProhibited(requestedNickname, account.getUuid())) { throw new UsernameNotAvailableException(); } @@ -346,7 +447,9 @@ public class AccountsManager { account, a -> true, // In the future, this may also check for any forbidden discriminators - a -> accounts.setUsername(a, usernameGenerator.generateAvailableUsername(requestedNickname, accounts::usernameAvailable)), + a -> accounts.setUsername( + a, + usernameGenerator.generateAvailableUsername(requestedNickname, accounts::usernameAvailable)), () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); } @@ -539,7 +642,6 @@ 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); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ReservedUsernames.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernames.java similarity index 79% rename from service/src/main/java/org/whispersystems/textsecuregcm/storage/ReservedUsernames.java rename to service/src/main/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernames.java index 625026a7a..c44cdb98c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ReservedUsernames.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernames.java @@ -26,7 +26,7 @@ import software.amazon.awssdk.services.dynamodb.model.ScanRequest; import software.amazon.awssdk.services.dynamodb.model.ScanResponse; import software.amazon.awssdk.services.dynamodb.paginators.ScanIterable; -public class ReservedUsernames { +public class ProhibitedUsernames { private final DynamoDbClient dynamoDbClient; private final String tableName; @@ -44,17 +44,17 @@ public class ReservedUsernames { static final String KEY_PATTERN = "P"; private static final String ATTR_RESERVED_FOR_UUID = "U"; - private static final Timer IS_RESERVED_TIMER = Metrics.timer(name(ReservedUsernames.class, "isReserved")); + private static final Timer IS_PROHIBITED_TIMER = Metrics.timer(name(ProhibitedUsernames.class, "isProhibited")); - private static final Logger log = LoggerFactory.getLogger(ReservedUsernames.class); + private static final Logger log = LoggerFactory.getLogger(ProhibitedUsernames.class); - public ReservedUsernames(final DynamoDbClient dynamoDbClient, final String tableName) { + public ProhibitedUsernames(final DynamoDbClient dynamoDbClient, final String tableName) { this.dynamoDbClient = dynamoDbClient; this.tableName = tableName; } - public boolean isReserved(final String nickname, final UUID accountIdentifier) { - return IS_RESERVED_TIMER.record(() -> { + public boolean isProhibited(final String nickname, final UUID accountIdentifier) { + return IS_PROHIBITED_TIMER.record(() -> { final ScanIterable scanIterable = dynamoDbClient.scanPaginator(ScanRequest.builder() .tableName(tableName) .build()); @@ -80,7 +80,13 @@ public class ReservedUsernames { }); } - public void reserveUsername(final String pattern, final UUID reservedFor) { + /** + * Prohibits username except for all accounts except `reservedFor` + * + * @param pattern pattern to prohibit + * @param reservedFor an account that is allowed to use names in the pattern + */ + public void prohibitUsername(final String pattern, final UUID reservedFor) { dynamoDbClient.putItem(PutItemRequest.builder() .tableName(tableName) .item(Map.of( diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameReservationNotFoundException.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameReservationNotFoundException.java new file mode 100644 index 000000000..066e89994 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameReservationNotFoundException.java @@ -0,0 +1,10 @@ +/* + * Copyright 2022 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.storage; + +public class UsernameReservationNotFoundException extends Exception { + +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java index 222d00954..d8000715a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java @@ -13,6 +13,7 @@ import io.micrometer.core.instrument.Metrics; import org.apache.commons.lang3.StringUtils; import org.whispersystems.textsecuregcm.configuration.UsernameConfiguration; import org.whispersystems.textsecuregcm.storage.UsernameNotAvailableException; +import java.time.Duration; import java.util.concurrent.ThreadLocalRandom; import java.util.function.Predicate; import java.util.regex.Pattern; @@ -41,16 +42,21 @@ public class UsernameGenerator { private final int initialWidth; private final int discriminatorMaxWidth; private final int attemptsPerWidth; + private final Duration reservationTtl; public UsernameGenerator(UsernameConfiguration configuration) { - this(configuration.getDiscriminatorInitialWidth(), configuration.getDiscriminatorMaxWidth(), configuration.getAttemptsPerWidth()); + this(configuration.getDiscriminatorInitialWidth(), + configuration.getDiscriminatorMaxWidth(), + configuration.getAttemptsPerWidth(), + configuration.getReservationTtl()); } @VisibleForTesting - public UsernameGenerator(int initialWidth, int discriminatorMaxWidth, int attemptsPerWidth) { + public UsernameGenerator(int initialWidth, int discriminatorMaxWidth, int attemptsPerWidth, final Duration reservationTtl) { this.initialWidth = initialWidth; this.discriminatorMaxWidth = discriminatorMaxWidth; this.attemptsPerWidth = attemptsPerWidth; + this.reservationTtl = reservationTtl; } /** @@ -109,6 +115,10 @@ public class UsernameGenerator { return String.format("%s#%0" + initialWidth + "d", nickname, discriminator); } + public Duration getReservationTtl() { + return reservationTtl; + } + public static boolean isValidNickname(final String nickname) { return StringUtils.isNotBlank(nickname) && NICKNAME_PATTERN.matcher(nickname).matches(); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java index 1f122b744..1696b61cf 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java @@ -47,7 +47,7 @@ import org.whispersystems.textsecuregcm.storage.Profiles; 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.ProhibitedUsernames; import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; import org.whispersystems.textsecuregcm.storage.UsernameNotAvailableException; import org.whispersystems.textsecuregcm.storage.VerificationCodeStore; @@ -152,7 +152,7 @@ public class AssignUsernameCommand extends EnvironmentCommand phoneNumberIdentifiersByE164; @@ -87,6 +87,8 @@ class AccountsManagerTest { return null; }; + private static final UUID RESERVATION_TOKEN = UUID.randomUUID(); + @BeforeEach void setup() throws InterruptedException { accounts = mock(Accounts.class); @@ -95,7 +97,7 @@ class AccountsManagerTest { keys = mock(Keys.class); messagesManager = mock(MessagesManager.class); profilesManager = mock(ProfilesManager.class); - reservedUsernames = mock(ReservedUsernames.class); + prohibitedUsernames = mock(ProhibitedUsernames.class); //noinspection unchecked commands = mock(RedisAdvancedClusterCommands.class); @@ -149,7 +151,7 @@ class AccountsManagerTest { directoryQueue, keys, messagesManager, - reservedUsernames, + prohibitedUsernames, profilesManager, mock(StoredVerificationCodeManager.class), storageClient, @@ -737,6 +739,65 @@ class AccountsManagerTest { verify(accounts).setUsername(eq(account), startsWith(nickname)); } + @Test + void testReserveUsername() throws UsernameNotAvailableException { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + final String nickname = "beethoven"; + accountsManager.reserveUsername(account, nickname); + verify(accounts).reserveUsername(eq(account), startsWith(nickname), any()); + } + + @Test + void testSetReservedUsername() throws UsernameNotAvailableException, UsernameReservationNotFoundException { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + final String reserved = "scooby#1234"; + setReservationHash(account, reserved); + when(accounts.usernameAvailable(eq(Optional.of(RESERVATION_TOKEN)), eq(reserved))).thenReturn(true); + accountsManager.confirmReservedUsername(account, reserved, RESERVATION_TOKEN); + verify(accounts).confirmUsername(eq(account), eq(reserved), eq(RESERVATION_TOKEN)); + } + + @Test + void testSetReservedHashNameMismatch() { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + setReservationHash(account, "pluto#1234"); + when(accounts.usernameAvailable(eq(Optional.of(RESERVATION_TOKEN)), eq("pluto#1234"))).thenReturn(true); + assertThrows(UsernameReservationNotFoundException.class, + () -> accountsManager.confirmReservedUsername(account, "goofy#1234", RESERVATION_TOKEN)); + } + + @Test + void testSetReservedHashAciMismatch() { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + final String reserved = "toto#1234"; + account.setReservedUsernameHash(Accounts.reservedUsernameHash(UUID.randomUUID(), reserved)); + when(accounts.usernameAvailable(eq(Optional.of(RESERVATION_TOKEN)), eq(reserved))).thenReturn(true); + assertThrows(UsernameReservationNotFoundException.class, + () -> accountsManager.confirmReservedUsername(account, reserved, RESERVATION_TOKEN)); + } + + @Test + void testSetReservedLapsed() { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + final String reserved = "porkchop#1234"; + // name was reserved, but the reservation lapsed and another account took it + setReservationHash(account, reserved); + when(accounts.usernameAvailable(eq(Optional.of(RESERVATION_TOKEN)), eq(reserved))).thenReturn(false); + assertThrows(UsernameNotAvailableException.class, () -> accountsManager.confirmReservedUsername(account, reserved, RESERVATION_TOKEN)); + verify(accounts, never()).confirmUsername(any(), any(), any()); + } + + @Test + void testSetReservedRetry() throws UsernameNotAvailableException, UsernameReservationNotFoundException { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + final String username = "santaslittlehelper#1234"; + account.setUsername(username); + + // reserved username already set, should be treated as a replay + accountsManager.confirmReservedUsername(account, username, RESERVATION_TOKEN); + verifyNoInteractions(accounts); + } + @Test void testSetUsernameSameUsername() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); @@ -761,7 +822,29 @@ class AccountsManagerTest { } @Test - void testSetUsernameExpandDiscriminator() throws UsernameNotAvailableException { + void testReserveUsernameReroll() throws UsernameNotAvailableException { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + final String nickname = "clifford"; + final String username = nickname + "#ZZZ"; + account.setUsername(username); + + // given the correct old username, should reroll discriminator even if the nick matches + accountsManager.reserveUsername(account, nickname); + verify(accounts).reserveUsername(eq(account), and(startsWith(nickname), not(eq(username))), any()); + } + + @Test + void testSetReservedUsernameWithNoReservation() { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), + new ArrayList<>(), new byte[16]); + assertThrows(UsernameReservationNotFoundException.class, + () -> accountsManager.confirmReservedUsername(account, "laika#1234", RESERVATION_TOKEN)); + verify(accounts, never()).confirmUsername(any(), any(), any()); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testUsernameExpandDiscriminator(boolean reserve) throws UsernameNotAvailableException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); final String nickname = "test"; @@ -775,8 +858,14 @@ class AccountsManagerTest { when(accounts.usernameAvailable(any())).thenReturn(false); when(accounts.usernameAvailable(argThat(isWide))).thenReturn(true); - accountsManager.setUsername(account, nickname, null); - verify(accounts).setUsername(eq(account), and(startsWith(nickname), argThat(isWide))); + if (reserve) { + accountsManager.reserveUsername(account, nickname); + verify(accounts).reserveUsername(eq(account), and(startsWith(nickname), argThat(isWide)), any()); + + } else { + accountsManager.setUsername(account, nickname, null); + verify(accounts).setUsername(eq(account), and(startsWith(nickname), argThat(isWide))); + } } @Test @@ -801,7 +890,7 @@ class AccountsManagerTest { @Test void testSetUsernameReserved() { final String nickname = "reserved"; - when(reservedUsernames.isReserved(eq(nickname), any())).thenReturn(true); + when(prohibitedUsernames.isProhibited(eq(nickname), any())).thenReturn(true); final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); @@ -823,6 +912,10 @@ class AccountsManagerTest { assertThrows(UsernameNotAvailableException.class, () -> accountsManager.setUsername(account, "n00bkiller", null)); } + private void setReservationHash(final Account account, final String reservedUsername) { + account.setReservedUsernameHash(Accounts.reservedUsernameHash(account.getUuid(), reservedUsername)); + } + private static Device generateTestDevice(final long lastSeen) { final Device device = new Device(); device.setId(Device.MASTER_ID); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java index 136bc16b5..c8f7d6257 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java @@ -8,6 +8,8 @@ package org.whispersystems.textsecuregcm.storage; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; @@ -22,6 +24,8 @@ import org.whispersystems.textsecuregcm.util.AttributeValues; import org.whispersystems.textsecuregcm.util.UsernameGenerator; import software.amazon.awssdk.services.dynamodb.model.*; import java.time.Clock; +import java.time.Duration; +import java.time.Instant; import java.util.*; import java.util.function.Consumer; @@ -110,7 +114,11 @@ class AccountsManagerUsernameIntegrationTest { .build(); ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbClient().createTable(createPhoneNumberIdentifierTableRequest); + buildAccountsManager(1, 2, 10); + } + private void buildAccountsManager(final int initialWidth, int discriminatorMaxWidth, int attemptsPerWidth) + throws InterruptedException { @SuppressWarnings("unchecked") final DynamicConfigurationManager dynamicConfigurationManager = mock(DynamicConfigurationManager.class); @@ -127,6 +135,8 @@ class AccountsManagerUsernameIntegrationTest { USERNAMES_TABLE_NAME, SCAN_PAGE_SIZE)); + usernameGenerator = new UsernameGenerator(initialWidth, discriminatorMaxWidth, attemptsPerWidth, + Duration.ofDays(1)); final DeletedAccountsManager deletedAccountsManager = mock(DeletedAccountsManager.class); doAnswer((final InvocationOnMock invocationOnMock) -> { @SuppressWarnings("unchecked") @@ -141,8 +151,6 @@ class AccountsManagerUsernameIntegrationTest { final ExperimentEnrollmentManager experimentEnrollmentManager = mock(ExperimentEnrollmentManager.class); when(experimentEnrollmentManager.isEnrolled(any(UUID.class), eq(AccountsManager.USERNAME_EXPERIMENT_NAME))) .thenReturn(true); - - usernameGenerator = new UsernameGenerator(1, 2, 10); accountsManager = new AccountsManager( accounts, phoneNumberIdentifiers, @@ -151,7 +159,7 @@ class AccountsManagerUsernameIntegrationTest { mock(DirectoryQueue.class), mock(Keys.class), mock(MessagesManager.class), - mock(ReservedUsernames.class), + mock(ProhibitedUsernames.class), mock(ProfilesManager.class), mock(StoredVerificationCodeManager.class), mock(SecureStorageClient.class), @@ -190,19 +198,32 @@ class AccountsManagerUsernameIntegrationTest { assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsername()).isEmpty(); } - @Test - void testNoUsernames() throws InterruptedException { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testNoUsernames(boolean reserve) throws InterruptedException { Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), new ArrayList<>()); for (int i = 1; i <= 99; i++) { + final Map item = new HashMap<>(Map.of( + Accounts.KEY_ACCOUNT_UUID, AttributeValues.fromUUID(UUID.randomUUID()), + Accounts.ATTR_USERNAME, AttributeValues.fromString(usernameGenerator.fromParts("n00bkiller", i)))); + // half of these are taken usernames, half are only reservations (have a TTL) + if (i % 2 == 0) { + item.put(Accounts.ATTR_TTL, + AttributeValues.fromLong(Instant.now().plus(Duration.ofMinutes(1)).getEpochSecond())); + } ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbClient().putItem(PutItemRequest.builder() .tableName(USERNAMES_TABLE_NAME) - .item(Map.of( - Accounts.KEY_ACCOUNT_UUID, AttributeValues.fromUUID(UUID.randomUUID()), - Accounts.ATTR_USERNAME, AttributeValues.fromString(usernameGenerator.fromParts("n00bkiller", i)))) + .item(item) .build()); } - assertThrows(UsernameNotAvailableException.class, () -> accountsManager.setUsername(account, "n00bkiller", null)); + assertThrows(UsernameNotAvailableException.class, () -> { + if (reserve) { + accountsManager.reserveUsername(account, "n00bkiller"); + } else { + accountsManager.setUsername(account, "n00bkiller", null); + } + }); assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsername()).isEmpty(); } @@ -237,4 +258,112 @@ class AccountsManagerUsernameIntegrationTest { verify(accounts, times(1)).usernameAvailable(argThat(un -> discriminator(un) >= 10)); } + @Test + public void testReserveSetClear() + throws InterruptedException, UsernameNotAvailableException, UsernameReservationNotFoundException { + Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), + new ArrayList<>()); + AccountsManager.UsernameReservation reservation = accountsManager.reserveUsername(account, "n00bkiller"); + account = reservation.account(); + assertThat(account.getReservedUsernameHash()).isPresent(); + assertThat(reservation.reservedUsername()).startsWith("n00bkiller"); + int discriminator = discriminator(reservation.reservedUsername()); + assertThat(discriminator).isGreaterThan(0).isLessThan(10); + assertThat(accountsManager.getByUsername(reservation.reservedUsername())).isEmpty(); + + account = accountsManager.confirmReservedUsername( + account, + reservation.reservedUsername(), + reservation.reservationToken()); + + assertThat(account.getUsername().get()).startsWith("n00bkiller"); + assertThat(accountsManager.getByUsername(account.getUsername().get()).orElseThrow().getUuid()).isEqualTo( + account.getUuid()); + + // reroll + reservation = accountsManager.reserveUsername(account, "n00bkiller"); + account = reservation.account(); + account = accountsManager.confirmReservedUsername( + account, + reservation.reservedUsername(), + reservation.reservationToken()); + + final String newUsername = account.getUsername().orElseThrow(); + assertThat(discriminator(account.getUsername().orElseThrow())).isNotEqualTo(discriminator); + + // clear + account = accountsManager.clearUsername(account); + assertThat(accountsManager.getByUsername(newUsername)).isEmpty(); + assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsername()).isEmpty(); + + } + + @Test + public void testReservationLapsed() + throws InterruptedException, UsernameNotAvailableException, UsernameReservationNotFoundException { + // use a username generator that can retry a lot + buildAccountsManager(1, 1, 1000000); + + final Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), + new ArrayList<>()); + AccountsManager.UsernameReservation reservation1 = accountsManager.reserveUsername(account, "n00bkiller"); + final String reservedUsername = reservation1.reservedUsername(); + + long past = Instant.now().minus(Duration.ofMinutes(1)).getEpochSecond(); + // force expiration + ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbClient().updateItem(UpdateItemRequest.builder() + .tableName(USERNAMES_TABLE_NAME) + .key(Map.of(Accounts.ATTR_USERNAME, AttributeValues.fromString(reservedUsername))) + .updateExpression("SET #ttl = :ttl") + .expressionAttributeNames(Map.of("#ttl", Accounts.ATTR_TTL)) + .expressionAttributeValues(Map.of(":ttl", AttributeValues.fromLong(past))) + .build()); + + int discriminator = discriminator(reservedUsername); + + // use up all names except the reserved one + for (int i = 1; i <= 9; i++) { + if (i == discriminator) { + continue; + } + + ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbClient().putItem(PutItemRequest.builder() + .tableName(USERNAMES_TABLE_NAME) + .item(Map.of( + Accounts.KEY_ACCOUNT_UUID, AttributeValues.fromUUID(UUID.randomUUID()), + Accounts.ATTR_USERNAME, AttributeValues.fromString(usernameGenerator.fromParts("n00bkiller", i)))) + .build()); + } + + // a different account should be able to reserve it + Account account2 = accountsManager.create("+18005552222", "password", null, new AccountAttributes(), + new ArrayList<>()); + final AccountsManager.UsernameReservation reservation2 = accountsManager.reserveUsername(account2, "n00bkiller" + ); + assertThat(reservation2.reservedUsername()).isEqualTo(reservedUsername); + + assertThrows(UsernameNotAvailableException.class, + () -> accountsManager.confirmReservedUsername(reservation1.account(), reservedUsername, reservation1.reservationToken())); + accountsManager.confirmReservedUsername(reservation2.account(), reservedUsername, reservation2.reservationToken()); + } + + @Test + void testUsernameReserveClearSetReserved() + throws InterruptedException, UsernameNotAvailableException, UsernameReservationNotFoundException { + Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), + new ArrayList<>()); + account = accountsManager.setUsername(account, "n00bkiller", null); + final AccountsManager.UsernameReservation reservation = accountsManager.reserveUsername(account, "other"); + account = reservation.account(); + + assertThat(reservation.reservedUsername()).startsWith("other"); + assertThat(account.getUsername()).hasValueSatisfying(s -> s.startsWith("n00bkiller")); + + account = accountsManager.clearUsername(account); + assertThat(account.getReservedUsernameHash()).isPresent(); + assertThat(account.getUsername()).isEmpty(); + + account = accountsManager.confirmReservedUsername(account, reservation.reservedUsername(), reservation.reservationToken()); + assertThat(account.getUsername()).hasValueSatisfying(s -> s.startsWith("other")); + } } 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 03ec5b583..b2130f14e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java @@ -17,6 +17,9 @@ import static org.mockito.Mockito.when; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.uuid.UUIDComparator; import java.nio.charset.StandardCharsets; +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -26,6 +29,7 @@ import java.util.Random; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; +import java.util.function.Supplier; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -74,6 +78,7 @@ class AccountsTest { .build()) .build(); + private Clock mockClock; private DynamicConfigurationManager mockDynamicConfigManager; private Accounts accounts; @@ -130,7 +135,11 @@ class AccountsTest { when(mockDynamicConfigManager.getConfiguration()) .thenReturn(new DynamicConfiguration()); + mockClock = mock(Clock.class); + when(mockClock.instant()).thenReturn(Instant.EPOCH); + this.accounts = new Accounts( + mockClock, mockDynamicConfigManager, dynamoDbExtension.getDynamoDbClient(), dynamoDbExtension.getDynamoDbAsyncClient(), @@ -608,7 +617,7 @@ class AccountsTest { } @Test - void testSetUsername() throws UsernameNotAvailableException { + void testSetUsername() { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); @@ -679,7 +688,7 @@ class AccountsTest { } @Test - void testClearUsername() throws UsernameNotAvailableException { + void testClearUsername() { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); @@ -704,7 +713,7 @@ class AccountsTest { } @Test - void testClearUsernameVersionMismatch() throws UsernameNotAvailableException { + void testClearUsernameVersionMismatch() { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); @@ -719,6 +728,154 @@ class AccountsTest { assertThat(account.getUsername()).hasValueSatisfying(u -> assertThat(u).isEqualTo(username)); } + @Test + void testReservedUsername() { + final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account1); + final Account account2 = generateAccount("+18005552222", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account2); + + final UUID token = accounts.reserveUsername(account1, "garfield", Duration.ofDays(1)); + assertThat(account1.getReservedUsernameHash()).get().isEqualTo(Accounts.reservedUsernameHash(account1.getUuid(), "garfield")); + assertThat(account1.getUsername()).isEmpty(); + + // account 2 shouldn't be able to reserve the username + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.reserveUsername(account2, "garfield", Duration.ofDays(1))); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.confirmUsername(account2, "garfield", UUID.randomUUID())); + assertThat(accounts.getByUsername("garfield")).isEmpty(); + + accounts.confirmUsername(account1, "garfield", token); + assertThat(account1.getReservedUsernameHash()).isEmpty(); + assertThat(account1.getUsername()).get().isEqualTo("garfield"); + assertThat(accounts.getByUsername("garfield").get().getUuid()).isEqualTo(account1.getUuid()); + + assertThat(dynamoDbExtension.getDynamoDbClient() + .getItem(GetItemRequest.builder() + .tableName(USERNAME_CONSTRAINT_TABLE_NAME) + .key(Map.of(Accounts.ATTR_USERNAME, AttributeValues.fromString("garfield"))) + .build()) + .item()) + .doesNotContainKey(Accounts.ATTR_TTL); + } + + @Test + void testUsernameAvailable() { + final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account1); + + final String username = "unsinkablesam"; + + final UUID token = accounts.reserveUsername(account1, username, Duration.ofDays(1)); + assertThat(accounts.usernameAvailable(username)).isFalse(); + assertThat(accounts.usernameAvailable(Optional.empty(), username)).isFalse(); + assertThat(accounts.usernameAvailable(Optional.of(UUID.randomUUID()), username)).isFalse(); + assertThat(accounts.usernameAvailable(Optional.of(token), username)).isTrue(); + + accounts.confirmUsername(account1, username, token); + assertThat(accounts.usernameAvailable(username)).isFalse(); + assertThat(accounts.usernameAvailable(Optional.empty(), username)).isFalse(); + assertThat(accounts.usernameAvailable(Optional.of(UUID.randomUUID()), username)).isFalse(); + assertThat(accounts.usernameAvailable(Optional.of(token), username)).isFalse(); + } + + + @Test + void testReservedUsernameWrongToken() { + final Account account = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account); + accounts.reserveUsername(account, "grumpy", Duration.ofDays(1)); + assertThat(account.getReservedUsernameHash()) + .get() + .isEqualTo(Accounts.reservedUsernameHash(account.getUuid(), "grumpy")); + assertThat(account.getUsername()).isEmpty(); + + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.confirmUsername(account, "grumpy", UUID.randomUUID())); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.setUsername(account, "grumpy")); + } + + @Test + void testReserveExpiredReservedUsername() { + final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account1); + final Account account2 = generateAccount("+18005552222", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account2); + + accounts.reserveUsername(account1, "snowball#0002", Duration.ofDays(2)); + + Supplier take = () -> accounts.reserveUsername(account2, "snowball#0002", Duration.ofDays(2)); + + for (int i = 0; i <= 2; i++) { + when(mockClock.instant()).thenReturn(Instant.EPOCH.plus(Duration.ofDays(i))); + assertThrows(ContestedOptimisticLockException.class, take::get); + } + + // after 2 days, can take the name + when(mockClock.instant()).thenReturn(Instant.EPOCH.plus(Duration.ofDays(2)).plus(Duration.ofSeconds(1))); + final UUID token = take.get(); + + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.reserveUsername(account1, "snowball#0002", Duration.ofDays(2))); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.setUsername(account1, "snowball#0002")); + + accounts.confirmUsername(account2, "snowball#0002", token); + assertThat(accounts.getByUsername("snowball#0002").get().getUuid()).isEqualTo(account2.getUuid()); + } + + @Test + void testTakeExpiredReservedUsername() { + final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account1); + final Account account2 = generateAccount("+18005552222", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account2); + + accounts.reserveUsername(account1, "snowball#0002", Duration.ofDays(2)); + + Runnable take = () -> accounts.setUsername(account2, "snowball#0002"); + + for (int i = 0; i <= 2; i++) { + when(mockClock.instant()).thenReturn(Instant.EPOCH.plus(Duration.ofDays(i))); + assertThrows(ContestedOptimisticLockException.class, take::run); + } + + // after 2 days, can take the name + when(mockClock.instant()).thenReturn(Instant.EPOCH.plus(Duration.ofDays(2)).plus(Duration.ofSeconds(1))); + take.run(); + + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.reserveUsername(account1, "snowball#0002", Duration.ofDays(2))); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.setUsername(account1, "snowball#0002")); + assertThat(accounts.getByUsername("snowball#0002").get().getUuid()).isEqualTo(account2.getUuid()); + } + + @Test + void testRetryReserveUsername() { + final Account account = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account); + accounts.reserveUsername(account, "jorts", Duration.ofDays(2)); + + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.reserveUsername(account, "jorts", Duration.ofDays(2)), + "Shouldn't be able to re-reserve same username (would extend ttl)"); + } + + @Test + void testReserveUsernameVersionConflict() { + final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); + accounts.create(account); + account.setVersion(account.getVersion() + 12); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.reserveUsername(account, "salem", Duration.ofDays(1))); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.setUsername(account, "salem")); + + } + private Device generateDevice(long id) { return DevicesHelper.createDevice(id); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ReservedUsernamesTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernamesTest.java similarity index 79% rename from service/src/test/java/org/whispersystems/textsecuregcm/storage/ReservedUsernamesTest.java rename to service/src/test/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernamesTest.java index e40dd2040..211840be3 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ReservedUsernamesTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernamesTest.java @@ -17,37 +17,37 @@ import org.junit.jupiter.params.provider.MethodSource; import software.amazon.awssdk.services.dynamodb.model.AttributeDefinition; import software.amazon.awssdk.services.dynamodb.model.ScalarAttributeType; -class ReservedUsernamesTest { +class ProhibitedUsernamesTest { private static final String RESERVED_USERNAMES_TABLE_NAME = "reserved_usernames_test"; @RegisterExtension static DynamoDbExtension dynamoDbExtension = DynamoDbExtension.builder() .tableName(RESERVED_USERNAMES_TABLE_NAME) - .hashKey(ReservedUsernames.KEY_PATTERN) + .hashKey(ProhibitedUsernames.KEY_PATTERN) .attributeDefinition(AttributeDefinition.builder() - .attributeName(ReservedUsernames.KEY_PATTERN) + .attributeName(ProhibitedUsernames.KEY_PATTERN) .attributeType(ScalarAttributeType.S) .build()) .build(); private static final UUID RESERVED_FOR_UUID = UUID.randomUUID(); - private ReservedUsernames reservedUsernames; + private ProhibitedUsernames prohibitedUsernames; @BeforeEach void setUp() { - reservedUsernames = - new ReservedUsernames(dynamoDbExtension.getDynamoDbClient(), RESERVED_USERNAMES_TABLE_NAME); + prohibitedUsernames = + new ProhibitedUsernames(dynamoDbExtension.getDynamoDbClient(), RESERVED_USERNAMES_TABLE_NAME); } @ParameterizedTest @MethodSource void isReserved(final String username, final UUID uuid, final boolean expectReserved) { - reservedUsernames.reserveUsername(".*myusername.*", RESERVED_FOR_UUID); - reservedUsernames.reserveUsername("^foobar$", RESERVED_FOR_UUID); + prohibitedUsernames.prohibitUsername(".*myusername.*", RESERVED_FOR_UUID); + prohibitedUsernames.prohibitUsername("^foobar$", RESERVED_FOR_UUID); - assertEquals(expectReserved, reservedUsernames.isReserved(username, uuid)); + assertEquals(expectReserved, prohibitedUsernames.isProhibited(username, uuid)); } private static Stream isReserved() { 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 1d177af18..67a2fce5c 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 @@ -73,10 +73,13 @@ import org.whispersystems.textsecuregcm.entities.AccountIdentifierResponse; import org.whispersystems.textsecuregcm.entities.AccountIdentityResponse; import org.whispersystems.textsecuregcm.entities.ApnRegistrationId; import org.whispersystems.textsecuregcm.entities.ChangePhoneNumberRequest; +import org.whispersystems.textsecuregcm.entities.ConfirmUsernameRequest; import org.whispersystems.textsecuregcm.entities.GcmRegistrationId; import org.whispersystems.textsecuregcm.entities.IncomingMessage; import org.whispersystems.textsecuregcm.entities.RegistrationLock; import org.whispersystems.textsecuregcm.entities.RegistrationLockFailure; +import org.whispersystems.textsecuregcm.entities.ReserveUsernameRequest; +import org.whispersystems.textsecuregcm.entities.ReserveUsernameResponse; import org.whispersystems.textsecuregcm.entities.SignedPreKey; import org.whispersystems.textsecuregcm.entities.UsernameRequest; import org.whispersystems.textsecuregcm.entities.UsernameResponse; @@ -99,6 +102,7 @@ import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; import org.whispersystems.textsecuregcm.storage.UsernameNotAvailableException; +import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundException; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; import org.whispersystems.textsecuregcm.tests.util.AuthHelper; import org.whispersystems.textsecuregcm.util.Hex; @@ -121,6 +125,7 @@ class AccountControllerTest { private static final UUID SENDER_REG_LOCK_UUID = UUID.randomUUID(); private static final UUID SENDER_TRANSFER_UUID = UUID.randomUUID(); + private static final UUID RESERVATION_TOKEN = UUID.randomUUID(); private static final String ABUSIVE_HOST = "192.168.1.1"; private static final String NICE_HOST = "127.0.0.1"; @@ -144,6 +149,7 @@ class AccountControllerTest { private static RateLimiter smsVoicePrefixLimiter = mock(RateLimiter.class); private static RateLimiter autoBlockLimiter = mock(RateLimiter.class); private static RateLimiter usernameSetLimiter = mock(RateLimiter.class); + private static RateLimiter usernameReserveLimiter = mock(RateLimiter.class); private static RateLimiter usernameLookupLimiter = mock(RateLimiter.class); private static SmsSender smsSender = mock(SmsSender.class); private static TurnTokenGenerator turnTokenGenerator = mock(TurnTokenGenerator.class); @@ -208,6 +214,7 @@ class AccountControllerTest { when(rateLimiters.getSmsVoicePrefixLimiter()).thenReturn(smsVoicePrefixLimiter); when(rateLimiters.getAutoBlockLimiter()).thenReturn(autoBlockLimiter); when(rateLimiters.getUsernameSetLimiter()).thenReturn(usernameSetLimiter); + when(rateLimiters.getUsernameReserveLimiter()).thenReturn(usernameReserveLimiter); when(rateLimiters.getUsernameLookupLimiter()).thenReturn(usernameLookupLimiter); when(senderPinAccount.getLastSeen()).thenReturn(System.currentTimeMillis()); @@ -319,6 +326,7 @@ class AccountControllerTest { smsVoicePrefixLimiter, autoBlockLimiter, usernameSetLimiter, + usernameReserveLimiter, usernameLookupLimiter, smsSender, turnTokenGenerator, @@ -1733,6 +1741,63 @@ class AccountControllerTest { assertThat(response.readEntity(UsernameResponse.class).username()).isEqualTo("n00bkiller#1234"); } + @Test + void testReserveUsername() throws UsernameNotAvailableException { + when(accountsManager.reserveUsername(any(), eq("n00bkiller"))) + .thenReturn(new AccountsManager.UsernameReservation(null, "n00bkiller#1234", RESERVATION_TOKEN)); + Response response = + resources.getJerseyTest() + .target("/v1/accounts/username/reserved") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.json(new ReserveUsernameRequest("n00bkiller"))); + assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.readEntity(ReserveUsernameResponse.class)) + .satisfies(r -> r.username().equals("n00bkiller#1234")) + .satisfies(r -> r.reservationToken().equals(RESERVATION_TOKEN)); + } + + @Test + void testCommitUsername() throws UsernameNotAvailableException, UsernameReservationNotFoundException { + Account account = mock(Account.class); + when(account.getUsername()).thenReturn(Optional.of("n00bkiller#1234")); + when(accountsManager.confirmReservedUsername(any(), eq("n00bkiller#1234"), eq(RESERVATION_TOKEN))).thenReturn(account); + Response response = + resources.getJerseyTest() + .target("/v1/accounts/username/confirm") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.json(new ConfirmUsernameRequest("n00bkiller#1234", RESERVATION_TOKEN))); + assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.readEntity(UsernameResponse.class).username()).isEqualTo("n00bkiller#1234"); + } + + @Test + void testCommitUnreservedUsername() throws UsernameNotAvailableException, UsernameReservationNotFoundException { + when(accountsManager.confirmReservedUsername(any(), eq("n00bkiller#1234"), eq(RESERVATION_TOKEN))) + .thenThrow(new UsernameReservationNotFoundException()); + Response response = + resources.getJerseyTest() + .target("/v1/accounts/username/confirm") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.json(new ConfirmUsernameRequest("n00bkiller#1234", RESERVATION_TOKEN))); + assertThat(response.getStatus()).isEqualTo(409); + } + + @Test + void testCommitLapsedUsername() throws UsernameNotAvailableException, UsernameReservationNotFoundException { + when(accountsManager.confirmReservedUsername(any(), eq("n00bkiller#1234"), eq(RESERVATION_TOKEN))) + .thenThrow(new UsernameNotAvailableException()); + Response response = + resources.getJerseyTest() + .target("/v1/accounts/username/confirm") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.json(new ConfirmUsernameRequest("n00bkiller#1234", RESERVATION_TOKEN))); + assertThat(response.getStatus()).isEqualTo(410); + } + @Test void testSetTakenUsername() { Response response = diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameGeneratorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameGeneratorTest.java index 8ad69d348..b3b43facf 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameGeneratorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameGeneratorTest.java @@ -8,6 +8,7 @@ import org.junit.jupiter.params.provider.MethodSource; import org.whispersystems.textsecuregcm.storage.UsernameNotAvailableException; import org.whispersystems.textsecuregcm.util.UsernameGenerator; +import java.time.Duration; import java.util.HashSet; import java.util.Set; import java.util.function.Predicate; @@ -17,6 +18,8 @@ import static org.assertj.core.api.Assertions.assertThat; public class UsernameGeneratorTest { + private static final Duration TTL = Duration.ofMinutes(5); + @ParameterizedTest(name = "[{index}]:{0} ({2})") @MethodSource public void nicknameValidation(String nickname, boolean valid, String testCaseName) { @@ -64,7 +67,7 @@ public class UsernameGeneratorTest { @Test public void zeroPadDiscriminators() { - final UsernameGenerator generator = new UsernameGenerator(4, 5, 1); + final UsernameGenerator generator = new UsernameGenerator(4, 5, 1, TTL); assertThat(generator.fromParts("test", 1)).isEqualTo("test#0001"); assertThat(generator.fromParts("test", 123)).isEqualTo("test#0123"); assertThat(generator.fromParts("test", 9999)).isEqualTo("test#9999"); @@ -73,16 +76,16 @@ public class UsernameGeneratorTest { @Test public void expectedWidth() throws UsernameNotAvailableException { - String username = new UsernameGenerator(1, 6, 1).generateAvailableUsername("test", t -> true); + String username = new UsernameGenerator(1, 6, 1, TTL).generateAvailableUsername("test", t -> true); assertThat(extractDiscriminator(username)).isGreaterThan(0).isLessThan(10); - username = new UsernameGenerator(2, 6, 1).generateAvailableUsername("test", t -> true); + username = new UsernameGenerator(2, 6, 1, TTL).generateAvailableUsername("test", t -> true); assertThat(extractDiscriminator(username)).isGreaterThan(0).isLessThan(100); } @Test public void expandDiscriminator() throws UsernameNotAvailableException { - UsernameGenerator ug = new UsernameGenerator(1, 6, 10); + UsernameGenerator ug = new UsernameGenerator(1, 6, 10, TTL); final String username = ug.generateAvailableUsername("test", allowDiscriminator(d -> d >= 10000)); int discriminator = extractDiscriminator(username); assertThat(discriminator).isGreaterThanOrEqualTo(10000).isLessThan(100000); @@ -90,7 +93,7 @@ public class UsernameGeneratorTest { @Test public void expandDiscriminatorToMax() throws UsernameNotAvailableException { - UsernameGenerator ug = new UsernameGenerator(1, 6, 10); + UsernameGenerator ug = new UsernameGenerator(1, 6, 10, TTL); final String username = ug.generateAvailableUsername("test", allowDiscriminator(d -> d >= 100000)); int discriminator = extractDiscriminator(username); assertThat(discriminator).isGreaterThanOrEqualTo(100000).isLessThan(1000000); @@ -98,7 +101,7 @@ public class UsernameGeneratorTest { @Test public void exhaustDiscriminator() { - UsernameGenerator ug = new UsernameGenerator(1, 6, 10); + UsernameGenerator ug = new UsernameGenerator(1, 6, 10, TTL); Assertions.assertThrows(UsernameNotAvailableException.class, () -> { // allow greater than our max width ug.generateAvailableUsername("test", allowDiscriminator(d -> d >= 1000000)); @@ -107,7 +110,7 @@ public class UsernameGeneratorTest { @Test public void randomCoverageMinWidth() throws UsernameNotAvailableException { - UsernameGenerator ug = new UsernameGenerator(1, 6, 10); + UsernameGenerator ug = new UsernameGenerator(1, 6, 10, TTL); final Set seen = new HashSet<>(); for (int i = 0; i < 1000 && seen.size() < 9; i++) { seen.add(extractDiscriminator(ug.generateAvailableUsername("test", ignored -> true))); @@ -120,7 +123,7 @@ public class UsernameGeneratorTest { @Test public void randomCoverageMidWidth() throws UsernameNotAvailableException { - UsernameGenerator ug = new UsernameGenerator(1, 6, 10); + UsernameGenerator ug = new UsernameGenerator(1, 6, 10, TTL); final Set seen = new HashSet<>(); for (int i = 0; i < 100000 && seen.size() < 90; i++) { seen.add(extractDiscriminator(ug.generateAvailableUsername("test", allowDiscriminator(d -> d >= 10))));