Move handleAccountCreated() call to outside re-registration

This commit is contained in:
Chris Eager
2026-05-19 12:26:53 -05:00
committed by Chris Eager
parent 9d3b6ebb4a
commit b9a24fedea
11 changed files with 30 additions and 51 deletions
@@ -53,7 +53,7 @@ public class IntegrationTools {
new RegistrationRecoveryPasswordsManager(registrationRecoveryPasswords),
new VerificationSessionManager(verificationSessions),
new PhoneNumberIdentifiers(dynamoDbAsyncClient, config.dynamoDbTables().phoneNumberIdentifiers()),
new ChangeNumberWaitingPeriods(config.dynamoDbTables().changeNumberWaitingPeriods(), dynamoDbAsyncClient, dynamoDbClient)
new ChangeNumberWaitingPeriods(config.dynamoDbTables().changeNumberWaitingPeriods(), dynamoDbClient)
);
}
@@ -708,7 +708,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
MessagesManager messagesManager = new MessagesManager(messagesDynamoDb, messagesCache, redisMessageAvailabilityManager,
reportMessageManager, messageDeletionAsyncExecutor, Clock.systemUTC());
final ChangeNumberWaitingPeriods changeNumberWaitingPeriods = new ChangeNumberWaitingPeriods(
config.getDynamoDbTables().getChangeNumberWaitingPeriods().getTableName(), dynamoDbAsyncClient, dynamoDbClient);
config.getDynamoDbTables().getChangeNumberWaitingPeriods().getTableName(), dynamoDbClient);
final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager = new ChangeNumberWaitingPeriodManager(
changeNumberWaitingPeriods, config.getChangeNumber().postRegistrationWaitingPeriod(), clock);
AccountLockManager accountLockManager = new AccountLockManager(dynamoDbClient,
@@ -37,7 +37,6 @@ import java.util.Arrays;
import java.util.Base64;
import java.util.Collection;
import java.util.HashSet;
import java.util.HexFormat;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -405,8 +404,7 @@ public class AccountsManager extends RedisPubSubAdapter<String, String> implemen
return CompletableFuture.allOf(keysManager.deleteSingleUsePreKeys(aci),
keysManager.deleteSingleUsePreKeys(pni),
messagesManager.clear(aci),
profilesManager.deleteAll(aci, false),
changeNumberWaitingPeriodManager.handleAccountCreated(aci, clock.instant()));
profilesManager.deleteAll(aci, false));
})
.join();
}
@@ -419,6 +417,7 @@ public class AccountsManager extends RedisPubSubAdapter<String, String> implemen
.join())
.orElse(false);
changeNumberWaitingPeriodManager.handleAccountCreated(account.getUuid(), clock.instant());
Tags tags = Tags.of(UserAgentTagUtil.getPlatformTag(userAgent),
Tag.of("type", accountCreationType),
@@ -11,7 +11,6 @@ import java.time.Duration;
import java.time.Instant;
import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
/// Manages post-registration change number waiting period expiration data
public class ChangeNumberWaitingPeriodManager {
@@ -29,8 +28,8 @@ public class ChangeNumberWaitingPeriodManager {
/// Must be called when an account is created, including re-registration
@VisibleForTesting
public CompletableFuture<Void> handleAccountCreated(final UUID aci, final Instant created) {
return changeNumberWaitingPeriods.setExpiration(aci, created.plus(waitingPeriod));
public void handleAccountCreated(final UUID aci, final Instant created) {
changeNumberWaitingPeriods.setExpiration(aci, created.plus(waitingPeriod));
}
/// Returns the waiting period duration remaining, if any. If present, {@code duration} will always be positive.
@@ -10,10 +10,7 @@ import java.time.Instant;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import org.whispersystems.textsecuregcm.util.AttributeValues;
import org.whispersystems.textsecuregcm.util.Util;
import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient;
import software.amazon.awssdk.services.dynamodb.DynamoDbClient;
import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
import software.amazon.awssdk.services.dynamodb.model.DeleteItemRequest;
@@ -29,23 +26,20 @@ public class ChangeNumberWaitingPeriods {
static final String ATTR_TTL = "E";
private final String tableName;
private final DynamoDbAsyncClient dynamoDbAsyncClient;
private final DynamoDbClient dynamoDbClient;
public ChangeNumberWaitingPeriods(final String tableName, final DynamoDbAsyncClient dynamoDbAsyncClient, final DynamoDbClient dynamoDbClient) {
public ChangeNumberWaitingPeriods(final String tableName, final DynamoDbClient dynamoDbClient) {
this.tableName = tableName;
this.dynamoDbAsyncClient = dynamoDbAsyncClient;
this.dynamoDbClient = dynamoDbClient;
}
public CompletableFuture<Void> setExpiration(final UUID aci, final Instant expiration) {
return dynamoDbAsyncClient.putItem(PutItemRequest.builder()
public void setExpiration(final UUID aci, final Instant expiration) {
dynamoDbClient.putItem(PutItemRequest.builder()
.tableName(tableName)
.item(Map.of(
KEY_ACCOUNT_UUID, AttributeValues.fromUUID(aci),
ATTR_TTL, AttributeValues.fromLong(expiration.getEpochSecond())))
.build())
.thenRun(Util.NOOP);
.build());
}
public Optional<Instant> getExpiration(final UUID aci) {
@@ -284,7 +284,7 @@ public record CommandDependencies(
RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager =
new RegistrationRecoveryPasswordsManager(registrationRecoveryPasswords);
final ChangeNumberWaitingPeriods changeNumberWaitingPeriods = new ChangeNumberWaitingPeriods(
configuration.getDynamoDbTables().getChangeNumberWaitingPeriods().getTableName(), dynamoDbAsyncClient, dynamoDbClient);
configuration.getDynamoDbTables().getChangeNumberWaitingPeriods().getTableName(), dynamoDbClient);
final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager = new ChangeNumberWaitingPeriodManager(
changeNumberWaitingPeriods, configuration.getChangeNumber().postRegistrationWaitingPeriod(), clock);
AccountsManager accountsManager = new AccountsManager(accounts, phoneNumberIdentifiers, cacheCluster,
@@ -140,10 +140,6 @@ public class AccountCreationDeletionIntegrationTest {
disconnectionRequestManager = mock(DisconnectionRequestManager.class);
when(disconnectionRequestManager.requestDisconnection(any())).thenReturn(CompletableFuture.completedFuture(null));
final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager = mock(ChangeNumberWaitingPeriodManager.class);
when(changeNumberWaitingPeriodManager.handleAccountCreated(any(UUID.class), any(Instant.class)))
.thenReturn(CompletableFuture.completedFuture(null));
accountsManager = new AccountsManager(
accounts,
phoneNumberIdentifiers,
@@ -153,7 +149,7 @@ public class AccountCreationDeletionIntegrationTest {
keysManager,
messagesManager,
profilesManager,
changeNumberWaitingPeriodManager,
mock(ChangeNumberWaitingPeriodManager.class),
secureStorageClient,
svr2Client,
disconnectionRequestManager,
@@ -49,7 +49,6 @@ import java.util.Base64;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HexFormat;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -123,6 +122,7 @@ class AccountsManagerTest {
private MessagesManager messagesManager;
private ProfilesManager profilesManager;
private DisconnectionRequestManager disconnectionRequestManager;
private ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager;
private Map<String, UUID> phoneNumberIdentifiersByE164;
@@ -147,7 +147,7 @@ class AccountsManagerTest {
messagesManager = mock(MessagesManager.class);
profilesManager = mock(ProfilesManager.class);
disconnectionRequestManager = mock(DisconnectionRequestManager.class);
final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager = mock(ChangeNumberWaitingPeriodManager.class);
changeNumberWaitingPeriodManager = mock(ChangeNumberWaitingPeriodManager.class);
//noinspection unchecked
asyncCommands = mock(RedisAsyncCommands.class);
@@ -215,8 +215,6 @@ class AccountsManagerTest {
.build();
when(disconnectionRequestManager.requestDisconnection(any())).thenReturn(CompletableFuture.completedFuture(null));
when(changeNumberWaitingPeriodManager.handleAccountCreated(any(UUID.class), any(Instant.class)))
.thenReturn(CompletableFuture.completedFuture(null));
accountsManager = new AccountsManager(
accounts,
@@ -725,6 +723,8 @@ class AccountsManagerTest {
notNull(),
notNull());
verify(changeNumberWaitingPeriodManager).handleAccountCreated(eq(createdAccount.getUuid()), any(Instant.class));
verifyNoInteractions(messagesManager);
verifyNoInteractions(profilesManager);
}
@@ -782,6 +782,7 @@ class AccountsManagerTest {
verify(profilesManager, times(2)).deleteAll(existingUuid, false);
verify(disconnectionRequestManager).requestDisconnection(argThat(account ->
account.getIdentifier(IdentityType.ACI).equals(existingUuid) && account != reregisteredAccount));
verify(changeNumberWaitingPeriodManager).handleAccountCreated(eq(existingUuid), any(Instant.class));
}
@Test
@@ -134,10 +134,6 @@ class AccountsManagerUsernameIntegrationTest {
final DisconnectionRequestManager disconnectionRequestManager = mock(DisconnectionRequestManager.class);
when(disconnectionRequestManager.requestDisconnection(any())).thenReturn(CompletableFuture.completedFuture(null));
final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager = mock(ChangeNumberWaitingPeriodManager.class);
when(changeNumberWaitingPeriodManager.handleAccountCreated(any(UUID.class), any(Instant.class)))
.thenReturn(CompletableFuture.completedFuture(null));
accountsManager = new AccountsManager(
accounts,
phoneNumberIdentifiers,
@@ -147,7 +143,7 @@ class AccountsManagerUsernameIntegrationTest {
keysManager,
messageManager,
profileManager,
changeNumberWaitingPeriodManager,
mock(ChangeNumberWaitingPeriodManager.class),
mock(SecureStorageClient.class),
mock(SecureValueRecoveryClient.class),
disconnectionRequestManager,
@@ -11,7 +11,6 @@ import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -31,31 +30,27 @@ class ChangeNumberWaitingPeriodManagerTest {
void setUp() {
changeNumberWaitingPeriodManager = new ChangeNumberWaitingPeriodManager(
new ChangeNumberWaitingPeriods(Tables.CHANGE_NUMBER_WAITING_PERIODS.tableName(),
DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient(),
DYNAMO_DB_EXTENSION.getDynamoDbClient()),
WAITING_PERIOD,
Clock.systemUTC());
}
@Test
void testNewAccount() throws Exception {
void testNewAccount() {
final UUID aci = UUID.randomUUID();
assertTrue(changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(aci).isEmpty());
changeNumberWaitingPeriodManager.handleAccountCreated(aci, Instant.now())
.get(5, TimeUnit.SECONDS);
changeNumberWaitingPeriodManager.handleAccountCreated(aci, Instant.now());
assertTrue(changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(aci).isPresent());
}
@Test
void testOldAccount() throws Exception {
void testOldAccount() {
final UUID aci = UUID.randomUUID();
changeNumberWaitingPeriodManager.handleAccountCreated(aci,
Instant.now().minus(WAITING_PERIOD).minus(Duration.ofHours(1)))
.get(5, TimeUnit.SECONDS);
changeNumberWaitingPeriodManager.handleAccountCreated(aci, Instant.now().minus(WAITING_PERIOD).minus(Duration.ofHours(1)));
assertTrue(changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(aci).isEmpty());
}
@@ -30,22 +30,21 @@ class ChangeNumberWaitingPeriodsTest {
void setUp() {
changeNumberWaitingPeriods = new ChangeNumberWaitingPeriods(
Tables.CHANGE_NUMBER_WAITING_PERIODS.tableName(),
DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient(),
DYNAMO_DB_EXTENSION.getDynamoDbClient());
}
@Test
void getExpiration_unknownAci() throws Exception {
void getExpiration_unknownAci() {
assertTrue(changeNumberWaitingPeriods.getExpiration(UUID.randomUUID()).isEmpty());
}
@Test
void setAndGetExpiration() throws Exception {
void setAndGetExpiration() {
final UUID aci = UUID.randomUUID();
// truncate to seconds because the TTL attribute stores epoch seconds
final Instant expiration = Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.SECONDS);
changeNumberWaitingPeriods.setExpiration(aci, expiration).get();
changeNumberWaitingPeriods.setExpiration(aci, expiration);
final Optional<Instant> result = changeNumberWaitingPeriods.getExpiration(aci);
assertTrue(result.isPresent());
@@ -55,12 +54,12 @@ class ChangeNumberWaitingPeriodsTest {
}
@Test
void setExpiration_overwritesExistingEntry() throws Exception {
void setExpiration_overwritesExistingEntry() {
final UUID aci = UUID.randomUUID();
final Instant first = Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.SECONDS);
final Instant second = Instant.now().plusSeconds(7200).truncatedTo(ChronoUnit.SECONDS);
changeNumberWaitingPeriods.setExpiration(aci, first).get();
changeNumberWaitingPeriods.setExpiration(aci, first);
{
final Optional<Instant> result = changeNumberWaitingPeriods.getExpiration(aci);
@@ -68,7 +67,7 @@ class ChangeNumberWaitingPeriodsTest {
assertEquals(first, result.get());
}
changeNumberWaitingPeriods.setExpiration(aci, second).get();
changeNumberWaitingPeriods.setExpiration(aci, second);
{
final Optional<Instant> result = changeNumberWaitingPeriods.getExpiration(aci);
@@ -78,9 +77,9 @@ class ChangeNumberWaitingPeriodsTest {
}
@Test
void delete_removesExisting() throws Exception {
void delete_removesExisting() {
final UUID aci = UUID.randomUUID();
changeNumberWaitingPeriods.setExpiration(aci, Instant.now().plusSeconds(3600)).get();
changeNumberWaitingPeriods.setExpiration(aci, Instant.now().plusSeconds(3600));
assertTrue( changeNumberWaitingPeriods.getExpiration(aci).isPresent());