MirrorMirror
/
Signal-Server
mirror of https://github.com/signalapp/Signal-Server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Enable case-sensitive usernames

This commit is contained in:
Katherine Yen 2022-12-13 07:59:37 -08:00 committed by GitHub
parent a883426402
commit 26f5ffdde3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 83 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java
View File

@ -39,6 +39,7 @@ import org.whispersystems.textsecuregcm.util.AttributeValues;
import org.whispersystems.textsecuregcm.util.ExceptionUtils;
import org.whispersystems.textsecuregcm.util.SystemMapper;
import org.whispersystems.textsecuregcm.util.UUIDUtil;
import org.whispersystems.textsecuregcm.util.UsernameNormalizer;
import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient;
import software.amazon.awssdk.services.dynamodb.DynamoDbClient;
import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
@ -304,7 +305,6 @@ public class Accounts extends AbstractDynamoDbStore {
final String reservedUsername,
final Duration ttl) {
final long startNanos = System.nanoTime();
// if there is an existing old reservation it will be cleaned up via ttl
final Optional<byte[]> maybeOriginalReservation = account.getReservedUsernameHash();
account.setReservedUsernameHash(reservedUsernameHash(account.getUuid(), reservedUsername));
@ -322,7 +322,7 @@ public class Accounts extends AbstractDynamoDbStore {
.tableName(usernamesConstraintTableName)
.item(Map.of(
KEY_ACCOUNT_UUID, AttributeValues.fromUUID(reservationToken),
ATTR_USERNAME, AttributeValues.fromString(reservedUsername),
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))
@ -411,12 +411,13 @@ public class Accounts extends AbstractDynamoDbStore {
final List<TransactWriteItem> 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
writeItems.add(TransactWriteItem.builder()
.put(Put.builder()
.tableName(usernamesConstraintTableName)
.item(Map.of(
KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()),
ATTR_USERNAME, AttributeValues.fromString(username)))
ATTR_USERNAME, AttributeValues.fromString(UsernameNormalizer.normalize(username))))
// it's not in the constraint table OR it's expired OR it was reserved by us
.conditionExpression("attribute_not_exists(#username) OR #ttl < :now OR #aci = :reservation ")
.expressionAttributeNames(Map.of("#username", ATTR_USERNAME, "#ttl", ATTR_TTL, "#aci", KEY_ACCOUNT_UUID))
@ -446,7 +447,7 @@ public class Accounts extends AbstractDynamoDbStore {
.build());
maybeOriginalUsername.ifPresent(originalUsername -> writeItems.add(
buildDelete(usernamesConstraintTableName, ATTR_USERNAME, originalUsername)));
buildDelete(usernamesConstraintTableName, ATTR_USERNAME, UsernameNormalizer.normalize(originalUsername))));
final TransactWriteItemsRequest request = TransactWriteItemsRequest.builder()
.transactItems(writeItems)
@ -499,7 +500,7 @@ public class Accounts extends AbstractDynamoDbStore {
.build())
.build());
writeItems.add(buildDelete(usernamesConstraintTableName, ATTR_USERNAME, username));
writeItems.add(buildDelete(usernamesConstraintTableName, ATTR_USERNAME, UsernameNormalizer.normalize(username)));
final TransactWriteItemsRequest request = TransactWriteItemsRequest.builder()
.transactItems(writeItems)
@ -606,7 +607,7 @@ public class Accounts extends AbstractDynamoDbStore {
public boolean usernameAvailable(final Optional<UUID> reservationToken, final String username) {
final Optional<Map<String, AttributeValue>> usernameItem = itemByKey(
usernamesConstraintTableName, ATTR_USERNAME, AttributeValues.fromString(username));
usernamesConstraintTableName, ATTR_USERNAME, AttributeValues.fromString(UsernameNormalizer.normalize(username)));
if (usernameItem.isEmpty()) {
// username is free
@ -643,7 +644,7 @@ public class Accounts extends AbstractDynamoDbStore {
GET_BY_USERNAME_TIMER,
usernamesConstraintTableName,
ATTR_USERNAME,
AttributeValues.fromString(username),
AttributeValues.fromString(UsernameNormalizer.normalize(username)),
item -> !item.containsKey(ATTR_TTL) // ignore items with a ttl (reservations)
);
}
@ -665,11 +666,10 @@ public class Accounts extends AbstractDynamoDbStore {
));
account.getUsername().ifPresent(username -> transactWriteItems.add(
buildDelete(usernamesConstraintTableName, ATTR_USERNAME, username)));
buildDelete(usernamesConstraintTableName, ATTR_USERNAME, UsernameNormalizer.normalize(username))));
final TransactWriteItemsRequest request = TransactWriteItemsRequest.builder()
.transactItems(transactWriteItems).build();
db().transactWriteItems(request);
}));
}
@ -852,7 +852,7 @@ public class Accounts extends AbstractDynamoDbStore {
throw new AssertionError(e);
}
final ByteBuffer byteBuffer = ByteBuffer.allocate(32 + 1);
sha256.update(reservedUsername.getBytes(StandardCharsets.UTF_8));
sha256.update(UsernameNormalizer.normalize(reservedUsername).getBytes(StandardCharsets.UTF_8));
sha256.update(UUIDUtil.toBytes(accountId));
byteBuffer.put(RESERVED_USERNAME_HASH_VERSION);
byteBuffer.put(sha256.digest());

7
service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java
View File

@ -51,6 +51,7 @@ 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 {
@ -391,7 +392,7 @@ public class AccountsManager {
return account;
}
final byte[] newHash = Accounts.reservedUsernameHash(account.getUuid(), reservedUsername);
final byte[] newHash = Accounts.reservedUsernameHash(account.getUuid(), UsernameNormalizer.normalize(reservedUsername));
if (!account.getReservedUsernameHash().map(oldHash -> Arrays.equals(oldHash, newHash)).orElse(false)) {
// no such reservation existed, either there was no previous call to reserveUsername
// or the reservation changed
@ -720,8 +721,8 @@ public class AccountsManager {
clientPresenceManager.disconnectPresence(account.getUuid(), device.getId())));
}
private String getUsernameAccountMapKey(String key) {
return "UAccountMap::" + key;
private String getUsernameAccountMapKey(String username) {
return "UAccountMap::" + UsernameNormalizer.normalize(username);
}
private String getAccountMapKey(String key) {

11
service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameGenerator.java
View File

@ -22,18 +22,17 @@ import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name;
public class UsernameGenerator {
/**
* Nicknames are
* Nicknames
* <list>
* <li> lowercase </li>
* <li> do not start with a number </li>
* <li> alphanumeric or underscores only </li>
* <li> minimum length 3 </li>
* <li> maximum length 32 </li>
* <li> are alphanumeric or underscores only </li>
* <li> have minimum length 3 </li>
* <li> have maximum length 32 </li>
* </list>
*
* Usernames typically consist of a nickname and an integer discriminator
*/
public static final Pattern NICKNAME_PATTERN = Pattern.compile("^[_a-z][_a-z0-9]{2,31}$");
public static final 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"));

10
service/src/main/java/org/whispersystems/textsecuregcm/util/UsernameNormalizer.java Normal file
View File

@ -0,0 +1,10 @@
package org.whispersystems.textsecuregcm.util;
import java.util.Locale;
public final class UsernameNormalizer {
private UsernameNormalizer() {}
public static String normalize(final String username) {
return username.toLowerCase(Locale.ROOT);
}
}

16
service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java
View File

@ -1496,32 +1496,32 @@ 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()))
when(account.getUsername()).thenReturn(Optional.of("N00bkilleR.1234"));
when(accountsManager.setUsername(any(), eq("N00bkilleR"), isNull()))
.thenReturn(account);
Response response =
resources.getJerseyTest()
.target("/v1/accounts/username")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
.put(Entity.json(new UsernameRequest("n00bkiller", null)));
.put(Entity.json(new UsernameRequest("N00bkilleR", null)));
assertThat(response.getStatus()).isEqualTo(200);
assertThat(response.readEntity(UsernameResponse.class).username()).isEqualTo("n00bkiller.1234");
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));
when(accountsManager.reserveUsername(any(), eq("N00bkilleR")))
.thenReturn(new AccountsManager.UsernameReservation(null, "N00bkilleR.1234", RESERVATION_TOKEN));
Response response =
resources.getJerseyTest()
.target("/v1/accounts/username/reserved")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
.put(Entity.json(new ReserveUsernameRequest("n00bkiller")));
.put(Entity.json(new ReserveUsernameRequest("N00bkilleR")));
assertThat(response.getStatus()).isEqualTo(200);
assertThat(response.readEntity(ReserveUsernameResponse.class))
.satisfies(r -> r.username().equals("n00bkiller.1234"))
.satisfies(r -> r.username().equals("N00bkilleR.1234"))
.satisfies(r -> r.reservationToken().equals(RESERVATION_TOKEN));
}

3
service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java
View File

@ -63,6 +63,7 @@ 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 {
@ -751,7 +752,7 @@ class AccountsManagerTest {
@Test
void testSetReservedUsername() throws UsernameNotAvailableException, UsernameReservationNotFoundException {
final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]);
final String reserved = "scooby.1234";
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);

39
service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java
View File

@ -43,6 +43,7 @@ import org.whispersystems.textsecuregcm.util.TestClock;
import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient;
import software.amazon.awssdk.services.dynamodb.DynamoDbClient;
import software.amazon.awssdk.services.dynamodb.model.AttributeDefinition;
import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
import software.amazon.awssdk.services.dynamodb.model.ConditionalCheckFailedException;
import software.amazon.awssdk.services.dynamodb.model.CreateTableRequest;
import software.amazon.awssdk.services.dynamodb.model.GetItemRequest;
@ -617,7 +618,7 @@ class AccountsTest {
final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID());
accounts.create(account);
final String username = "test";
final String username = "TeST";
assertThat(accounts.getByUsername(username)).isEmpty();
@ -638,6 +639,12 @@ class AccountsTest {
accounts.setUsername(account, secondUsername);
assertThat(accounts.getByUsername(username)).isEmpty();
assertThat(dynamoDbExtension.getDynamoDbClient()
.getItem(GetItemRequest.builder()
.tableName(USERNAME_CONSTRAINT_TABLE_NAME)
.key(Map.of(Accounts.ATTR_USERNAME, AttributeValues.fromString("test")))
.build())
.item()).isEmpty();
{
final Optional<Account> maybeAccount = accounts.getByUsername(secondUsername);
@ -688,7 +695,7 @@ class AccountsTest {
final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID());
accounts.create(account);
final String username = "test";
final String username = "TeST";
accounts.setUsername(account, username);
assertThat(accounts.getByUsername(username)).isPresent();
@ -731,29 +738,31 @@ class AccountsTest {
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"));
final UUID token = accounts.reserveUsername(account1, "GarfielD", Duration.ofDays(1));
assertThat(account1.getReservedUsernameHash()).get().isEqualTo(Accounts.reservedUsernameHash(account1.getUuid(), "GarfielD"));
assertThat(account1.getUsername()).isEmpty();
// account 2 shouldn't be able to reserve the username
// account 2 shouldn't be able to reserve the username if it's the same when normalized
assertThrows(ContestedOptimisticLockException.class,
() -> accounts.reserveUsername(account2, "garfield", Duration.ofDays(1)));
() -> accounts.reserveUsername(account2, "gARFIELd", Duration.ofDays(1)));
assertThrows(ContestedOptimisticLockException.class,
() -> accounts.confirmUsername(account2, "garfield", UUID.randomUUID()));
assertThat(accounts.getByUsername("garfield")).isEmpty();
() -> accounts.confirmUsername(account2, "gARFIELd", UUID.randomUUID()));
assertThat(accounts.getByUsername("gARFIELd")).isEmpty();
accounts.confirmUsername(account1, "garfield", token);
accounts.confirmUsername(account1, "GarfielD", token);
assertThat(account1.getReservedUsernameHash()).isEmpty();
assertThat(account1.getUsername()).get().isEqualTo("garfield");
assertThat(accounts.getByUsername("garfield").get().getUuid()).isEqualTo(account1.getUuid());
assertThat(account1.getUsername()).get().isEqualTo("GarfielD");
assertThat(accounts.getByUsername("GarfielD").get().getUuid()).isEqualTo(account1.getUuid());
assertThat(dynamoDbExtension.getDynamoDbClient()
final Map<String, AttributeValue> usernameConstraintRecord = dynamoDbExtension.getDynamoDbClient()
.getItem(GetItemRequest.builder()
.tableName(USERNAME_CONSTRAINT_TABLE_NAME)
.key(Map.of(Accounts.ATTR_USERNAME, AttributeValues.fromString("garfield")))
.build())
.item())
.doesNotContainKey(Accounts.ATTR_TTL);
.item();
assertThat(usernameConstraintRecord).containsKey(Accounts.ATTR_USERNAME);
assertThat(usernameConstraintRecord).doesNotContainKey(Accounts.ATTR_TTL);
}
@Test
@ -761,7 +770,7 @@ class AccountsTest {
final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID());
accounts.create(account1);
final String username = "unsinkablesam";
final String username = "UnSinkaBlesam";
final UUID token = accounts.reserveUsername(account1, username, Duration.ofDays(1));
assertThat(accounts.usernameAvailable(username)).isFalse();

6
service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameGeneratorTest.java
View File

@ -28,8 +28,8 @@ public class UsernameGeneratorTest {
static Stream<Arguments> nicknameValidation() {
return Stream.of(
Arguments.of("Test", false, "upper case"),
Arguments.of("tesT", false, "upper case"),
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"),
@ -55,7 +55,7 @@ public class UsernameGeneratorTest {
static Stream<Arguments> nonStandardUsernames() {
return Stream.of(
Arguments.of("Test.123", false),
Arguments.of("Test.123", true),
Arguments.of("test.-123", false),
Arguments.of("test.0", false),
Arguments.of("test.", false),

17
service/src/test/java/org/whispersystems/textsecuregcm/tests/util/UsernameNormalizerTest.java Normal file
View File

@ -0,0 +1,17 @@
package org.whispersystems.textsecuregcm.tests.util;
import org.junit.jupiter.api.Test;
import org.whispersystems.textsecuregcm.util.UsernameNormalizer;
import static org.assertj.core.api.Assertions.assertThat;
public class UsernameNormalizerTest {
@Test
public void usernameNormalization() {
assertThat(UsernameNormalizer.normalize("TeST")).isEqualTo("test");
assertThat(UsernameNormalizer.normalize("TeST_")).isEqualTo("test_");
assertThat(UsernameNormalizer.normalize("TeST_.123")).isEqualTo("test_.123");
}
}