From c98b54ff1550b5cfa52daa219bd6299618c6fa9d Mon Sep 17 00:00:00 2001 From: Katherine Yen Date: Wed, 1 Feb 2023 14:31:44 -0800 Subject: [PATCH] Revert "Stored hashed username" --- .../WhisperServerConfiguration.java | 10 + .../textsecuregcm/WhisperServerService.java | 13 +- .../configuration/UsernameConfiguration.java | 44 +++ .../controllers/AccountController.java | 122 ++++---- .../entities/AccountIdentityResponse.java | 2 +- .../entities/ConfirmUsernameHashRequest.java | 19 -- .../entities/ConfirmUsernameRequest.java | 12 + .../entities/ReserveUsernameHashRequest.java | 23 -- .../entities/ReserveUsernameHashResponse.java | 20 -- .../entities/ReserveUsernameRequest.java | 12 + .../entities/ReserveUsernameResponse.java | 10 + .../entities/UsernameHashResponse.java | 21 -- .../entities/UsernameRequest.java | 12 + .../entities/UsernameResponse.java | 8 + .../mappers/JsonMappingExceptionMapper.java | 12 - .../textsecuregcm/storage/Account.java | 17 +- .../storage/AccountChangeValidator.java | 27 +- .../textsecuregcm/storage/Accounts.java | 174 ++++++----- .../storage/AccountsManager.java | 163 +++++++---- ...ava => UsernameNotAvailableException.java} | 2 +- .../util/ByteArrayBase64UrlAdapter.java | 27 -- .../textsecuregcm/util/Nickname.java | 27 ++ .../textsecuregcm/util/NicknameValidator.java | 17 ++ .../textsecuregcm/util/UsernameGenerator.java | 152 ++++++++++ .../util/UsernameNormalizer.java | 10 + .../workers/AssignUsernameCommand.java | 33 +-- .../workers/DeleteUserCommand.java | 6 +- .../SetUserDiscoverabilityCommand.java | 6 +- .../controllers/AccountControllerTest.java | 264 ++++++++--------- .../controllers/ProfileControllerTest.java | 7 +- .../storage/AccountChangeValidatorTest.java | 15 +- ...ntsManagerChangeNumberIntegrationTest.java | 3 + ...ConcurrentModificationIntegrationTest.java | 3 + .../storage/AccountsManagerTest.java | 276 ++++++++++++------ ...ccountsManagerUsernameIntegrationTest.java | 221 ++++++++------ .../textsecuregcm/storage/AccountsTest.java | 241 +++++++-------- .../storage/ProhibitedUsernamesTest.java | 69 +++++ .../tests/util/AccountsHelper.java | 2 +- .../tests/util/UsernameGeneratorTest.java | 145 +++++++++ .../tests/util/UsernameNormalizerTest.java | 17 ++ .../util/NicknameValidatorTest.java | 41 +++ 41 files changed, 1490 insertions(+), 815 deletions(-) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/configuration/UsernameConfiguration.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameHashRequest.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameRequest.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameHashRequest.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameHashResponse.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 delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameHashResponse.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameRequest.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameResponse.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/mappers/JsonMappingExceptionMapper.java rename service/src/main/java/org/whispersystems/textsecuregcm/storage/{UsernameHashNotAvailableException.java => UsernameNotAvailableException.java} (68%) delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/ByteArrayBase64UrlAdapter.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/Nickname.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/NicknameValidator.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameNormalizer.java create mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernamesTest.java create mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameGeneratorTest.java create mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameNormalizerTest.java create mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/util/NicknameValidatorTest.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java index 30929bdd8..b784c897a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java @@ -48,6 +48,7 @@ import org.whispersystems.textsecuregcm.configuration.StripeConfiguration; import org.whispersystems.textsecuregcm.configuration.SubscriptionConfiguration; import org.whispersystems.textsecuregcm.configuration.TestDeviceConfiguration; import org.whispersystems.textsecuregcm.configuration.UnidentifiedDeliveryConfiguration; +import org.whispersystems.textsecuregcm.configuration.UsernameConfiguration; import org.whispersystems.textsecuregcm.configuration.ZkConfig; import org.whispersystems.websocket.configuration.WebSocketConfiguration; @@ -254,6 +255,11 @@ public class WhisperServerConfiguration extends Configuration { @JsonProperty private ReportMessageConfiguration reportMessage = new ReportMessageConfiguration(); + @Valid + @NotNull + @JsonProperty + private UsernameConfiguration username = new UsernameConfiguration(); + @Valid @JsonProperty private SpamFilterConfiguration spamFilterConfiguration; @@ -441,6 +447,10 @@ public class WhisperServerConfiguration extends Configuration { return spamFilterConfiguration; } + public UsernameConfiguration getUsername() { + return username; + } + public RegistrationServiceConfiguration getRegistrationServiceConfiguration() { return registrationService; } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index f6810d455..2dc69451e 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -123,7 +123,6 @@ import org.whispersystems.textsecuregcm.mappers.DeviceLimitExceededExceptionMapp import org.whispersystems.textsecuregcm.mappers.IOExceptionMapper; import org.whispersystems.textsecuregcm.mappers.ImpossiblePhoneNumberExceptionMapper; import org.whispersystems.textsecuregcm.mappers.InvalidWebsocketAddressExceptionMapper; -import org.whispersystems.textsecuregcm.mappers.JsonMappingExceptionMapper; import org.whispersystems.textsecuregcm.mappers.NonNormalizedPhoneNumberExceptionMapper; import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper; import org.whispersystems.textsecuregcm.mappers.ServerRejectedExceptionMapper; @@ -193,6 +192,7 @@ import org.whispersystems.textsecuregcm.storage.NonNormalizedAccountCrawlerListe import org.whispersystems.textsecuregcm.storage.PhoneNumberIdentifiers; import org.whispersystems.textsecuregcm.storage.Profiles; import org.whispersystems.textsecuregcm.storage.ProfilesManager; +import org.whispersystems.textsecuregcm.storage.ProhibitedUsernames; import org.whispersystems.textsecuregcm.storage.PubSubManager; import org.whispersystems.textsecuregcm.storage.PushChallengeDynamoDb; import org.whispersystems.textsecuregcm.storage.PushFeedbackProcessor; @@ -209,6 +209,7 @@ import org.whispersystems.textsecuregcm.subscriptions.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; @@ -350,6 +351,8 @@ public class WhisperServerService extends Application { environment.jersey().register(exceptionMapper); webSocketEnvironment.jersey().register(exceptionMapper); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/UsernameConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/UsernameConfiguration.java new file mode 100644 index 000000000..623287053 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/UsernameConfiguration.java @@ -0,0 +1,44 @@ +/* + * Copyright 2013-2020 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.configuration; + +import com.fasterxml.jackson.annotation.JsonProperty; +import javax.validation.constraints.Min; +import java.time.Duration; + +public class UsernameConfiguration { + + @JsonProperty + @Min(1) + private int discriminatorInitialWidth = 2; + + @JsonProperty + @Min(1) + private int discriminatorMaxWidth = 9; + + @JsonProperty + @Min(1) + private int attemptsPerWidth = 10; + + @JsonProperty + private Duration reservationTtl = Duration.ofMinutes(5); + + public int getDiscriminatorInitialWidth() { + return discriminatorInitialWidth; + } + + public int getDiscriminatorMaxWidth() { + return discriminatorMaxWidth; + } + + public int getAttemptsPerWidth() { + return attemptsPerWidth; + } + + public Duration getReservationTtl() { + return reservationTtl; + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index 8061f3a84..1eb3e642a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -26,7 +26,6 @@ import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; -import java.util.Base64; import java.util.Map; import java.util.Optional; import java.util.UUID; @@ -76,17 +75,18 @@ 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.ConfirmUsernameHashRequest; +import org.whispersystems.textsecuregcm.entities.ConfirmUsernameRequest; import org.whispersystems.textsecuregcm.entities.DeviceName; import org.whispersystems.textsecuregcm.entities.GcmRegistrationId; import org.whispersystems.textsecuregcm.entities.MismatchedDevices; import org.whispersystems.textsecuregcm.entities.RegistrationLock; import org.whispersystems.textsecuregcm.entities.RegistrationLockFailure; -import org.whispersystems.textsecuregcm.entities.ReserveUsernameHashRequest; -import org.whispersystems.textsecuregcm.entities.ReserveUsernameHashResponse; +import org.whispersystems.textsecuregcm.entities.ReserveUsernameRequest; +import org.whispersystems.textsecuregcm.entities.ReserveUsernameResponse; import org.whispersystems.textsecuregcm.entities.StaleDevices; +import org.whispersystems.textsecuregcm.entities.UsernameRequest; +import org.whispersystems.textsecuregcm.entities.UsernameResponse; import org.whispersystems.textsecuregcm.limits.RateLimitedByIp; -import org.whispersystems.textsecuregcm.entities.UsernameHashResponse; import org.whispersystems.textsecuregcm.limits.RateLimiter; import org.whispersystems.textsecuregcm.limits.RateLimiters; import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil; @@ -103,7 +103,7 @@ import org.whispersystems.textsecuregcm.storage.ChangeNumberManager; import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; -import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableException; +import org.whispersystems.textsecuregcm.storage.UsernameNotAvailableException; import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundException; import org.whispersystems.textsecuregcm.util.Constants; import org.whispersystems.textsecuregcm.util.HeaderUtils; @@ -111,13 +111,13 @@ import org.whispersystems.textsecuregcm.util.Hex; import org.whispersystems.textsecuregcm.util.ImpossiblePhoneNumberException; import org.whispersystems.textsecuregcm.util.NonNormalizedPhoneNumberException; import org.whispersystems.textsecuregcm.util.Optionals; +import org.whispersystems.textsecuregcm.util.UsernameGenerator; import org.whispersystems.textsecuregcm.util.Util; @SuppressWarnings("OptionalUsedAsFieldOrParameterType") @Path("/v1/accounts") public class AccountController { - public static final int MAXIMUM_USERNAME_HASHES_LIST_LENGTH = 20; - public static final int USERNAME_HASH_LENGTH = 32; + private final Logger logger = LoggerFactory.getLogger(AccountController.class); private final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); private final Meter countryFilteredHostMeter = metricRegistry.meter(name(AccountController.class, "country_limited_host" )); @@ -136,7 +136,11 @@ public class AccountController { .publishPercentiles(0.75, 0.95, 0.99, 0.999) .distributionStatisticExpiry(Duration.ofHours(2)) .register(Metrics.globalRegistry); + + private static final String NONSTANDARD_USERNAME_COUNTER_NAME = name(AccountController.class, "nonStandardUsername"); + private static final String LOCKED_ACCOUNT_COUNTER_NAME = name(AccountController.class, "lockedAccount"); + private static final String CHALLENGE_PRESENT_TAG_NAME = "present"; private static final String CHALLENGE_MATCH_TAG_NAME = "matches"; private static final String COUNTRY_CODE_TAG_NAME = "countryCode"; @@ -443,7 +447,7 @@ public class AccountController { return new AccountIdentityResponse(account.getUuid(), account.getNumber(), account.getPhoneNumberIdentifier(), - account.getUsernameHash().orElse(null), + account.getUsername().orElse(null), existingAccount.map(Account::isStorageSupported).orElse(false)); } @@ -504,7 +508,7 @@ public class AccountController { updatedAccount.getUuid(), updatedAccount.getNumber(), updatedAccount.getPhoneNumberIdentifier(), - updatedAccount.getUsernameHash().orElse(null), + updatedAccount.getUsername().orElse(null), updatedAccount.isStorageSupported()); } catch (MismatchedDevicesException e) { throw new WebApplicationException(Response.status(409) @@ -683,78 +687,96 @@ public class AccountController { return new AccountIdentityResponse(auth.getAccount().getUuid(), auth.getAccount().getNumber(), auth.getAccount().getPhoneNumberIdentifier(), - auth.getAccount().getUsernameHash().orElse(null), + auth.getAccount().getUsername().orElse(null), auth.getAccount().isStorageSupported()); } @Timed @DELETE - @Path("/username_hash") + @Path("/username") @Produces(MediaType.APPLICATION_JSON) - public void deleteUsernameHash(@Auth AuthenticatedAccount auth) { - accounts.clearUsernameHash(auth.getAccount()); + public void deleteUsername(@Auth AuthenticatedAccount auth) { + accounts.clearUsername(auth.getAccount()); } + @Timed @PUT - @Path("/username_hash/reserve") + @Path("/username/reserved") @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) - public ReserveUsernameHashResponse reserveUsernameHash(@Auth AuthenticatedAccount auth, + public ReserveUsernameResponse reserveUsername(@Auth AuthenticatedAccount auth, @HeaderParam(HeaderUtils.X_SIGNAL_AGENT) String userAgent, - @NotNull @Valid ReserveUsernameHashRequest usernameRequest) throws RateLimitExceededException { + @NotNull @Valid ReserveUsernameRequest usernameRequest) throws RateLimitExceededException { rateLimiters.getUsernameReserveLimiter().validate(auth.getAccount().getUuid()); - for (byte[] hash : usernameRequest.usernameHashes()) { - if (hash.length != USERNAME_HASH_LENGTH) { - throw new WebApplicationException(Response.status(422).build()); - } - } - try { - final AccountsManager.UsernameReservation reservation = accounts.reserveUsernameHash( + final AccountsManager.UsernameReservation reservation = accounts.reserveUsername( auth.getAccount(), - usernameRequest.usernameHashes() + usernameRequest.nickname() ); - return new ReserveUsernameHashResponse(reservation.reservedUsernameHash()); - } catch (final UsernameHashNotAvailableException e) { + return new ReserveUsernameResponse(reservation.reservedUsername(), reservation.reservationToken()); + } catch (final UsernameNotAvailableException e) { throw new WebApplicationException(Status.CONFLICT); } } @Timed @PUT - @Path("/username_hash/confirm") + @Path("/username/confirm") @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) - public UsernameHashResponse confirmUsernameHash(@Auth AuthenticatedAccount auth, + public UsernameResponse confirmUsername(@Auth AuthenticatedAccount auth, @HeaderParam(HeaderUtils.X_SIGNAL_AGENT) String userAgent, - @NotNull @Valid ConfirmUsernameHashRequest confirmRequest) throws RateLimitExceededException { + @NotNull @Valid ConfirmUsernameRequest confirmRequest) throws RateLimitExceededException { rateLimiters.getUsernameSetLimiter().validate(auth.getAccount().getUuid()); try { - final Account account = accounts.confirmReservedUsernameHash(auth.getAccount(), confirmRequest.usernameHash()); + final Account account = accounts.confirmReservedUsername(auth.getAccount(), confirmRequest.usernameToConfirm(), confirmRequest.reservationToken()); return account - .getUsernameHash() - .map(UsernameHashResponse::new) + .getUsername() + .map(UsernameResponse::new) .orElseThrow(() -> new IllegalStateException("Could not get username after setting")); } catch (final UsernameReservationNotFoundException e) { throw new WebApplicationException(Status.CONFLICT); - } catch (final UsernameHashNotAvailableException e) { + } catch (final UsernameNotAvailableException e) { throw new WebApplicationException(Status.GONE); } } + @Timed + @PUT + @Path("/username") + @Produces(MediaType.APPLICATION_JSON) + @Consumes(MediaType.APPLICATION_JSON) + public UsernameResponse setUsername( + @Auth AuthenticatedAccount auth, + @HeaderParam(HeaderUtils.X_SIGNAL_AGENT) String userAgent, + @NotNull @Valid UsernameRequest usernameRequest) throws RateLimitExceededException { + rateLimiters.getUsernameSetLimiter().validate(auth.getAccount().getUuid()); + checkUsername(usernameRequest.existingUsername(), userAgent); + + try { + final Account account = accounts.setUsername(auth.getAccount(), usernameRequest.nickname(), + usernameRequest.existingUsername()); + return account + .getUsername() + .map(UsernameResponse::new) + .orElseThrow(() -> new IllegalStateException("Could not get username after setting")); + } catch (final UsernameNotAvailableException e) { + throw new WebApplicationException(Status.CONFLICT); + } + } + @Timed @GET - @Path("/username_hash/{usernameHash}") + @Path("/username/{username}") @Produces(MediaType.APPLICATION_JSON) @RateLimitedByIp(RateLimiters.Handle.USERNAME_LOOKUP) - public AccountIdentifierResponse lookupUsernameHash( + public AccountIdentifierResponse lookupUsername( @HeaderParam(HeaderUtils.X_SIGNAL_AGENT) final String userAgent, - @HeaderParam(HttpHeaders.X_FORWARDED_FOR) final String forwardedFor, - @PathParam("usernameHash") final String usernameHash, + @PathParam("username") final String username, @Context final HttpServletRequest request) throws RateLimitExceededException { // Disallow clients from making authenticated requests to this endpoint @@ -762,21 +784,10 @@ public class AccountController { throw new BadRequestException(); } - rateLimitByClientIp(rateLimiters.getUsernameLookupLimiter(), forwardedFor); - - final byte[] hash; - try { - hash = Base64.getUrlDecoder().decode(usernameHash); - } catch (IllegalArgumentException | AssertionError e) { - throw new WebApplicationException(Response.status(422).build()); - } - - if (hash.length != USERNAME_HASH_LENGTH) { - throw new WebApplicationException(Response.status(422).build()); - } + checkUsername(username, userAgent); return accounts - .getByUsernameHash(hash) + .getByUsername(username) .map(Account::getUuid) .map(AccountIdentifierResponse::new) .orElseThrow(() -> new WebApplicationException(Status.NOT_FOUND)); @@ -933,6 +944,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 String generatePushChallenge() { SecureRandom random = new SecureRandom(); byte[] challenge = new byte[16]; diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountIdentityResponse.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountIdentityResponse.java index 450ea51a6..d1b2c5762 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountIdentityResponse.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountIdentityResponse.java @@ -11,6 +11,6 @@ import javax.annotation.Nullable; public record AccountIdentityResponse(UUID uuid, String number, UUID pni, - @Nullable byte[] usernameHash, + @Nullable String username, boolean storageCapable) { } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameHashRequest.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameHashRequest.java deleted file mode 100644 index 0ead70093..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameHashRequest.java +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Copyright 2022 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.entities; - -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import com.fasterxml.jackson.databind.annotation.JsonSerialize; -import org.whispersystems.textsecuregcm.controllers.AccountController; -import org.whispersystems.textsecuregcm.util.ByteArrayBase64UrlAdapter; -import org.whispersystems.textsecuregcm.util.ExactlySize; - -public record ConfirmUsernameHashRequest( - @JsonSerialize(using = ByteArrayBase64UrlAdapter.Serializing.class) - @JsonDeserialize(using = ByteArrayBase64UrlAdapter.Deserializing.class) - @ExactlySize(AccountController.USERNAME_HASH_LENGTH) - byte[] usernameHash -) {} 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/ReserveUsernameHashRequest.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameHashRequest.java deleted file mode 100644 index 4d25312ff..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameHashRequest.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright 2022 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.entities; - - -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import com.fasterxml.jackson.databind.annotation.JsonSerialize; -import org.whispersystems.textsecuregcm.controllers.AccountController; -import org.whispersystems.textsecuregcm.util.ByteArrayBase64UrlAdapter; -import javax.validation.Valid; -import javax.validation.constraints.Size; -import java.util.List; - -public record ReserveUsernameHashRequest( - @Valid - @Size(min=1, max=AccountController.MAXIMUM_USERNAME_HASHES_LIST_LENGTH) - @JsonSerialize(contentUsing = ByteArrayBase64UrlAdapter.Serializing.class) - @JsonDeserialize(contentUsing = ByteArrayBase64UrlAdapter.Deserializing.class) - List usernameHashes -) {} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameHashResponse.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameHashResponse.java deleted file mode 100644 index b04b22efd..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameHashResponse.java +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Copyright 2022 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.entities; - -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import com.fasterxml.jackson.databind.annotation.JsonSerialize; -import org.whispersystems.textsecuregcm.controllers.AccountController; -import org.whispersystems.textsecuregcm.util.ByteArrayBase64UrlAdapter; -import org.whispersystems.textsecuregcm.util.ExactlySize; -import java.util.UUID; - -public record ReserveUsernameHashResponse( - @JsonSerialize(using = ByteArrayBase64UrlAdapter.Serializing.class) - @JsonDeserialize(using = ByteArrayBase64UrlAdapter.Deserializing.class) - @ExactlySize(AccountController.USERNAME_HASH_LENGTH) - byte[] usernameHash -) {} 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/entities/UsernameHashResponse.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameHashResponse.java deleted file mode 100644 index 2a7653181..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameHashResponse.java +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright 2022 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.entities; - -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import com.fasterxml.jackson.databind.annotation.JsonSerialize; -import org.whispersystems.textsecuregcm.controllers.AccountController; -import org.whispersystems.textsecuregcm.util.ByteArrayBase64UrlAdapter; -import org.whispersystems.textsecuregcm.util.ExactlySize; -import javax.validation.Valid; - -public record UsernameHashResponse( - @Valid - @JsonSerialize(using = ByteArrayBase64UrlAdapter.Serializing.class) - @JsonDeserialize(using = ByteArrayBase64UrlAdapter.Deserializing.class) - @ExactlySize(AccountController.USERNAME_HASH_LENGTH) - byte[] usernameHash -) {} 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/mappers/JsonMappingExceptionMapper.java b/service/src/main/java/org/whispersystems/textsecuregcm/mappers/JsonMappingExceptionMapper.java deleted file mode 100644 index 515a27296..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/mappers/JsonMappingExceptionMapper.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.whispersystems.textsecuregcm.mappers; - -import com.fasterxml.jackson.databind.JsonMappingException; -import javax.ws.rs.core.Response; -import javax.ws.rs.ext.ExceptionMapper; - -public class JsonMappingExceptionMapper implements ExceptionMapper { - @Override - public Response toResponse(final JsonMappingException exception) { - return Response.status(422).build(); - } -} 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 25719fae5..2192c5c67 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java @@ -16,15 +16,12 @@ import java.util.Optional; import java.util.UUID; import java.util.function.Predicate; import javax.annotation.Nullable; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import com.fasterxml.jackson.databind.annotation.JsonSerialize; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.auth.SaltedTokenHash; import org.whispersystems.textsecuregcm.auth.StoredRegistrationLock; import org.whispersystems.textsecuregcm.entities.AccountAttributes; import org.whispersystems.textsecuregcm.storage.Device.DeviceCapabilities; -import org.whispersystems.textsecuregcm.util.ByteArrayBase64UrlAdapter; import org.whispersystems.textsecuregcm.util.Util; public class Account { @@ -42,14 +39,10 @@ public class Account { private String number; @JsonProperty - @JsonSerialize(using = ByteArrayBase64UrlAdapter.Serializing.class) - @JsonDeserialize(using = ByteArrayBase64UrlAdapter.Deserializing.class) @Nullable - private byte[] usernameHash; + private String username; @JsonProperty - @JsonSerialize(using = ByteArrayBase64UrlAdapter.Serializing.class) - @JsonDeserialize(using = ByteArrayBase64UrlAdapter.Deserializing.class) @Nullable private byte[] reservedUsernameHash; @@ -133,16 +126,16 @@ public class Account { this.phoneNumberIdentifier = phoneNumberIdentifier; } - public Optional getUsernameHash() { + public Optional getUsername() { requireNotStale(); - return Optional.ofNullable(usernameHash); + return Optional.ofNullable(username); } - public void setUsernameHash(final byte[] usernameHash) { + public void setUsername(final String username) { requireNotStale(); - this.usernameHash = usernameHash; + this.username = username; } public Optional getReservedUsernameHash() { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidator.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidator.java index 507b6c0a8..2db069324 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidator.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidator.java @@ -7,16 +7,11 @@ package org.whispersystems.textsecuregcm.storage; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.whispersystems.textsecuregcm.util.Optionals; -import java.security.MessageDigest; -import java.security.SecureRandom; -import java.util.Arrays; - class AccountChangeValidator { private final boolean allowNumberChange; - private final boolean allowUsernameHashChange; + private final boolean allowUsernameChange; static final AccountChangeValidator GENERAL_CHANGE_VALIDATOR = new AccountChangeValidator(false, false); static final AccountChangeValidator NUMBER_CHANGE_VALIDATOR = new AccountChangeValidator(true, false); @@ -25,10 +20,10 @@ class AccountChangeValidator { private static final Logger logger = LoggerFactory.getLogger(AccountChangeValidator.class); AccountChangeValidator(final boolean allowNumberChange, - final boolean allowUsernameHashChange) { + final boolean allowUsernameChange) { this.allowNumberChange = allowNumberChange; - this.allowUsernameHashChange = allowUsernameHashChange; + this.allowUsernameChange = allowUsernameChange; } public void validateChange(final Account originalAccount, final Account updatedAccount) { @@ -49,21 +44,13 @@ class AccountChangeValidator { } } - if (!allowUsernameHashChange) { - // We can potentially replace this with the actual hash of some invalid username (e.g. 1nickname.123) - final byte[] dummyHash = new byte[32]; - new SecureRandom().nextBytes(dummyHash); + if (!allowUsernameChange) { + assert updatedAccount.getUsername().equals(originalAccount.getUsername()); - final byte[] updatedAccountUsernameHash = updatedAccount.getUsernameHash().orElse(dummyHash); - final byte[] originalAccountUsernameHash = originalAccount.getUsernameHash().orElse(dummyHash); - - boolean usernameUnchanged = MessageDigest.isEqual(updatedAccountUsernameHash, originalAccountUsernameHash); - - if (!usernameUnchanged) { - logger.error("Username hash changed via \"normal\" update; username hashes must be changed via reserveUsernameHash and confirmUsernameHash methods", + if (!updatedAccount.getUsername().equals(originalAccount.getUsername())) { + logger.error("Username changed via \"normal\" update; usernames must be changed via setUsername method", new RuntimeException()); } - assert usernameUnchanged; } } } 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 fe6aa1f9c..b862957fa 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java @@ -39,6 +39,7 @@ import org.whispersystems.textsecuregcm.util.AttributeValues; import org.whispersystems.textsecuregcm.util.ExceptionUtils; import org.whispersystems.textsecuregcm.util.SystemMapper; import org.whispersystems.textsecuregcm.util.UUIDUtil; +import org.whispersystems.textsecuregcm.util.UsernameNormalizer; import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; import software.amazon.awssdk.services.dynamodb.model.AttributeValue; @@ -63,14 +64,16 @@ public class Accounts extends AbstractDynamoDbStore { private static final Logger log = LoggerFactory.getLogger(Accounts.class); + 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_HASH_TIMER = Metrics.timer(name(Accounts.class, "clearUsernameHash")); + private static final Timer CLEAR_USERNAME_TIMER = Metrics.timer(name(Accounts.class, "clearUsername")); private static final Timer UPDATE_TIMER = Metrics.timer(name(Accounts.class, "update")); private static final Timer GET_BY_NUMBER_TIMER = Metrics.timer(name(Accounts.class, "getByNumber")); - private static final Timer GET_BY_USERNAME_HASH_TIMER = Metrics.timer(name(Accounts.class, "getByUsernameHash")); + private static final Timer GET_BY_USERNAME_TIMER = Metrics.timer(name(Accounts.class, "getByUsername")); private static final Timer GET_BY_PNI_TIMER = Metrics.timer(name(Accounts.class, "getByPni")); private static final Timer GET_BY_UUID_TIMER = Metrics.timer(name(Accounts.class, "getByUuid")); private static final Timer GET_ALL_FROM_START_TIMER = Metrics.timer(name(Accounts.class, "getAllFrom")); @@ -93,10 +96,8 @@ public class Accounts extends AbstractDynamoDbStore { static final String ATTR_VERSION = "V"; // canonically discoverable static final String ATTR_CANONICALLY_DISCOVERABLE = "C"; - // username hash; byte[] or null - static final String ATTR_USERNAME_HASH = "N"; - // confirmed; bool - static final String ATTR_CONFIRMED = "F"; + // username; string + static final String ATTR_USERNAME = "N"; // unidentified access key; byte[] or null static final String ATTR_UAK = "UAK"; // time to live; number @@ -295,23 +296,24 @@ public class Accounts extends AbstractDynamoDbStore { } /** - * Reserve a username hash under the account UUID + * Reserve a username under a token + * + * @return a reservation token that must be provided when {@link #confirmUsername(Account, String, UUID)} is called */ - public void reserveUsernameHash( + public UUID reserveUsername( final Account account, - final byte[] reservedUsernameHash, + final String reservedUsername, final Duration ttl) { final long startNanos = System.nanoTime(); // if there is an existing old reservation it will be cleaned up via ttl final Optional maybeOriginalReservation = account.getReservedUsernameHash(); - account.setReservedUsernameHash(reservedUsernameHash); + account.setReservedUsernameHash(reservedUsernameHash(account.getUuid(), reservedUsername)); boolean succeeded = false; final long expirationTime = clock.instant().plus(ttl).getEpochSecond(); - // Use account UUID as a "reservation token" - by providing this, the client proves ownership of the hash - UUID uuid = account.getUuid(); + final UUID reservationToken = UUID.randomUUID(); try { final List writeItems = new ArrayList<>(); @@ -319,12 +321,11 @@ public class Accounts extends AbstractDynamoDbStore { .put(Put.builder() .tableName(usernamesConstraintTableName) .item(Map.of( - KEY_ACCOUNT_UUID, AttributeValues.fromUUID(uuid), - ATTR_USERNAME_HASH, AttributeValues.fromByteArray(reservedUsernameHash), - ATTR_TTL, AttributeValues.fromLong(expirationTime), - ATTR_CONFIRMED, AttributeValues.fromBool(false))) - .conditionExpression("attribute_not_exists(#username_hash) OR (#ttl < :now)") - .expressionAttributeNames(Map.of("#username_hash", ATTR_USERNAME_HASH, "#ttl", ATTR_TTL)) + KEY_ACCOUNT_UUID, AttributeValues.fromUUID(reservationToken), + ATTR_USERNAME, AttributeValues.fromString(UsernameNormalizer.normalize(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()) @@ -334,7 +335,7 @@ public class Accounts extends AbstractDynamoDbStore { TransactWriteItem.builder() .update(Update.builder() .tableName(accountsTableName) - .key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(uuid))) + .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)) @@ -366,23 +367,42 @@ public class Accounts extends AbstractDynamoDbStore { } RESERVE_USERNAME_TIMER.record(System.nanoTime() - startNanos, TimeUnit.NANOSECONDS); } + return reservationToken; } /** - * Confirm (set) a previously reserved username hash + * Confirm (set) a previously reserved username * * @param account to update - * @param usernameHash believed to be available - * @throws ContestedOptimisticLockException if the account has been updated or the username has taken by someone else + * @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 confirmUsernameHash(final Account account, final byte[] usernameHash) + 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 maybeOriginalUsernameHash = account.getUsernameHash(); - final Optional maybeOriginalReservationHash = account.getReservedUsernameHash(); + final Optional maybeOriginalUsername = account.getUsername(); + final Optional maybeOriginalReservation = account.getReservedUsernameHash(); - account.setUsernameHash(usernameHash); + account.setUsername(username); account.setReservedUsernameHash(null); boolean succeeded = false; @@ -390,21 +410,20 @@ public class Accounts extends AbstractDynamoDbStore { try { final List writeItems = new ArrayList<>(); - // add the username hash to the constraint table, wiping out the ttl if we had already reserved the hash + // add the username to the constraint table, wiping out the ttl if we had already reserved the name + // Persist the normalized username in the usernamesConstraint table and the original username in the accounts table writeItems.add(TransactWriteItem.builder() .put(Put.builder() .tableName(usernamesConstraintTableName) .item(Map.of( KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()), - ATTR_USERNAME_HASH, AttributeValues.fromByteArray(usernameHash), - ATTR_CONFIRMED, AttributeValues.fromBool(true))) + ATTR_USERNAME, AttributeValues.fromString(UsernameNormalizer.normalize(username)))) // it's not in the constraint table OR it's expired OR it was reserved by us - .conditionExpression("attribute_not_exists(#username_hash) OR #ttl < :now OR (#aci = :aci AND #confirmed = :confirmed)") - .expressionAttributeNames(Map.of("#username_hash", ATTR_USERNAME_HASH, "#ttl", ATTR_TTL, "#aci", KEY_ACCOUNT_UUID, "#confirmed", ATTR_CONFIRMED)) + .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()), - ":aci", AttributeValues.fromUUID(account.getUuid()), - ":confirmed", AttributeValues.fromBool(false))) + ":reservation", AttributeValues.fromUUID(reservationToken.orElseGet(UUID::randomUUID)))) .returnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD) .build()) .build()); @@ -414,21 +433,21 @@ public class Accounts extends AbstractDynamoDbStore { .update(Update.builder() .tableName(accountsTableName) .key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()))) - .updateExpression("SET #data = :data, #username_hash = :username_hash ADD #version :version_increment") + .updateExpression("SET #data = :data, #username = :username ADD #version :version_increment") .conditionExpression("#version = :version") .expressionAttributeNames(Map.of("#data", ATTR_ACCOUNT_DATA, - "#username_hash", ATTR_USERNAME_HASH, + "#username", ATTR_USERNAME, "#version", ATTR_VERSION)) .expressionAttributeValues(Map.of( ":data", AttributeValues.fromByteArray(SystemMapper.getMapper().writeValueAsBytes(account)), - ":username_hash", AttributeValues.fromByteArray(usernameHash), + ":username", AttributeValues.fromString(username), ":version", AttributeValues.fromInt(account.getVersion()), ":version_increment", AttributeValues.fromInt(1))) .build()) .build()); - maybeOriginalUsernameHash.ifPresent(originalUsernameHash -> writeItems.add( - buildDelete(usernamesConstraintTableName, ATTR_USERNAME_HASH, originalUsernameHash))); + maybeOriginalUsername.ifPresent(originalUsername -> writeItems.add( + buildDelete(usernamesConstraintTableName, ATTR_USERNAME, UsernameNormalizer.normalize(originalUsername)))); final TransactWriteItemsRequest request = TransactWriteItemsRequest.builder() .transactItems(writeItems) @@ -447,17 +466,17 @@ public class Accounts extends AbstractDynamoDbStore { throw e; } finally { if (!succeeded) { - account.setUsernameHash(maybeOriginalUsernameHash.orElse(null)); - account.setReservedUsernameHash(maybeOriginalReservationHash.orElse(null)); + account.setUsername(maybeOriginalUsername.orElse(null)); + account.setReservedUsernameHash(maybeOriginalReservation.orElse(null)); } SET_USERNAME_TIMER.record(System.nanoTime() - startNanos, TimeUnit.NANOSECONDS); } } - public void clearUsernameHash(final Account account) { - account.getUsernameHash().ifPresent(usernameHash -> { - CLEAR_USERNAME_HASH_TIMER.record(() -> { - account.setUsernameHash(null); + public void clearUsername(final Account account) { + account.getUsername().ifPresent(username -> { + CLEAR_USERNAME_TIMER.record(() -> { + account.setUsername(null); boolean succeeded = false; @@ -469,10 +488,10 @@ public class Accounts extends AbstractDynamoDbStore { .update(Update.builder() .tableName(accountsTableName) .key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()))) - .updateExpression("SET #data = :data REMOVE #username_hash ADD #version :version_increment") + .updateExpression("SET #data = :data REMOVE #username ADD #version :version_increment") .conditionExpression("#version = :version") .expressionAttributeNames(Map.of("#data", ATTR_ACCOUNT_DATA, - "#username_hash", ATTR_USERNAME_HASH, + "#username", ATTR_USERNAME, "#version", ATTR_VERSION)) .expressionAttributeValues(Map.of( ":data", AttributeValues.fromByteArray(SystemMapper.getMapper().writeValueAsBytes(account)), @@ -481,7 +500,7 @@ public class Accounts extends AbstractDynamoDbStore { .build()) .build()); - writeItems.add(buildDelete(usernamesConstraintTableName, ATTR_USERNAME_HASH, usernameHash)); + writeItems.add(buildDelete(usernamesConstraintTableName, ATTR_USERNAME, UsernameNormalizer.normalize(username))); final TransactWriteItemsRequest request = TransactWriteItemsRequest.builder() .transactItems(writeItems) @@ -501,7 +520,7 @@ public class Accounts extends AbstractDynamoDbStore { throw e; } finally { if (!succeeded) { - account.setUsernameHash(usernameHash); + account.setUsername(username); } } }); @@ -582,27 +601,27 @@ public class Accounts extends AbstractDynamoDbStore { } } - public boolean usernameHashAvailable(final byte[] username) { - return usernameHashAvailable(Optional.empty(), username); + public boolean usernameAvailable(final String username) { + return usernameAvailable(Optional.empty(), username); } - public boolean usernameHashAvailable(final Optional accountUuid, final byte[] usernameHash) { - final Optional> usernameHashItem = itemByKey( - usernamesConstraintTableName, ATTR_USERNAME_HASH, AttributeValues.fromByteArray(usernameHash)); + public boolean usernameAvailable(final Optional reservationToken, final String username) { + final Optional> usernameItem = itemByKey( + usernamesConstraintTableName, ATTR_USERNAME, AttributeValues.fromString(UsernameNormalizer.normalize(username))); - if (usernameHashItem.isEmpty()) { - // username hash is free + if (usernameItem.isEmpty()) { + // username is free return true; } - final Map item = usernameHashItem.get(); + final Map item = usernameItem.get(); if (AttributeValues.getLong(item, ATTR_TTL, Long.MAX_VALUE) < clock.instant().getEpochSecond()) { - // username hash was reserved, but has expired + // username was reserved, but has expired return true; } - // username hash is reserved by us - return !AttributeValues.getBool(item, ATTR_CONFIRMED, true) && accountUuid + // username is reserved by us + return reservationToken .map(AttributeValues.getUUID(item, KEY_ACCOUNT_UUID, new UUID(0, 0))::equals) .orElse(false); } @@ -620,13 +639,13 @@ public class Accounts extends AbstractDynamoDbStore { } @Nonnull - public Optional getByUsernameHash(final byte[] usernameHash) { + public Optional getByUsername(final String username) { return getByIndirectLookup( - GET_BY_USERNAME_HASH_TIMER, + GET_BY_USERNAME_TIMER, usernamesConstraintTableName, - ATTR_USERNAME_HASH, - AttributeValues.fromByteArray(usernameHash), - item -> AttributeValues.getBool(item, ATTR_CONFIRMED, false) // ignore items that are reservations (not confirmed) + ATTR_USERNAME, + AttributeValues.fromString(UsernameNormalizer.normalize(username)), + item -> !item.containsKey(ATTR_TTL) // ignore items with a ttl (reservations) ); } @@ -646,8 +665,8 @@ public class Accounts extends AbstractDynamoDbStore { buildDelete(phoneNumberIdentifierConstraintTableName, ATTR_PNI_UUID, account.getPhoneNumberIdentifier()) )); - account.getUsernameHash().ifPresent(usernameHash -> transactWriteItems.add( - buildDelete(usernamesConstraintTableName, ATTR_USERNAME_HASH, usernameHash))); + account.getUsername().ifPresent(username -> transactWriteItems.add( + buildDelete(usernamesConstraintTableName, ATTR_USERNAME, UsernameNormalizer.normalize(username)))); final TransactWriteItemsRequest request = TransactWriteItemsRequest.builder() .transactItems(transactWriteItems).build(); @@ -788,11 +807,6 @@ public class Accounts extends AbstractDynamoDbStore { return buildDelete(tableName, keyName, AttributeValues.fromString(keyValue)); } - @Nonnull - private static TransactWriteItem buildDelete(final String tableName, final String keyName, final byte[] keyValue) { - return buildDelete(tableName, keyName, AttributeValues.fromByteArray(keyValue)); - } - @Nonnull private static TransactWriteItem buildDelete(final String tableName, final String keyName, final UUID keyValue) { return buildDelete(tableName, keyName, AttributeValues.fromUUID(keyValue)); @@ -829,6 +843,22 @@ public class Accounts extends AbstractDynamoDbStore { .collect(Collectors.joining(", ")); } + @Nonnull + public static byte[] reservedUsernameHash(final UUID accountId, final String reservedUsername) { + final MessageDigest sha256; + try { + sha256 = MessageDigest.getInstance("SHA-256"); + } catch (final NoSuchAlgorithmException e) { + throw new AssertionError(e); + } + final ByteBuffer byteBuffer = ByteBuffer.allocate(32 + 1); + sha256.update(UsernameNormalizer.normalize(reservedUsername).getBytes(StandardCharsets.UTF_8)); + sha256.update(UUIDUtil.toBytes(accountId)); + byteBuffer.put(RESERVED_USERNAME_HASH_VERSION); + byteBuffer.put(sha256.digest()); + return byteBuffer.array(); + } + @VisibleForTesting @Nonnull static Account fromItem(final Map item) { @@ -853,7 +883,7 @@ public class Accounts extends AbstractDynamoDbStore { account.setNumber(item.get(ATTR_ACCOUNT_E164).s(), phoneNumberIdentifierFromAttribute); account.setUuid(accountIdentifier); - account.setUsernameHash(AttributeValues.getByteArray(item, ATTR_USERNAME_HASH, null)); + account.setUsername(AttributeValues.getString(item, ATTR_USERNAME, null)); account.setVersion(Integer.parseInt(item.get(ATTR_VERSION).n())); account.setCanonicallyDiscoverable(Optional.ofNullable(item.get(ATTR_CANONICALLY_DISCOVERABLE)) .map(AttributeValue::bool) 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 8f2d59b19..8dd08df90 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -22,7 +22,6 @@ import java.io.IOException; import java.time.Clock; import java.time.Duration; import java.util.Arrays; -import java.util.Base64; import java.util.Collections; import java.util.List; import java.util.Map; @@ -51,6 +50,8 @@ 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.UsernameGenerator; +import org.whispersystems.textsecuregcm.util.UsernameNormalizer; import org.whispersystems.textsecuregcm.util.Util; public class AccountsManager { @@ -59,13 +60,13 @@ public class AccountsManager { private static final Timer createTimer = metricRegistry.timer(name(AccountsManager.class, "create")); private static final Timer updateTimer = metricRegistry.timer(name(AccountsManager.class, "update")); private static final Timer getByNumberTimer = metricRegistry.timer(name(AccountsManager.class, "getByNumber")); - private static final Timer getByUsernameHashTimer = metricRegistry.timer(name(AccountsManager.class, "getByUsernameHash")); + private static final Timer getByUsernameTimer = metricRegistry.timer(name(AccountsManager.class, "getByUsername")); private static final Timer getByUuidTimer = metricRegistry.timer(name(AccountsManager.class, "getByUuid")); private static final Timer deleteTimer = metricRegistry.timer(name(AccountsManager.class, "delete")); private static final Timer redisSetTimer = metricRegistry.timer(name(AccountsManager.class, "redisSet")); private static final Timer redisNumberGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisNumberGet")); - private static final Timer redisUsernameHashGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisUsernameHashGet")); + private static final Timer redisUsernameGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisUsernameGet")); private static final Timer redisPniGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisPniGet")); private static final Timer redisUuidGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisUuidGet")); private static final Timer redisDeleteTimer = metricRegistry.timer(name(AccountsManager.class, "redisDelete")); @@ -87,6 +88,7 @@ public class AccountsManager { private final DirectoryQueue directoryQueue; private final Keys keys; private final MessagesManager messagesManager; + private final ProhibitedUsernames prohibitedUsernames; private final ProfilesManager profilesManager; private final StoredVerificationCodeManager pendingAccounts; private final SecureStorageClient secureStorageClient; @@ -94,6 +96,7 @@ public class AccountsManager { private final ClientPresenceManager clientPresenceManager; private final ExperimentEnrollmentManager experimentEnrollmentManager; private final Clock clock; + private final UsernameGenerator usernameGenerator; private static final ObjectMapper mapper = SystemMapper.getMapper(); @@ -103,11 +106,9 @@ public class AccountsManager { // the owner. private static final long CACHE_TTL_SECONDS = Duration.ofDays(2).toSeconds(); - private static final Duration USERNAME_HASH_RESERVATION_TTL_MINUTES = Duration.ofMinutes(5); - @FunctionalInterface private interface AccountPersister { - void persistAccount(Account account) throws UsernameHashNotAvailableException; + void persistAccount(Account account) throws UsernameNotAvailableException; } public enum DeletionReason { @@ -129,11 +130,13 @@ public class AccountsManager { final DirectoryQueue directoryQueue, final Keys keys, final MessagesManager messagesManager, + final ProhibitedUsernames prohibitedUsernames, final ProfilesManager profilesManager, final StoredVerificationCodeManager pendingAccounts, final SecureStorageClient secureStorageClient, final SecureBackupClient secureBackupClient, final ClientPresenceManager clientPresenceManager, + final UsernameGenerator usernameGenerator, final ExperimentEnrollmentManager experimentEnrollmentManager, final Clock clock) { this.accounts = accounts; @@ -148,6 +151,8 @@ public class AccountsManager { this.secureStorageClient = secureStorageClient; this.secureBackupClient = secureBackupClient; this.clientPresenceManager = clientPresenceManager; + this.prohibitedUsernames = prohibitedUsernames; + this.usernameGenerator = usernameGenerator; this.experimentEnrollmentManager = experimentEnrollmentManager; this.clock = Objects.requireNonNull(clock); } @@ -319,43 +324,42 @@ public class AccountsManager { return updatedAccount.get(); } - public record UsernameReservation(Account account, byte[] reservedUsernameHash){} + public record UsernameReservation(Account account, String reservedUsername, UUID reservationToken){} /** - * Reserve a username hash so that no other accounts may take it. + * Generate a username from a nickname, and reserve it so no other accounts may take it. * - * The reserved hash can later be set with {@link #confirmReservedUsernameHash(Account, byte[])}. The reservation - * will eventually expire, after which point confirmReservedUsernameHash may fail if another account has taken the - * username hash. + * 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 requestedUsernameHashes the list of username hashes to attempt to reserve - * @return the reserved username hash and an updated Account object - * @throws UsernameHashNotAvailableException no username hash is available + * @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 reserveUsernameHash(final Account account, final List requestedUsernameHashes) throws UsernameHashNotAvailableException { + public UsernameReservation reserveUsername(final Account account, final String requestedNickname) throws UsernameNotAvailableException { if (!experimentEnrollmentManager.isEnrolled(account.getUuid(), USERNAME_EXPERIMENT_NAME)) { - throw new UsernameHashNotAvailableException(); + throw new UsernameNotAvailableException(); } + if (prohibitedUsernames.isProhibited(requestedNickname, account.getUuid())) { + throw new UsernameNotAvailableException(); + } redisDelete(account); class Reserver implements AccountPersister { - byte[] reservedUsernameHash; + UUID reservationToken; + String reservedUsername; @Override - public void persistAccount(final Account account) throws UsernameHashNotAvailableException { - for (byte[] usernameHash : requestedUsernameHashes) { - if (accounts.usernameHashAvailable(usernameHash)) { - reservedUsernameHash = usernameHash; - accounts.reserveUsernameHash( - account, - usernameHash, - USERNAME_HASH_RESERVATION_TTL_MINUTES); - return; - } - } - throw new UsernameHashNotAvailableException(); + 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(); @@ -365,28 +369,31 @@ public class AccountsManager { reserver, () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); - return new UsernameReservation(updatedAccount, reserver.reservedUsernameHash); + return new UsernameReservation(updatedAccount, reserver.reservedUsername, reserver.reservationToken); } /** - * Set a username hash previously reserved with {@link #reserveUsernameHash(Account, List)} + * Set a username previously reserved with {@link #reserveUsername(Account, String)} * * @param account the account to update - * @param reservedUsernameHash the previously reserved username hash - * @return the updated account with the username hash field set - * @throws UsernameHashNotAvailableException if the reserved username hash has been taken (because the reservation expired) - * @throws UsernameReservationNotFoundException if `reservedUsernameHash` was not reserved in the account + * @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 confirmReservedUsernameHash(final Account account, final byte[] reservedUsernameHash) throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { + 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 UsernameHashNotAvailableException(); + throw new UsernameNotAvailableException(); } - if (account.getUsernameHash().map(currentUsernameHash -> Arrays.equals(currentUsernameHash, reservedUsernameHash)).orElse(false)) { + + if (account.getUsername().map(reservedUsername::equals).orElse(false)) { // the client likely already succeeded and is retrying return account; } - if (!account.getReservedUsernameHash().map(oldHash -> Arrays.equals(oldHash, reservedUsernameHash)).orElse(false)) { + final byte[] newHash = Accounts.reservedUsernameHash(account.getUuid(), UsernameNormalizer.normalize(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(); @@ -398,23 +405,63 @@ public class AccountsManager { account, a -> true, a -> { - // though we know this username hash was reserved, the reservation could have lapsed - if (!accounts.usernameHashAvailable(Optional.of(account.getUuid()), reservedUsernameHash)) { - throw new UsernameHashNotAvailableException(); + // though we know this username was reserved, the reservation could have lapsed + if (!accounts.usernameAvailable(Optional.of(reservationToken), reservedUsername)) { + throw new UsernameNotAvailableException(); } - accounts.confirmUsernameHash(a, reservedUsernameHash); + accounts.confirmUsername(a, reservedUsername, reservationToken); }, () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); } - public Account clearUsernameHash(final Account account) { + /** + * 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 (prohibitedUsernames.isProhibited(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 failableUpdateWithRetries( + account, + a -> true, + // 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); + } + + public Account clearUsername(final Account account) { redisDelete(account); return updateWithRetries( account, a -> true, - accounts::clearUsernameHash, + accounts::clearUsername, () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); } @@ -500,7 +547,7 @@ public class AccountsManager { final AccountChangeValidator changeValidator) { try { return failableUpdateWithRetries(account, updater, persister::accept, retriever, changeValidator); - } catch (UsernameHashNotAvailableException e) { + } catch (UsernameNotAvailableException e) { // not possible throw new IllegalStateException(e); } @@ -510,7 +557,7 @@ public class AccountsManager { final Function updater, final AccountPersister persister, final Supplier retriever, - final AccountChangeValidator changeValidator) throws UsernameHashNotAvailableException { + final AccountChangeValidator changeValidator) throws UsernameNotAvailableException { Account originalAccount = cloneAccount(account); @@ -593,11 +640,11 @@ public class AccountsManager { } } - public Optional getByUsernameHash(final byte[] usernameHash) { - try (final Timer.Context ignored = getByUsernameHashTimer.time()) { - Optional account = redisGetByUsernameHash(usernameHash); + public Optional getByUsername(final String username) { + try (final Timer.Context ignored = getByUsernameTimer.time()) { + Optional account = redisGetByUsername(username); if (account.isEmpty()) { - account = accounts.getByUsernameHash(usernameHash); + account = accounts.getByUsername(username); account.ifPresent(this::redisSet); } @@ -674,8 +721,8 @@ public class AccountsManager { clientPresenceManager.disconnectPresence(account.getUuid(), device.getId()))); } - private String getUsernameHashAccountMapKey(byte[] usernameHash) { - return "UAccountMap::" + Base64.getUrlEncoder().withoutPadding().encodeToString(usernameHash); + private String getUsernameAccountMapKey(String username) { + return "UAccountMap::" + UsernameNormalizer.normalize(username); } private String getAccountMapKey(String key) { @@ -697,8 +744,8 @@ public class AccountsManager { commands.setex(getAccountMapKey(account.getNumber()), CACHE_TTL_SECONDS, account.getUuid().toString()); commands.setex(getAccountEntityKey(account.getUuid()), CACHE_TTL_SECONDS, accountJson); - account.getUsernameHash().ifPresent(usernameHash -> - commands.setex(getUsernameHashAccountMapKey(usernameHash), CACHE_TTL_SECONDS, account.getUuid().toString())); + account.getUsername().ifPresent(username -> + commands.setex(getUsernameAccountMapKey(username), CACHE_TTL_SECONDS, account.getUuid().toString())); }); } catch (JsonProcessingException e) { throw new IllegalStateException(e); @@ -713,8 +760,8 @@ public class AccountsManager { return redisGetBySecondaryKey(getAccountMapKey(e164), redisNumberGetTimer); } - private Optional redisGetByUsernameHash(byte[] usernameHash) { - return redisGetBySecondaryKey(getUsernameHashAccountMapKey(usernameHash), redisUsernameHashGetTimer); + private Optional redisGetByUsername(String username) { + return redisGetBySecondaryKey(getUsernameAccountMapKey(username), redisUsernameGetTimer); } private Optional redisGetBySecondaryKey(String secondaryKey, Timer timer) { @@ -765,7 +812,7 @@ public class AccountsManager { getAccountMapKey(account.getPhoneNumberIdentifier().toString()), getAccountEntityKey(account.getUuid())); - account.getUsernameHash().ifPresent(usernameHash -> connection.sync().del(getUsernameHashAccountMapKey(usernameHash))); + account.getUsername().ifPresent(username -> connection.sync().del(getUsernameAccountMapKey(username))); }); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameHashNotAvailableException.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameNotAvailableException.java similarity index 68% rename from service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameHashNotAvailableException.java rename to service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameNotAvailableException.java index 04f81f625..a2d25f49f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameHashNotAvailableException.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameNotAvailableException.java @@ -5,5 +5,5 @@ package org.whispersystems.textsecuregcm.storage; -public class UsernameHashNotAvailableException extends Exception { +public class UsernameNotAvailableException extends Exception { } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/ByteArrayBase64UrlAdapter.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/ByteArrayBase64UrlAdapter.java deleted file mode 100644 index 1e3c2933a..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/ByteArrayBase64UrlAdapter.java +++ /dev/null @@ -1,27 +0,0 @@ -package org.whispersystems.textsecuregcm.util; - -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.DeserializationContext; -import com.fasterxml.jackson.databind.JsonDeserializer; -import com.fasterxml.jackson.databind.JsonSerializer; -import com.fasterxml.jackson.databind.SerializerProvider; -import java.io.IOException; -import java.util.Base64; - -public class ByteArrayBase64UrlAdapter { - public static class Serializing extends JsonSerializer { - @Override - public void serialize(byte[] bytes, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) - throws IOException { - jsonGenerator.writeString(Base64.getUrlEncoder().withoutPadding().encodeToString(bytes)); - } - } - - public static class Deserializing extends JsonDeserializer { - @Override - public byte[] deserialize(JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException { - return Base64.getUrlDecoder().decode(jsonParser.getValueAsString()); - } - } -} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/Nickname.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/Nickname.java new file mode 100644 index 000000000..10c8f8a8f --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/Nickname.java @@ -0,0 +1,27 @@ +/* + * Copyright 2013-2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +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; + +@Target({ FIELD, PARAMETER }) +@Retention(RUNTIME) +@Constraint(validatedBy = NicknameValidator.class) +public @interface Nickname { + + String message() default "{org.whispersystems.textsecuregcm.util.Nickname.message}"; + + Class[] groups() default { }; + + Class[] payload() 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..0a1ff3168 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java @@ -0,0 +1,152 @@ +/* + * 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.time.Duration; +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 + * + *
  • do not start with a number
  • + *
  • are alphanumeric or underscores only
  • + *
  • have minimum length 3
  • + *
  • have maximum length 32
  • + *
    + * + * Usernames typically consist of a nickname and an integer discriminator + */ + public static final Pattern NICKNAME_PATTERN = Pattern.compile("^[_a-zA-Z][_a-zA-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; + private final Duration reservationTtl; + + public UsernameGenerator(UsernameConfiguration configuration) { + this(configuration.getDiscriminatorInitialWidth(), + configuration.getDiscriminatorMaxWidth(), + configuration.getAttemptsPerWidth(), + configuration.getReservationTtl()); + } + + @VisibleForTesting + public UsernameGenerator(int initialWidth, int discriminatorMaxWidth, int attemptsPerWidth, final Duration reservationTtl) { + this.initialWidth = initialWidth; + this.discriminatorMaxWidth = discriminatorMaxWidth; + this.attemptsPerWidth = attemptsPerWidth; + this.reservationTtl = reservationTtl; + } + + /** + * 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 = 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 String fromParts(final String nickname, final int discriminator) throws IllegalArgumentException { + if (!isValidNickname(nickname)) { + throw new IllegalArgumentException("Invalid nickname " + nickname); + } + // zero pad discriminators less than the discriminator initial width + return String.format("%s%s%0" + initialWidth + "d", nickname, SEPARATOR, discriminator); + } + + public Duration getReservationTtl() { + return reservationTtl; + } + + 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/UsernameNormalizer.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameNormalizer.java new file mode 100644 index 000000000..1d0ecaf6c --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameNormalizer.java @@ -0,0 +1,10 @@ +package org.whispersystems.textsecuregcm.util; + +import java.util.Locale; + +public final class UsernameNormalizer { + private UsernameNormalizer() {} + public static String normalize(final String username) { + return username.toLowerCase(Locale.ROOT); + } +} 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 191f95bcc..1e329f6df 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java @@ -17,8 +17,6 @@ import io.dropwizard.cli.EnvironmentCommand; import io.dropwizard.setup.Environment; import io.lettuce.core.resource.ClientResources; import java.time.Clock; -import java.util.Base64; -import java.util.List; import java.util.UUID; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -52,10 +50,10 @@ import org.whispersystems.textsecuregcm.storage.ProhibitedUsernames; import org.whispersystems.textsecuregcm.storage.ReportMessageDynamoDb; import org.whispersystems.textsecuregcm.storage.ReportMessageManager; import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; -import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableException; -import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundException; +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; @@ -67,18 +65,18 @@ public class AssignUsernameCommand extends EnvironmentCommand { try { - final AccountsManager.UsernameReservation reservation = accountsManager.reserveUsernameHash(account, - List.of(Base64.getUrlDecoder().decode(usernameHash))); - final Account result = accountsManager.confirmReservedUsernameHash(account, Base64.getUrlDecoder().decode(usernameHash)); - System.out.println("New username hash: " + usernameHash); - } catch (final UsernameHashNotAvailableException e) { - throw new IllegalArgumentException("Username hash already taken"); - } catch (final UsernameReservationNotFoundException e) { - throw new IllegalArgumentException("Username hash reservation not found"); + 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 3cf0d9134..7b7f9af1c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java @@ -55,6 +55,7 @@ import org.whispersystems.textsecuregcm.storage.ReportMessageManager; 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; @@ -194,9 +195,10 @@ public class DeleteUserCommand extends EnvironmentCommand maybeAccount; diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java index 54e125b19..57925e3a0 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java @@ -6,12 +6,12 @@ package org.whispersystems.textsecuregcm.controllers; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; 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; @@ -24,6 +24,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; +import com.fasterxml.jackson.core.JsonProcessingException; import com.google.common.collect.ImmutableSet; import com.google.common.net.HttpHeaders; import com.google.i18n.phonenumbers.NumberParseException; @@ -36,7 +37,6 @@ import java.nio.charset.StandardCharsets; import java.security.SecureRandom; import java.time.Duration; import java.util.Arrays; -import java.util.Base64; import java.util.Collections; import java.util.List; import java.util.Map; @@ -79,20 +79,20 @@ 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.ConfirmUsernameHashRequest; +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.ReserveUsernameHashRequest; -import org.whispersystems.textsecuregcm.entities.ReserveUsernameHashResponse; +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; import org.whispersystems.textsecuregcm.limits.RateLimitByIpFilter; -import org.whispersystems.textsecuregcm.entities.UsernameHashResponse; import org.whispersystems.textsecuregcm.limits.RateLimiter; import org.whispersystems.textsecuregcm.limits.RateLimiters; import org.whispersystems.textsecuregcm.mappers.ImpossiblePhoneNumberExceptionMapper; -import org.whispersystems.textsecuregcm.mappers.JsonMappingExceptionMapper; import org.whispersystems.textsecuregcm.mappers.NonNormalizedPhoneNumberExceptionMapper; import org.whispersystems.textsecuregcm.mappers.NonNormalizedPhoneNumberResponse; import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper; @@ -108,7 +108,7 @@ import org.whispersystems.textsecuregcm.storage.ChangeNumberManager; import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager; -import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableException; +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; @@ -119,6 +119,7 @@ import org.whispersystems.textsecuregcm.util.TestClock; @ExtendWith(DropwizardExtensionsSupport.class) class AccountControllerTest { + private static final String SENDER = "+14152222222"; private static final String SENDER_OLD = "+14151111111"; private static final String SENDER_PIN = "+14153333333"; @@ -130,18 +131,10 @@ class AccountControllerTest { private static final String SENDER_TRANSFER = "+14151111112"; private static final String RESTRICTED_COUNTRY = "800"; private static final String RESTRICTED_NUMBER = "+" + RESTRICTED_COUNTRY + "11111111"; - private static final String BASE_64_URL_USERNAME_HASH_1 = "9p6Tip7BFefFOJzv4kv4GyXEYsBVfk_WbjNejdlOvQE"; - private static final String BASE_64_URL_USERNAME_HASH_2 = "NLUom-CHwtemcdvOTTXdmXmzRIV7F05leS8lwkVK_vc"; - - private static final String INVALID_BASE_64_URL_USERNAME_HASH = "fA+VkNbvB6dVfx/6NpaRSK6mvhhAUBgDNWFaD7+7gvs="; - private static final String TOO_SHORT_BASE_64_URL_USERNAME_HASH = "P2oMuxx0xgGxSpTO0ACq3IztEOBDaV9t9YFu4bAGpQ"; - private static final byte[] USERNAME_HASH_1 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_1); - private static final byte[] USERNAME_HASH_2 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_2); - private static final byte[] INVALID_USERNAME_HASH = Base64.getDecoder().decode(INVALID_BASE_64_URL_USERNAME_HASH); - private static final byte[] TOO_SHORT_USERNAME_HASH = Base64.getUrlDecoder().decode(TOO_SHORT_BASE_64_URL_USERNAME_HASH); 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 NICE_HOST = "127.0.0.1"; private static final String RATE_LIMITED_IP_HOST = "10.0.0.1"; @@ -193,7 +186,6 @@ class AccountControllerTest { new PolymorphicAuthValueFactoryProvider.Binder<>( ImmutableSet.of(AuthenticatedAccount.class, DisabledPermittedAuthenticatedAccount.class))) - .addProvider(new JsonMappingExceptionMapper()) .addProvider(new RateLimitExceededExceptionMapper()) .addProvider(new ImpossiblePhoneNumberExceptionMapper()) .addProvider(new NonNormalizedPhoneNumberExceptionMapper()) @@ -280,6 +272,9 @@ class AccountControllerTest { return account; }); + when(accountsManager.setUsername(AuthHelper.VALID_ACCOUNT, "takenusername", null)) + .thenThrow(new UsernameNotAvailableException()); + when(changeNumberManager.changeNumber(any(), any(), any(), any(), any(), any())).thenAnswer((Answer) invocation -> { final Account account = invocation.getArgument(0, Account.class); final String number = invocation.getArgument(1, String.class); @@ -1653,140 +1648,143 @@ class AccountControllerTest { } @Test - void testReserveUsernameHash() throws UsernameHashNotAvailableException { - when(accountsManager.reserveUsernameHash(any(), any())) - .thenReturn(new AccountsManager.UsernameReservation(null, USERNAME_HASH_1)); - Response response = - resources.getJerseyTest() - .target("/v1/accounts/username_hash/reserve") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ReserveUsernameHashRequest(List.of(USERNAME_HASH_1, USERNAME_HASH_2)))); - assertThat(response.getStatus()).isEqualTo(200); - assertThat(response.readEntity(ReserveUsernameHashResponse.class)) - .satisfies(r -> assertThat(r.usernameHash()).hasSize(32)); - } - - @Test - void testReserveUsernameHashUnavailable() throws UsernameHashNotAvailableException { - when(accountsManager.reserveUsernameHash(any(), anyList())) - .thenThrow(new UsernameHashNotAvailableException()); - Response response = - resources.getJerseyTest() - .target("/v1/accounts/username_hash/reserve") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ReserveUsernameHashRequest(List.of(USERNAME_HASH_1, USERNAME_HASH_2)))); - assertThat(response.getStatus()).isEqualTo(409); - } - - @ParameterizedTest - @MethodSource - void testReserveUsernameHashListSizeInvalid(List usernameHashes) { - Response response = - resources.getJerseyTest() - .target("/v1/accounts/username_hash/reserve") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ReserveUsernameHashRequest(usernameHashes))); - assertThat(response.getStatus()).isEqualTo(422); - } - - static Stream testReserveUsernameHashListSizeInvalid() { - return Stream.of( - Arguments.of(Collections.nCopies(21, USERNAME_HASH_1)), - Arguments.of(Collections.emptyList()) - ); - } - - @Test - void testReserveUsernameHashInvalidHashSize() { - List usernameHashes = List.of(new byte[31]); - Response response = - resources.getJerseyTest() - .target("/v1/accounts/username_hash/reserve") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ReserveUsernameHashRequest(usernameHashes))); - assertThat(response.getStatus()).isEqualTo(422); - } - - @Test - void testReserveUsernameHashInvalidBase64UrlEncoding() { - Response response = - resources.getJerseyTest() - .target("/v1/accounts/username_hash/reserve") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json( - // Has '+' and '='characters which are invalid in base64url - """ - { - "usernameHashes": ["jh1jJ50oGn9wUXAFNtDus6AJgWOQ6XbZzF+wCv7OOQs="] - } - """)); - assertThat(response.getStatus()).isEqualTo(422); - } - - @Test - void testCommitUsername() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { + void testSetUsername() throws UsernameNotAvailableException { Account account = mock(Account.class); - when(account.getUsernameHash()).thenReturn(Optional.of(USERNAME_HASH_1)); - when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1))).thenReturn(account); + 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_hash/confirm") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1))); + .target("/v1/accounts/username") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.json(new UsernameRequest("N00bkilleR", null))); assertThat(response.getStatus()).isEqualTo(200); - assertArrayEquals(response.readEntity(UsernameHashResponse.class).usernameHash(), USERNAME_HASH_1); + assertThat(response.readEntity(UsernameResponse.class).username()).isEqualTo("N00bkilleR.1234"); } @Test - void testCommitUnreservedUsername() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { - when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1))) + 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_hash/confirm") + .target("/v1/accounts/username/confirm") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1))); + .put(Entity.json(new ConfirmUsernameRequest("n00bkiller.1234", RESERVATION_TOKEN))); assertThat(response.getStatus()).isEqualTo(409); } @Test - void testCommitLapsedUsername() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { - when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1))) - .thenThrow(new UsernameHashNotAvailableException()); + 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_hash/confirm") + .target("/v1/accounts/username/confirm") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1))); + .put(Entity.json(new ConfirmUsernameRequest("n00bkiller.1234", RESERVATION_TOKEN))); assertThat(response.getStatus()).isEqualTo(410); } + @Test + void testSetTakenUsername() { + Response response = + resources.getJerseyTest() + .target("/v1/accounts/username/") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.json(new UsernameRequest("takenusername", null))); + + assertThat(response.getStatus()).isEqualTo(409); + } + + @Test + void testSetInvalidUsername() { + Response response = + resources.getJerseyTest() + .target("/v1/accounts/username") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + // contains non-ascii character + .put(Entity.json(new UsernameRequest("pаypal", null))); + + assertThat(response.getStatus()).isEqualTo(422); + } + + @Test + void testSetInvalidPrefixUsername() throws JsonProcessingException { + Response response = + resources.getJerseyTest() + .target("/v1/accounts/username") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.json(new UsernameRequest("0n00bkiller", null))); + assertThat(response.getStatus()).isEqualTo(422); + } + + @Test + void testSetUsernameBadAuth() { + Response response = + resources.getJerseyTest() + .target("/v1/accounts/username") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.INVALID_PASSWORD)) + .put(Entity.json(new UsernameRequest("n00bkiller", null))); + assertThat(response.getStatus()).isEqualTo(401); + } + @Test void testDeleteUsername() { Response response = resources.getJerseyTest() - .target("/v1/accounts/username_hash/") + .target("/v1/accounts/username/") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) .delete(); assertThat(response.getStatus()).isEqualTo(204); - verify(accountsManager).clearUsernameHash(AuthHelper.VALID_ACCOUNT); + verify(accountsManager).clearUsername(AuthHelper.VALID_ACCOUNT); } @Test void testDeleteUsernameBadAuth() { Response response = resources.getJerseyTest() - .target("/v1/accounts/username_hash/") + .target("/v1/accounts/username/") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.INVALID_PASSWORD)) .delete(); @@ -2000,9 +1998,9 @@ class AccountControllerTest { final UUID uuid = UUID.randomUUID(); when(account.getUuid()).thenReturn(uuid); - when(accountsManager.getByUsernameHash(any())).thenReturn(Optional.of(account)); + when(accountsManager.getByUsername(eq("n00bkiller.1234"))).thenReturn(Optional.of(account)); Response response = resources.getJerseyTest() - .target(String.format("v1/accounts/username_hash/%s", BASE_64_URL_USERNAME_HASH_1)) + .target("v1/accounts/username/n00bkiller.1234") .request() .header(HttpHeaders.X_FORWARDED_FOR, "127.0.0.1") .get(); @@ -2012,9 +2010,9 @@ class AccountControllerTest { @Test void testLookupUsernameDoesNotExist() { - when(accountsManager.getByUsernameHash(any())).thenReturn(Optional.empty()); + when(accountsManager.getByUsername(eq("n00bkiller.1234"))).thenReturn(Optional.empty()); assertThat(resources.getJerseyTest() - .target(String.format("v1/accounts/username_hash/%s", BASE_64_URL_USERNAME_HASH_1)) + .target("v1/accounts/username/n00bkiller.1234") .request() .header(HttpHeaders.X_FORWARDED_FOR, "127.0.0.1") .get().getStatus()).isEqualTo(404); @@ -2026,7 +2024,7 @@ class AccountControllerTest { MockUtils.updateRateLimiterResponseToFail( rateLimiters, RateLimiters.Handle.USERNAME_LOOKUP, "127.0.0.1", expectedRetryAfter); final Response response = resources.getJerseyTest() - .target(String.format("v1/accounts/username_hash/%s", BASE_64_URL_USERNAME_HASH_1)) + .target("/v1/accounts/username/test.123") .request() .header(HttpHeaders.X_FORWARDED_FOR, "127.0.0.1") .get(); @@ -2035,34 +2033,6 @@ class AccountControllerTest { assertThat(response.getHeaderString("Retry-After")).isEqualTo(String.valueOf(expectedRetryAfter.toSeconds())); } - @Test - void testLookupUsernameAuthenticated() { - assertThat(resources.getJerseyTest() - .target(String.format("/v1/accounts/username_hash/%s", USERNAME_HASH_1)) - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .header(HttpHeaders.X_FORWARDED_FOR, "127.0.0.1") - .get() - .getStatus()).isEqualTo(400); - } - - @Test - void testLookupUsernameInvalidFormat() { - assertThat(resources.getJerseyTest() - .target(String.format("/v1/accounts/username_hash/%s", INVALID_USERNAME_HASH)) - .request() - .header(HttpHeaders.X_FORWARDED_FOR, "127.0.0.1") - .get() - .getStatus()).isEqualTo(422); - - assertThat(resources.getJerseyTest() - .target(String.format("/v1/accounts/username_hash/%s", TOO_SHORT_USERNAME_HASH)) - .request() - .header(HttpHeaders.X_FORWARDED_FOR, "127.0.0.1") - .get() - .getStatus()).isEqualTo(422); - } - @ParameterizedTest @MethodSource void pushTokensMatch(@Nullable final String pushChallenge, @Nullable final StoredVerificationCode storedVerificationCode, final boolean expectMatch) { 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 d435ea25a..2d2cace83 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java @@ -31,6 +31,7 @@ import java.security.SecureRandom; import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.time.ZoneId; import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Arrays; @@ -131,8 +132,6 @@ class ProfileControllerTest { private static final String ACCOUNT_PHONE_NUMBER_IDENTITY_KEY = "bazz"; private static final String ACCOUNT_TWO_IDENTITY_KEY = "bar"; private static final String ACCOUNT_TWO_PHONE_NUMBER_IDENTITY_KEY = "baz"; - private static final String BASE_64_URL_USERNAME_HASH = "9p6Tip7BFefFOJzv4kv4GyXEYsBVfk_WbjNejdlOvQE"; - private static final byte[] USERNAME_HASH = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH); @SuppressWarnings("unchecked") private static final DynamicConfigurationManager dynamicConfigurationManager = mock( DynamicConfigurationManager.class); @@ -198,7 +197,7 @@ class ProfileControllerTest { when(profileAccount.isAnnouncementGroupSupported()).thenReturn(false); when(profileAccount.isChangeNumberSupported()).thenReturn(false); when(profileAccount.getCurrentProfileVersion()).thenReturn(Optional.empty()); - when(profileAccount.getUsernameHash()).thenReturn(Optional.of(USERNAME_HASH)); + when(profileAccount.getUsername()).thenReturn(Optional.of("n00bkiller")); when(profileAccount.getUnidentifiedAccessKey()).thenReturn(Optional.of("1337".getBytes())); Account capabilitiesAccount = mock(Account.class); @@ -213,7 +212,7 @@ class ProfileControllerTest { when(accountsManager.getByE164(AuthHelper.VALID_NUMBER_TWO)).thenReturn(Optional.of(profileAccount)); when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID_TWO)).thenReturn(Optional.of(profileAccount)); when(accountsManager.getByPhoneNumberIdentifier(AuthHelper.VALID_PNI_TWO)).thenReturn(Optional.of(profileAccount)); - when(accountsManager.getByUsernameHash(USERNAME_HASH)).thenReturn(Optional.of(profileAccount)); + when(accountsManager.getByUsername("n00bkiller")).thenReturn(Optional.of(profileAccount)); when(accountsManager.getByE164(AuthHelper.VALID_NUMBER)).thenReturn(Optional.of(capabilitiesAccount)); when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(capabilitiesAccount)); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidatorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidatorTest.java index 484b05ff6..39ef6e24e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidatorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidatorTest.java @@ -10,7 +10,6 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import java.util.Base64; import java.util.Optional; import java.util.UUID; import java.util.stream.Stream; @@ -27,10 +26,8 @@ class AccountChangeValidatorTest { private static final UUID ORIGINAL_PNI = UUID.randomUUID(); private static final UUID CHANGED_PNI = UUID.randomUUID(); - private static final String BASE_64_URL_ORIGINAL_USERNAME = "9p6Tip7BFefFOJzv4kv4GyXEYsBVfk_WbjNejdlOvQE"; - private static final String BASE_64_URL_CHANGED_USERNAME = "NLUom-CHwtemcdvOTTXdmXmzRIV7F05leS8lwkVK_vc"; - private static final byte[] ORIGINAL_USERNAME_HASH = Base64.getUrlDecoder().decode(BASE_64_URL_ORIGINAL_USERNAME); - private static final byte[] CHANGED_USERNAME_HASH = Base64.getUrlDecoder().decode(BASE_64_URL_CHANGED_USERNAME); + private static final String ORIGINAL_USERNAME = "bruce_wayne"; + private static final String CHANGED_USERNAME = "batman"; @ParameterizedTest @MethodSource @@ -52,22 +49,22 @@ class AccountChangeValidatorTest { final Account originalAccount = mock(Account.class); when(originalAccount.getNumber()).thenReturn(ORIGINAL_NUMBER); when(originalAccount.getPhoneNumberIdentifier()).thenReturn(ORIGINAL_PNI); - when(originalAccount.getUsernameHash()).thenReturn(Optional.of(ORIGINAL_USERNAME_HASH)); + when(originalAccount.getUsername()).thenReturn(Optional.of(ORIGINAL_USERNAME)); final Account unchangedAccount = mock(Account.class); when(unchangedAccount.getNumber()).thenReturn(ORIGINAL_NUMBER); when(unchangedAccount.getPhoneNumberIdentifier()).thenReturn(ORIGINAL_PNI); - when(unchangedAccount.getUsernameHash()).thenReturn(Optional.of(ORIGINAL_USERNAME_HASH)); + when(unchangedAccount.getUsername()).thenReturn(Optional.of(ORIGINAL_USERNAME)); final Account changedNumberAccount = mock(Account.class); when(changedNumberAccount.getNumber()).thenReturn(CHANGED_NUMBER); when(changedNumberAccount.getPhoneNumberIdentifier()).thenReturn(CHANGED_PNI); - when(changedNumberAccount.getUsernameHash()).thenReturn(Optional.of(ORIGINAL_USERNAME_HASH)); + when(changedNumberAccount.getUsername()).thenReturn(Optional.of(ORIGINAL_USERNAME)); final Account changedUsernameAccount = mock(Account.class); when(changedUsernameAccount.getNumber()).thenReturn(ORIGINAL_NUMBER); when(changedUsernameAccount.getPhoneNumberIdentifier()).thenReturn(ORIGINAL_PNI); - when(changedUsernameAccount.getUsernameHash()).thenReturn(Optional.of(CHANGED_USERNAME_HASH)); + when(changedUsernameAccount.getUsername()).thenReturn(Optional.of(CHANGED_USERNAME)); return Stream.of( Arguments.of(originalAccount, unchangedAccount, AccountChangeValidator.GENERAL_CHANGE_VALIDATOR, true), 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 7a959036e..414aa74a7 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java @@ -33,6 +33,7 @@ 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; @@ -191,11 +192,13 @@ class AccountsManagerChangeNumberIntegrationTest { mock(DirectoryQueue.class), mock(Keys.class), mock(MessagesManager.class), + mock(ProhibitedUsernames.class), mock(ProfilesManager.class), mock(StoredVerificationCodeManager.class), 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 fdf42fd2a..e065b803e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java @@ -51,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; @@ -158,11 +159,13 @@ class AccountsManagerConcurrentModificationIntegrationTest { mock(DirectoryQueue.class), mock(Keys.class), mock(MessagesManager.class), + mock(ProhibitedUsernames.class), mock(ProfilesManager.class), mock(StoredVerificationCodeManager.class), 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 a5de6dca3..e078aa7a4 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -5,15 +5,18 @@ package org.whispersystems.textsecuregcm.storage; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; 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; @@ -28,17 +31,15 @@ import static org.mockito.Mockito.when; import io.lettuce.core.RedisException; import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; import java.time.Clock; -import java.time.Duration; import java.util.ArrayList; -import java.util.Base64; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.function.BiFunction; import java.util.function.Consumer; +import java.util.regex.Pattern; import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -46,7 +47,9 @@ 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; @@ -59,12 +62,10 @@ 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; +import org.whispersystems.textsecuregcm.util.UsernameNormalizer; class AccountsManagerTest { - private static final String BASE_64_URL_USERNAME_HASH_1 = "9p6Tip7BFefFOJzv4kv4GyXEYsBVfk_WbjNejdlOvQE"; - private static final String BASE_64_URL_USERNAME_HASH_2 = "NLUom-CHwtemcdvOTTXdmXmzRIV7F05leS8lwkVK_vc"; - private static final byte[] USERNAME_HASH_1 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_1); - private static final byte[] USERNAME_HASH_2 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_2); private Accounts accounts; private DeletedAccountsManager deletedAccountsManager; @@ -72,6 +73,7 @@ class AccountsManagerTest { private Keys keys; private MessagesManager messagesManager; private ProfilesManager profilesManager; + private ProhibitedUsernames prohibitedUsernames; private ExperimentEnrollmentManager enrollmentManager; private Map phoneNumberIdentifiersByE164; @@ -87,6 +89,8 @@ class AccountsManagerTest { return null; }; + private static final UUID RESERVATION_TOKEN = UUID.randomUUID(); + @BeforeEach void setup() throws InterruptedException { accounts = mock(Accounts.class); @@ -95,6 +99,7 @@ class AccountsManagerTest { keys = mock(Keys.class); messagesManager = mock(MessagesManager.class); profilesManager = mock(ProfilesManager.class); + prohibitedUsernames = mock(ProhibitedUsernames.class); //noinspection unchecked commands = mock(RedisAdvancedClusterCommands.class); @@ -138,7 +143,7 @@ class AccountsManagerTest { enrollmentManager = mock(ExperimentEnrollmentManager.class); when(enrollmentManager.isEnrolled(any(UUID.class), eq(AccountsManager.USERNAME_EXPERIMENT_NAME))).thenReturn(true); - when(accounts.usernameHashAvailable(any())).thenReturn(true); + when(accounts.usernameAvailable(any())).thenReturn(true); accountsManager = new AccountsManager( accounts, @@ -148,11 +153,13 @@ class AccountsManagerTest { directoryQueue, keys, messagesManager, + prohibitedUsernames, profilesManager, mock(StoredVerificationCodeManager.class), storageClient, backupClient, mock(ClientPresenceManager.class), + new UsernameGenerator(new UsernameConfiguration()), enrollmentManager, mock(Clock.class)); } @@ -162,8 +169,7 @@ class AccountsManagerTest { UUID uuid = UUID.randomUUID(); when(commands.get(eq("AccountMap::+14152222222"))).thenReturn(uuid.toString()); - when(commands.get(eq("Account3::" + uuid))).thenReturn( - "{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}"); + when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}"); Optional account = accountsManager.getByE164("+14152222222"); @@ -182,8 +188,7 @@ class AccountsManagerTest { void testGetAccountByUuidInCache() { UUID uuid = UUID.randomUUID(); - when(commands.get(eq("Account3::" + uuid))).thenReturn( - "{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}"); + when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}"); Optional account = accountsManager.getByAccountIdentifier(uuid); @@ -204,8 +209,7 @@ class AccountsManagerTest { UUID pni = UUID.randomUUID(); when(commands.get(eq("AccountMap::" + pni))).thenReturn(uuid.toString()); - when(commands.get(eq("Account3::" + uuid))).thenReturn( - "{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}"); + when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}"); Optional account = accountsManager.getByPhoneNumberIdentifier(pni); @@ -221,21 +225,21 @@ class AccountsManagerTest { } @Test - void testGetByUsernameHashInCache() { + void testGetByUsernameInCache() { UUID uuid = UUID.randomUUID(); - when(commands.get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1))).thenReturn(uuid.toString()); - when(commands.get(eq("Account3::" + uuid))).thenReturn( - String.format("{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\", \"usernameHash\": \"%s\"}", - BASE_64_URL_USERNAME_HASH_1)); + String username = "test"; - Optional account = accountsManager.getByUsernameHash(USERNAME_HASH_1); + when(commands.get(eq("UAccountMap::" + username))).thenReturn(uuid.toString()); + when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\", \"username\": \"test\"}"); + + Optional account = accountsManager.getByUsername(username); assertTrue(account.isPresent()); assertEquals(account.get().getNumber(), "+14152222222"); assertEquals(UUID.fromString("de24dc73-fbd8-41be-a7d5-764c70d9da7e"), account.get().getPhoneNumberIdentifier()); - assertArrayEquals(USERNAME_HASH_1, account.get().getUsernameHash().get()); + assertEquals(Optional.of(username), account.get().getUsername()); - verify(commands).get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1)); + verify(commands).get(eq("UAccountMap::" + username)); verify(commands).get(eq("Account3::" + uuid)); verifyNoMoreInteractions(commands); @@ -316,28 +320,29 @@ class AccountsManagerTest { } @Test - void testGetAccountByUsernameHashNotInCache() { + void testGetAccountByUsernameNotInCache() { UUID uuid = UUID.randomUUID(); + String username = "test"; Account account = AccountsHelper.generateTestAccount("+14152222222", uuid, UUID.randomUUID(), new ArrayList<>(), new byte[16]); - account.setUsernameHash(USERNAME_HASH_1); + account.setUsername(username); - when(commands.get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1))).thenReturn(null); - when(accounts.getByUsernameHash(USERNAME_HASH_1)).thenReturn(Optional.of(account)); + when(commands.get(eq("UAccountMap::" + username))).thenReturn(null); + when(accounts.getByUsername(username)).thenReturn(Optional.of(account)); - Optional retrieved = accountsManager.getByUsernameHash(USERNAME_HASH_1); + Optional retrieved = accountsManager.getByUsername(username); assertTrue(retrieved.isPresent()); assertSame(retrieved.get(), account); - verify(commands).get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1)); - verify(commands).setex(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1), anyLong(), eq(uuid.toString())); + verify(commands).get(eq("UAccountMap::" + username)); + verify(commands).setex(eq("UAccountMap::" + username), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("AccountMap::" + account.getPhoneNumberIdentifier()), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(commands); - verify(accounts).getByUsernameHash(USERNAME_HASH_1); + verify(accounts).getByUsername(username); verifyNoMoreInteractions(accounts); } @@ -417,26 +422,27 @@ class AccountsManagerTest { @Test void testGetAccountByUsernameBrokenCache() { UUID uuid = UUID.randomUUID(); + String username = "test"; Account account = AccountsHelper.generateTestAccount("+14152222222", uuid, UUID.randomUUID(), new ArrayList<>(), new byte[16]); - account.setUsernameHash(USERNAME_HASH_1); + account.setUsername(username); - when(commands.get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1))).thenThrow(new RedisException("OH NO")); - when(accounts.getByUsernameHash(USERNAME_HASH_1)).thenReturn(Optional.of(account)); + when(commands.get(eq("UAccountMap::" + username))).thenThrow(new RedisException("OH NO")); + when(accounts.getByUsername(username)).thenReturn(Optional.of(account)); - Optional retrieved = accountsManager.getByUsernameHash(USERNAME_HASH_1); + Optional retrieved = accountsManager.getByUsername(username); assertTrue(retrieved.isPresent()); assertSame(retrieved.get(), account); - verify(commands).get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1)); - verify(commands).setex(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1), anyLong(), eq(uuid.toString())); + verify(commands).get(eq("UAccountMap::" + username)); + verify(commands).setex(eq("UAccountMap::" + username), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("AccountMap::" + account.getPhoneNumberIdentifier()), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(commands); - verify(accounts).getByUsernameHash(USERNAME_HASH_1); + verify(accounts).getByUsername(username); verifyNoMoreInteractions(accounts); } @@ -728,96 +734,188 @@ class AccountsManagerTest { } @Test - void testReserveUsernameHash() throws UsernameHashNotAvailableException { + void testSetUsername() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - final List usernameHashes = List.of(new byte[32], new byte[32]); - when(accounts.usernameHashAvailable(any())).thenReturn(true); - accountsManager.reserveUsernameHash(account, usernameHashes); - verify(accounts).reserveUsernameHash(eq(account), eq(new byte[32]), eq(Duration.ofMinutes(5))); + final String nickname = "test"; + assertDoesNotThrow(() -> accountsManager.setUsername(account, nickname, null)); + verify(accounts).setUsername(eq(account), startsWith(nickname)); } @Test - void testReserveUsernameHashNotAvailable() { + void testReserveUsername() throws UsernameNotAvailableException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - when(accounts.usernameHashAvailable(any())).thenReturn(false); - - assertThrows(UsernameHashNotAvailableException.class, () -> accountsManager.reserveUsernameHash(account, List.of( - USERNAME_HASH_1, USERNAME_HASH_2))); + final String nickname = "beethoven"; + accountsManager.reserveUsername(account, nickname); + verify(accounts).reserveUsername(eq(account), startsWith(nickname), any()); } @Test - void testReserveUsernameDisabled() { + void testSetReservedUsername() throws UsernameNotAvailableException, UsernameReservationNotFoundException { 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(UsernameHashNotAvailableException.class, () -> accountsManager.reserveUsernameHash(account, List.of( - USERNAME_HASH_1))); + 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 testConfirmReservedUsernameHash() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { + void testSetReservedHashNameMismatch() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - setReservationHash(account, USERNAME_HASH_1); - when(accounts.usernameHashAvailable(eq(Optional.of(account.getUuid())), eq(USERNAME_HASH_1))).thenReturn(true); - accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1); - verify(accounts).confirmUsernameHash(eq(account), eq(USERNAME_HASH_1)); - } - - @Test - void testConfirmReservedHashNameMismatch() { - final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - setReservationHash(account, USERNAME_HASH_1); - when(accounts.usernameHashAvailable(eq(Optional.of(account.getUuid())), eq(USERNAME_HASH_1))).thenReturn(true); + setReservationHash(account, "pluto.1234"); + when(accounts.usernameAvailable(eq(Optional.of(RESERVATION_TOKEN)), eq("pluto.1234"))).thenReturn(true); assertThrows(UsernameReservationNotFoundException.class, - () -> accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_2)); + () -> accountsManager.confirmReservedUsername(account, "goofy.1234", RESERVATION_TOKEN)); } @Test - void testConfirmReservedLapsed() { + void testSetReservedHashAciMismatch() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - // hash was reserved, but the reservation lapsed and another account took it - setReservationHash(account, USERNAME_HASH_1); - when(accounts.usernameHashAvailable(eq(Optional.of(account.getUuid())), eq(USERNAME_HASH_1))).thenReturn(false); - assertThrows(UsernameHashNotAvailableException.class, () -> accountsManager.confirmReservedUsernameHash(account, - USERNAME_HASH_1)); - verify(accounts, never()).confirmUsernameHash(any(), any()); + 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 testConfirmReservedRetry() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { + void testSetReservedLapsed() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - account.setUsernameHash(USERNAME_HASH_1); + 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.confirmReservedUsernameHash(account, USERNAME_HASH_1); + accountsManager.confirmReservedUsername(account, username, RESERVATION_TOKEN); verifyNoInteractions(accounts); } @Test - void testConfirmReservedUsernameHashWithNoReservation() { - final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), - new ArrayList<>(), new byte[16]); - assertThrows(UsernameReservationNotFoundException.class, - () -> accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1)); - verify(accounts, never()).confirmUsernameHash(any(), any()); + void testSetUsernameSameUsername() { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + final String nickname = "test"; + account.setUsername(nickname + ".123"); + + // should be treated as a replayed request + assertDoesNotThrow(() -> accountsManager.setUsername(account, nickname, null)); + verify(accounts, never()).setUsername(eq(account), any()); } @Test - void testClearUsernameHash() { - Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - account.setUsernameHash(USERNAME_HASH_1); - accountsManager.clearUsernameHash(account); - verify(accounts).clearUsernameHash(eq(account)); + void testSetUsernameReroll() throws UsernameNotAvailableException { + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); + final String nickname = "test"; + final String username = nickname + ".ZZZ"; + account.setUsername(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)))); + } + + @Test + 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"; + + ArgumentMatcher isWide = (String username) -> { + String[] spl = username.split(Pattern.quote(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); + + 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 + 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 nickname = "reserved"; + when(prohibitedUsernames.isProhibited(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, nickname, null)); + assertTrue(account.getUsername().isEmpty()); } @Test void testSetUsernameViaUpdate() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - assertThrows(AssertionError.class, () -> accountsManager.update(account, a -> a.setUsernameHash(USERNAME_HASH_1))); + assertThrows(AssertionError.class, () -> accountsManager.update(account, a -> a.setUsername("test"))); } - private void setReservationHash(final Account account, final byte[] reservedUsernameHash) { - account.setReservedUsernameHash(reservedUsernameHash); + @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 void setReservationHash(final Account account, final String reservedUsername) { + account.setReservedUsernameHash(Accounts.reservedUsernameHash(account.getUuid(), reservedUsername)); } private static Device generateTestDevice(final long lastSeen) { 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 2a9818a13..c3c952ba3 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java @@ -6,10 +6,9 @@ package org.whispersystems.textsecuregcm.storage; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.argThat; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; @@ -18,15 +17,11 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.security.SecureRandom; import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Base64; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.UUID; @@ -34,6 +29,8 @@ import java.util.function.Consumer; 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; @@ -45,6 +42,7 @@ 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.AttributeDefinition; import software.amazon.awssdk.services.dynamodb.model.AttributeValue; import software.amazon.awssdk.services.dynamodb.model.CreateTableRequest; @@ -61,11 +59,7 @@ class AccountsManagerUsernameIntegrationTest { private static final String PNI_ASSIGNMENT_TABLE_NAME = "pni_assignment_test"; private static final String USERNAMES_TABLE_NAME = "usernames_test"; private static final String PNI_TABLE_NAME = "pni_test"; - private static final String BASE_64_URL_USERNAME_HASH_1 = "9p6Tip7BFefFOJzv4kv4GyXEYsBVfk_WbjNejdlOvQE"; - private static final String BASE_64_URL_USERNAME_HASH_2 = "NLUom-CHwtemcdvOTTXdmXmzRIV7F05leS8lwkVK_vc"; private static final int SCAN_PAGE_SIZE = 1; - private static final byte[] USERNAME_HASH_1 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_1); - private static final byte[] USERNAME_HASH_2 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_2); @RegisterExtension static DynamoDbExtension ACCOUNTS_DYNAMO_EXTENSION = DynamoDbExtension.builder() @@ -92,6 +86,7 @@ class AccountsManagerUsernameIntegrationTest { private AccountsManager accountsManager; private Accounts accounts; + private UsernameGenerator usernameGenerator; @BeforeEach void setup() throws InterruptedException { @@ -112,12 +107,12 @@ class AccountsManagerUsernameIntegrationTest { CreateTableRequest createUsernamesTableRequest = CreateTableRequest.builder() .tableName(USERNAMES_TABLE_NAME) .keySchema(KeySchemaElement.builder() - .attributeName(Accounts.ATTR_USERNAME_HASH) + .attributeName(Accounts.ATTR_USERNAME) .keyType(KeyType.HASH) .build()) .attributeDefinitions(AttributeDefinition.builder() - .attributeName(Accounts.ATTR_USERNAME_HASH) - .attributeType(ScalarAttributeType.B) + .attributeName(Accounts.ATTR_USERNAME) + .attributeType(ScalarAttributeType.S) .build()) .provisionedThroughput(DynamoDbExtension.DEFAULT_PROVISIONED_THROUGHPUT) .build(); @@ -157,6 +152,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") @@ -179,159 +176,211 @@ class AccountsManagerUsernameIntegrationTest { mock(DirectoryQueue.class), mock(Keys.class), mock(MessagesManager.class), + mock(ProhibitedUsernames.class), mock(ProfilesManager.class), mock(StoredVerificationCodeManager.class), mock(SecureStorageClient.class), mock(SecureBackupClient.class), mock(ClientPresenceManager.class), + usernameGenerator, experimentEnrollmentManager, mock(Clock.class)); } + private static int discriminator(String username) { + return Integer.parseInt(username.substring(username.indexOf(UsernameGenerator.SEPARATOR) + 1)); + } + @Test - void testNoUsernames() throws InterruptedException { + void testSetClearUsername() throws UsernameNotAvailableException, InterruptedException { Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), new ArrayList<>()); - List usernameHashes = List.of(USERNAME_HASH_1, USERNAME_HASH_2); - int i = 0; - for (byte[] hash : usernameHashes) { + 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(); + } + + @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_HASH, AttributeValues.fromByteArray(hash))); + 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())); } - i++; ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbClient().putItem(PutItemRequest.builder() .tableName(USERNAMES_TABLE_NAME) .item(item) .build()); } - assertThrows(UsernameHashNotAvailableException.class, () -> {accountsManager.reserveUsernameHash(account, usernameHashes);}); - assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsernameHash()).isEmpty(); + assertThrows(UsernameNotAvailableException.class, () -> { + if (reserve) { + accountsManager.reserveUsername(account, "n00bkiller"); + } else { + accountsManager.setUsername(account, "n00bkiller", null); + } + }); + assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsername()).isEmpty(); } @Test - void testReserveUsernameSnatched() throws InterruptedException, UsernameHashNotAvailableException { + void testUsernameSnatched() throws InterruptedException, UsernameNotAvailableException { final Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), new ArrayList<>()); - ArrayList usernameHashes = new ArrayList<>(Arrays.asList(USERNAME_HASH_1, USERNAME_HASH_2)); - for (byte[] hash : usernameHashes) { + 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_HASH, AttributeValues.fromByteArray(hash))) + Accounts.ATTR_USERNAME, AttributeValues.fromString(usernameGenerator.fromParts("n00bkiller", i)))) .build()); } - - byte[] availableHash = new byte[32]; - new SecureRandom().nextBytes(availableHash); - usernameHashes.add(availableHash); - // 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).usernameHashAvailable(any()); - final byte[] username = accountsManager - .reserveUsernameHash(account, usernameHashes) - .reservedUsernameHash(); - - assertArrayEquals(username, availableHash); + 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), - // 5 more attempts until "availableHash" returns true - verify(accounts, times(4)).usernameHashAvailable(any()); + // 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)); } @Test - public void testReserveConfirmClear() - throws InterruptedException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { + 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(); - // reserve - AccountsManager.UsernameReservation reservation = accountsManager.reserveUsernameHash(account, List.of( - USERNAME_HASH_1)); - assertArrayEquals(reservation.account().getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1); - assertThat(accountsManager.getByUsernameHash(reservation.reservedUsernameHash())).isEmpty(); + account = accountsManager.confirmReservedUsername( + account, + reservation.reservedUsername(), + reservation.reservationToken()); - // confirm - account = accountsManager.confirmReservedUsernameHash( - reservation.account(), - reservation.reservedUsernameHash()); - assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_1); - assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1).orElseThrow().getUuid()).isEqualTo( + 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.clearUsernameHash(account); - assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); - assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsernameHash()).isEmpty(); + account = accountsManager.clearUsername(account); + assertThat(accountsManager.getByUsername(newUsername)).isEmpty(); + assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsername()).isEmpty(); + } @Test public void testReservationLapsed() - throws InterruptedException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { + 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.reserveUsernameHash(account, List.of( - USERNAME_HASH_1)); + 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_HASH, AttributeValues.fromByteArray(USERNAME_HASH_1))) + .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.reserveUsernameHash(account2, List.of( - USERNAME_HASH_1)); - assertArrayEquals(reservation2.reservedUsernameHash(), USERNAME_HASH_1); + final AccountsManager.UsernameReservation reservation2 = accountsManager.reserveUsername(account2, "n00bkiller" + ); + assertThat(reservation2.reservedUsername()).isEqualTo(reservedUsername); - assertThrows(UsernameHashNotAvailableException.class, - () -> accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1)); - account2 = accountsManager.confirmReservedUsernameHash(reservation2.account(), USERNAME_HASH_1); - assertEquals(accountsManager.getByUsernameHash(USERNAME_HASH_1).orElseThrow().getUuid(), account2.getUuid()); - assertArrayEquals(account2.getUsernameHash().orElseThrow(), USERNAME_HASH_1); + assertThrows(UsernameNotAvailableException.class, + () -> accountsManager.confirmReservedUsername(reservation1.account(), reservedUsername, reservation1.reservationToken())); + accountsManager.confirmReservedUsername(reservation2.account(), reservedUsername, reservation2.reservationToken()); } @Test - void testUsernameSetReserveAnotherClearSetReserved() - throws InterruptedException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { + 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(); - // Set username hash - final AccountsManager.UsernameReservation reservation1 = accountsManager.reserveUsernameHash(account, List.of( - USERNAME_HASH_1)); - account = accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1); + assertThat(reservation.reservedUsername()).startsWith("other"); + assertThat(account.getUsername()).hasValueSatisfying(s -> s.startsWith("n00bkiller")); - // Reserve another hash on the same account - final AccountsManager.UsernameReservation reservation2 = accountsManager.reserveUsernameHash(account, List.of( - USERNAME_HASH_2)); - account = reservation2.account(); - - assertArrayEquals(account.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_2); - assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_1); - - // Clear the set username hash but not the reserved one - account = accountsManager.clearUsernameHash(account); + account = accountsManager.clearUsername(account); assertThat(account.getReservedUsernameHash()).isPresent(); - assertThat(account.getUsernameHash()).isEmpty(); + assertThat(account.getUsername()).isEmpty(); - // Confirm second reservation - account = accountsManager.confirmReservedUsernameHash(account, reservation2.reservedUsernameHash()); - assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_2); + 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 6ce81a94a..773174c84 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java @@ -9,8 +9,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -23,7 +21,6 @@ import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; -import java.util.Base64; import java.util.List; import java.util.Map; import java.util.Optional; @@ -31,6 +28,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; @@ -68,15 +66,9 @@ class AccountsTest { private static final String NUMBER_CONSTRAINT_TABLE_NAME = "numbers_test"; private static final String PNI_CONSTRAINT_TABLE_NAME = "pni_test"; private static final String USERNAME_CONSTRAINT_TABLE_NAME = "username_test"; - private static final String BASE_64_URL_USERNAME_HASH_1 = "9p6Tip7BFefFOJzv4kv4GyXEYsBVfk_WbjNejdlOvQE"; - private static final String BASE_64_URL_USERNAME_HASH_2 = "NLUom-CHwtemcdvOTTXdmXmzRIV7F05leS8lwkVK_vc"; - private static final byte[] USERNAME_HASH_1 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_1); - private static final byte[] USERNAME_HASH_2 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_2); private static final int SCAN_PAGE_SIZE = 1; - - @RegisterExtension static DynamoDbExtension dynamoDbExtension = DynamoDbExtension.builder() .tableName(ACCOUNTS_TABLE_NAME) @@ -126,12 +118,12 @@ class AccountsTest { CreateTableRequest createUsernamesTableRequest = CreateTableRequest.builder() .tableName(USERNAME_CONSTRAINT_TABLE_NAME) .keySchema(KeySchemaElement.builder() - .attributeName(Accounts.ATTR_USERNAME_HASH) + .attributeName(Accounts.ATTR_USERNAME) .keyType(KeyType.HASH) .build()) .attributeDefinitions(AttributeDefinition.builder() - .attributeName(Accounts.ATTR_USERNAME_HASH) - .attributeType(ScalarAttributeType.B) + .attributeName(Accounts.ATTR_USERNAME) + .attributeType(ScalarAttributeType.S) .build()) .provisionedThroughput(DynamoDbExtension.DEFAULT_PROVISIONED_THROUGHPUT) .build(); @@ -622,38 +614,40 @@ class AccountsTest { } @Test - void testSwitchUsernameHashes() { + void testSetUsername() { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); - assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); + final String username = "TeST"; - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); - accounts.confirmUsernameHash(account, USERNAME_HASH_1); + assertThat(accounts.getByUsername(username)).isEmpty(); + + accounts.setUsername(account, username); { - final Optional maybeAccount = accounts.getByUsernameHash(USERNAME_HASH_1); + final Optional maybeAccount = accounts.getByUsername(username); assertThat(maybeAccount).hasValueSatisfying(retrievedAccount -> - assertThat(retrievedAccount.getUsernameHash()).hasValueSatisfying(retrievedUsernameHash -> - assertArrayEquals(retrievedUsernameHash, USERNAME_HASH_1))); + assertThat(retrievedAccount.getUsername()).hasValueSatisfying(retrievedUsername -> + assertThat(retrievedUsername).isEqualTo(username))); verifyStoredState(account.getNumber(), account.getUuid(), account.getPhoneNumberIdentifier(), maybeAccount.orElseThrow(), account); } - accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofDays(1)); - accounts.confirmUsernameHash(account, USERNAME_HASH_2); + final String secondUsername = username + "2"; - assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); + accounts.setUsername(account, secondUsername); + + assertThat(accounts.getByUsername(username)).isEmpty(); assertThat(dynamoDbExtension.getDynamoDbClient() .getItem(GetItemRequest.builder() .tableName(USERNAME_CONSTRAINT_TABLE_NAME) - .key(Map.of(Accounts.ATTR_USERNAME_HASH, AttributeValues.fromByteArray(USERNAME_HASH_1))) + .key(Map.of(Accounts.ATTR_USERNAME, AttributeValues.fromString("test"))) .build()) .item()).isEmpty(); { - final Optional maybeAccount = accounts.getByUsernameHash(USERNAME_HASH_2); + final Optional maybeAccount = accounts.getByUsername(secondUsername); assertThat(maybeAccount).isPresent(); verifyStoredState(account.getNumber(), account.getUuid(), account.getPhoneNumberIdentifier(), @@ -662,51 +656,38 @@ class AccountsTest { } @Test - void testUsernameHashConflict() { + void testSetUsernameConflict() { final Account firstAccount = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); final Account secondAccount = generateAccount("+18005559876", UUID.randomUUID(), UUID.randomUUID()); accounts.create(firstAccount); accounts.create(secondAccount); - // first account reserves and confirms username hash - assertThatNoException().isThrownBy(() -> { - accounts.reserveUsernameHash(firstAccount, USERNAME_HASH_1, Duration.ofDays(1)); - accounts.confirmUsernameHash(firstAccount, USERNAME_HASH_1); - }); + final String username = "test"; - final Optional maybeAccount = accounts.getByUsernameHash(USERNAME_HASH_1); + assertThatNoException().isThrownBy(() -> accounts.setUsername(firstAccount, username)); + + final Optional maybeAccount = accounts.getByUsername(username); assertThat(maybeAccount).isPresent(); verifyStoredState(firstAccount.getNumber(), firstAccount.getUuid(), firstAccount.getPhoneNumberIdentifier(), maybeAccount.get(), firstAccount); - // throw an error if second account tries to reserve or confirm the same username hash assertThatExceptionOfType(ContestedOptimisticLockException.class) - .isThrownBy(() -> accounts.reserveUsernameHash(secondAccount, USERNAME_HASH_1, Duration.ofDays(1))); - assertThatExceptionOfType(ContestedOptimisticLockException.class) - .isThrownBy(() -> accounts.confirmUsernameHash(secondAccount, USERNAME_HASH_1)); + .isThrownBy(() -> accounts.setUsername(secondAccount, username)); - // throw an error if first account tries to reserve or confirm the username hash that it has already confirmed - assertThatExceptionOfType(ContestedOptimisticLockException.class) - .isThrownBy(() -> accounts.reserveUsernameHash(firstAccount, USERNAME_HASH_1, Duration.ofDays(1))); - assertThatExceptionOfType(ContestedOptimisticLockException.class) - .isThrownBy(() -> accounts.confirmUsernameHash(firstAccount, USERNAME_HASH_1)); - - assertThat(secondAccount.getReservedUsernameHash()).isEmpty(); - assertThat(secondAccount.getUsernameHash()).isEmpty(); + assertThat(secondAccount.getUsername()).isEmpty(); } @Test - void testConfirmUsernameHashVersionMismatch() { + void testSetUsernameVersionMismatch() { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); account.setVersion(account.getVersion() + 77); assertThatExceptionOfType(ContestedOptimisticLockException.class) - .isThrownBy(() -> accounts.confirmUsernameHash(account, USERNAME_HASH_1)); + .isThrownBy(() -> accounts.setUsername(account, "test")); - assertThat(account.getUsernameHash()).isEmpty(); + assertThat(account.getUsername()).isEmpty(); } @Test @@ -714,15 +695,16 @@ class AccountsTest { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); - accounts.confirmUsernameHash(account, USERNAME_HASH_1); - assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isPresent(); + final String username = "TeST"; - accounts.clearUsernameHash(account); + accounts.setUsername(account, username); + assertThat(accounts.getByUsername(username)).isPresent(); - assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); + accounts.clearUsername(account); + + assertThat(accounts.getByUsername(username)).isEmpty(); assertThat(accounts.getByAccountIdentifier(account.getUuid())) - .hasValueSatisfying(clearedAccount -> assertThat(clearedAccount.getUsernameHash()).isEmpty()); + .hasValueSatisfying(clearedAccount -> assertThat(clearedAccount.getUsername()).isEmpty()); } @Test @@ -730,7 +712,7 @@ class AccountsTest { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); - assertThatNoException().isThrownBy(() -> accounts.clearUsernameHash(account)); + assertThatNoException().isThrownBy(() -> accounts.clearUsername(account)); } @Test @@ -738,136 +720,167 @@ class AccountsTest { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); - accounts.confirmUsernameHash(account, USERNAME_HASH_1); + final String username = "test"; + + accounts.setUsername(account, username); account.setVersion(account.getVersion() + 12); - assertThatExceptionOfType(ContestedOptimisticLockException.class).isThrownBy(() -> accounts.clearUsernameHash(account)); + assertThatExceptionOfType(ContestedOptimisticLockException.class).isThrownBy(() -> accounts.clearUsername(account)); - assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_1); + assertThat(account.getUsername()).hasValueSatisfying(u -> assertThat(u).isEqualTo(username)); } @Test - void testReservedUsernameHash() { + 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); - accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(1)); - assertArrayEquals(account1.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1); - assertThat(account1.getUsernameHash()).isEmpty(); + 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 or confirm the same username hash + // account 2 shouldn't be able to reserve the username if it's the same when normalized assertThrows(ContestedOptimisticLockException.class, - () -> accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1))); + () -> accounts.reserveUsername(account2, "gARFIELd", Duration.ofDays(1))); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.confirmUsernameHash(account2, USERNAME_HASH_1)); - assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); + () -> accounts.confirmUsername(account2, "gARFIELd", UUID.randomUUID())); + assertThat(accounts.getByUsername("gARFIELd")).isEmpty(); - accounts.confirmUsernameHash(account1, USERNAME_HASH_1); + accounts.confirmUsername(account1, "GarfielD", token); assertThat(account1.getReservedUsernameHash()).isEmpty(); - assertArrayEquals(account1.getUsernameHash().orElseThrow(), USERNAME_HASH_1); - assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).get().getUuid()).isEqualTo(account1.getUuid()); + assertThat(account1.getUsername()).get().isEqualTo("GarfielD"); + assertThat(accounts.getByUsername("GarfielD").get().getUuid()).isEqualTo(account1.getUuid()); final Map usernameConstraintRecord = dynamoDbExtension.getDynamoDbClient() .getItem(GetItemRequest.builder() .tableName(USERNAME_CONSTRAINT_TABLE_NAME) - .key(Map.of(Accounts.ATTR_USERNAME_HASH, AttributeValues.fromByteArray(USERNAME_HASH_1))) + .key(Map.of(Accounts.ATTR_USERNAME, AttributeValues.fromString("garfield"))) .build()) .item(); - assertThat(usernameConstraintRecord).containsKey(Accounts.ATTR_USERNAME_HASH); + assertThat(usernameConstraintRecord).containsKey(Accounts.ATTR_USERNAME); assertThat(usernameConstraintRecord).doesNotContainKey(Accounts.ATTR_TTL); } @Test - void testUsernameHashAvailable() { + void testUsernameAvailable() { final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account1); - accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(1)); - assertThat(accounts.usernameHashAvailable(USERNAME_HASH_1)).isFalse(); - assertThat(accounts.usernameHashAvailable(Optional.empty(), USERNAME_HASH_1)).isFalse(); - assertThat(accounts.usernameHashAvailable(Optional.of(UUID.randomUUID()), USERNAME_HASH_1)).isFalse(); - assertThat(accounts.usernameHashAvailable(Optional.of(account1.getUuid()), USERNAME_HASH_1)).isTrue(); + final String username = "UnSinkaBlesam"; - accounts.confirmUsernameHash(account1, USERNAME_HASH_1); - assertThat(accounts.usernameHashAvailable(USERNAME_HASH_1)).isFalse(); - assertThat(accounts.usernameHashAvailable(Optional.empty(), USERNAME_HASH_1)).isFalse(); - assertThat(accounts.usernameHashAvailable(Optional.of(UUID.randomUUID()), USERNAME_HASH_1)).isFalse(); - assertThat(accounts.usernameHashAvailable(Optional.of(account1.getUuid()), USERNAME_HASH_1)).isFalse(); + 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 testConfirmReservedUsernameHashWrongAccountUuid() { - 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.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(1)); - assertArrayEquals(account1.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1); - assertThat(account1.getUsernameHash()).isEmpty(); + @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(); - // only account1 should be able to confirm the reserved hash assertThrows(ContestedOptimisticLockException.class, - () -> accounts.confirmUsernameHash(account2, USERNAME_HASH_1)); + () -> accounts.confirmUsername(account, "grumpy", UUID.randomUUID())); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.setUsername(account, "grumpy")); } @Test - void testConfirmExpiredReservedUsernameHash() { + 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); + final String username = "snowball.02"; - accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(2)); + accounts.reserveUsername(account1, username, Duration.ofDays(2)); - Runnable runnable = () -> accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1)); + Supplier take = () -> accounts.reserveUsername(account2, username, Duration.ofDays(2)); for (int i = 0; i <= 2; i++) { clock.pin(Instant.EPOCH.plus(Duration.ofDays(i))); - assertThrows(ContestedOptimisticLockException.class, runnable::run); + assertThrows(ContestedOptimisticLockException.class, take::get); } - // after 2 days, can reserve and confirm the hash + // after 2 days, can take the name clock.pin(Instant.EPOCH.plus(Duration.ofDays(2)).plus(Duration.ofSeconds(1))); - runnable.run(); - assertEquals(account2.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1); - - accounts.confirmUsernameHash(account2, USERNAME_HASH_1); + final UUID token = take.get(); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(2))); + () -> accounts.reserveUsername(account1, username, Duration.ofDays(2))); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.confirmUsernameHash(account1, USERNAME_HASH_1)); - assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).get().getUuid()).isEqualTo(account2.getUuid()); + () -> accounts.setUsername(account1, username)); + + accounts.confirmUsername(account2, username, token); + assertThat(accounts.getByUsername(username).get().getUuid()).isEqualTo(account2.getUuid()); } @Test - void testRetryReserveUsernameHash() { + 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); + final String username = "simon.123"; + + accounts.reserveUsername(account1, username, Duration.ofDays(2)); + + Runnable take = () -> accounts.setUsername(account2, username); + + for (int i = 0; i <= 2; i++) { + clock.pin(Instant.EPOCH.plus(Duration.ofDays(i))); + assertThrows(ContestedOptimisticLockException.class, take::run); + } + + // after 2 days, can take the name + clock.pin(Instant.EPOCH.plus(Duration.ofDays(2)).plus(Duration.ofSeconds(1))); + take.run(); + + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.reserveUsername(account1, username, Duration.ofDays(2))); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.setUsername(account1, username)); + assertThat(accounts.getByUsername(username).get().getUuid()).isEqualTo(account2.getUuid()); + } + + @Test + void testRetryReserveUsername() { final Account account = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(2)); + accounts.reserveUsername(account, "jorts", Duration.ofDays(2)); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(2)), - "Shouldn't be able to re-reserve same username hash (would extend ttl)"); + () -> accounts.reserveUsername(account, "jorts", Duration.ofDays(2)), + "Shouldn't be able to re-reserve same username (would extend ttl)"); } @Test - void testReserveConfirmUsernameHashVersionConflict() { + void testReserveUsernameVersionConflict() { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); account.setVersion(account.getVersion() + 12); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1))); + () -> accounts.reserveUsername(account, "salem", Duration.ofDays(1))); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.confirmUsernameHash(account, USERNAME_HASH_1)); - assertThat(account.getReservedUsernameHash()).isEmpty(); - assertThat(account.getUsernameHash()).isEmpty(); + () -> accounts.setUsername(account, "salem")); + } private Device generateDevice(long id) { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernamesTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernamesTest.java new file mode 100644 index 000000000..211840be3 --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernamesTest.java @@ -0,0 +1,69 @@ +/* + * Copyright 2013-2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.storage; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.UUID; +import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import software.amazon.awssdk.services.dynamodb.model.AttributeDefinition; +import software.amazon.awssdk.services.dynamodb.model.ScalarAttributeType; + +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(ProhibitedUsernames.KEY_PATTERN) + .attributeDefinition(AttributeDefinition.builder() + .attributeName(ProhibitedUsernames.KEY_PATTERN) + .attributeType(ScalarAttributeType.S) + .build()) + .build(); + + private static final UUID RESERVED_FOR_UUID = UUID.randomUUID(); + + private ProhibitedUsernames prohibitedUsernames; + + @BeforeEach + void setUp() { + prohibitedUsernames = + new ProhibitedUsernames(dynamoDbExtension.getDynamoDbClient(), RESERVED_USERNAMES_TABLE_NAME); + } + + @ParameterizedTest + @MethodSource + void isReserved(final String username, final UUID uuid, final boolean expectReserved) { + prohibitedUsernames.prohibitUsername(".*myusername.*", RESERVED_FOR_UUID); + prohibitedUsernames.prohibitUsername("^foobar$", RESERVED_FOR_UUID); + + assertEquals(expectReserved, prohibitedUsernames.isProhibited(username, uuid)); + } + + private static Stream isReserved() { + return Stream.of( + Arguments.of("myusername", UUID.randomUUID(), true), + Arguments.of("myusername", RESERVED_FOR_UUID, false), + Arguments.of("thyusername", UUID.randomUUID(), false), + Arguments.of("somemyusername", UUID.randomUUID(), true), + Arguments.of("myusernamesome", UUID.randomUUID(), true), + Arguments.of("somemyusernamesome", UUID.randomUUID(), true), + Arguments.of("MYUSERNAME", UUID.randomUUID(), true), + Arguments.of("foobar", UUID.randomUUID(), true), + Arguments.of("foobar", RESERVED_FOR_UUID, false), + Arguments.of("somefoobar", UUID.randomUUID(), false), + Arguments.of("foobarsome", UUID.randomUUID(), false), + Arguments.of("somefoobarsome", UUID.randomUUID(), false), + Arguments.of("FOOBAR", UUID.randomUUID(), true)); + } +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java index 94fe45301..622112e0a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java @@ -113,7 +113,7 @@ public class AccountsHelper { case "getUuid" -> when(updatedAccount.getUuid()).thenAnswer(stubbing); case "getPhoneNumberIdentifier" -> when(updatedAccount.getPhoneNumberIdentifier()).thenAnswer(stubbing); case "getNumber" -> when(updatedAccount.getNumber()).thenAnswer(stubbing); - case "getUsername" -> when(updatedAccount.getUsernameHash()).thenAnswer(stubbing); + case "getUsername" -> when(updatedAccount.getUsername()).thenAnswer(stubbing); case "getDevices" -> when(updatedAccount.getDevices()).thenAnswer(stubbing); case "getDevice" -> when(updatedAccount.getDevice(stubbing.getInvocation().getArgument(0))).thenAnswer(stubbing); case "getMasterDevice" -> when(updatedAccount.getMasterDevice()).thenAnswer(stubbing); 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..d494d76ed --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameGeneratorTest.java @@ -0,0 +1,145 @@ +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.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; +import java.util.stream.Stream; + +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) { + assertThat(UsernameGenerator.isValidNickname(nickname)).isEqualTo(valid); + } + + static Stream nicknameValidation() { + return Stream.of( + Arguments.of("Test", true, "upper case"), + Arguments.of("tesT", true, "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("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", true), + 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 zeroPadDiscriminators() { + 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"); + assertThat(generator.fromParts("test", 99999)).isEqualTo("test.99999"); + } + + @Test + public void expectedWidth() throws UsernameNotAvailableException { + 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, 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, TTL); + 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, TTL); + 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, TTL); + 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, TTL); + 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, 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)))); + } + // 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.substring(username.indexOf(UsernameGenerator.SEPARATOR) + 1)); + } +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameNormalizerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameNormalizerTest.java new file mode 100644 index 000000000..432b318fe --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameNormalizerTest.java @@ -0,0 +1,17 @@ +package org.whispersystems.textsecuregcm.tests.util; + +import org.junit.jupiter.api.Test; +import org.whispersystems.textsecuregcm.util.UsernameNormalizer; + +import static org.assertj.core.api.Assertions.assertThat; + +public class UsernameNormalizerTest { + + @Test + public void usernameNormalization() { + assertThat(UsernameNormalizer.normalize("TeST")).isEqualTo("test"); + assertThat(UsernameNormalizer.normalize("TeST_")).isEqualTo("test_"); + assertThat(UsernameNormalizer.normalize("TeST_.123")).isEqualTo("test_.123"); + } + +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/util/NicknameValidatorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/util/NicknameValidatorTest.java new file mode 100644 index 000000000..bdc1e6998 --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/util/NicknameValidatorTest.java @@ -0,0 +1,41 @@ +/* + * Copyright 2013-2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.stream.Stream; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +class NicknameValidatorTest { + + @ParameterizedTest + @MethodSource + void isValid(final String username, final boolean expectValid) { + final NicknameValidator nicknameValidator = new NicknameValidator(); + + assertEquals(expectValid, nicknameValidator.isValid(username, null)); + } + + private static Stream isValid() { + return Stream.of( + Arguments.of("test", true), + Arguments.of("_test", true), + Arguments.of("test123", true), + Arguments.of("a", false), // Too short + 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 + Arguments.of(" ", false), + Arguments.of("", false), + Arguments.of(null, false) + ); + } +}