diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java index 3332b1b09..7e87e4806 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java @@ -45,6 +45,7 @@ import org.whispersystems.textsecuregcm.configuration.SubscriptionConfiguration; import org.whispersystems.textsecuregcm.configuration.TestDeviceConfiguration; import org.whispersystems.textsecuregcm.configuration.TwilioConfiguration; import org.whispersystems.textsecuregcm.configuration.UnidentifiedDeliveryConfiguration; +import org.whispersystems.textsecuregcm.configuration.UsernameConfiguration; import org.whispersystems.textsecuregcm.configuration.VoiceVerificationConfiguration; import org.whispersystems.textsecuregcm.configuration.ZkConfig; import org.whispersystems.websocket.configuration.WebSocketConfiguration; @@ -247,6 +248,11 @@ public class WhisperServerConfiguration extends Configuration { @JsonProperty private ReportMessageConfiguration reportMessage = new ReportMessageConfiguration(); + @Valid + @NotNull + @JsonProperty + private UsernameConfiguration username = new UsernameConfiguration(); + @Valid @JsonProperty private AbusiveMessageFilterConfiguration abusiveMessageFilter; @@ -424,4 +430,8 @@ public class WhisperServerConfiguration extends Configuration { public AbusiveMessageFilterConfiguration getAbusiveMessageFilterConfiguration() { return abusiveMessageFilter; } + + public UsernameConfiguration getUsername() { + return username; + } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 3457b6582..f0b1085e6 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -205,6 +205,7 @@ import org.whispersystems.textsecuregcm.stripe.StripeManager; import org.whispersystems.textsecuregcm.util.Constants; import org.whispersystems.textsecuregcm.util.DynamoDbFromConfig; import org.whispersystems.textsecuregcm.util.HostnameUtil; +import org.whispersystems.textsecuregcm.util.UsernameGenerator; import org.whispersystems.textsecuregcm.util.logging.LoggingUnhandledExceptionMapper; import org.whispersystems.textsecuregcm.util.logging.UncaughtExceptionHandler; import org.whispersystems.textsecuregcm.websocket.AuthenticatedConnectListener; @@ -444,11 +445,13 @@ public class WhisperServerService extends Application new IllegalStateException("Could not get username after setting")); + } catch (final UsernameNotAvailableException e) { + throw new WebApplicationException(Status.CONFLICT); + } + } + + @Timed + @GET + @Path("/username/{username}") + @Produces(MediaType.APPLICATION_JSON) + public AccountIdentifierResponse lookupUsername( + @HeaderParam("X-Signal-Agent") final String userAgent, + @HeaderParam("X-Forwarded-For") final String forwardedFor, + @PathParam("username") final String username, + @Context final HttpServletRequest request) throws RateLimitExceededException { + + // Disallow clients from making authenticated requests to this endpoint + if (StringUtils.isNotBlank(request.getHeader("Authorization"))) { + 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); + + return accounts + .getByUsername(username) + .map(Account::getUuid) + .map(AccountIdentifierResponse::new) + .orElseThrow(() -> new WebApplicationException(Status.NOT_FOUND)); } @HEAD @@ -662,17 +715,7 @@ public class AccountController { if (StringUtils.isNotBlank(request.getHeader("Authorization"))) { throw new BadRequestException(); } - - final String mostRecentProxy = ForwardedIpUtil.getMostRecentProxy(forwardedFor) - .orElseThrow(() -> { - // Missing/malformed Forwarded-For, so we cannot check for a rate-limit. - // This shouldn't happen, so conservatively assume we're over the rate-limit - // and indicate that the client should retry - logger.error("Missing/bad Forwarded-For, cannot check account {}", uuid.toString()); - return new RateLimitExceededException(Duration.ofHours(1)); - }); - - rateLimiters.getCheckAccountExistenceLimiter().validate(mostRecentProxy); + rateLimitByClientIp(rateLimiters.getCheckAccountExistenceLimiter(), forwardedFor); final Status status = accounts.getByAccountIdentifier(uuid) .or(() -> accounts.getByPhoneNumberIdentifier(uuid)) @@ -681,6 +724,19 @@ public class AccountController { return Response.status(status).build(); } + private void rateLimitByClientIp(final RateLimiter rateLimiter, final String forwardedFor) throws RateLimitExceededException { + final String mostRecentProxy = ForwardedIpUtil.getMostRecentProxy(forwardedFor) + .orElseThrow(() -> { + // Missing/malformed Forwarded-For, so we cannot check for a rate-limit. + // This shouldn't happen, so conservatively assume we're over the rate-limit + // and indicate that the client should retry + logger.error("Missing/bad Forwarded-For: {}", forwardedFor); + return new RateLimitExceededException(Duration.ofHours(1)); + }); + + rateLimiter.validate(mostRecentProxy); + } + private void verifyRegistrationLock(final Account existingAccount, @Nullable final String clientRegistrationLock) throws RateLimitExceededException, WebApplicationException { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java index a48c68e3e..4f81567bb 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java @@ -503,24 +503,6 @@ public class ProfileController { account.getPhoneNumberIdentifier()); } - @Timed - @GET - @Produces(MediaType.APPLICATION_JSON) - @Path("/username/{username}") - public BaseProfileResponse getProfileByUsername( - @Auth AuthenticatedAccount auth, - @Context ContainerRequestContext containerRequestContext, - @PathParam("username") String username) - throws RateLimitExceededException { - - rateLimiters.getUsernameLookupLimiter().validate(auth.getAccount().getUuid()); - - final Account targetAccount = accountsManager.getByUsername(username).orElseThrow(NotFoundException::new); - final boolean isSelf = auth.getAccount().getUuid().equals(targetAccount.getUuid()); - - return buildBaseProfileResponseForAccountIdentity(targetAccount, isSelf, containerRequestContext); - } - private ProfileKeyCredentialResponse getProfileCredential(final String encodedProfileCredentialRequest, final VersionedProfile profile, final UUID uuid) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountIdentifierResponse.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountIdentifierResponse.java new file mode 100644 index 000000000..dfd33fd7e --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountIdentifierResponse.java @@ -0,0 +1,10 @@ +/* + * Copyright 2022 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.entities; +import javax.validation.constraints.NotNull; +import java.util.UUID; + +public record AccountIdentifierResponse(@NotNull UUID uuid) {} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameRequest.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameRequest.java new file mode 100644 index 000000000..7a5844b9a --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameRequest.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.annotation.Nullable; +import javax.validation.Valid; + +public record UsernameRequest(@Valid @Nickname String nickname, @Nullable String existingUsername) {} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameResponse.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameResponse.java new file mode 100644 index 000000000..ed47fce71 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameResponse.java @@ -0,0 +1,8 @@ +/* + * Copyright 2022 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.entities; + +public record UsernameResponse(String username) {} 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 8b253c534..d2ed0deac 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java @@ -345,8 +345,15 @@ public class Accounts extends AbstractDynamoDbStore { }); } + /** + * 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, UsernameNotAvailableException { + throws ContestedOptimisticLockException { final long startNanos = System.nanoTime(); final Optional maybeOriginalUsername = account.getUsername(); @@ -405,18 +412,14 @@ public class Accounts extends AbstractDynamoDbStore { } 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())) { + if (e.cancellationReasons().stream().map(CancellationReason::code).anyMatch("ConditionalCheckFailed"::equals)) { throw new ContestedOptimisticLockException(); } - throw e; } finally { if (!succeeded) { account.setUsername(maybeOriginalUsername.orElse(null)); } - SET_USERNAME_TIMER.record(System.nanoTime() - startNanos, TimeUnit.NANOSECONDS); } } @@ -563,6 +566,14 @@ public class Accounts extends AbstractDynamoDbStore { } } + public boolean usernameAvailable(final String username) { + final GetItemResponse response = client.getItem(GetItemRequest.builder() + .tableName(usernamesConstraintTableName) + .key(Map.of(ATTR_USERNAME, AttributeValues.fromString(username))) + .build()); + return !response.hasItem(); + } + public Optional getByE164(String number) { return GET_BY_NUMBER_TIMER.record(() -> { 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 d285b5173..94e173ae2 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -12,6 +12,7 @@ import com.codahale.metrics.SharedMetricRegistries; import com.codahale.metrics.Timer; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.annotations.VisibleForTesting; import io.lettuce.core.RedisException; import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; import io.micrometer.core.instrument.Metrics; @@ -37,6 +38,7 @@ import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials; import org.whispersystems.textsecuregcm.controllers.MismatchedDevicesException; import org.whispersystems.textsecuregcm.entities.AccountAttributes; import org.whispersystems.textsecuregcm.entities.SignedPreKey; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.push.ClientPresenceManager; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; import org.whispersystems.textsecuregcm.redis.RedisOperation; @@ -46,7 +48,7 @@ import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.util.Constants; import org.whispersystems.textsecuregcm.util.DestinationDeviceValidator; import org.whispersystems.textsecuregcm.util.SystemMapper; -import org.whispersystems.textsecuregcm.util.UsernameValidator; +import org.whispersystems.textsecuregcm.util.UsernameGenerator; import org.whispersystems.textsecuregcm.util.Util; public class AccountsManager { @@ -71,6 +73,9 @@ public class AccountsManager { private static final String COUNTRY_CODE_TAG_NAME = "country"; private static final String DELETION_REASON_TAG_NAME = "reason"; + @VisibleForTesting + public static final String USERNAME_EXPERIMENT_NAME = "usernames"; + private final Logger logger = LoggerFactory.getLogger(AccountsManager.class); private final Accounts accounts; @@ -86,7 +91,9 @@ public class AccountsManager { private final SecureStorageClient secureStorageClient; private final SecureBackupClient secureBackupClient; private final ClientPresenceManager clientPresenceManager; + private final ExperimentEnrollmentManager experimentEnrollmentManager; private final Clock clock; + private final UsernameGenerator usernameGenerator; private static final ObjectMapper mapper = SystemMapper.getMapper(); @@ -126,6 +133,8 @@ public class AccountsManager { final SecureStorageClient secureStorageClient, final SecureBackupClient secureBackupClient, final ClientPresenceManager clientPresenceManager, + final UsernameGenerator usernameGenerator, + final ExperimentEnrollmentManager experimentEnrollmentManager, final Clock clock) { this.accounts = accounts; this.phoneNumberIdentifiers = phoneNumberIdentifiers; @@ -140,6 +149,8 @@ public class AccountsManager { this.secureBackupClient = secureBackupClient; this.clientPresenceManager = clientPresenceManager; this.reservedUsernames = reservedUsernames; + this.usernameGenerator = usernameGenerator; + this.experimentEnrollmentManager = experimentEnrollmentManager; this.clock = Objects.requireNonNull(clock); } @@ -276,32 +287,27 @@ public class AccountsManager { final Account numberChangedAccount; - try { - numberChangedAccount = updateWithRetries( - account, - a -> { - //noinspection ConstantConditions - if (pniSignedPreKeys != null && pniRegistrationIds != null) { - pniSignedPreKeys.forEach((deviceId, signedPreKey) -> - a.getDevice(deviceId).ifPresent(device -> device.setPhoneNumberIdentitySignedPreKey(signedPreKey))); + numberChangedAccount = updateWithRetries( + account, + a -> { + //noinspection ConstantConditions + if (pniSignedPreKeys != null && pniRegistrationIds != null) { + pniSignedPreKeys.forEach((deviceId, signedPreKey) -> + a.getDevice(deviceId).ifPresent(device -> device.setPhoneNumberIdentitySignedPreKey(signedPreKey))); - pniRegistrationIds.forEach((deviceId, registrationId) -> - a.getDevice(deviceId).ifPresent(device -> device.setPhoneNumberIdentityRegistrationId(registrationId))); - } + pniRegistrationIds.forEach((deviceId, registrationId) -> + a.getDevice(deviceId).ifPresent(device -> device.setPhoneNumberIdentityRegistrationId(registrationId))); + } - if (pniIdentityKey != null) { - a.setPhoneNumberIdentityKey(pniIdentityKey); - } + if (pniIdentityKey != null) { + a.setPhoneNumberIdentityKey(pniIdentityKey); + } - return true; - }, - a -> accounts.changeNumber(a, number, phoneNumberIdentifier), - () -> accounts.getByAccountIdentifier(uuid).orElseThrow(), - AccountChangeValidator.NUMBER_CHANGE_VALIDATOR); - } catch (UsernameNotAvailableException e) { - // This should never happen when changing numbers - throw new RuntimeException(e); - } + return true; + }, + a -> accounts.changeNumber(a, number, phoneNumberIdentifier), + () -> accounts.getByAccountIdentifier(uuid).orElseThrow(), + AccountChangeValidator.NUMBER_CHANGE_VALIDATOR); updatedAccount.set(numberChangedAccount); directoryQueue.changePhoneNumber(numberChangedAccount, originalNumber, number); @@ -315,23 +321,31 @@ public class AccountsManager { return updatedAccount.get(); } - public Account setUsername(final Account account, final String username) throws UsernameNotAvailableException { - final String canonicalUsername = UsernameValidator.getCanonicalUsername(username); - - if (account.getUsername().map(canonicalUsername::equals).orElse(false)) { - return account; + 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(canonicalUsername, account.getUuid())) { + if (reservedUsernames.isReserved(requestedNickname, account.getUuid())) { throw new UsernameNotAvailableException(); } + final Optional currentUsername = account.getUsername(); + final Optional currentNickname = currentUsername.map(UsernameGenerator::extractNickname); + if (currentNickname.map(requestedNickname::equals).orElse(false) && !Objects.equals(expectedOldUsername, currentUsername.orElse(null))) { + // The requested nickname matches what the server already has, and the + // client provided the wrong existing username. Treat this as a replayed + // request, assuming that the client has previously succeeded + return account; + } + redisDelete(account); - return updateWithRetries( + return failableUpdateWithRetries( account, a -> true, - a -> accounts.setUsername(a, canonicalUsername), + // In the future, this may also check for any forbidden discriminators + a -> accounts.setUsername(a, usernameGenerator.generateAvailableUsername(requestedNickname, accounts::usernameAvailable)), () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); } @@ -339,31 +353,20 @@ public class AccountsManager { public Account clearUsername(final Account account) { redisDelete(account); - try { - return updateWithRetries( - account, - a -> true, - accounts::clearUsername, - () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), - AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); - } catch (UsernameNotAvailableException e) { - // This should never happen - throw new RuntimeException(e); - } + return updateWithRetries( + account, + a -> true, + accounts::clearUsername, + () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), + AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); } public Account update(Account account, Consumer updater) { - - 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); - } + return update(account, a -> { + updater.accept(a); + // assume that all updaters passed to the public method actually modify the account + return true; + }); } /** @@ -371,34 +374,28 @@ public class AccountsManager { * redundant updates of {@code device.lastSeen} */ 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); - }); - } catch (UsernameNotAvailableException e) { - // This should never happen when updating last-seen timestamps - throw new RuntimeException(e); - } + }).orElse(false); + }); } /** * @param account account to update * @param updater must return {@code true} if the account was actually updated */ - private Account update(Account account, Function updater) throws UsernameNotAvailableException { + private Account update(Account account, Function updater) { final boolean wasVisibleBeforeUpdate = account.shouldBeVisibleInDirectory(); @@ -429,6 +426,19 @@ public class AccountsManager { } private Account updateWithRetries(Account account, + final Function updater, + final Consumer persister, + final Supplier retriever, + final AccountChangeValidator changeValidator) { + try { + return failableUpdateWithRetries(account, updater, persister::accept, retriever, changeValidator); + } catch (UsernameNotAvailableException e) { + // not possible + throw new IllegalStateException(e); + } + } + + private Account failableUpdateWithRetries(Account account, final Function updater, final AccountPersister persister, final Supplier retriever, @@ -482,16 +492,11 @@ public class AccountsManager { } public Account updateDevice(Account account, long deviceId, Consumer deviceUpdater) { - 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); - } + return update(account, a -> { + a.getDevice(deviceId).ifPresent(deviceUpdater); + // assume that all updaters passed to the public method actually modify the device + return true; + }); } public Optional getByE164(String number) { @@ -522,12 +527,10 @@ public class AccountsManager { public Optional getByUsername(final String username) { try (final Timer.Context ignored = getByUsernameTimer.time()) { - final String canonicalUsername = UsernameValidator.getCanonicalUsername(username); - - Optional account = redisGetByUsername(canonicalUsername); + Optional account = redisGetByUsername(username); if (account.isEmpty()) { - account = accounts.getByUsername(canonicalUsername); + 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/ReservedUsernames.java index 752595c3f..625026a7a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ReservedUsernames.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ReservedUsernames.java @@ -53,7 +53,7 @@ public class ReservedUsernames { this.tableName = tableName; } - public boolean isReserved(final String username, final UUID accountIdentifier) { + public boolean isReserved(final String nickname, final UUID accountIdentifier) { return IS_RESERVED_TIMER.record(() -> { final ScanIterable scanIterable = dynamoDbClient.scanPaginator(ScanRequest.builder() .tableName(tableName) @@ -66,7 +66,7 @@ public class ReservedUsernames { final Pattern pattern = patternCache.get(item.get(KEY_PATTERN).s()); final UUID reservedFor = AttributeValues.getUUID(item, ATTR_RESERVED_FOR_UUID, null); - if (pattern.matcher(username).matches() && !accountIdentifier.equals(reservedFor)) { + if (pattern.matcher(nickname).matches() && !accountIdentifier.equals(reservedFor)) { return true; } } catch (final Exception e) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/Username.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/Nickname.java similarity index 86% rename from service/src/main/java/org/whispersystems/textsecuregcm/util/Username.java rename to service/src/main/java/org/whispersystems/textsecuregcm/util/Nickname.java index b9d7af788..10c8f8a8f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/Username.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/Nickname.java @@ -5,21 +5,21 @@ package org.whispersystems.textsecuregcm.util; +import javax.validation.Constraint; +import javax.validation.Payload; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + import static java.lang.annotation.ElementType.FIELD; import static java.lang.annotation.ElementType.PARAMETER; import static java.lang.annotation.RetentionPolicy.RUNTIME; -import java.lang.annotation.Retention; -import java.lang.annotation.Target; -import javax.validation.Constraint; -import javax.validation.Payload; - @Target({ FIELD, PARAMETER }) @Retention(RUNTIME) -@Constraint(validatedBy = UsernameValidator.class) -public @interface Username { +@Constraint(validatedBy = NicknameValidator.class) +public @interface Nickname { - String message() default "{org.whispersystems.textsecuregcm.util.Username.message}"; + String message() default "{org.whispersystems.textsecuregcm.util.Nickname.message}"; Class[] groups() default { }; diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/NicknameValidator.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/NicknameValidator.java new file mode 100644 index 000000000..82ea5e96d --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/NicknameValidator.java @@ -0,0 +1,17 @@ +/* + * Copyright 2013-2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.util; + +import javax.validation.ConstraintValidator; +import javax.validation.ConstraintValidatorContext; + + +public class NicknameValidator implements ConstraintValidator { + @Override + public boolean isValid(final String nickname, final ConstraintValidatorContext context) { + return UsernameGenerator.isValidNickname(nickname); + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java new file mode 100644 index 000000000..a941bff25 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java @@ -0,0 +1,142 @@ +/* + * Copyright 2022 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.util; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.math.IntMath; +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.DistributionSummary; +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.util.concurrent.ThreadLocalRandom; +import java.util.function.Predicate; +import java.util.regex.Pattern; + +import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name; + +public class UsernameGenerator { + /** + * Nicknames are + * + *
  • lowercase
    • + *
  • do not start with a number
    • + *
  • alphanumeric or underscores only
    • + *
  • minimum length 3
    • + *
  • maximum length 32
    • + *     + * + * Usernames typically consist of a nickname and an integer discriminator + */ + public static final Pattern NICKNAME_PATTERN = Pattern.compile("^[_a-z][_a-z0-9]{2,31}$"); + public static final String SEPARATOR = "#"; + + private static final Counter USERNAME_NOT_AVAILABLE_COUNTER = Metrics.counter(name(UsernameGenerator.class, "usernameNotAvailable")); + private static final DistributionSummary DISCRIMINATOR_ATTEMPT_COUNTER = Metrics.summary(name(UsernameGenerator.class, "discriminatorAttempts")); + + private final int initialWidth; + private final int discriminatorMaxWidth; + private final int attemptsPerWidth; + + public UsernameGenerator(UsernameConfiguration configuration) { + this(configuration.getDiscriminatorInitialWidth(), configuration.getDiscriminatorMaxWidth(), configuration.getAttemptsPerWidth()); + } + + @VisibleForTesting + public UsernameGenerator(int initialWidth, int discriminatorMaxWidth, int attemptsPerWidth) { + this.initialWidth = initialWidth; + this.discriminatorMaxWidth = discriminatorMaxWidth; + this.attemptsPerWidth = attemptsPerWidth; + } + + /** + * Generate a username with a random discriminator + * + * @param nickname The string nickname + * @param usernameAvailableFun A {@link Predicate} that returns true if the provided username is available + * @return The nickname appended with a random discriminator + * @throws UsernameNotAvailableException if we failed to find a nickname+discriminator pair that was available + */ + public String generateAvailableUsername(final String nickname, final Predicate usernameAvailableFun) throws UsernameNotAvailableException { + int rangeMin = 1; + int rangeMax = IntMath.pow(10, initialWidth); + int totalMax = IntMath.pow(10, discriminatorMaxWidth); + int attempts = 0; + while (rangeMax <= totalMax) { + // check discriminators of the current width up to attemptsPerWidth times + for (int i = 0; i < attemptsPerWidth; i++) { + int discriminator = ThreadLocalRandom.current().nextInt(rangeMin, rangeMax); + String username = UsernameGenerator.fromParts(nickname, discriminator); + attempts++; + if (usernameAvailableFun.test(username)) { + DISCRIMINATOR_ATTEMPT_COUNTER.record(attempts); + return username; + } + } + + // update the search range to look for numbers of one more digit + // than the previous iteration + rangeMin = rangeMax; + rangeMax *= 10; + } + USERNAME_NOT_AVAILABLE_COUNTER.increment(); + throw new UsernameNotAvailableException(); + } + + /** + * Strips the discriminator from a username, if it is present + * + * @param username the string username + * @return the nickname prefix of the username + */ + public static String extractNickname(final String username) { + int sep = username.indexOf(SEPARATOR); + return sep == -1 ? username : username.substring(0, sep); + } + + /** + * Generate a username from a nickname and discriminator + */ + public static String fromParts(final String nickname, final int discriminator) throws IllegalArgumentException { + if (!isValidNickname(nickname)) { + throw new IllegalArgumentException("Invalid nickname " + nickname); + } + return nickname + SEPARATOR + discriminator; + } + + public static boolean isValidNickname(final String nickname) { + return StringUtils.isNotBlank(nickname) && NICKNAME_PATTERN.matcher(nickname).matches(); + } + + /** + * Checks if the username consists of a valid nickname followed by an integer discriminator + * + * @param username string username to check + * @return true if the username is in standard form + */ + public static boolean isStandardFormat(final String username) { + if (username == null) { + return false; + } + int sep = username.indexOf(SEPARATOR); + if (sep == -1) { + return false; + } + final String nickname = username.substring(0, sep); + if (!isValidNickname(nickname)) { + return false; + } + + try { + int discriminator = Integer.parseInt(username.substring(sep + 1)); + return discriminator > 0; + } catch (NumberFormatException e) { + return false; + } + } + +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameValidator.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameValidator.java deleted file mode 100644 index 89cd2112d..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameValidator.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright 2013-2021 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.util; - -import org.apache.commons.lang3.StringUtils; -import javax.validation.ConstraintValidator; -import javax.validation.ConstraintValidatorContext; -import java.util.regex.Pattern; - -public class UsernameValidator implements ConstraintValidator { - - private static final Pattern USERNAME_PATTERN = - Pattern.compile("^[a-z_][a-z0-9_]{3,25}$", Pattern.CASE_INSENSITIVE); - - @Override - public boolean isValid(final String username, final ConstraintValidatorContext context) { - return StringUtils.isNotBlank(username) && USERNAME_PATTERN.matcher(getCanonicalUsername(username)).matches(); - } - - public static String getCanonicalUsername(final String username) { - return username != null ? username.toLowerCase() : null; - } -} 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 c7ea9f713..1f122b744 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java @@ -26,11 +26,13 @@ import org.whispersystems.textsecuregcm.WhisperServerConfiguration; import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentialGenerator; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.push.PushLatencyManager; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.push.ClientPresenceManager; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; 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.DeletedAccounts; @@ -50,6 +52,7 @@ import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; import org.whispersystems.textsecuregcm.storage.UsernameNotAvailableException; import org.whispersystems.textsecuregcm.storage.VerificationCodeStore; import org.whispersystems.textsecuregcm.util.DynamoDbFromConfig; +import org.whispersystems.textsecuregcm.util.UsernameGenerator; import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; @@ -68,11 +71,11 @@ public class AssignUsernameCommand extends EnvironmentCommand { try { - accountsManager.setUsername(account, username); + final Account result = accountsManager.setUsername(account, nickname, null); + System.out.println("New username: " + result.getUsername()); } catch (final UsernameNotAvailableException e) { throw new IllegalArgumentException("Username already taken"); } 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 01913708a..b4c5b4eb6 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java @@ -28,6 +28,7 @@ import org.whispersystems.textsecuregcm.WhisperServerConfiguration; import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentialGenerator; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.push.PushLatencyManager; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.push.ClientPresenceManager; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; @@ -53,6 +54,7 @@ import org.whispersystems.textsecuregcm.storage.ReservedUsernames; import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; import org.whispersystems.textsecuregcm.storage.VerificationCodeStore; import org.whispersystems.textsecuregcm.util.DynamoDbFromConfig; +import org.whispersystems.textsecuregcm.util.UsernameGenerator; import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; @@ -112,6 +114,9 @@ public class DeleteUserCommand extends EnvironmentCommand account = accountsManager.getByE164(user); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/SetUserDiscoverabilityCommand.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/SetUserDiscoverabilityCommand.java index 5919f4687..08991d011 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/SetUserDiscoverabilityCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/SetUserDiscoverabilityCommand.java @@ -27,6 +27,7 @@ import org.whispersystems.textsecuregcm.WhisperServerConfiguration; import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentialGenerator; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.push.PushLatencyManager; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.push.ClientPresenceManager; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; @@ -51,6 +52,7 @@ import org.whispersystems.textsecuregcm.storage.ReservedUsernames; import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; import org.whispersystems.textsecuregcm.storage.VerificationCodeStore; import org.whispersystems.textsecuregcm.util.DynamoDbFromConfig; +import org.whispersystems.textsecuregcm.util.UsernameGenerator; import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; @@ -115,6 +117,9 @@ public class SetUserDiscoverabilityCommand extends EnvironmentCommand maybeAccount; diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java index fa48ccc47..cfcab4f3e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java @@ -80,8 +80,6 @@ import org.whispersystems.textsecuregcm.configuration.BadgeConfiguration; import org.whispersystems.textsecuregcm.configuration.BadgesConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicPaymentsConfiguration; -import org.whispersystems.textsecuregcm.controllers.ProfileController; -import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException; import org.whispersystems.textsecuregcm.entities.Badge; import org.whispersystems.textsecuregcm.entities.BadgeSvg; import org.whispersystems.textsecuregcm.entities.BaseProfileResponse; @@ -362,23 +360,6 @@ class ProfileControllerTest { assertThat(response.getStatus()).isEqualTo(401); } - @Test - void testProfileGetByUsername() throws RateLimitExceededException { - BaseProfileResponse profile = resources.getJerseyTest() - .target("/v1/profile/username/n00bkiller") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .get(BaseProfileResponse.class); - - assertThat(profile.getIdentityKey()).isEqualTo("bar"); - assertThat(profile.getUuid()).isEqualTo(AuthHelper.VALID_UUID_TWO); - assertThat(profile.getBadges()).hasSize(1).element(0).has(new Condition<>( - badge -> "Test Badge".equals(badge.getName()), "has badge with expected name")); - - verify(accountsManager).getByUsername("n00bkiller"); - verify(usernameRateLimiter, times(1)).validate(eq(AuthHelper.VALID_UUID)); - } - @Test void testProfileGetUnauthorized() { Response response = resources.getJerseyTest() @@ -389,31 +370,6 @@ class ProfileControllerTest { assertThat(response.getStatus()).isEqualTo(401); } - @Test - void testProfileGetByUsernameUnauthorized() { - Response response = resources.getJerseyTest() - .target("/v1/profile/username/n00bkiller") - .request() - .get(); - - assertThat(response.getStatus()).isEqualTo(401); - } - - - @Test - void testProfileGetByUsernameNotFound() throws RateLimitExceededException { - Response response = resources.getJerseyTest() - .target("/v1/profile/username/n00bkillerzzzzz") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .get(); - - assertThat(response.getStatus()).isEqualTo(404); - - verify(accountsManager).getByUsername("n00bkillerzzzzz"); - verify(usernameRateLimiter).validate(eq(AuthHelper.VALID_UUID)); - } - @Test void testProfileGetDisabled() { 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 ff4c01ee5..5ef5832fb 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java @@ -27,11 +27,13 @@ import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfigurati import org.whispersystems.textsecuregcm.controllers.MismatchedDevicesException; import org.whispersystems.textsecuregcm.entities.AccountAttributes; import org.whispersystems.textsecuregcm.entities.SignedPreKey; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.push.ClientPresenceManager; import org.whispersystems.textsecuregcm.redis.RedisClusterExtension; import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; +import org.whispersystems.textsecuregcm.util.UsernameGenerator; import software.amazon.awssdk.services.dynamodb.model.AttributeDefinition; import software.amazon.awssdk.services.dynamodb.model.CreateTableRequest; import software.amazon.awssdk.services.dynamodb.model.GlobalSecondaryIndex; @@ -197,6 +199,8 @@ class AccountsManagerChangeNumberIntegrationTest { secureStorageClient, secureBackupClient, clientPresenceManager, + mock(UsernameGenerator.class), + mock(ExperimentEnrollmentManager.class), mock(Clock.class)); } } 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 b580e8e0a..a52cd3c0f 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java @@ -42,6 +42,7 @@ import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.entities.AccountAttributes; import org.whispersystems.textsecuregcm.entities.SignedPreKey; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.push.ClientPresenceManager; import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient; @@ -50,6 +51,7 @@ import org.whispersystems.textsecuregcm.tests.util.DevicesHelper; import org.whispersystems.textsecuregcm.tests.util.JsonHelpers; import org.whispersystems.textsecuregcm.tests.util.RedisClusterHelper; import org.whispersystems.textsecuregcm.util.Pair; +import org.whispersystems.textsecuregcm.util.UsernameGenerator; import software.amazon.awssdk.services.dynamodb.model.AttributeDefinition; import software.amazon.awssdk.services.dynamodb.model.CreateTableRequest; import software.amazon.awssdk.services.dynamodb.model.KeySchemaElement; @@ -164,6 +166,8 @@ class AccountsManagerConcurrentModificationIntegrationTest { mock(SecureStorageClient.class), mock(SecureBackupClient.class), mock(ClientPresenceManager.class), + mock(UsernameGenerator.class), + mock(ExperimentEnrollmentManager.class), mock(Clock.class) ); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java index 76b63047b..ceea14f29 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -10,10 +10,13 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.AdditionalMatchers.and; +import static org.mockito.AdditionalMatchers.not; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.startsWith; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; @@ -43,11 +46,14 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentMatcher; import org.mockito.stubbing.Answer; +import org.whispersystems.textsecuregcm.configuration.UsernameConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.controllers.MismatchedDevicesException; import org.whispersystems.textsecuregcm.entities.AccountAttributes; import org.whispersystems.textsecuregcm.entities.SignedPreKey; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.push.ClientPresenceManager; import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient; @@ -55,6 +61,7 @@ import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.Device.DeviceCapabilities; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; import org.whispersystems.textsecuregcm.tests.util.RedisClusterHelper; +import org.whispersystems.textsecuregcm.util.UsernameGenerator; class AccountsManagerTest { @@ -65,6 +72,7 @@ class AccountsManagerTest { private MessagesManager messagesManager; private ProfilesManager profilesManager; private ReservedUsernames reservedUsernames; + private ExperimentEnrollmentManager enrollmentManager; private Map phoneNumberIdentifiersByE164; @@ -129,6 +137,10 @@ class AccountsManagerTest { when(dynamicConfigurationManager.getConfiguration()).thenReturn(dynamicConfiguration); + enrollmentManager = mock(ExperimentEnrollmentManager.class); + when(enrollmentManager.isEnrolled(any(UUID.class), eq(AccountsManager.USERNAME_EXPERIMENT_NAME))).thenReturn(true); + when(accounts.usernameAvailable(any())).thenReturn(true); + accountsManager = new AccountsManager( accounts, phoneNumberIdentifiers, @@ -143,6 +155,8 @@ class AccountsManagerTest { storageClient, backupClient, mock(ClientPresenceManager.class), + new UsernameGenerator(new UsernameConfiguration()), + enrollmentManager, mock(Clock.class)); } @@ -716,45 +730,82 @@ class AccountsManagerTest { } @Test - void testSetUsername() throws UsernameNotAvailableException { + void testSetUsername() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - final String username = "test"; - - assertDoesNotThrow(() -> accountsManager.setUsername(account, username)); - verify(accounts).setUsername(account, username); + final String nickname = "test"; + assertDoesNotThrow(() -> accountsManager.setUsername(account, nickname, null)); + verify(accounts).setUsername(eq(account), startsWith(nickname)); } @Test - void testSetUsernameSameUsername() throws UsernameNotAvailableException { + void testSetUsernameSameUsername() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - final String username = "test"; - account.setUsername(username); + final String nickname = "test"; + account.setUsername(nickname + "#123"); - assertDoesNotThrow(() -> accountsManager.setUsername(account, username)); + // should be treated as a replayed request + assertDoesNotThrow(() -> accountsManager.setUsername(account, nickname, null)); verify(accounts, never()).setUsername(eq(account), any()); } @Test - void testSetUsernameNotAvailable() throws UsernameNotAvailableException { + void testSetUsernameReroll() throws UsernameNotAvailableException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - final String username = "test"; + final String nickname = "test"; + final String username = nickname + "#ZZZ"; + account.setUsername(username); - doThrow(new UsernameNotAvailableException()).when(accounts).setUsername(account, username); + // given the correct old username, should reroll discriminator even if the nick matches + accountsManager.setUsername(account, nickname, username); + verify(accounts).setUsername(eq(account), and(startsWith(nickname), not(eq(username)))); + } - assertThrows(UsernameNotAvailableException.class, () -> accountsManager.setUsername(account, username)); - verify(accounts).setUsername(account, username); + @Test + void testSetUsernameExpandDiscriminator() throws UsernameNotAvailableException { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + final String nickname = "test"; + ArgumentMatcher isWide = (String username) -> { + String[] spl = username.split(UsernameGenerator.SEPARATOR); + assertEquals(spl.length, 2); + int discriminator = Integer.parseInt(spl[1]); + // require a 7 digit discriminator + return discriminator > 1_000_000; + }; + 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))); + } + + @Test + void testChangeUsername() throws UsernameNotAvailableException { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + final String nickname = "test"; + account.setUsername("old#123"); + accountsManager.setUsername(account, nickname, "old#123"); + verify(accounts).setUsername(eq(account), startsWith(nickname)); + } + + @Test + void testSetUsernameNotAvailable() { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + final String nickname = "unavailable"; + when(accounts.usernameAvailable(startsWith(nickname))).thenReturn(false); + assertThrows(UsernameNotAvailableException.class, () -> accountsManager.setUsername(account, nickname, null)); + verify(accounts, never()).setUsername(any(), any()); assertTrue(account.getUsername().isEmpty()); } @Test void testSetUsernameReserved() { - final String username = "reserved"; - when(reservedUsernames.isReserved(eq(username), any())).thenReturn(true); + final String nickname = "reserved"; + when(reservedUsernames.isReserved(eq(nickname), any())).thenReturn(true); final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - assertThrows(UsernameNotAvailableException.class, () -> accountsManager.setUsername(account, username)); + assertThrows(UsernameNotAvailableException.class, () -> accountsManager.setUsername(account, nickname, null)); assertTrue(account.getUsername().isEmpty()); } @@ -765,6 +816,13 @@ class AccountsManagerTest { assertThrows(AssertionError.class, () -> accountsManager.update(account, a -> a.setUsername("test"))); } + @Test + void testSetUsernameDisabled() { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + when(enrollmentManager.isEnrolled(account.getUuid(), AccountsManager.USERNAME_EXPERIMENT_NAME)).thenReturn(false); + assertThrows(UsernameNotAvailableException.class, () -> accountsManager.setUsername(account, "n00bkiller", null)); + } + 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 new file mode 100644 index 000000000..8bbf72725 --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java @@ -0,0 +1,238 @@ +/* + * Copyright 2013-2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +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.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; +import org.whispersystems.textsecuregcm.entities.AccountAttributes; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; +import org.whispersystems.textsecuregcm.push.ClientPresenceManager; +import org.whispersystems.textsecuregcm.redis.RedisClusterExtension; +import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient; +import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient; +import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; +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.util.*; +import java.util.function.Consumer; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +class AccountsManagerUsernameIntegrationTest { + + 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 int SCAN_PAGE_SIZE = 1; + + @RegisterExtension + static DynamoDbExtension ACCOUNTS_DYNAMO_EXTENSION = DynamoDbExtension.builder() + .tableName(ACCOUNTS_TABLE_NAME) + .hashKey(Accounts.KEY_ACCOUNT_UUID) + .attributeDefinition(AttributeDefinition.builder() + .attributeName(Accounts.KEY_ACCOUNT_UUID) + .attributeType(ScalarAttributeType.B) + .build()) + .build(); + + @RegisterExtension + static DynamoDbExtension PNI_DYNAMO_EXTENSION = DynamoDbExtension.builder() + .tableName(PNI_TABLE_NAME) + .hashKey(PhoneNumberIdentifiers.KEY_E164) + .attributeDefinition(AttributeDefinition.builder() + .attributeName(PhoneNumberIdentifiers.KEY_E164) + .attributeType(ScalarAttributeType.S) + .build()) + .build(); + + @RegisterExtension + static RedisClusterExtension CACHE_CLUSTER_EXTENSION = RedisClusterExtension.builder().build(); + + private AccountsManager accountsManager; + private Accounts accounts; + + @BeforeEach + void setup() throws InterruptedException { + CreateTableRequest createNumbersTableRequest = CreateTableRequest.builder() + .tableName(NUMBERS_TABLE_NAME) + .keySchema(KeySchemaElement.builder() + .attributeName(Accounts.ATTR_ACCOUNT_E164) + .keyType(KeyType.HASH) + .build()) + .attributeDefinitions(AttributeDefinition.builder() + .attributeName(Accounts.ATTR_ACCOUNT_E164) + .attributeType(ScalarAttributeType.S) + .build()) + .provisionedThroughput(DynamoDbExtension.DEFAULT_PROVISIONED_THROUGHPUT) + .build(); + + ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbClient().createTable(createNumbersTableRequest); + CreateTableRequest createUsernamesTableRequest = CreateTableRequest.builder() + .tableName(USERNAMES_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(); + + ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbClient().createTable(createUsernamesTableRequest); + CreateTableRequest createPhoneNumberIdentifierTableRequest = CreateTableRequest.builder() + .tableName(PNI_ASSIGNMENT_TABLE_NAME) + .keySchema(KeySchemaElement.builder() + .attributeName(Accounts.ATTR_PNI_UUID) + .keyType(KeyType.HASH) + .build()) + .attributeDefinitions(AttributeDefinition.builder() + .attributeName(Accounts.ATTR_PNI_UUID) + .attributeType(ScalarAttributeType.B) + .build()) + .provisionedThroughput(DynamoDbExtension.DEFAULT_PROVISIONED_THROUGHPUT) + .build(); + + ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbClient().createTable(createPhoneNumberIdentifierTableRequest); + + @SuppressWarnings("unchecked") final DynamicConfigurationManager dynamicConfigurationManager = + mock(DynamicConfigurationManager.class); + + DynamicConfiguration dynamicConfiguration = new DynamicConfiguration(); + when(dynamicConfigurationManager.getConfiguration()).thenReturn(dynamicConfiguration); + + accounts = Mockito.spy(new Accounts( + dynamicConfigurationManager, + ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbClient(), + ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbAsyncClient(), + ACCOUNTS_DYNAMO_EXTENSION.getTableName(), + NUMBERS_TABLE_NAME, + PNI_ASSIGNMENT_TABLE_NAME, + USERNAMES_TABLE_NAME, + SCAN_PAGE_SIZE)); + + final DeletedAccountsManager deletedAccountsManager = mock(DeletedAccountsManager.class); + doAnswer((final InvocationOnMock invocationOnMock) -> { + @SuppressWarnings("unchecked") + Consumer> consumer = invocationOnMock.getArgument(1, Consumer.class); + consumer.accept(Optional.empty()); + return null; + }).when(deletedAccountsManager).lockAndTake(any(), any()); + + final PhoneNumberIdentifiers phoneNumberIdentifiers = + new PhoneNumberIdentifiers(PNI_DYNAMO_EXTENSION.getDynamoDbClient(), PNI_TABLE_NAME); + + final ExperimentEnrollmentManager experimentEnrollmentManager = mock(ExperimentEnrollmentManager.class); + when(experimentEnrollmentManager.isEnrolled(any(UUID.class), eq(AccountsManager.USERNAME_EXPERIMENT_NAME))) + .thenReturn(true); + + accountsManager = new AccountsManager( + accounts, + phoneNumberIdentifiers, + CACHE_CLUSTER_EXTENSION.getRedisCluster(), + deletedAccountsManager, + mock(DirectoryQueue.class), + mock(Keys.class), + mock(MessagesManager.class), + mock(ReservedUsernames.class), + mock(ProfilesManager.class), + mock(StoredVerificationCodeManager.class), + mock(SecureStorageClient.class), + mock(SecureBackupClient.class), + mock(ClientPresenceManager.class), + new UsernameGenerator(1, 2, 10), + experimentEnrollmentManager, + mock(Clock.class)); + } + + private static int discriminator(String username) { + return Integer.parseInt(username.split(UsernameGenerator.SEPARATOR)[1]); + } + + @Test + void testSetClearUsername() throws UsernameNotAvailableException, InterruptedException { + Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), + new ArrayList<>()); + account = accountsManager.setUsername(account, "n00bkiller", null); + assertThat(account.getUsername()).isPresent(); + assertThat(account.getUsername().get()).startsWith("n00bkiller"); + int discriminator = discriminator(account.getUsername().get()); + assertThat(discriminator).isGreaterThan(0).isLessThan(10); + + assertThat(accountsManager.getByUsername(account.getUsername().get()).orElseThrow().getUuid()).isEqualTo( + account.getUuid()); + + // reroll + account = accountsManager.setUsername(account, "n00bkiller", account.getUsername().get()); + 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 + void testNoUsernames() throws InterruptedException { + Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), + new ArrayList<>()); + for (int i = 1; i <= 99; i++) { + 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()); + } + assertThrows(UsernameNotAvailableException.class, () -> accountsManager.setUsername(account, "n00bkiller", null)); + assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsername()).isEmpty(); + } + + @Test + void testUsernameSnatched() throws InterruptedException, UsernameNotAvailableException { + final Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), + new ArrayList<>()); + for (int i = 1; i <= 9; i++) { + 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()); + } + + // first time this is called lie and say the username is available + // this simulates seeing an available username and then it being taken + // by someone before the write + doReturn(true).doCallRealMethod().when(accounts).usernameAvailable(any()); + final String username = accountsManager + .setUsername(account, "n00bkiller", null) + .getUsername().orElseThrow(); + assertThat(username).startsWith("n00bkiller"); + assertThat(discriminator(username)).isGreaterThanOrEqualTo(10).isLessThan(100); + + // 1 attempt on first try (returns true), + // 10 (attempts per width) on width=2 discriminators (all taken) + verify(accounts, times(11)).usernameAvailable(argThat(un -> discriminator(un) < 10)); + + // 1 final attempt on width=3 discriminators + verify(accounts, times(1)).usernameAvailable(argThat(un -> discriminator(un) >= 10)); + } + +} 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 6ac00be8a..35e93bdc1 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java @@ -722,7 +722,7 @@ class AccountsTest { assertThat(maybeAccount).isPresent(); verifyStoredState(firstAccount.getNumber(), firstAccount.getUuid(), firstAccount.getPhoneNumberIdentifier(), maybeAccount.get(), firstAccount); - assertThatExceptionOfType(UsernameNotAvailableException.class) + assertThatExceptionOfType(ContestedOptimisticLockException.class) .isThrownBy(() -> accounts.setUsername(secondAccount, username)); assertThat(secondAccount.getUsername()).isEmpty(); 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 ebffde14f..66f68b72c 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -10,6 +10,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.anyLong; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doThrow; @@ -23,6 +24,7 @@ import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import com.fasterxml.jackson.core.JsonProcessingException; import com.google.common.collect.ImmutableSet; import io.dropwizard.auth.PolymorphicAuthValueFactoryProvider; import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; @@ -66,6 +68,7 @@ import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfigurati import org.whispersystems.textsecuregcm.controllers.AccountController; import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException; import org.whispersystems.textsecuregcm.entities.AccountAttributes; +import org.whispersystems.textsecuregcm.entities.AccountIdentifierResponse; import org.whispersystems.textsecuregcm.entities.AccountIdentityResponse; import org.whispersystems.textsecuregcm.entities.ApnRegistrationId; import org.whispersystems.textsecuregcm.entities.ChangePhoneNumberRequest; @@ -74,6 +77,8 @@ import org.whispersystems.textsecuregcm.entities.IncomingMessage; import org.whispersystems.textsecuregcm.entities.RegistrationLock; import org.whispersystems.textsecuregcm.entities.RegistrationLockFailure; import org.whispersystems.textsecuregcm.entities.SignedPreKey; +import org.whispersystems.textsecuregcm.entities.UsernameRequest; +import org.whispersystems.textsecuregcm.entities.UsernameResponse; import org.whispersystems.textsecuregcm.limits.RateLimiter; import org.whispersystems.textsecuregcm.limits.RateLimiters; import org.whispersystems.textsecuregcm.mappers.ImpossiblePhoneNumberExceptionMapper; @@ -138,6 +143,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 usernameLookupLimiter = mock(RateLimiter.class); private static SmsSender smsSender = mock(SmsSender.class); private static TurnTokenGenerator turnTokenGenerator = mock(TurnTokenGenerator.class); private static Account senderPinAccount = mock(Account.class); @@ -201,6 +207,7 @@ class AccountControllerTest { when(rateLimiters.getSmsVoicePrefixLimiter()).thenReturn(smsVoicePrefixLimiter); when(rateLimiters.getAutoBlockLimiter()).thenReturn(autoBlockLimiter); when(rateLimiters.getUsernameSetLimiter()).thenReturn(usernameSetLimiter); + when(rateLimiters.getUsernameLookupLimiter()).thenReturn(usernameLookupLimiter); when(senderPinAccount.getLastSeen()).thenReturn(System.currentTimeMillis()); when(senderPinAccount.getRegistrationLock()).thenReturn(new StoredRegistrationLock(Optional.empty(), Optional.empty(), System.currentTimeMillis())); @@ -246,7 +253,7 @@ class AccountControllerTest { return account; }); - when(accountsManager.setUsername(AuthHelper.VALID_ACCOUNT, "takenusername")) + when(accountsManager.setUsername(AuthHelper.VALID_ACCOUNT, "takenusername", null)) .thenThrow(new UsernameNotAvailableException()); when(changeNumberManager.changeNumber(any(), any(), any(), any(), any(), any())).thenAnswer((Answer) invocation -> { @@ -311,6 +318,7 @@ class AccountControllerTest { smsVoicePrefixLimiter, autoBlockLimiter, usernameSetLimiter, + usernameLookupLimiter, smsSender, turnTokenGenerator, senderPinAccount, @@ -1660,25 +1668,29 @@ class AccountControllerTest { } @Test - void testSetUsername() { + void testSetUsername() throws UsernameNotAvailableException { + Account account = mock(Account.class); + when(account.getUsername()).thenReturn(Optional.of("n00bkiller#1234")); + when(accountsManager.setUsername(any(), eq("n00bkiller"), isNull())) + .thenReturn(account); Response response = resources.getJerseyTest() - .target("/v1/accounts/username/n00bkiller") + .target("/v1/accounts/username") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.text("")); - + .put(Entity.json(new UsernameRequest("n00bkiller", null))); assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.readEntity(UsernameResponse.class).username()).isEqualTo("n00bkiller#1234"); } @Test void testSetTakenUsername() { Response response = resources.getJerseyTest() - .target("/v1/accounts/username/takenusername") + .target("/v1/accounts/username/") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.text("")); + .put(Entity.json(new UsernameRequest("takenusername", null))); assertThat(response.getStatus()).isEqualTo(409); } @@ -1687,35 +1699,34 @@ class AccountControllerTest { void testSetInvalidUsername() { Response response = resources.getJerseyTest() - .target("/v1/accounts/username/pаypal") + .target("/v1/accounts/username") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.text("")); + // contains non-ascii character + .put(Entity.json(new UsernameRequest("pаypal", null))); - assertThat(response.getStatus()).isEqualTo(400); + assertThat(response.getStatus()).isEqualTo(422); } @Test - void testSetInvalidPrefixUsername() { + void testSetInvalidPrefixUsername() throws JsonProcessingException { Response response = resources.getJerseyTest() - .target("/v1/accounts/username/0n00bkiller") + .target("/v1/accounts/username") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.text("")); - - assertThat(response.getStatus()).isEqualTo(400); + .put(Entity.json(new UsernameRequest("0n00bkiller", null))); + assertThat(response.getStatus()).isEqualTo(422); } @Test void testSetUsernameBadAuth() { Response response = resources.getJerseyTest() - .target("/v1/accounts/username/n00bkiller") + .target("/v1/accounts/username") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.INVALID_PASSWORD)) - .put(Entity.text("")); - + .put(Entity.json(new UsernameRequest("n00bkiller", null))); assertThat(response.getStatus()).isEqualTo(401); } @@ -1935,4 +1946,43 @@ class AccountControllerTest { .head() .getStatus()).isEqualTo(400); } + + @Test + void testLookupUsername() { + final Account account = mock(Account.class); + final UUID uuid = UUID.randomUUID(); + when(account.getUuid()).thenReturn(uuid); + + when(accountsManager.getByUsername(eq("n00bkiller#1234"))).thenReturn(Optional.of(account)); + Response response = resources.getJerseyTest() + .target("v1/accounts/username/n00bkiller#1234") + .request() + .header("X-Forwarded-For", "127.0.0.1") + .get(); + assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.readEntity(AccountIdentifierResponse.class).uuid()).isEqualTo(uuid); + } + + @Test + void testLookupUsernameDoesNotExist() { + when(accountsManager.getByUsername(eq("n00bkiller#1234"))).thenReturn(Optional.empty()); + assertThat(resources.getJerseyTest() + .target("v1/accounts/username/n00bkiller#1234") + .request() + .header("X-Forwarded-For", "127.0.0.1") + .get().getStatus()).isEqualTo(404); + } + + @Test + void testLookupUsernameRateLimited() throws RateLimitExceededException { + doThrow(new RateLimitExceededException(Duration.ofSeconds(13))).when(usernameLookupLimiter).validate("127.0.0.1"); + final Response response = resources.getJerseyTest() + .target("/v1/accounts/username/test#123") + .request() + .header("X-Forwarded-For", "127.0.0.1") + .get(); + + assertThat(response.getStatus()).isEqualTo(413); + assertThat(response.getHeaderString("Retry-After")).isEqualTo(String.valueOf(Duration.ofSeconds(13).toSeconds())); + } } 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 new file mode 100644 index 000000000..c19d40b4d --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameGeneratorTest.java @@ -0,0 +1,133 @@ +package org.whispersystems.textsecuregcm.tests.util; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.whispersystems.textsecuregcm.storage.UsernameNotAvailableException; +import org.whispersystems.textsecuregcm.util.UsernameGenerator; + +import java.util.HashSet; +import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; + +public class UsernameGeneratorTest { + + @ParameterizedTest(name = "[{index}]:{0} ({2})") + @MethodSource + public void nicknameValidation(String nickname, boolean valid, String testCaseName) { + assertThat(UsernameGenerator.isValidNickname(nickname)).isEqualTo(valid); + } + + static Stream nicknameValidation() { + return Stream.of( + Arguments.of("Test", false, "upper case"), + Arguments.of("tesT", false, "upper case"), + Arguments.of("te-st", false, "illegal character"), + Arguments.of("ab\uD83D\uDC1B", false, "illegal character"), + Arguments.of("1test", false, "illegal start"), + Arguments.of("test#123", false, "illegal character"), + Arguments.of("ab", false, "too short"), + Arguments.of("", false, ""), + Arguments.of("_123456789_123456789_123456789123", false, "33 characters"), + + Arguments.of("_test", true, ""), + Arguments.of("test", true, ""), + Arguments.of("test123", true, ""), + Arguments.of("abc", true, ""), + Arguments.of("_123456789_123456789_12345678912", true, "32 characters") + ); + } + + @ParameterizedTest(name="[{index}]: {0}") + @MethodSource + public void nonStandardUsernames(final String username, final boolean isStandard) { + assertThat(UsernameGenerator.isStandardFormat(username)).isEqualTo(isStandard); + } + + static Stream nonStandardUsernames() { + return Stream.of( + Arguments.of("Test#123", false), + Arguments.of("test#-123", false), + Arguments.of("test#0", false), + Arguments.of("test#", false), + Arguments.of("test#1_00", false), + + Arguments.of("test#1", true), + Arguments.of("abc#1234", true) + ); + } + + @Test + public void expectedWidth() throws UsernameNotAvailableException { + String username = new UsernameGenerator(1, 6, 1).generateAvailableUsername("test", t -> true); + assertThat(extractDiscriminator(username)).isGreaterThan(0).isLessThan(10); + + username = new UsernameGenerator(2, 6, 1).generateAvailableUsername("test", t -> true); + assertThat(extractDiscriminator(username)).isGreaterThan(0).isLessThan(100); + } + + @Test + public void expandDiscriminator() throws UsernameNotAvailableException { + UsernameGenerator ug = new UsernameGenerator(1, 6, 10); + final String username = ug.generateAvailableUsername("test", allowDiscriminator(d -> d >= 10000)); + int discriminator = extractDiscriminator(username); + assertThat(discriminator).isGreaterThanOrEqualTo(10000).isLessThan(100000); + } + + @Test + public void expandDiscriminatorToMax() throws UsernameNotAvailableException { + UsernameGenerator ug = new UsernameGenerator(1, 6, 10); + final String username = ug.generateAvailableUsername("test", allowDiscriminator(d -> d >= 100000)); + int discriminator = extractDiscriminator(username); + assertThat(discriminator).isGreaterThanOrEqualTo(100000).isLessThan(1000000); + } + + @Test + public void exhaustDiscriminator() { + UsernameGenerator ug = new UsernameGenerator(1, 6, 10); + Assertions.assertThrows(UsernameNotAvailableException.class, () -> { + // allow greater than our max width + ug.generateAvailableUsername("test", allowDiscriminator(d -> d >= 1000000)); + }); + } + + @Test + public void randomCoverageMinWidth() throws UsernameNotAvailableException { + UsernameGenerator ug = new UsernameGenerator(1, 6, 10); + final Set seen = new HashSet<>(); + for (int i = 0; i < 1000 && seen.size() < 9; i++) { + seen.add(extractDiscriminator(ug.generateAvailableUsername("test", ignored -> true))); + } + // after 1K iterations, probability of a missed value is (9/10)^999 + assertThat(seen.size()).isEqualTo(9); + assertThat(seen).allMatch(i -> i > 0 && i < 10); + + } + + @Test + public void randomCoverageMidWidth() throws UsernameNotAvailableException { + UsernameGenerator ug = new UsernameGenerator(1, 6, 10); + 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)))); + } + // after 100K iterations, probability of a missed value is (99/100)^99999 + assertThat(seen.size()).isEqualTo(90); + assertThat(seen).allMatch(i -> i >= 10 && i < 100); + + } + + private static Predicate allowDiscriminator(Predicate p) { + return username -> p.test(extractDiscriminator(username)); + } + + private static int extractDiscriminator(final String username) { + return Integer.parseInt(username.split(UsernameGenerator.SEPARATOR)[1]); + } +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/util/UsernameValidatorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/util/NicknameValidatorTest.java similarity index 57% rename from service/src/test/java/org/whispersystems/textsecuregcm/util/UsernameValidatorTest.java rename to service/src/test/java/org/whispersystems/textsecuregcm/util/NicknameValidatorTest.java index 32e7c3d83..bdc1e6998 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/util/UsernameValidatorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/util/NicknameValidatorTest.java @@ -12,14 +12,14 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -class UsernameValidatorTest { +class NicknameValidatorTest { @ParameterizedTest @MethodSource void isValid(final String username, final boolean expectValid) { - final UsernameValidator usernameValidator = new UsernameValidator(); + final NicknameValidator nicknameValidator = new NicknameValidator(); - assertEquals(expectValid, usernameValidator.isValid(username, null)); + assertEquals(expectValid, nicknameValidator.isValid(username, null)); } private static Stream isValid() { @@ -28,8 +28,8 @@ class UsernameValidatorTest { Arguments.of("_test", true), Arguments.of("test123", true), Arguments.of("a", false), // Too short - Arguments.of("thisIsAReallyReallyReallyLongUsernameThatWeWouldNotAllow", false), - Arguments.of("Illegal character", false), + Arguments.of("thisisareallyreallyreallylongusernamethatwewouldnotalllow", false), + Arguments.of("illegal character", false), Arguments.of("0test", false), // Illegal first character Arguments.of("pаypal", false), // Unicode confusable characters Arguments.of("test\uD83D\uDC4E", false), // Emoji @@ -38,19 +38,4 @@ class UsernameValidatorTest { Arguments.of(null, false) ); } - - @ParameterizedTest - @MethodSource - void getCanonicalUsername(final String username, final String expectedCanonicalUsername) { - assertEquals(expectedCanonicalUsername, UsernameValidator.getCanonicalUsername(username)); - } - - private static Stream getCanonicalUsername() { - return Stream.of( - Arguments.of("test", "test"), - Arguments.of("TEst", "test"), - Arguments.of("t_e_S_T", "t_e_s_t"), - Arguments.of(null, null) - ); - } }