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

Change discriminator seperator and default width

This commit is contained in:
Ravi Khadiwala 2022-09-13 17:10:34 -05:00 committed by ravi-signal
parent 65dbcb3e5f
commit d0a8899daf
7 changed files with 62 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
service/src/main/java/org/whispersystems/textsecuregcm/configuration/UsernameConfiguration.java
View File

@ -13,7 +13,7 @@ public class UsernameConfiguration {
@JsonProperty
@Min(1)
private int discriminatorInitialWidth = 4;
private int discriminatorInitialWidth = 2;
@JsonProperty
@Min(1)

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

@ -34,7 +34,7 @@ public class UsernameGenerator {
* Usernames typically consist of a nickname and an integer discriminator
*/
public static final Pattern NICKNAME_PATTERN = Pattern.compile("^[_a-z][_a-z0-9]{2,31}$");
public static final String SEPARATOR = "#";
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"));
@ -112,7 +112,7 @@ public class UsernameGenerator {
throw new IllegalArgumentException("Invalid nickname " + nickname);
}
// zero pad discriminators less than the discriminator initial width
return String.format("%s#%0" + initialWidth + "d", nickname, discriminator);
return String.format("%s%s%0" + initialWidth + "d", nickname, SEPARATOR, discriminator);
}
public Duration getReservationTtl() {

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

@ -39,6 +39,7 @@ 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;
@ -750,7 +751,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);
@ -760,16 +761,16 @@ class AccountsManagerTest {
@Test
void testSetReservedHashNameMismatch() {
final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]);
setReservationHash(account, "pluto#1234");
when(accounts.usernameAvailable(eq(Optional.of(RESERVATION_TOKEN)), eq("pluto#1234"))).thenReturn(true);
setReservationHash(account, "pluto.1234");
when(accounts.usernameAvailable(eq(Optional.of(RESERVATION_TOKEN)), eq("pluto.1234"))).thenReturn(true);
assertThrows(UsernameReservationNotFoundException.class,
() -> accountsManager.confirmReservedUsername(account, "goofy#1234", RESERVATION_TOKEN));
() -> accountsManager.confirmReservedUsername(account, "goofy.1234", RESERVATION_TOKEN));
}
@Test
void testSetReservedHashAciMismatch() {
final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]);
final String reserved = "toto#1234";
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,
@ -779,7 +780,7 @@ class AccountsManagerTest {
@Test
void testSetReservedLapsed() {
final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]);
final String reserved = "porkchop#1234";
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);
@ -790,7 +791,7 @@ class AccountsManagerTest {
@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";
final String username = "santaslittlehelper.1234";
account.setUsername(username);
// reserved username already set, should be treated as a replay
@ -802,7 +803,7 @@ class AccountsManagerTest {
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");
account.setUsername(nickname + ".123");
// should be treated as a replayed request
assertDoesNotThrow(() -> accountsManager.setUsername(account, nickname, null));
@ -813,7 +814,7 @@ class AccountsManagerTest {
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";
final String username = nickname + ".ZZZ";
account.setUsername(username);
// given the correct old username, should reroll discriminator even if the nick matches
@ -825,7 +826,7 @@ class AccountsManagerTest {
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";
final String username = nickname + ".ZZZ";
account.setUsername(username);
// given the correct old username, should reroll discriminator even if the nick matches
@ -838,7 +839,7 @@ class AccountsManagerTest {
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));
() -> accountsManager.confirmReservedUsername(account, "laika.1234", RESERVATION_TOKEN));
verify(accounts, never()).confirmUsername(any(), any(), any());
}
@ -849,7 +850,7 @@ class AccountsManagerTest {
final String nickname = "test";
ArgumentMatcher<String> isWide = (String username) -> {
String[] spl = username.split(UsernameGenerator.SEPARATOR);
String[] spl = username.split(Pattern.quote(UsernameGenerator.SEPARATOR));
assertEquals(spl.length, 2);
int discriminator = Integer.parseInt(spl[1]);
// require a 7 digit discriminator
@ -872,8 +873,8 @@ class AccountsManagerTest {
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");
account.setUsername("old.123");
accountsManager.setUsername(account, nickname, "old.123");
verify(accounts).setUsername(eq(account), startsWith(nickname));
}

2
service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java
View File

@ -171,7 +171,7 @@ class AccountsManagerUsernameIntegrationTest {
}
private static int discriminator(String username) {
return Integer.parseInt(username.split(UsernameGenerator.SEPARATOR)[1]);
return Integer.parseInt(username.substring(username.indexOf(UsernameGenerator.SEPARATOR) + 1));
}
@Test

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

@ -803,10 +803,11 @@ class AccountsTest {
accounts.create(account1);
final Account account2 = generateAccount("+18005552222", UUID.randomUUID(), UUID.randomUUID());
accounts.create(account2);
final String username = "snowball.02";
accounts.reserveUsername(account1, "snowball#0002", Duration.ofDays(2));
accounts.reserveUsername(account1, username, Duration.ofDays(2));
Supplier<UUID> take = () -> accounts.reserveUsername(account2, "snowball#0002", Duration.ofDays(2));
Supplier<UUID> take = () -> accounts.reserveUsername(account2, username, Duration.ofDays(2));
for (int i = 0; i <= 2; i++) {
when(mockClock.instant()).thenReturn(Instant.EPOCH.plus(Duration.ofDays(i)));
@ -818,12 +819,12 @@ class AccountsTest {
final UUID token = take.get();
assertThrows(ContestedOptimisticLockException.class,
() -> accounts.reserveUsername(account1, "snowball#0002", Duration.ofDays(2)));
() -> accounts.reserveUsername(account1, username, Duration.ofDays(2)));
assertThrows(ContestedOptimisticLockException.class,
() -> accounts.setUsername(account1, "snowball#0002"));
() -> accounts.setUsername(account1, username));
accounts.confirmUsername(account2, "snowball#0002", token);
assertThat(accounts.getByUsername("snowball#0002").get().getUuid()).isEqualTo(account2.getUuid());
accounts.confirmUsername(account2, username, token);
assertThat(accounts.getByUsername(username).get().getUuid()).isEqualTo(account2.getUuid());
}
@Test
@ -832,10 +833,11 @@ class AccountsTest {
accounts.create(account1);
final Account account2 = generateAccount("+18005552222", UUID.randomUUID(), UUID.randomUUID());
accounts.create(account2);
final String username = "simon.123";
accounts.reserveUsername(account1, "snowball#0002", Duration.ofDays(2));
accounts.reserveUsername(account1, username, Duration.ofDays(2));
Runnable take = () -> accounts.setUsername(account2, "snowball#0002");
Runnable take = () -> accounts.setUsername(account2, username);
for (int i = 0; i <= 2; i++) {
when(mockClock.instant()).thenReturn(Instant.EPOCH.plus(Duration.ofDays(i)));
@ -847,10 +849,10 @@ class AccountsTest {
take.run();
assertThrows(ContestedOptimisticLockException.class,
() -> accounts.reserveUsername(account1, "snowball#0002", Duration.ofDays(2)));
() -> accounts.reserveUsername(account1, username, Duration.ofDays(2)));
assertThrows(ContestedOptimisticLockException.class,
() -> accounts.setUsername(account1, "snowball#0002"));
assertThat(accounts.getByUsername("snowball#0002").get().getUuid()).isEqualTo(account2.getUuid());
() -> accounts.setUsername(account1, username));
assertThat(accounts.getByUsername(username).get().getUuid()).isEqualTo(account2.getUuid());
}
@Test

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

@ -1728,7 +1728,7 @@ class AccountControllerTest {
@Test
void testSetUsername() throws UsernameNotAvailableException {
Account account = mock(Account.class);
when(account.getUsername()).thenReturn(Optional.of("n00bkiller#1234"));
when(account.getUsername()).thenReturn(Optional.of("n00bkiller.1234"));
when(accountsManager.setUsername(any(), eq("n00bkiller"), isNull()))
.thenReturn(account);
Response response =
@ -1738,13 +1738,13 @@ class AccountControllerTest {
.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");
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));
.thenReturn(new AccountsManager.UsernameReservation(null, "n00bkiller.1234", RESERVATION_TOKEN));
Response response =
resources.getJerseyTest()
.target("/v1/accounts/username/reserved")
@ -1753,48 +1753,48 @@ class AccountControllerTest {
.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));
}
@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);
when(account.getUsername()).thenReturn(Optional.of("n00bkiller.1234"));
when(accountsManager.confirmReservedUsername(any(), eq("n00bkiller.1234"), eq(RESERVATION_TOKEN))).thenReturn(account);
Response response =
resources.getJerseyTest()
.target("/v1/accounts/username/confirm")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
.put(Entity.json(new ConfirmUsernameRequest("n00bkiller#1234", RESERVATION_TOKEN)));
.put(Entity.json(new ConfirmUsernameRequest("n00bkiller.1234", RESERVATION_TOKEN)));
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 testCommitUnreservedUsername() throws UsernameNotAvailableException, UsernameReservationNotFoundException {
when(accountsManager.confirmReservedUsername(any(), eq("n00bkiller#1234"), eq(RESERVATION_TOKEN)))
when(accountsManager.confirmReservedUsername(any(), eq("n00bkiller.1234"), eq(RESERVATION_TOKEN)))
.thenThrow(new UsernameReservationNotFoundException());
Response response =
resources.getJerseyTest()
.target("/v1/accounts/username/confirm")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
.put(Entity.json(new ConfirmUsernameRequest("n00bkiller#1234", RESERVATION_TOKEN)));
.put(Entity.json(new ConfirmUsernameRequest("n00bkiller.1234", RESERVATION_TOKEN)));
assertThat(response.getStatus()).isEqualTo(409);
}
@Test
void testCommitLapsedUsername() throws UsernameNotAvailableException, UsernameReservationNotFoundException {
when(accountsManager.confirmReservedUsername(any(), eq("n00bkiller#1234"), eq(RESERVATION_TOKEN)))
when(accountsManager.confirmReservedUsername(any(), eq("n00bkiller.1234"), eq(RESERVATION_TOKEN)))
.thenThrow(new UsernameNotAvailableException());
Response response =
resources.getJerseyTest()
.target("/v1/accounts/username/confirm")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
.put(Entity.json(new ConfirmUsernameRequest("n00bkiller#1234", RESERVATION_TOKEN)));
.put(Entity.json(new ConfirmUsernameRequest("n00bkiller.1234", RESERVATION_TOKEN)));
assertThat(response.getStatus()).isEqualTo(410);
}
@ -2068,9 +2068,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.getByUsername(eq("n00bkiller.1234"))).thenReturn(Optional.of(account));
Response response = resources.getJerseyTest()
.target("v1/accounts/username/n00bkiller#1234")
.target("v1/accounts/username/n00bkiller.1234")
.request()
.header("X-Forwarded-For", "127.0.0.1")
.get();
@ -2080,9 +2080,9 @@ class AccountControllerTest {
@Test
void testLookupUsernameDoesNotExist() {
when(accountsManager.getByUsername(eq("n00bkiller#1234"))).thenReturn(Optional.empty());
when(accountsManager.getByUsername(eq("n00bkiller.1234"))).thenReturn(Optional.empty());
assertThat(resources.getJerseyTest()
.target("v1/accounts/username/n00bkiller#1234")
.target("v1/accounts/username/n00bkiller.1234")
.request()
.header("X-Forwarded-For", "127.0.0.1")
.get().getStatus()).isEqualTo(404);
@ -2092,7 +2092,7 @@ class AccountControllerTest {
void testLookupUsernameRateLimited() throws RateLimitExceededException {
doThrow(new RateLimitExceededException(Duration.ofSeconds(13))).when(usernameLookupLimiter).validate("127.0.0.1");
final Response response = resources.getJerseyTest()
.target("/v1/accounts/username/test#123")
.target("/v1/accounts/username/test.123")
.request()
.header("X-Forwarded-For", "127.0.0.1")
.get();

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

@ -34,6 +34,7 @@ public class UsernameGeneratorTest {
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"),
@ -54,24 +55,24 @@ public class UsernameGeneratorTest {
static Stream<Arguments> nonStandardUsernames() {
return Stream.of(
Arguments.of("Test#123", false),
Arguments.of("test#-123", false),
Arguments.of("test#0", false),
Arguments.of("test#", false),
Arguments.of("test#1_00", false),
Arguments.of("Test.123", false),
Arguments.of("test.-123", false),
Arguments.of("test.0", false),
Arguments.of("test.", false),
Arguments.of("test.1_00", false),
Arguments.of("test#1", true),
Arguments.of("abc#1234", true)
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");
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
@ -139,6 +140,6 @@ public class UsernameGeneratorTest {
}
private static int extractDiscriminator(final String username) {
return Integer.parseInt(username.split(UsernameGenerator.SEPARATOR)[1]);
return Integer.parseInt(username.substring(username.indexOf(UsernameGenerator.SEPARATOR) + 1));
}
}