From 24f515ccb41136f2feae051fc276963c50fd19e6 Mon Sep 17 00:00:00 2001 From: Katherine Yen Date: Thu, 2 Feb 2023 11:20:44 -0800 Subject: [PATCH] Revert "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 ++++------- ...=> UsernameHashNotAvailableException.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 | 248 +++++++++------- .../controllers/ProfileControllerTest.java | 7 +- .../storage/AccountChangeValidatorTest.java | 15 +- ...ntsManagerChangeNumberIntegrationTest.java | 3 - ...ConcurrentModificationIntegrationTest.java | 3 - .../storage/AccountsManagerTest.java | 270 ++++++------------ ...ccountsManagerUsernameIntegrationTest.java | 221 ++++++-------- .../textsecuregcm/storage/AccountsTest.java | 231 +++++++-------- .../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, 799 insertions(+), 1474 deletions(-) delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/configuration/UsernameConfiguration.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameHashRequest.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameRequest.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameHashRequest.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameHashResponse.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameRequest.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameResponse.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameHashResponse.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameRequest.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameResponse.java create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/mappers/JsonMappingExceptionMapper.java rename service/src/main/java/org/whispersystems/textsecuregcm/storage/{UsernameNotAvailableException.java => UsernameHashNotAvailableException.java} (68%) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/ByteArrayBase64UrlAdapter.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/Nickname.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/NicknameValidator.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameNormalizer.java delete mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernamesTest.java delete mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameGeneratorTest.java delete mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameNormalizerTest.java delete 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 b784c897a..30929bdd8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java @@ -48,7 +48,6 @@ 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; @@ -255,11 +254,6 @@ public class WhisperServerConfiguration extends Configuration { @JsonProperty private ReportMessageConfiguration reportMessage = new ReportMessageConfiguration(); - @Valid - @NotNull - @JsonProperty - private UsernameConfiguration username = new UsernameConfiguration(); - @Valid @JsonProperty private SpamFilterConfiguration spamFilterConfiguration; @@ -447,10 +441,6 @@ 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 2dc69451e..f6810d455 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -123,6 +123,7 @@ 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; @@ -192,7 +193,6 @@ 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,7 +209,6 @@ 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; @@ -351,8 +350,6 @@ 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 deleted file mode 100644 index 623287053..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/UsernameConfiguration.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * 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 1eb3e642a..8061f3a84 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -26,6 +26,7 @@ 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; @@ -75,18 +76,17 @@ import org.whispersystems.textsecuregcm.entities.AccountIdentifierResponse; import org.whispersystems.textsecuregcm.entities.AccountIdentityResponse; import org.whispersystems.textsecuregcm.entities.ApnRegistrationId; import org.whispersystems.textsecuregcm.entities.ChangePhoneNumberRequest; -import org.whispersystems.textsecuregcm.entities.ConfirmUsernameRequest; +import org.whispersystems.textsecuregcm.entities.ConfirmUsernameHashRequest; 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.ReserveUsernameRequest; -import org.whispersystems.textsecuregcm.entities.ReserveUsernameResponse; +import org.whispersystems.textsecuregcm.entities.ReserveUsernameHashRequest; +import org.whispersystems.textsecuregcm.entities.ReserveUsernameHashResponse; 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.UsernameNotAvailableException; +import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableException; 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,11 +136,7 @@ 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"; @@ -447,7 +443,7 @@ public class AccountController { return new AccountIdentityResponse(account.getUuid(), account.getNumber(), account.getPhoneNumberIdentifier(), - account.getUsername().orElse(null), + account.getUsernameHash().orElse(null), existingAccount.map(Account::isStorageSupported).orElse(false)); } @@ -508,7 +504,7 @@ public class AccountController { updatedAccount.getUuid(), updatedAccount.getNumber(), updatedAccount.getPhoneNumberIdentifier(), - updatedAccount.getUsername().orElse(null), + updatedAccount.getUsernameHash().orElse(null), updatedAccount.isStorageSupported()); } catch (MismatchedDevicesException e) { throw new WebApplicationException(Response.status(409) @@ -687,96 +683,78 @@ public class AccountController { return new AccountIdentityResponse(auth.getAccount().getUuid(), auth.getAccount().getNumber(), auth.getAccount().getPhoneNumberIdentifier(), - auth.getAccount().getUsername().orElse(null), + auth.getAccount().getUsernameHash().orElse(null), auth.getAccount().isStorageSupported()); } @Timed @DELETE - @Path("/username") + @Path("/username_hash") @Produces(MediaType.APPLICATION_JSON) - public void deleteUsername(@Auth AuthenticatedAccount auth) { - accounts.clearUsername(auth.getAccount()); + public void deleteUsernameHash(@Auth AuthenticatedAccount auth) { + accounts.clearUsernameHash(auth.getAccount()); } - @Timed @PUT - @Path("/username/reserved") + @Path("/username_hash/reserve") @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) - public ReserveUsernameResponse reserveUsername(@Auth AuthenticatedAccount auth, + public ReserveUsernameHashResponse reserveUsernameHash(@Auth AuthenticatedAccount auth, @HeaderParam(HeaderUtils.X_SIGNAL_AGENT) String userAgent, - @NotNull @Valid ReserveUsernameRequest usernameRequest) throws RateLimitExceededException { + @NotNull @Valid ReserveUsernameHashRequest 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.reserveUsername( + final AccountsManager.UsernameReservation reservation = accounts.reserveUsernameHash( auth.getAccount(), - usernameRequest.nickname() + usernameRequest.usernameHashes() ); - return new ReserveUsernameResponse(reservation.reservedUsername(), reservation.reservationToken()); - } catch (final UsernameNotAvailableException e) { + return new ReserveUsernameHashResponse(reservation.reservedUsernameHash()); + } catch (final UsernameHashNotAvailableException e) { throw new WebApplicationException(Status.CONFLICT); } } @Timed @PUT - @Path("/username/confirm") + @Path("/username_hash/confirm") @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) - public UsernameResponse confirmUsername(@Auth AuthenticatedAccount auth, + public UsernameHashResponse confirmUsernameHash(@Auth AuthenticatedAccount auth, @HeaderParam(HeaderUtils.X_SIGNAL_AGENT) String userAgent, - @NotNull @Valid ConfirmUsernameRequest confirmRequest) throws RateLimitExceededException { + @NotNull @Valid ConfirmUsernameHashRequest confirmRequest) throws RateLimitExceededException { rateLimiters.getUsernameSetLimiter().validate(auth.getAccount().getUuid()); try { - final Account account = accounts.confirmReservedUsername(auth.getAccount(), confirmRequest.usernameToConfirm(), confirmRequest.reservationToken()); + final Account account = accounts.confirmReservedUsernameHash(auth.getAccount(), confirmRequest.usernameHash()); return account - .getUsername() - .map(UsernameResponse::new) + .getUsernameHash() + .map(UsernameHashResponse::new) .orElseThrow(() -> new IllegalStateException("Could not get username after setting")); } catch (final UsernameReservationNotFoundException e) { throw new WebApplicationException(Status.CONFLICT); - } catch (final UsernameNotAvailableException e) { + } catch (final UsernameHashNotAvailableException 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/{username}") + @Path("/username_hash/{usernameHash}") @Produces(MediaType.APPLICATION_JSON) @RateLimitedByIp(RateLimiters.Handle.USERNAME_LOOKUP) - public AccountIdentifierResponse lookupUsername( + public AccountIdentifierResponse lookupUsernameHash( @HeaderParam(HeaderUtils.X_SIGNAL_AGENT) final String userAgent, - @PathParam("username") final String username, + @HeaderParam(HttpHeaders.X_FORWARDED_FOR) final String forwardedFor, + @PathParam("usernameHash") final String usernameHash, @Context final HttpServletRequest request) throws RateLimitExceededException { // Disallow clients from making authenticated requests to this endpoint @@ -784,10 +762,21 @@ public class AccountController { throw new BadRequestException(); } - checkUsername(username, userAgent); + 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()); + } return accounts - .getByUsername(username) + .getByUsernameHash(hash) .map(Account::getUuid) .map(AccountIdentifierResponse::new) .orElseThrow(() -> new WebApplicationException(Status.NOT_FOUND)); @@ -944,15 +933,6 @@ 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 d1b2c5762..450ea51a6 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 String username, + @Nullable byte[] usernameHash, 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 new file mode 100644 index 000000000..0ead70093 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameHashRequest.java @@ -0,0 +1,19 @@ +/* + * 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 deleted file mode 100644 index 49110012e..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameRequest.java +++ /dev/null @@ -1,12 +0,0 @@ -/* - * 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 new file mode 100644 index 000000000..4d25312ff --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameHashRequest.java @@ -0,0 +1,23 @@ +/* + * 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 new file mode 100644 index 000000000..b04b22efd --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameHashResponse.java @@ -0,0 +1,20 @@ +/* + * 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 deleted file mode 100644 index 4c1433860..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameRequest.java +++ /dev/null @@ -1,12 +0,0 @@ -/* - * 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 deleted file mode 100644 index de16f488f..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ReserveUsernameResponse.java +++ /dev/null @@ -1,10 +0,0 @@ -/* - * 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 new file mode 100644 index 000000000..2a7653181 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameHashResponse.java @@ -0,0 +1,21 @@ +/* + * 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 deleted file mode 100644 index 7a5844b9a..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameRequest.java +++ /dev/null @@ -1,12 +0,0 @@ -/* - * 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 deleted file mode 100644 index ed47fce71..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameResponse.java +++ /dev/null @@ -1,8 +0,0 @@ -/* - * 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 new file mode 100644 index 000000000..515a27296 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/mappers/JsonMappingExceptionMapper.java @@ -0,0 +1,12 @@ +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 2192c5c67..25719fae5 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java @@ -16,12 +16,15 @@ 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 { @@ -39,10 +42,14 @@ public class Account { private String number; @JsonProperty + @JsonSerialize(using = ByteArrayBase64UrlAdapter.Serializing.class) + @JsonDeserialize(using = ByteArrayBase64UrlAdapter.Deserializing.class) @Nullable - private String username; + private byte[] usernameHash; @JsonProperty + @JsonSerialize(using = ByteArrayBase64UrlAdapter.Serializing.class) + @JsonDeserialize(using = ByteArrayBase64UrlAdapter.Deserializing.class) @Nullable private byte[] reservedUsernameHash; @@ -126,16 +133,16 @@ public class Account { this.phoneNumberIdentifier = phoneNumberIdentifier; } - public Optional getUsername() { + public Optional getUsernameHash() { requireNotStale(); - return Optional.ofNullable(username); + return Optional.ofNullable(usernameHash); } - public void setUsername(final String username) { + public void setUsernameHash(final byte[] usernameHash) { requireNotStale(); - this.username = username; + this.usernameHash = usernameHash; } 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 2db069324..507b6c0a8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidator.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidator.java @@ -7,11 +7,16 @@ 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 allowUsernameChange; + private final boolean allowUsernameHashChange; static final AccountChangeValidator GENERAL_CHANGE_VALIDATOR = new AccountChangeValidator(false, false); static final AccountChangeValidator NUMBER_CHANGE_VALIDATOR = new AccountChangeValidator(true, false); @@ -20,10 +25,10 @@ class AccountChangeValidator { private static final Logger logger = LoggerFactory.getLogger(AccountChangeValidator.class); AccountChangeValidator(final boolean allowNumberChange, - final boolean allowUsernameChange) { + final boolean allowUsernameHashChange) { this.allowNumberChange = allowNumberChange; - this.allowUsernameChange = allowUsernameChange; + this.allowUsernameHashChange = allowUsernameHashChange; } public void validateChange(final Account originalAccount, final Account updatedAccount) { @@ -44,13 +49,21 @@ class AccountChangeValidator { } } - if (!allowUsernameChange) { - assert updatedAccount.getUsername().equals(originalAccount.getUsername()); + 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 (!updatedAccount.getUsername().equals(originalAccount.getUsername())) { - logger.error("Username changed via \"normal\" update; usernames must be changed via setUsername method", + 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", 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 b862957fa..fe6aa1f9c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java @@ -39,7 +39,6 @@ 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; @@ -64,16 +63,14 @@ 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_TIMER = Metrics.timer(name(Accounts.class, "clearUsername")); + private static final Timer CLEAR_USERNAME_HASH_TIMER = Metrics.timer(name(Accounts.class, "clearUsernameHash")); private static final Timer UPDATE_TIMER = Metrics.timer(name(Accounts.class, "update")); private static final Timer GET_BY_NUMBER_TIMER = Metrics.timer(name(Accounts.class, "getByNumber")); - private static final Timer GET_BY_USERNAME_TIMER = Metrics.timer(name(Accounts.class, "getByUsername")); + private static final Timer GET_BY_USERNAME_HASH_TIMER = Metrics.timer(name(Accounts.class, "getByUsernameHash")); 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")); @@ -96,8 +93,10 @@ public class Accounts extends AbstractDynamoDbStore { static final String ATTR_VERSION = "V"; // canonically discoverable static final String ATTR_CANONICALLY_DISCOVERABLE = "C"; - // username; string - static final String ATTR_USERNAME = "N"; + // username hash; byte[] or null + static final String ATTR_USERNAME_HASH = "N"; + // confirmed; bool + static final String ATTR_CONFIRMED = "F"; // unidentified access key; byte[] or null static final String ATTR_UAK = "UAK"; // time to live; number @@ -296,24 +295,23 @@ public class Accounts extends AbstractDynamoDbStore { } /** - * Reserve a username under a token - * - * @return a reservation token that must be provided when {@link #confirmUsername(Account, String, UUID)} is called + * Reserve a username hash under the account UUID */ - public UUID reserveUsername( + public void reserveUsernameHash( final Account account, - final String reservedUsername, + final byte[] reservedUsernameHash, 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.getUuid(), reservedUsername)); + account.setReservedUsernameHash(reservedUsernameHash); boolean succeeded = false; final long expirationTime = clock.instant().plus(ttl).getEpochSecond(); - final UUID reservationToken = UUID.randomUUID(); + // Use account UUID as a "reservation token" - by providing this, the client proves ownership of the hash + UUID uuid = account.getUuid(); try { final List writeItems = new ArrayList<>(); @@ -321,11 +319,12 @@ public class Accounts extends AbstractDynamoDbStore { .put(Put.builder() .tableName(usernamesConstraintTableName) .item(Map.of( - 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)) + 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)) .expressionAttributeValues(Map.of(":now", AttributeValues.fromLong(clock.instant().getEpochSecond()))) .returnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD) .build()) @@ -335,7 +334,7 @@ public class Accounts extends AbstractDynamoDbStore { TransactWriteItem.builder() .update(Update.builder() .tableName(accountsTableName) - .key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()))) + .key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(uuid))) .updateExpression("SET #data = :data ADD #version :version_increment") .conditionExpression("#version = :version") .expressionAttributeNames(Map.of("#data", ATTR_ACCOUNT_DATA, "#version", ATTR_VERSION)) @@ -367,42 +366,23 @@ public class Accounts extends AbstractDynamoDbStore { } RESERVE_USERNAME_TIMER.record(System.nanoTime() - startNanos, TimeUnit.NANOSECONDS); } - return reservationToken; } /** - * Confirm (set) a previously reserved username + * Confirm (set) a previously reserved username hash * * @param account to update - * @param username believed to be available - * @param reservationToken a token returned by the call to {@link #reserveUsername(Account, String, Duration)}, - * only required if setting a reserved username - * @throws ContestedOptimisticLockException if the account has been updated or the username taken by someone else + * @param usernameHash believed to be available + * @throws ContestedOptimisticLockException if the account has been updated or the username has taken by someone else */ - public void confirmUsername(final Account account, final String username, final UUID reservationToken) - throws ContestedOptimisticLockException { - setUsername(account, username, Optional.of(reservationToken)); - } - - /** - * Set the account username - * - * @param account to update - * @param username believed to be available - * @throws ContestedOptimisticLockException if the account has been updated or the username taken by someone else - */ - public void setUsername(final Account account, final String username) throws ContestedOptimisticLockException { - setUsername(account, username, Optional.empty()); - } - - private void setUsername(final Account account, final String username, final Optional reservationToken) + public void confirmUsernameHash(final Account account, final byte[] usernameHash) throws ContestedOptimisticLockException { final long startNanos = System.nanoTime(); - final Optional maybeOriginalUsername = account.getUsername(); - final Optional maybeOriginalReservation = account.getReservedUsernameHash(); + final Optional maybeOriginalUsernameHash = account.getUsernameHash(); + final Optional maybeOriginalReservationHash = account.getReservedUsernameHash(); - account.setUsername(username); + account.setUsernameHash(usernameHash); account.setReservedUsernameHash(null); boolean succeeded = false; @@ -410,20 +390,21 @@ public class Accounts extends AbstractDynamoDbStore { try { final List writeItems = new ArrayList<>(); - // 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 + // add the username hash to the constraint table, wiping out the ttl if we had already reserved the hash writeItems.add(TransactWriteItem.builder() .put(Put.builder() .tableName(usernamesConstraintTableName) .item(Map.of( KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()), - ATTR_USERNAME, AttributeValues.fromString(UsernameNormalizer.normalize(username)))) + ATTR_USERNAME_HASH, AttributeValues.fromByteArray(usernameHash), + ATTR_CONFIRMED, AttributeValues.fromBool(true))) // it's not in the constraint table OR it's expired OR it was reserved by us - .conditionExpression("attribute_not_exists(#username) OR #ttl < :now OR #aci = :reservation ") - .expressionAttributeNames(Map.of("#username", ATTR_USERNAME, "#ttl", ATTR_TTL, "#aci", KEY_ACCOUNT_UUID)) + .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)) .expressionAttributeValues(Map.of( ":now", AttributeValues.fromLong(clock.instant().getEpochSecond()), - ":reservation", AttributeValues.fromUUID(reservationToken.orElseGet(UUID::randomUUID)))) + ":aci", AttributeValues.fromUUID(account.getUuid()), + ":confirmed", AttributeValues.fromBool(false))) .returnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD) .build()) .build()); @@ -433,21 +414,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 = :username ADD #version :version_increment") + .updateExpression("SET #data = :data, #username_hash = :username_hash ADD #version :version_increment") .conditionExpression("#version = :version") .expressionAttributeNames(Map.of("#data", ATTR_ACCOUNT_DATA, - "#username", ATTR_USERNAME, + "#username_hash", ATTR_USERNAME_HASH, "#version", ATTR_VERSION)) .expressionAttributeValues(Map.of( ":data", AttributeValues.fromByteArray(SystemMapper.getMapper().writeValueAsBytes(account)), - ":username", AttributeValues.fromString(username), + ":username_hash", AttributeValues.fromByteArray(usernameHash), ":version", AttributeValues.fromInt(account.getVersion()), ":version_increment", AttributeValues.fromInt(1))) .build()) .build()); - maybeOriginalUsername.ifPresent(originalUsername -> writeItems.add( - buildDelete(usernamesConstraintTableName, ATTR_USERNAME, UsernameNormalizer.normalize(originalUsername)))); + maybeOriginalUsernameHash.ifPresent(originalUsernameHash -> writeItems.add( + buildDelete(usernamesConstraintTableName, ATTR_USERNAME_HASH, originalUsernameHash))); final TransactWriteItemsRequest request = TransactWriteItemsRequest.builder() .transactItems(writeItems) @@ -466,17 +447,17 @@ public class Accounts extends AbstractDynamoDbStore { throw e; } finally { if (!succeeded) { - account.setUsername(maybeOriginalUsername.orElse(null)); - account.setReservedUsernameHash(maybeOriginalReservation.orElse(null)); + account.setUsernameHash(maybeOriginalUsernameHash.orElse(null)); + account.setReservedUsernameHash(maybeOriginalReservationHash.orElse(null)); } SET_USERNAME_TIMER.record(System.nanoTime() - startNanos, TimeUnit.NANOSECONDS); } } - public void clearUsername(final Account account) { - account.getUsername().ifPresent(username -> { - CLEAR_USERNAME_TIMER.record(() -> { - account.setUsername(null); + public void clearUsernameHash(final Account account) { + account.getUsernameHash().ifPresent(usernameHash -> { + CLEAR_USERNAME_HASH_TIMER.record(() -> { + account.setUsernameHash(null); boolean succeeded = false; @@ -488,10 +469,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 ADD #version :version_increment") + .updateExpression("SET #data = :data REMOVE #username_hash ADD #version :version_increment") .conditionExpression("#version = :version") .expressionAttributeNames(Map.of("#data", ATTR_ACCOUNT_DATA, - "#username", ATTR_USERNAME, + "#username_hash", ATTR_USERNAME_HASH, "#version", ATTR_VERSION)) .expressionAttributeValues(Map.of( ":data", AttributeValues.fromByteArray(SystemMapper.getMapper().writeValueAsBytes(account)), @@ -500,7 +481,7 @@ public class Accounts extends AbstractDynamoDbStore { .build()) .build()); - writeItems.add(buildDelete(usernamesConstraintTableName, ATTR_USERNAME, UsernameNormalizer.normalize(username))); + writeItems.add(buildDelete(usernamesConstraintTableName, ATTR_USERNAME_HASH, usernameHash)); final TransactWriteItemsRequest request = TransactWriteItemsRequest.builder() .transactItems(writeItems) @@ -520,7 +501,7 @@ public class Accounts extends AbstractDynamoDbStore { throw e; } finally { if (!succeeded) { - account.setUsername(username); + account.setUsernameHash(usernameHash); } } }); @@ -601,27 +582,27 @@ public class Accounts extends AbstractDynamoDbStore { } } - public boolean usernameAvailable(final String username) { - return usernameAvailable(Optional.empty(), username); + public boolean usernameHashAvailable(final byte[] username) { + return usernameHashAvailable(Optional.empty(), username); } - public boolean usernameAvailable(final Optional reservationToken, final String username) { - final Optional> usernameItem = itemByKey( - usernamesConstraintTableName, ATTR_USERNAME, AttributeValues.fromString(UsernameNormalizer.normalize(username))); + public boolean usernameHashAvailable(final Optional accountUuid, final byte[] usernameHash) { + final Optional> usernameHashItem = itemByKey( + usernamesConstraintTableName, ATTR_USERNAME_HASH, AttributeValues.fromByteArray(usernameHash)); - if (usernameItem.isEmpty()) { - // username is free + if (usernameHashItem.isEmpty()) { + // username hash is free return true; } - final Map item = usernameItem.get(); + final Map item = usernameHashItem.get(); if (AttributeValues.getLong(item, ATTR_TTL, Long.MAX_VALUE) < clock.instant().getEpochSecond()) { - // username was reserved, but has expired + // username hash was reserved, but has expired return true; } - // username is reserved by us - return reservationToken + // username hash is reserved by us + return !AttributeValues.getBool(item, ATTR_CONFIRMED, true) && accountUuid .map(AttributeValues.getUUID(item, KEY_ACCOUNT_UUID, new UUID(0, 0))::equals) .orElse(false); } @@ -639,13 +620,13 @@ public class Accounts extends AbstractDynamoDbStore { } @Nonnull - public Optional getByUsername(final String username) { + public Optional getByUsernameHash(final byte[] usernameHash) { return getByIndirectLookup( - GET_BY_USERNAME_TIMER, + GET_BY_USERNAME_HASH_TIMER, usernamesConstraintTableName, - ATTR_USERNAME, - AttributeValues.fromString(UsernameNormalizer.normalize(username)), - item -> !item.containsKey(ATTR_TTL) // ignore items with a ttl (reservations) + ATTR_USERNAME_HASH, + AttributeValues.fromByteArray(usernameHash), + item -> AttributeValues.getBool(item, ATTR_CONFIRMED, false) // ignore items that are reservations (not confirmed) ); } @@ -665,8 +646,8 @@ public class Accounts extends AbstractDynamoDbStore { buildDelete(phoneNumberIdentifierConstraintTableName, ATTR_PNI_UUID, account.getPhoneNumberIdentifier()) )); - account.getUsername().ifPresent(username -> transactWriteItems.add( - buildDelete(usernamesConstraintTableName, ATTR_USERNAME, UsernameNormalizer.normalize(username)))); + account.getUsernameHash().ifPresent(usernameHash -> transactWriteItems.add( + buildDelete(usernamesConstraintTableName, ATTR_USERNAME_HASH, usernameHash))); final TransactWriteItemsRequest request = TransactWriteItemsRequest.builder() .transactItems(transactWriteItems).build(); @@ -807,6 +788,11 @@ 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)); @@ -843,22 +829,6 @@ 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) { @@ -883,7 +853,7 @@ public class Accounts extends AbstractDynamoDbStore { account.setNumber(item.get(ATTR_ACCOUNT_E164).s(), phoneNumberIdentifierFromAttribute); account.setUuid(accountIdentifier); - account.setUsername(AttributeValues.getString(item, ATTR_USERNAME, null)); + account.setUsernameHash(AttributeValues.getByteArray(item, ATTR_USERNAME_HASH, 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 8dd08df90..8f2d59b19 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -22,6 +22,7 @@ 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; @@ -50,8 +51,6 @@ 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 { @@ -60,13 +59,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 getByUsernameTimer = metricRegistry.timer(name(AccountsManager.class, "getByUsername")); + private static final Timer getByUsernameHashTimer = metricRegistry.timer(name(AccountsManager.class, "getByUsernameHash")); 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 redisUsernameGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisUsernameGet")); + private static final Timer redisUsernameHashGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisUsernameHashGet")); 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")); @@ -88,7 +87,6 @@ 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; @@ -96,7 +94,6 @@ 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(); @@ -106,9 +103,11 @@ 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 UsernameNotAvailableException; + void persistAccount(Account account) throws UsernameHashNotAvailableException; } public enum DeletionReason { @@ -130,13 +129,11 @@ 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; @@ -151,8 +148,6 @@ 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); } @@ -324,42 +319,43 @@ public class AccountsManager { return updatedAccount.get(); } - public record UsernameReservation(Account account, String reservedUsername, UUID reservationToken){} + public record UsernameReservation(Account account, byte[] reservedUsernameHash){} /** - * Generate a username from a nickname, and reserve it so no other accounts may take it. + * Reserve a username hash so that no other accounts may take it. * - * The reserved username can later be set with {@link #confirmReservedUsername(Account, String, UUID)}. The reservation - * will eventually expire, after which point confirmReservedUsername may fail if another account has taken the - * username. + * 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. * * @param account the account to update - * @param requestedNickname the nickname to reserve a username for - * @return the reserved username and an updated Account object - * @throws UsernameNotAvailableException no username is available for the requested nickname + * @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 */ - public UsernameReservation reserveUsername(final Account account, final String requestedNickname) throws UsernameNotAvailableException { + public UsernameReservation reserveUsernameHash(final Account account, final List requestedUsernameHashes) throws UsernameHashNotAvailableException { if (!experimentEnrollmentManager.isEnrolled(account.getUuid(), USERNAME_EXPERIMENT_NAME)) { - throw new UsernameNotAvailableException(); + throw new UsernameHashNotAvailableException(); } - if (prohibitedUsernames.isProhibited(requestedNickname, account.getUuid())) { - throw new UsernameNotAvailableException(); - } redisDelete(account); class Reserver implements AccountPersister { - UUID reservationToken; - String reservedUsername; + byte[] reservedUsernameHash; @Override - public void persistAccount(final Account account) throws UsernameNotAvailableException { - // In the future, this may also check for any forbidden discriminators - reservedUsername = usernameGenerator.generateAvailableUsername(requestedNickname, accounts::usernameAvailable); - reservationToken = accounts.reserveUsername( - account, - reservedUsername, - usernameGenerator.getReservationTtl()); + 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(); } } final Reserver reserver = new Reserver(); @@ -369,31 +365,28 @@ public class AccountsManager { reserver, () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); - return new UsernameReservation(updatedAccount, reserver.reservedUsername, reserver.reservationToken); + return new UsernameReservation(updatedAccount, reserver.reservedUsernameHash); } /** - * Set a username previously reserved with {@link #reserveUsername(Account, String)} + * Set a username hash previously reserved with {@link #reserveUsernameHash(Account, List)} * * @param account the account to update - * @param reservedUsername the previously reserved username - * @param reservationToken the UUID returned from the reservation - * @return the updated account with the username field set - * @throws UsernameNotAvailableException if the reserved username has been taken (because the reservation expired) - * @throws UsernameReservationNotFoundException if `reservedUsername` was not reserved in the account + * @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 */ - public Account confirmReservedUsername(final Account account, final String reservedUsername, final UUID reservationToken) throws UsernameNotAvailableException, UsernameReservationNotFoundException { + public Account confirmReservedUsernameHash(final Account account, final byte[] reservedUsernameHash) throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { if (!experimentEnrollmentManager.isEnrolled(account.getUuid(), USERNAME_EXPERIMENT_NAME)) { - throw new UsernameNotAvailableException(); + throw new UsernameHashNotAvailableException(); } - - if (account.getUsername().map(reservedUsername::equals).orElse(false)) { + if (account.getUsernameHash().map(currentUsernameHash -> Arrays.equals(currentUsernameHash, reservedUsernameHash)).orElse(false)) { // the client likely already succeeded and is retrying return account; } - final byte[] newHash = Accounts.reservedUsernameHash(account.getUuid(), UsernameNormalizer.normalize(reservedUsername)); - if (!account.getReservedUsernameHash().map(oldHash -> Arrays.equals(oldHash, newHash)).orElse(false)) { + if (!account.getReservedUsernameHash().map(oldHash -> Arrays.equals(oldHash, reservedUsernameHash)).orElse(false)) { // no such reservation existed, either there was no previous call to reserveUsername // or the reservation changed throw new UsernameReservationNotFoundException(); @@ -405,63 +398,23 @@ public class AccountsManager { account, a -> true, a -> { - // though we know this username was reserved, the reservation could have lapsed - if (!accounts.usernameAvailable(Optional.of(reservationToken), reservedUsername)) { - throw new UsernameNotAvailableException(); + // though we know this username hash was reserved, the reservation could have lapsed + if (!accounts.usernameHashAvailable(Optional.of(account.getUuid()), reservedUsernameHash)) { + throw new UsernameHashNotAvailableException(); } - accounts.confirmUsername(a, reservedUsername, reservationToken); + accounts.confirmUsernameHash(a, reservedUsernameHash); }, () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); } - /** - * Sets a username generated from `requestedNickname` with no prior reservation - * - * @param account the account to update - * @param requestedNickname the nickname to generate a username from - * @param expectedOldUsername the expected existing username of the account (for replay detection) - * @return the updated account with the username field set - * @throws UsernameNotAvailableException if no free username could be set for `requestedNickname` - */ - public Account setUsername(final Account account, final String requestedNickname, final @Nullable String expectedOldUsername) throws UsernameNotAvailableException { - if (!experimentEnrollmentManager.isEnrolled(account.getUuid(), USERNAME_EXPERIMENT_NAME)) { - throw new UsernameNotAvailableException(); - } - - if (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) { + public Account clearUsernameHash(final Account account) { redisDelete(account); return updateWithRetries( account, a -> true, - accounts::clearUsername, + accounts::clearUsernameHash, () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); } @@ -547,7 +500,7 @@ public class AccountsManager { final AccountChangeValidator changeValidator) { try { return failableUpdateWithRetries(account, updater, persister::accept, retriever, changeValidator); - } catch (UsernameNotAvailableException e) { + } catch (UsernameHashNotAvailableException e) { // not possible throw new IllegalStateException(e); } @@ -557,7 +510,7 @@ public class AccountsManager { final Function updater, final AccountPersister persister, final Supplier retriever, - final AccountChangeValidator changeValidator) throws UsernameNotAvailableException { + final AccountChangeValidator changeValidator) throws UsernameHashNotAvailableException { Account originalAccount = cloneAccount(account); @@ -640,11 +593,11 @@ public class AccountsManager { } } - public Optional getByUsername(final String username) { - try (final Timer.Context ignored = getByUsernameTimer.time()) { - Optional account = redisGetByUsername(username); + public Optional getByUsernameHash(final byte[] usernameHash) { + try (final Timer.Context ignored = getByUsernameHashTimer.time()) { + Optional account = redisGetByUsernameHash(usernameHash); if (account.isEmpty()) { - account = accounts.getByUsername(username); + account = accounts.getByUsernameHash(usernameHash); account.ifPresent(this::redisSet); } @@ -721,8 +674,8 @@ public class AccountsManager { clientPresenceManager.disconnectPresence(account.getUuid(), device.getId()))); } - private String getUsernameAccountMapKey(String username) { - return "UAccountMap::" + UsernameNormalizer.normalize(username); + private String getUsernameHashAccountMapKey(byte[] usernameHash) { + return "UAccountMap::" + Base64.getUrlEncoder().withoutPadding().encodeToString(usernameHash); } private String getAccountMapKey(String key) { @@ -744,8 +697,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.getUsername().ifPresent(username -> - commands.setex(getUsernameAccountMapKey(username), CACHE_TTL_SECONDS, account.getUuid().toString())); + account.getUsernameHash().ifPresent(usernameHash -> + commands.setex(getUsernameHashAccountMapKey(usernameHash), CACHE_TTL_SECONDS, account.getUuid().toString())); }); } catch (JsonProcessingException e) { throw new IllegalStateException(e); @@ -760,8 +713,8 @@ public class AccountsManager { return redisGetBySecondaryKey(getAccountMapKey(e164), redisNumberGetTimer); } - private Optional redisGetByUsername(String username) { - return redisGetBySecondaryKey(getUsernameAccountMapKey(username), redisUsernameGetTimer); + private Optional redisGetByUsernameHash(byte[] usernameHash) { + return redisGetBySecondaryKey(getUsernameHashAccountMapKey(usernameHash), redisUsernameHashGetTimer); } private Optional redisGetBySecondaryKey(String secondaryKey, Timer timer) { @@ -812,7 +765,7 @@ public class AccountsManager { getAccountMapKey(account.getPhoneNumberIdentifier().toString()), getAccountEntityKey(account.getUuid())); - account.getUsername().ifPresent(username -> connection.sync().del(getUsernameAccountMapKey(username))); + account.getUsernameHash().ifPresent(usernameHash -> connection.sync().del(getUsernameHashAccountMapKey(usernameHash))); }); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameNotAvailableException.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameHashNotAvailableException.java similarity index 68% rename from service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameNotAvailableException.java rename to service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameHashNotAvailableException.java index a2d25f49f..04f81f625 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameNotAvailableException.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/UsernameHashNotAvailableException.java @@ -5,5 +5,5 @@ package org.whispersystems.textsecuregcm.storage; -public class UsernameNotAvailableException extends Exception { +public class UsernameHashNotAvailableException 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 new file mode 100644 index 000000000..1e3c2933a --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/ByteArrayBase64UrlAdapter.java @@ -0,0 +1,27 @@ +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 deleted file mode 100644 index 10c8f8a8f..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/Nickname.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * 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 deleted file mode 100644 index 82ea5e96d..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/NicknameValidator.java +++ /dev/null @@ -1,17 +0,0 @@ -/* - * 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 deleted file mode 100644 index 0a1ff3168..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java +++ /dev/null @@ -1,152 +0,0 @@ -/* - * 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 deleted file mode 100644 index 1d0ecaf6c..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameNormalizer.java +++ /dev/null @@ -1,10 +0,0 @@ -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 1e329f6df..191f95bcc 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java @@ -17,6 +17,8 @@ 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; @@ -50,10 +52,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.UsernameNotAvailableException; +import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableException; +import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundException; 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; @@ -65,18 +67,18 @@ public class AssignUsernameCommand extends EnvironmentCommand { try { - 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"); + 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"); } }, () -> { 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 7b7f9af1c..3cf0d9134 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java @@ -55,7 +55,6 @@ 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; @@ -195,10 +194,9 @@ 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 57925e3a0..54e125b19 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,7 +24,6 @@ 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; @@ -37,6 +36,7 @@ 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.ConfirmUsernameRequest; +import org.whispersystems.textsecuregcm.entities.ConfirmUsernameHashRequest; import org.whispersystems.textsecuregcm.entities.GcmRegistrationId; import org.whispersystems.textsecuregcm.entities.IncomingMessage; import org.whispersystems.textsecuregcm.entities.RegistrationLock; import org.whispersystems.textsecuregcm.entities.RegistrationLockFailure; -import org.whispersystems.textsecuregcm.entities.ReserveUsernameRequest; -import org.whispersystems.textsecuregcm.entities.ReserveUsernameResponse; +import org.whispersystems.textsecuregcm.entities.ReserveUsernameHashRequest; +import org.whispersystems.textsecuregcm.entities.ReserveUsernameHashResponse; 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.UsernameNotAvailableException; +import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableException; import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundException; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; import org.whispersystems.textsecuregcm.tests.util.AuthHelper; @@ -119,7 +119,6 @@ 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"; @@ -131,10 +130,18 @@ 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"; @@ -186,6 +193,7 @@ class AccountControllerTest { new PolymorphicAuthValueFactoryProvider.Binder<>( ImmutableSet.of(AuthenticatedAccount.class, DisabledPermittedAuthenticatedAccount.class))) + .addProvider(new JsonMappingExceptionMapper()) .addProvider(new RateLimitExceededExceptionMapper()) .addProvider(new ImpossiblePhoneNumberExceptionMapper()) .addProvider(new NonNormalizedPhoneNumberExceptionMapper()) @@ -272,9 +280,6 @@ 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); @@ -1648,143 +1653,140 @@ class AccountControllerTest { } @Test - void testSetUsername() throws UsernameNotAvailableException { - Account account = mock(Account.class); - when(account.getUsername()).thenReturn(Optional.of("N00bkilleR.1234")); - when(accountsManager.setUsername(any(), eq("N00bkilleR"), isNull())) - .thenReturn(account); + 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") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new UsernameRequest("N00bkilleR", null))); - assertThat(response.getStatus()).isEqualTo(200); - assertThat(response.readEntity(UsernameResponse.class).username()).isEqualTo("N00bkilleR.1234"); - } - - @Test - void testReserveUsername() throws UsernameNotAvailableException { - when(accountsManager.reserveUsername(any(), eq("N00bkilleR"))) - .thenReturn(new AccountsManager.UsernameReservation(null, "N00bkilleR.1234", RESERVATION_TOKEN)); - Response response = - resources.getJerseyTest() - .target("/v1/accounts/username/reserved") + .target("/v1/accounts/username_hash/reserve") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ReserveUsernameRequest("N00bkilleR"))); + .put(Entity.json(new ReserveUsernameHashRequest(List.of(USERNAME_HASH_1, USERNAME_HASH_2)))); assertThat(response.getStatus()).isEqualTo(200); - assertThat(response.readEntity(ReserveUsernameResponse.class)) - .satisfies(r -> r.username().equals("N00bkilleR.1234")) - .satisfies(r -> r.reservationToken().equals(RESERVATION_TOKEN)); + assertThat(response.readEntity(ReserveUsernameHashResponse.class)) + .satisfies(r -> assertThat(r.usernameHash()).hasSize(32)); } @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); + void testReserveUsernameHashUnavailable() throws UsernameHashNotAvailableException { + when(accountsManager.reserveUsernameHash(any(), anyList())) + .thenThrow(new UsernameHashNotAvailableException()); Response response = resources.getJerseyTest() - .target("/v1/accounts/username/confirm") + .target("/v1/accounts/username_hash/reserve") .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"); + .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 testCommitUnreservedUsername() throws UsernameNotAvailableException, UsernameReservationNotFoundException { - when(accountsManager.confirmReservedUsername(any(), eq("n00bkiller.1234"), eq(RESERVATION_TOKEN))) + 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 { + Account account = mock(Account.class); + when(account.getUsernameHash()).thenReturn(Optional.of(USERNAME_HASH_1)); + when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1))).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))); + assertThat(response.getStatus()).isEqualTo(200); + assertArrayEquals(response.readEntity(UsernameHashResponse.class).usernameHash(), USERNAME_HASH_1); + } + + @Test + void testCommitUnreservedUsername() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { + when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1))) .thenThrow(new UsernameReservationNotFoundException()); Response response = resources.getJerseyTest() - .target("/v1/accounts/username/confirm") + .target("/v1/accounts/username_hash/confirm") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ConfirmUsernameRequest("n00bkiller.1234", RESERVATION_TOKEN))); + .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1))); assertThat(response.getStatus()).isEqualTo(409); } @Test - void testCommitLapsedUsername() throws UsernameNotAvailableException, UsernameReservationNotFoundException { - when(accountsManager.confirmReservedUsername(any(), eq("n00bkiller.1234"), eq(RESERVATION_TOKEN))) - .thenThrow(new UsernameNotAvailableException()); + void testCommitLapsedUsername() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { + when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1))) + .thenThrow(new UsernameHashNotAvailableException()); Response response = resources.getJerseyTest() - .target("/v1/accounts/username/confirm") + .target("/v1/accounts/username_hash/confirm") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ConfirmUsernameRequest("n00bkiller.1234", RESERVATION_TOKEN))); + .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1))); 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/") + .target("/v1/accounts/username_hash/") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) .delete(); assertThat(response.getStatus()).isEqualTo(204); - verify(accountsManager).clearUsername(AuthHelper.VALID_ACCOUNT); + verify(accountsManager).clearUsernameHash(AuthHelper.VALID_ACCOUNT); } @Test void testDeleteUsernameBadAuth() { Response response = resources.getJerseyTest() - .target("/v1/accounts/username/") + .target("/v1/accounts/username_hash/") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.INVALID_PASSWORD)) .delete(); @@ -1998,9 +2000,9 @@ class AccountControllerTest { final UUID uuid = UUID.randomUUID(); when(account.getUuid()).thenReturn(uuid); - when(accountsManager.getByUsername(eq("n00bkiller.1234"))).thenReturn(Optional.of(account)); + when(accountsManager.getByUsernameHash(any())).thenReturn(Optional.of(account)); Response response = resources.getJerseyTest() - .target("v1/accounts/username/n00bkiller.1234") + .target(String.format("v1/accounts/username_hash/%s", BASE_64_URL_USERNAME_HASH_1)) .request() .header(HttpHeaders.X_FORWARDED_FOR, "127.0.0.1") .get(); @@ -2010,9 +2012,9 @@ class AccountControllerTest { @Test void testLookupUsernameDoesNotExist() { - when(accountsManager.getByUsername(eq("n00bkiller.1234"))).thenReturn(Optional.empty()); + when(accountsManager.getByUsernameHash(any())).thenReturn(Optional.empty()); assertThat(resources.getJerseyTest() - .target("v1/accounts/username/n00bkiller.1234") + .target(String.format("v1/accounts/username_hash/%s", BASE_64_URL_USERNAME_HASH_1)) .request() .header(HttpHeaders.X_FORWARDED_FOR, "127.0.0.1") .get().getStatus()).isEqualTo(404); @@ -2024,7 +2026,7 @@ class AccountControllerTest { MockUtils.updateRateLimiterResponseToFail( rateLimiters, RateLimiters.Handle.USERNAME_LOOKUP, "127.0.0.1", expectedRetryAfter); final Response response = resources.getJerseyTest() - .target("/v1/accounts/username/test.123") + .target(String.format("v1/accounts/username_hash/%s", BASE_64_URL_USERNAME_HASH_1)) .request() .header(HttpHeaders.X_FORWARDED_FOR, "127.0.0.1") .get(); @@ -2033,6 +2035,34 @@ 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 2d2cace83..d435ea25a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java @@ -31,7 +31,6 @@ 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; @@ -132,6 +131,8 @@ 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); @@ -197,7 +198,7 @@ class ProfileControllerTest { when(profileAccount.isAnnouncementGroupSupported()).thenReturn(false); when(profileAccount.isChangeNumberSupported()).thenReturn(false); when(profileAccount.getCurrentProfileVersion()).thenReturn(Optional.empty()); - when(profileAccount.getUsername()).thenReturn(Optional.of("n00bkiller")); + when(profileAccount.getUsernameHash()).thenReturn(Optional.of(USERNAME_HASH)); when(profileAccount.getUnidentifiedAccessKey()).thenReturn(Optional.of("1337".getBytes())); Account capabilitiesAccount = mock(Account.class); @@ -212,7 +213,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.getByUsername("n00bkiller")).thenReturn(Optional.of(profileAccount)); + when(accountsManager.getByUsernameHash(USERNAME_HASH)).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 39ef6e24e..484b05ff6 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidatorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountChangeValidatorTest.java @@ -10,6 +10,7 @@ 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; @@ -26,8 +27,10 @@ class AccountChangeValidatorTest { private static final UUID ORIGINAL_PNI = UUID.randomUUID(); private static final UUID CHANGED_PNI = UUID.randomUUID(); - private static final String ORIGINAL_USERNAME = "bruce_wayne"; - private static final String CHANGED_USERNAME = "batman"; + 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); @ParameterizedTest @MethodSource @@ -49,22 +52,22 @@ class AccountChangeValidatorTest { final Account originalAccount = mock(Account.class); when(originalAccount.getNumber()).thenReturn(ORIGINAL_NUMBER); when(originalAccount.getPhoneNumberIdentifier()).thenReturn(ORIGINAL_PNI); - when(originalAccount.getUsername()).thenReturn(Optional.of(ORIGINAL_USERNAME)); + when(originalAccount.getUsernameHash()).thenReturn(Optional.of(ORIGINAL_USERNAME_HASH)); final Account unchangedAccount = mock(Account.class); when(unchangedAccount.getNumber()).thenReturn(ORIGINAL_NUMBER); when(unchangedAccount.getPhoneNumberIdentifier()).thenReturn(ORIGINAL_PNI); - when(unchangedAccount.getUsername()).thenReturn(Optional.of(ORIGINAL_USERNAME)); + when(unchangedAccount.getUsernameHash()).thenReturn(Optional.of(ORIGINAL_USERNAME_HASH)); final Account changedNumberAccount = mock(Account.class); when(changedNumberAccount.getNumber()).thenReturn(CHANGED_NUMBER); when(changedNumberAccount.getPhoneNumberIdentifier()).thenReturn(CHANGED_PNI); - when(changedNumberAccount.getUsername()).thenReturn(Optional.of(ORIGINAL_USERNAME)); + when(changedNumberAccount.getUsernameHash()).thenReturn(Optional.of(ORIGINAL_USERNAME_HASH)); final Account changedUsernameAccount = mock(Account.class); when(changedUsernameAccount.getNumber()).thenReturn(ORIGINAL_NUMBER); when(changedUsernameAccount.getPhoneNumberIdentifier()).thenReturn(ORIGINAL_PNI); - when(changedUsernameAccount.getUsername()).thenReturn(Optional.of(CHANGED_USERNAME)); + when(changedUsernameAccount.getUsernameHash()).thenReturn(Optional.of(CHANGED_USERNAME_HASH)); 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 414aa74a7..7a959036e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java @@ -33,7 +33,6 @@ 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; @@ -192,13 +191,11 @@ 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 e065b803e..fdf42fd2a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java @@ -51,7 +51,6 @@ 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; @@ -159,13 +158,11 @@ 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 e078aa7a4..a5de6dca3 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -5,18 +5,15 @@ package org.whispersystems.textsecuregcm.storage; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; 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; @@ -31,15 +28,17 @@ 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; @@ -47,9 +46,7 @@ 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; @@ -62,10 +59,12 @@ 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; @@ -73,7 +72,6 @@ class AccountsManagerTest { private Keys keys; private MessagesManager messagesManager; private ProfilesManager profilesManager; - private ProhibitedUsernames prohibitedUsernames; private ExperimentEnrollmentManager enrollmentManager; private Map phoneNumberIdentifiersByE164; @@ -89,8 +87,6 @@ class AccountsManagerTest { return null; }; - private static final UUID RESERVATION_TOKEN = UUID.randomUUID(); - @BeforeEach void setup() throws InterruptedException { accounts = mock(Accounts.class); @@ -99,7 +95,6 @@ class AccountsManagerTest { keys = mock(Keys.class); messagesManager = mock(MessagesManager.class); profilesManager = mock(ProfilesManager.class); - prohibitedUsernames = mock(ProhibitedUsernames.class); //noinspection unchecked commands = mock(RedisAdvancedClusterCommands.class); @@ -143,7 +138,7 @@ class AccountsManagerTest { enrollmentManager = mock(ExperimentEnrollmentManager.class); when(enrollmentManager.isEnrolled(any(UUID.class), eq(AccountsManager.USERNAME_EXPERIMENT_NAME))).thenReturn(true); - when(accounts.usernameAvailable(any())).thenReturn(true); + when(accounts.usernameHashAvailable(any())).thenReturn(true); accountsManager = new AccountsManager( accounts, @@ -153,13 +148,11 @@ class AccountsManagerTest { directoryQueue, keys, messagesManager, - prohibitedUsernames, profilesManager, mock(StoredVerificationCodeManager.class), storageClient, backupClient, mock(ClientPresenceManager.class), - new UsernameGenerator(new UsernameConfiguration()), enrollmentManager, mock(Clock.class)); } @@ -169,7 +162,8 @@ 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"); @@ -188,7 +182,8 @@ 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); @@ -209,7 +204,8 @@ 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); @@ -225,21 +221,21 @@ class AccountsManagerTest { } @Test - void testGetByUsernameInCache() { + void testGetByUsernameHashInCache() { UUID uuid = UUID.randomUUID(); - String username = "test"; + 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)); - 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); + Optional account = accountsManager.getByUsernameHash(USERNAME_HASH_1); assertTrue(account.isPresent()); assertEquals(account.get().getNumber(), "+14152222222"); assertEquals(UUID.fromString("de24dc73-fbd8-41be-a7d5-764c70d9da7e"), account.get().getPhoneNumberIdentifier()); - assertEquals(Optional.of(username), account.get().getUsername()); + assertArrayEquals(USERNAME_HASH_1, account.get().getUsernameHash().get()); - verify(commands).get(eq("UAccountMap::" + username)); + verify(commands).get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1)); verify(commands).get(eq("Account3::" + uuid)); verifyNoMoreInteractions(commands); @@ -320,29 +316,28 @@ class AccountsManagerTest { } @Test - void testGetAccountByUsernameNotInCache() { + void testGetAccountByUsernameHashNotInCache() { UUID uuid = UUID.randomUUID(); - String username = "test"; Account account = AccountsHelper.generateTestAccount("+14152222222", uuid, UUID.randomUUID(), new ArrayList<>(), new byte[16]); - account.setUsername(username); + account.setUsernameHash(USERNAME_HASH_1); - when(commands.get(eq("UAccountMap::" + username))).thenReturn(null); - when(accounts.getByUsername(username)).thenReturn(Optional.of(account)); + when(commands.get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1))).thenReturn(null); + when(accounts.getByUsernameHash(USERNAME_HASH_1)).thenReturn(Optional.of(account)); - Optional retrieved = accountsManager.getByUsername(username); + Optional retrieved = accountsManager.getByUsernameHash(USERNAME_HASH_1); assertTrue(retrieved.isPresent()); assertSame(retrieved.get(), account); - verify(commands).get(eq("UAccountMap::" + username)); - verify(commands).setex(eq("UAccountMap::" + username), anyLong(), eq(uuid.toString())); + 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).setex(eq("AccountMap::" + account.getPhoneNumberIdentifier()), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(commands); - verify(accounts).getByUsername(username); + verify(accounts).getByUsernameHash(USERNAME_HASH_1); verifyNoMoreInteractions(accounts); } @@ -422,27 +417,26 @@ 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.setUsername(username); + account.setUsernameHash(USERNAME_HASH_1); - when(commands.get(eq("UAccountMap::" + username))).thenThrow(new RedisException("OH NO")); - when(accounts.getByUsername(username)).thenReturn(Optional.of(account)); + 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)); - Optional retrieved = accountsManager.getByUsername(username); + Optional retrieved = accountsManager.getByUsernameHash(USERNAME_HASH_1); assertTrue(retrieved.isPresent()); assertSame(retrieved.get(), account); - verify(commands).get(eq("UAccountMap::" + username)); - verify(commands).setex(eq("UAccountMap::" + username), anyLong(), eq(uuid.toString())); + 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).setex(eq("AccountMap::" + account.getPhoneNumberIdentifier()), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(commands); - verify(accounts).getByUsername(username); + verify(accounts).getByUsernameHash(USERNAME_HASH_1); verifyNoMoreInteractions(accounts); } @@ -734,188 +728,96 @@ class AccountsManagerTest { } @Test - void testSetUsername() { + void testReserveUsernameHash() throws UsernameHashNotAvailableException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - final String nickname = "test"; - assertDoesNotThrow(() -> accountsManager.setUsername(account, nickname, null)); - verify(accounts).setUsername(eq(account), startsWith(nickname)); + 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))); } @Test - void testReserveUsername() throws UsernameNotAvailableException { + void testReserveUsernameHashNotAvailable() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - final String nickname = "beethoven"; - accountsManager.reserveUsername(account, nickname); - verify(accounts).reserveUsername(eq(account), startsWith(nickname), any()); + when(accounts.usernameHashAvailable(any())).thenReturn(false); + + assertThrows(UsernameHashNotAvailableException.class, () -> accountsManager.reserveUsernameHash(account, List.of( + USERNAME_HASH_1, USERNAME_HASH_2))); } @Test - void testSetReservedUsername() throws UsernameNotAvailableException, UsernameReservationNotFoundException { + void testReserveUsernameDisabled() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - final String reserved = "sCoObY.1234"; - setReservationHash(account, reserved); - when(accounts.usernameAvailable(eq(Optional.of(RESERVATION_TOKEN)), eq(reserved))).thenReturn(true); - accountsManager.confirmReservedUsername(account, reserved, RESERVATION_TOKEN); - verify(accounts).confirmUsername(eq(account), eq(reserved), eq(RESERVATION_TOKEN)); + when(enrollmentManager.isEnrolled(account.getUuid(), AccountsManager.USERNAME_EXPERIMENT_NAME)).thenReturn(false); + assertThrows(UsernameHashNotAvailableException.class, () -> accountsManager.reserveUsernameHash(account, List.of( + USERNAME_HASH_1))); } @Test - void testSetReservedHashNameMismatch() { + void testConfirmReservedUsernameHash() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - setReservationHash(account, "pluto.1234"); - when(accounts.usernameAvailable(eq(Optional.of(RESERVATION_TOKEN)), eq("pluto.1234"))).thenReturn(true); + 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); assertThrows(UsernameReservationNotFoundException.class, - () -> accountsManager.confirmReservedUsername(account, "goofy.1234", RESERVATION_TOKEN)); + () -> accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_2)); } @Test - void testSetReservedHashAciMismatch() { + void testConfirmReservedLapsed() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - final String reserved = "toto.1234"; - account.setReservedUsernameHash(Accounts.reservedUsernameHash(UUID.randomUUID(), reserved)); - when(accounts.usernameAvailable(eq(Optional.of(RESERVATION_TOKEN)), eq(reserved))).thenReturn(true); - assertThrows(UsernameReservationNotFoundException.class, - () -> accountsManager.confirmReservedUsername(account, reserved, RESERVATION_TOKEN)); + // 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()); } @Test - void testSetReservedLapsed() { + void testConfirmReservedRetry() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - final String reserved = "porkchop.1234"; - // name was reserved, but the reservation lapsed and another account took it - setReservationHash(account, reserved); - when(accounts.usernameAvailable(eq(Optional.of(RESERVATION_TOKEN)), eq(reserved))).thenReturn(false); - assertThrows(UsernameNotAvailableException.class, () -> accountsManager.confirmReservedUsername(account, reserved, RESERVATION_TOKEN)); - verify(accounts, never()).confirmUsername(any(), any(), any()); - } - - @Test - void testSetReservedRetry() throws UsernameNotAvailableException, UsernameReservationNotFoundException { - final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); - final String username = "santaslittlehelper.1234"; - account.setUsername(username); + account.setUsernameHash(USERNAME_HASH_1); // reserved username already set, should be treated as a replay - accountsManager.confirmReservedUsername(account, username, RESERVATION_TOKEN); + accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1); verifyNoInteractions(accounts); } @Test - 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 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() { + void testConfirmReservedUsernameHashWithNoReservation() { 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))); - } + () -> accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1)); + verify(accounts, never()).confirmUsernameHash(any(), any()); } @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()); + 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)); } @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.setUsername("test"))); + assertThrows(AssertionError.class, () -> accountsManager.update(account, a -> a.setUsernameHash(USERNAME_HASH_1))); } - @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 void setReservationHash(final Account account, final byte[] reservedUsernameHash) { + account.setReservedUsernameHash(reservedUsernameHash); } 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 c3c952ba3..2a9818a13 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java @@ -6,9 +6,10 @@ 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; @@ -17,11 +18,15 @@ 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; @@ -29,8 +34,6 @@ 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; @@ -42,7 +45,6 @@ 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; @@ -59,7 +61,11 @@ 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() @@ -86,7 +92,6 @@ class AccountsManagerUsernameIntegrationTest { private AccountsManager accountsManager; private Accounts accounts; - private UsernameGenerator usernameGenerator; @BeforeEach void setup() throws InterruptedException { @@ -107,12 +112,12 @@ class AccountsManagerUsernameIntegrationTest { CreateTableRequest createUsernamesTableRequest = CreateTableRequest.builder() .tableName(USERNAMES_TABLE_NAME) .keySchema(KeySchemaElement.builder() - .attributeName(Accounts.ATTR_USERNAME) + .attributeName(Accounts.ATTR_USERNAME_HASH) .keyType(KeyType.HASH) .build()) .attributeDefinitions(AttributeDefinition.builder() - .attributeName(Accounts.ATTR_USERNAME) - .attributeType(ScalarAttributeType.S) + .attributeName(Accounts.ATTR_USERNAME_HASH) + .attributeType(ScalarAttributeType.B) .build()) .provisionedThroughput(DynamoDbExtension.DEFAULT_PROVISIONED_THROUGHPUT) .build(); @@ -152,8 +157,6 @@ 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") @@ -176,211 +179,159 @@ 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 testSetClearUsername() throws UsernameNotAvailableException, InterruptedException { + void testNoUsernames() throws InterruptedException { Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), new ArrayList<>()); - account = accountsManager.setUsername(account, "n00bkiller", null); - assertThat(account.getUsername()).isPresent(); - assertThat(account.getUsername().get()).startsWith("n00bkiller"); - int discriminator = discriminator(account.getUsername().get()); - assertThat(discriminator).isGreaterThan(0).isLessThan(10); - - assertThat(accountsManager.getByUsername(account.getUsername().get()).orElseThrow().getUuid()).isEqualTo( - account.getUuid()); - - // reroll - account = accountsManager.setUsername(account, "n00bkiller", account.getUsername().get()); - final String newUsername = account.getUsername().orElseThrow(); - assertThat(discriminator(account.getUsername().orElseThrow())).isNotEqualTo(discriminator); - - // clear - account = accountsManager.clearUsername(account); - assertThat(accountsManager.getByUsername(newUsername)).isEmpty(); - assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsername()).isEmpty(); - } - - @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++) { + List usernameHashes = List.of(USERNAME_HASH_1, USERNAME_HASH_2); + int i = 0; + for (byte[] hash : usernameHashes) { final Map item = new HashMap<>(Map.of( Accounts.KEY_ACCOUNT_UUID, AttributeValues.fromUUID(UUID.randomUUID()), - Accounts.ATTR_USERNAME, AttributeValues.fromString(usernameGenerator.fromParts("n00bkiller", i)))); + Accounts.ATTR_USERNAME_HASH, AttributeValues.fromByteArray(hash))); // 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(UsernameNotAvailableException.class, () -> { - if (reserve) { - accountsManager.reserveUsername(account, "n00bkiller"); - } else { - accountsManager.setUsername(account, "n00bkiller", null); - } - }); - assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsername()).isEmpty(); + assertThrows(UsernameHashNotAvailableException.class, () -> {accountsManager.reserveUsernameHash(account, usernameHashes);}); + assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsernameHash()).isEmpty(); } @Test - void testUsernameSnatched() throws InterruptedException, UsernameNotAvailableException { + void testReserveUsernameSnatched() throws InterruptedException, UsernameHashNotAvailableException { final Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), new ArrayList<>()); - for (int i = 1; i <= 9; i++) { + ArrayList usernameHashes = new ArrayList<>(Arrays.asList(USERNAME_HASH_1, USERNAME_HASH_2)); + for (byte[] hash : usernameHashes) { 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)))) + Accounts.ATTR_USERNAME_HASH, AttributeValues.fromByteArray(hash))) .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).usernameAvailable(any()); - final String username = accountsManager - .setUsername(account, "n00bkiller", null) - .getUsername().orElseThrow(); - assertThat(username).startsWith("n00bkiller"); - assertThat(discriminator(username)).isGreaterThanOrEqualTo(10).isLessThan(100); + doReturn(true).doCallRealMethod().when(accounts).usernameHashAvailable(any()); + final byte[] username = accountsManager + .reserveUsernameHash(account, usernameHashes) + .reservedUsernameHash(); + + assertArrayEquals(username, availableHash); // 1 attempt on first try (returns true), - // 10 (attempts per width) on width=2 discriminators (all taken) - verify(accounts, times(11)).usernameAvailable(argThat(un -> discriminator(un) < 10)); - - // 1 final attempt on width=3 discriminators - verify(accounts, times(1)).usernameAvailable(argThat(un -> discriminator(un) >= 10)); + // 5 more attempts until "availableHash" returns true + verify(accounts, times(4)).usernameHashAvailable(any()); } @Test - public void testReserveSetClear() - throws InterruptedException, UsernameNotAvailableException, UsernameReservationNotFoundException { + public void testReserveConfirmClear() + throws InterruptedException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), new ArrayList<>()); - AccountsManager.UsernameReservation reservation = accountsManager.reserveUsername(account, "n00bkiller"); - account = reservation.account(); - assertThat(account.getReservedUsernameHash()).isPresent(); - assertThat(reservation.reservedUsername()).startsWith("n00bkiller"); - int discriminator = discriminator(reservation.reservedUsername()); - assertThat(discriminator).isGreaterThan(0).isLessThan(10); - assertThat(accountsManager.getByUsername(reservation.reservedUsername())).isEmpty(); - account = accountsManager.confirmReservedUsername( - account, - reservation.reservedUsername(), - reservation.reservationToken()); + // 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(); - assertThat(account.getUsername().get()).startsWith("n00bkiller"); - assertThat(accountsManager.getByUsername(account.getUsername().get()).orElseThrow().getUuid()).isEqualTo( + // confirm + account = accountsManager.confirmReservedUsernameHash( + reservation.account(), + reservation.reservedUsernameHash()); + assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_1); + assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1).orElseThrow().getUuid()).isEqualTo( account.getUuid()); - // reroll - reservation = accountsManager.reserveUsername(account, "n00bkiller"); - account = reservation.account(); - account = accountsManager.confirmReservedUsername( - account, - reservation.reservedUsername(), - reservation.reservationToken()); - - final String newUsername = account.getUsername().orElseThrow(); - assertThat(discriminator(account.getUsername().orElseThrow())).isNotEqualTo(discriminator); - // clear - account = accountsManager.clearUsername(account); - assertThat(accountsManager.getByUsername(newUsername)).isEmpty(); - assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsername()).isEmpty(); - + account = accountsManager.clearUsernameHash(account); + assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); + assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsernameHash()).isEmpty(); } @Test public void testReservationLapsed() - throws InterruptedException, UsernameNotAvailableException, UsernameReservationNotFoundException { - // use a username generator that can retry a lot - buildAccountsManager(1, 1, 1000000); + throws InterruptedException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { final Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), new ArrayList<>()); - AccountsManager.UsernameReservation reservation1 = accountsManager.reserveUsername(account, "n00bkiller"); - final String reservedUsername = reservation1.reservedUsername(); + AccountsManager.UsernameReservation reservation1 = accountsManager.reserveUsernameHash(account, List.of( + USERNAME_HASH_1)); long past = Instant.now().minus(Duration.ofMinutes(1)).getEpochSecond(); // force expiration ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbClient().updateItem(UpdateItemRequest.builder() .tableName(USERNAMES_TABLE_NAME) - .key(Map.of(Accounts.ATTR_USERNAME, AttributeValues.fromString(reservedUsername))) + .key(Map.of(Accounts.ATTR_USERNAME_HASH, AttributeValues.fromByteArray(USERNAME_HASH_1))) .updateExpression("SET #ttl = :ttl") .expressionAttributeNames(Map.of("#ttl", Accounts.ATTR_TTL)) .expressionAttributeValues(Map.of(":ttl", AttributeValues.fromLong(past))) .build()); - int discriminator = discriminator(reservedUsername); - - // use up all names except the reserved one - for (int i = 1; i <= 9; i++) { - if (i == discriminator) { - continue; - } - - ACCOUNTS_DYNAMO_EXTENSION.getDynamoDbClient().putItem(PutItemRequest.builder() - .tableName(USERNAMES_TABLE_NAME) - .item(Map.of( - Accounts.KEY_ACCOUNT_UUID, AttributeValues.fromUUID(UUID.randomUUID()), - Accounts.ATTR_USERNAME, AttributeValues.fromString(usernameGenerator.fromParts("n00bkiller", i)))) - .build()); - } - // a different account should be able to reserve it Account account2 = accountsManager.create("+18005552222", "password", null, new AccountAttributes(), new ArrayList<>()); - final AccountsManager.UsernameReservation reservation2 = accountsManager.reserveUsername(account2, "n00bkiller" - ); - assertThat(reservation2.reservedUsername()).isEqualTo(reservedUsername); + final AccountsManager.UsernameReservation reservation2 = accountsManager.reserveUsernameHash(account2, List.of( + USERNAME_HASH_1)); + assertArrayEquals(reservation2.reservedUsernameHash(), USERNAME_HASH_1); - assertThrows(UsernameNotAvailableException.class, - () -> accountsManager.confirmReservedUsername(reservation1.account(), reservedUsername, reservation1.reservationToken())); - accountsManager.confirmReservedUsername(reservation2.account(), reservedUsername, reservation2.reservationToken()); + 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); } @Test - void testUsernameReserveClearSetReserved() - throws InterruptedException, UsernameNotAvailableException, UsernameReservationNotFoundException { + void testUsernameSetReserveAnotherClearSetReserved() + throws InterruptedException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { Account account = accountsManager.create("+18005551111", "password", null, new AccountAttributes(), new ArrayList<>()); - account = accountsManager.setUsername(account, "n00bkiller", null); - final AccountsManager.UsernameReservation reservation = accountsManager.reserveUsername(account, "other"); - account = reservation.account(); - assertThat(reservation.reservedUsername()).startsWith("other"); - assertThat(account.getUsername()).hasValueSatisfying(s -> s.startsWith("n00bkiller")); + // Set username hash + final AccountsManager.UsernameReservation reservation1 = accountsManager.reserveUsernameHash(account, List.of( + USERNAME_HASH_1)); + account = accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1); - account = accountsManager.clearUsername(account); + // 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); assertThat(account.getReservedUsernameHash()).isPresent(); - assertThat(account.getUsername()).isEmpty(); + assertThat(account.getUsernameHash()).isEmpty(); - account = accountsManager.confirmReservedUsername(account, reservation.reservedUsername(), reservation.reservationToken()); - assertThat(account.getUsername()).hasValueSatisfying(s -> s.startsWith("other")); + // Confirm second reservation + account = accountsManager.confirmReservedUsernameHash(account, reservation2.reservedUsernameHash()); + assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_2); } } 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 773174c84..6ce81a94a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java @@ -9,6 +9,8 @@ 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; @@ -21,6 +23,7 @@ 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; @@ -28,7 +31,6 @@ 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; @@ -66,9 +68,15 @@ 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) @@ -118,12 +126,12 @@ class AccountsTest { CreateTableRequest createUsernamesTableRequest = CreateTableRequest.builder() .tableName(USERNAME_CONSTRAINT_TABLE_NAME) .keySchema(KeySchemaElement.builder() - .attributeName(Accounts.ATTR_USERNAME) + .attributeName(Accounts.ATTR_USERNAME_HASH) .keyType(KeyType.HASH) .build()) .attributeDefinitions(AttributeDefinition.builder() - .attributeName(Accounts.ATTR_USERNAME) - .attributeType(ScalarAttributeType.S) + .attributeName(Accounts.ATTR_USERNAME_HASH) + .attributeType(ScalarAttributeType.B) .build()) .provisionedThroughput(DynamoDbExtension.DEFAULT_PROVISIONED_THROUGHPUT) .build(); @@ -614,40 +622,38 @@ class AccountsTest { } @Test - void testSetUsername() { + void testSwitchUsernameHashes() { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); - final String username = "TeST"; + assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); - assertThat(accounts.getByUsername(username)).isEmpty(); - - accounts.setUsername(account, username); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1); { - final Optional maybeAccount = accounts.getByUsername(username); + final Optional maybeAccount = accounts.getByUsernameHash(USERNAME_HASH_1); assertThat(maybeAccount).hasValueSatisfying(retrievedAccount -> - assertThat(retrievedAccount.getUsername()).hasValueSatisfying(retrievedUsername -> - assertThat(retrievedUsername).isEqualTo(username))); + assertThat(retrievedAccount.getUsernameHash()).hasValueSatisfying(retrievedUsernameHash -> + assertArrayEquals(retrievedUsernameHash, USERNAME_HASH_1))); verifyStoredState(account.getNumber(), account.getUuid(), account.getPhoneNumberIdentifier(), maybeAccount.orElseThrow(), account); } - final String secondUsername = username + "2"; + accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_2); - accounts.setUsername(account, secondUsername); - - assertThat(accounts.getByUsername(username)).isEmpty(); + assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); assertThat(dynamoDbExtension.getDynamoDbClient() .getItem(GetItemRequest.builder() .tableName(USERNAME_CONSTRAINT_TABLE_NAME) - .key(Map.of(Accounts.ATTR_USERNAME, AttributeValues.fromString("test"))) + .key(Map.of(Accounts.ATTR_USERNAME_HASH, AttributeValues.fromByteArray(USERNAME_HASH_1))) .build()) .item()).isEmpty(); { - final Optional maybeAccount = accounts.getByUsername(secondUsername); + final Optional maybeAccount = accounts.getByUsernameHash(USERNAME_HASH_2); assertThat(maybeAccount).isPresent(); verifyStoredState(account.getNumber(), account.getUuid(), account.getPhoneNumberIdentifier(), @@ -656,38 +662,51 @@ class AccountsTest { } @Test - void testSetUsernameConflict() { + void testUsernameHashConflict() { final Account firstAccount = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); final Account secondAccount = generateAccount("+18005559876", UUID.randomUUID(), UUID.randomUUID()); accounts.create(firstAccount); accounts.create(secondAccount); - final String username = "test"; + // first account reserves and confirms username hash + assertThatNoException().isThrownBy(() -> { + accounts.reserveUsernameHash(firstAccount, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(firstAccount, USERNAME_HASH_1); + }); - assertThatNoException().isThrownBy(() -> accounts.setUsername(firstAccount, username)); - - final Optional maybeAccount = accounts.getByUsername(username); + final Optional maybeAccount = accounts.getByUsernameHash(USERNAME_HASH_1); 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.setUsername(secondAccount, username)); + .isThrownBy(() -> accounts.reserveUsernameHash(secondAccount, USERNAME_HASH_1, Duration.ofDays(1))); + assertThatExceptionOfType(ContestedOptimisticLockException.class) + .isThrownBy(() -> accounts.confirmUsernameHash(secondAccount, USERNAME_HASH_1)); - assertThat(secondAccount.getUsername()).isEmpty(); + // 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(); } @Test - void testSetUsernameVersionMismatch() { + void testConfirmUsernameHashVersionMismatch() { 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.setUsername(account, "test")); + .isThrownBy(() -> accounts.confirmUsernameHash(account, USERNAME_HASH_1)); - assertThat(account.getUsername()).isEmpty(); + assertThat(account.getUsernameHash()).isEmpty(); } @Test @@ -695,16 +714,15 @@ class AccountsTest { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); - final String username = "TeST"; + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1); + assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isPresent(); - accounts.setUsername(account, username); - assertThat(accounts.getByUsername(username)).isPresent(); + accounts.clearUsernameHash(account); - accounts.clearUsername(account); - - assertThat(accounts.getByUsername(username)).isEmpty(); + assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); assertThat(accounts.getByAccountIdentifier(account.getUuid())) - .hasValueSatisfying(clearedAccount -> assertThat(clearedAccount.getUsername()).isEmpty()); + .hasValueSatisfying(clearedAccount -> assertThat(clearedAccount.getUsernameHash()).isEmpty()); } @Test @@ -712,7 +730,7 @@ class AccountsTest { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); - assertThatNoException().isThrownBy(() -> accounts.clearUsername(account)); + assertThatNoException().isThrownBy(() -> accounts.clearUsernameHash(account)); } @Test @@ -720,167 +738,136 @@ class AccountsTest { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); - final String username = "test"; - - accounts.setUsername(account, username); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1); account.setVersion(account.getVersion() + 12); - assertThatExceptionOfType(ContestedOptimisticLockException.class).isThrownBy(() -> accounts.clearUsername(account)); + assertThatExceptionOfType(ContestedOptimisticLockException.class).isThrownBy(() -> accounts.clearUsernameHash(account)); - assertThat(account.getUsername()).hasValueSatisfying(u -> assertThat(u).isEqualTo(username)); + assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_1); } @Test - void testReservedUsername() { + void testReservedUsernameHash() { final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account1); final Account account2 = generateAccount("+18005552222", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account2); - final UUID token = accounts.reserveUsername(account1, "GarfielD", Duration.ofDays(1)); - assertThat(account1.getReservedUsernameHash()).get().isEqualTo(Accounts.reservedUsernameHash(account1.getUuid(), "GarfielD")); - assertThat(account1.getUsername()).isEmpty(); + accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(1)); + assertArrayEquals(account1.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1); + assertThat(account1.getUsernameHash()).isEmpty(); - // account 2 shouldn't be able to reserve the username if it's the same when normalized + // account 2 shouldn't be able to reserve or confirm the same username hash assertThrows(ContestedOptimisticLockException.class, - () -> accounts.reserveUsername(account2, "gARFIELd", Duration.ofDays(1))); + () -> accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1))); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.confirmUsername(account2, "gARFIELd", UUID.randomUUID())); - assertThat(accounts.getByUsername("gARFIELd")).isEmpty(); + () -> accounts.confirmUsernameHash(account2, USERNAME_HASH_1)); + assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); - accounts.confirmUsername(account1, "GarfielD", token); + accounts.confirmUsernameHash(account1, USERNAME_HASH_1); assertThat(account1.getReservedUsernameHash()).isEmpty(); - assertThat(account1.getUsername()).get().isEqualTo("GarfielD"); - assertThat(accounts.getByUsername("GarfielD").get().getUuid()).isEqualTo(account1.getUuid()); + assertArrayEquals(account1.getUsernameHash().orElseThrow(), USERNAME_HASH_1); + assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).get().getUuid()).isEqualTo(account1.getUuid()); final Map usernameConstraintRecord = dynamoDbExtension.getDynamoDbClient() .getItem(GetItemRequest.builder() .tableName(USERNAME_CONSTRAINT_TABLE_NAME) - .key(Map.of(Accounts.ATTR_USERNAME, AttributeValues.fromString("garfield"))) + .key(Map.of(Accounts.ATTR_USERNAME_HASH, AttributeValues.fromByteArray(USERNAME_HASH_1))) .build()) .item(); - assertThat(usernameConstraintRecord).containsKey(Accounts.ATTR_USERNAME); + assertThat(usernameConstraintRecord).containsKey(Accounts.ATTR_USERNAME_HASH); assertThat(usernameConstraintRecord).doesNotContainKey(Accounts.ATTR_TTL); } @Test - void testUsernameAvailable() { + void testUsernameHashAvailable() { final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account1); - final String username = "UnSinkaBlesam"; + 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 UUID token = accounts.reserveUsername(account1, username, Duration.ofDays(1)); - assertThat(accounts.usernameAvailable(username)).isFalse(); - assertThat(accounts.usernameAvailable(Optional.empty(), username)).isFalse(); - assertThat(accounts.usernameAvailable(Optional.of(UUID.randomUUID()), username)).isFalse(); - assertThat(accounts.usernameAvailable(Optional.of(token), username)).isTrue(); - - accounts.confirmUsername(account1, username, token); - assertThat(accounts.usernameAvailable(username)).isFalse(); - assertThat(accounts.usernameAvailable(Optional.empty(), username)).isFalse(); - assertThat(accounts.usernameAvailable(Optional.of(UUID.randomUUID()), username)).isFalse(); - assertThat(accounts.usernameAvailable(Optional.of(token), username)).isFalse(); - } - - - @Test - void testReservedUsernameWrongToken() { - final Account account = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); - accounts.create(account); - accounts.reserveUsername(account, "grumpy", Duration.ofDays(1)); - assertThat(account.getReservedUsernameHash()) - .get() - .isEqualTo(Accounts.reservedUsernameHash(account.getUuid(), "grumpy")); - assertThat(account.getUsername()).isEmpty(); - - assertThrows(ContestedOptimisticLockException.class, - () -> accounts.confirmUsername(account, "grumpy", UUID.randomUUID())); - assertThrows(ContestedOptimisticLockException.class, - () -> accounts.setUsername(account, "grumpy")); + 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(); } @Test - void testReserveExpiredReservedUsername() { + 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); - final String username = "snowball.02"; - accounts.reserveUsername(account1, username, Duration.ofDays(2)); - - 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, take::get); - } - - // after 2 days, can take the name - clock.pin(Instant.EPOCH.plus(Duration.ofDays(2)).plus(Duration.ofSeconds(1))); - final UUID token = take.get(); + accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(1)); + assertArrayEquals(account1.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1); + assertThat(account1.getUsernameHash()).isEmpty(); + // only account1 should be able to confirm the reserved hash assertThrows(ContestedOptimisticLockException.class, - () -> accounts.reserveUsername(account1, username, Duration.ofDays(2))); - assertThrows(ContestedOptimisticLockException.class, - () -> accounts.setUsername(account1, username)); - - accounts.confirmUsername(account2, username, token); - assertThat(accounts.getByUsername(username).get().getUuid()).isEqualTo(account2.getUuid()); + () -> accounts.confirmUsernameHash(account2, USERNAME_HASH_1)); } @Test - void testTakeExpiredReservedUsername() { + void testConfirmExpiredReservedUsernameHash() { 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)); + accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(2)); - Runnable take = () -> accounts.setUsername(account2, username); + Runnable runnable = () -> accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1)); for (int i = 0; i <= 2; i++) { clock.pin(Instant.EPOCH.plus(Duration.ofDays(i))); - assertThrows(ContestedOptimisticLockException.class, take::run); + assertThrows(ContestedOptimisticLockException.class, runnable::run); } - // after 2 days, can take the name + // after 2 days, can reserve and confirm the hash clock.pin(Instant.EPOCH.plus(Duration.ofDays(2)).plus(Duration.ofSeconds(1))); - take.run(); + runnable.run(); + assertEquals(account2.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1); + + accounts.confirmUsernameHash(account2, USERNAME_HASH_1); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.reserveUsername(account1, username, Duration.ofDays(2))); + () -> accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(2))); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.setUsername(account1, username)); - assertThat(accounts.getByUsername(username).get().getUuid()).isEqualTo(account2.getUuid()); + () -> accounts.confirmUsernameHash(account1, USERNAME_HASH_1)); + assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).get().getUuid()).isEqualTo(account2.getUuid()); } @Test - void testRetryReserveUsername() { + void testRetryReserveUsernameHash() { final Account account = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); - accounts.reserveUsername(account, "jorts", Duration.ofDays(2)); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(2)); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.reserveUsername(account, "jorts", Duration.ofDays(2)), - "Shouldn't be able to re-reserve same username (would extend ttl)"); + () -> accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(2)), + "Shouldn't be able to re-reserve same username hash (would extend ttl)"); } @Test - void testReserveUsernameVersionConflict() { + void testReserveConfirmUsernameHashVersionConflict() { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); accounts.create(account); account.setVersion(account.getVersion() + 12); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.reserveUsername(account, "salem", Duration.ofDays(1))); + () -> accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1))); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.setUsername(account, "salem")); - + () -> accounts.confirmUsernameHash(account, USERNAME_HASH_1)); + assertThat(account.getReservedUsernameHash()).isEmpty(); + assertThat(account.getUsernameHash()).isEmpty(); } 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 deleted file mode 100644 index 211840be3..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ProhibitedUsernamesTest.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * 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 622112e0a..94fe45301 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.getUsername()).thenAnswer(stubbing); + case "getUsername" -> when(updatedAccount.getUsernameHash()).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 deleted file mode 100644 index d494d76ed..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameGeneratorTest.java +++ /dev/null @@ -1,145 +0,0 @@ -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 deleted file mode 100644 index 432b318fe..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameNormalizerTest.java +++ /dev/null @@ -1,17 +0,0 @@ -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 deleted file mode 100644 index bdc1e6998..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/util/NicknameValidatorTest.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * 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) - ); - } -}