Skip to content

Commit

Permalink
Allow reserving a hash previously reserved (but not committed) by the…
Browse files Browse the repository at this point in the history 
… same user
  • Loading branch information
@jkt-signal
jkt-signal authored Jan 5, 2024
1 parent f495ff4 commit 1e5fadc
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 26 deletions.
9 changes: 6 additions & 3 deletions service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,12 @@ public CompletableFuture<Void> reserveUsernameHash(
ATTR_TTL, AttributeValues.fromLong(expirationTime),
ATTR_CONFIRMED, AttributeValues.fromBool(false),
ATTR_RECLAIMABLE, 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())))
.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()),
":aci", AttributeValues.fromUUID(uuid),
":confirmed", AttributeValues.fromBool(false)))
.returnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD)
.build())
.build());
Expand Down
91 changes: 68 additions & 23 deletions service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -910,12 +910,7 @@ void testSwitchUsernameHashes() {
final UUID newHandle = account.getUsernameLinkHandle();

assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty();
assertThat(DYNAMO_DB_EXTENSION.getDynamoDbClient()
.getItem(GetItemRequest.builder()
.tableName(Tables.USERNAMES.tableName())
.key(Map.of(Accounts.ATTR_USERNAME_HASH, AttributeValues.fromByteArray(USERNAME_HASH_1)))
.build())
.item()).isEmpty();
assertThat(getUsernameConstraintTableItem(USERNAME_HASH_1)).isEmpty();
assertThat(accounts.getByUsernameLinkHandle(oldHandle).join()).isEmpty();

{
Expand Down Expand Up @@ -1045,17 +1040,69 @@ void testReservedUsernameHash() {
assertArrayEquals(account1.getUsernameHash().orElseThrow(), USERNAME_HASH_1);
assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join().get().getUuid()).isEqualTo(account1.getUuid());

final Map<String, AttributeValue> usernameConstraintRecord = DYNAMO_DB_EXTENSION.getDynamoDbClient()
.getItem(GetItemRequest.builder()
.tableName(Tables.USERNAMES.tableName())
.key(Map.of(Accounts.ATTR_USERNAME_HASH, AttributeValues.fromByteArray(USERNAME_HASH_1)))
.build())
.item();
final Map<String, AttributeValue> usernameConstraintRecord = getUsernameConstraintTableItem(USERNAME_HASH_1);

assertThat(usernameConstraintRecord).containsKey(Accounts.ATTR_USERNAME_HASH);
assertThat(usernameConstraintRecord).doesNotContainKey(Accounts.ATTR_TTL);
}

@Test
void switchBetweenReservedUsernameHashes() {
final Account account = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID());
createAccount(account);

accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join();
assertArrayEquals(account.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1);
assertThat(account.getUsernameHash()).isEmpty();

accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofDays(1)).join();
assertArrayEquals(account.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_2);
assertThat(account.getUsernameHash()).isEmpty();

final Map<String, AttributeValue> usernameConstraintRecord1 = getUsernameConstraintTableItem(USERNAME_HASH_1);
final Map<String, AttributeValue> usernameConstraintRecord2 = getUsernameConstraintTableItem(USERNAME_HASH_2);
assertThat(usernameConstraintRecord1).containsKey(Accounts.ATTR_USERNAME_HASH);
assertThat(usernameConstraintRecord2).containsKey(Accounts.ATTR_USERNAME_HASH);
assertThat(usernameConstraintRecord1).containsKey(Accounts.ATTR_TTL);
assertThat(usernameConstraintRecord2).containsKey(Accounts.ATTR_TTL);

clock.pin(Instant.EPOCH.plus(Duration.ofMinutes(1)));

accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join();
assertArrayEquals(account.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1);
assertThat(account.getUsernameHash()).isEmpty();

final Map<String, AttributeValue> newUsernameConstraintRecord1 = getUsernameConstraintTableItem(USERNAME_HASH_1);
assertThat(newUsernameConstraintRecord1).containsKey(Accounts.ATTR_USERNAME_HASH);
assertThat(newUsernameConstraintRecord1).containsKey(Accounts.ATTR_TTL);
assertThat(usernameConstraintRecord1.get(Accounts.ATTR_TTL))
.isNotEqualTo(newUsernameConstraintRecord1.get(Accounts.ATTR_TTL));
}

@Test
void reserveOwnConfirmedUsername() {
final Account account = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID());
createAccount(account);

accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join();
assertArrayEquals(account.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1);
assertThat(account.getUsernameHash()).isEmpty();
assertThat(getUsernameConstraintTableItem(USERNAME_HASH_1)).containsKey(Accounts.ATTR_TTL);


accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join();
assertThat(account.getReservedUsernameHash()).isEmpty();
assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_1);
assertThat(getUsernameConstraintTableItem(USERNAME_HASH_1)).doesNotContainKey(Accounts.ATTR_TTL);

CompletableFutureTestUtil.assertFailsWithCause(ContestedOptimisticLockException.class,
accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)));
assertThat(account.getReservedUsernameHash()).isEmpty();
assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_1);
assertThat(getUsernameConstraintTableItem(USERNAME_HASH_1)).containsKey(Accounts.ATTR_USERNAME_HASH);
assertThat(getUsernameConstraintTableItem(USERNAME_HASH_1)).doesNotContainKey(Accounts.ATTR_TTL);
}

@Test
void testUsernameHashAvailable() {
final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID());
Expand Down Expand Up @@ -1119,17 +1166,6 @@ void testConfirmExpiredReservedUsernameHash() {
assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join().get().getUuid()).isEqualTo(account2.getUuid());
}

@Test
void testRetryReserveUsernameHash() {
final Account account = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID());
createAccount(account);
accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(2)).join();

CompletableFutureTestUtil.assertFailsWithCause(ContestedOptimisticLockException.class,
accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(2)),
"Shouldn't be able to re-reserve same username hash (would extend ttl)");
}

@Test
void testReserveConfirmUsernameHashVersionConflict() {
final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID());
Expand Down Expand Up @@ -1300,6 +1336,15 @@ private Map<String, AttributeValue> readAccount(final UUID uuid) {
return get.item();
}

private Map<String, AttributeValue> getUsernameConstraintTableItem(final byte[] usernameHash) {
return DYNAMO_DB_EXTENSION.getDynamoDbClient()
.getItem(GetItemRequest.builder()
.tableName(Tables.USERNAMES.tableName())
.key(Map.of(Accounts.ATTR_USERNAME_HASH, AttributeValues.fromByteArray(usernameHash)))
.build())
.item();
}

private void verifyStoredState(String number, UUID uuid, UUID pni, byte[] usernameHash, Account expecting, boolean canonicallyDiscoverable) {
final DynamoDbClient db = DYNAMO_DB_EXTENSION.getDynamoDbClient();

Expand Down

0 comments on commit 1e5fadc

Please sign in to comment.