diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 07d627f69..7072ffd91 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -235,6 +235,7 @@ import org.whispersystems.textsecuregcm.storage.AccountLockManager; import org.whispersystems.textsecuregcm.storage.Accounts; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.ChangeNumberManager; +import org.whispersystems.textsecuregcm.storage.ChangeNumberWaitingPeriodManager; import org.whispersystems.textsecuregcm.storage.ClientReleaseManager; import org.whispersystems.textsecuregcm.storage.ClientReleases; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; @@ -705,11 +706,13 @@ public class WhisperServerService extends Application commonControllers = Lists.newArrayList( new AccountController(accountsManager, rateLimiters, registrationRecoveryPasswordsManager, 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 1774d25e3..eae1de94c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -129,6 +129,7 @@ public class AccountsManager extends RedisPubSubAdapter implemen private final KeysManager keysManager; private final MessagesManager messagesManager; private final ProfilesManager profilesManager; + private final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager; private final SecureStorageClient secureStorageClient; private final SecureValueRecoveryClient secureValueRecovery2Client; private final DisconnectionRequestManager disconnectionRequestManager; @@ -210,7 +211,7 @@ public class AccountsManager extends RedisPubSubAdapter implemen final AccountLockManager accountLockManager, final KeysManager keysManager, final MessagesManager messagesManager, - final ProfilesManager profilesManager, + final ProfilesManager profilesManager, final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager, final SecureStorageClient secureStorageClient, final SecureValueRecoveryClient secureValueRecovery2Client, final DisconnectionRequestManager disconnectionRequestManager, @@ -227,6 +228,7 @@ public class AccountsManager extends RedisPubSubAdapter implemen this.keysManager = keysManager; this.messagesManager = messagesManager; this.profilesManager = profilesManager; + this.changeNumberWaitingPeriodManager = changeNumberWaitingPeriodManager; this.secureStorageClient = secureStorageClient; this.secureValueRecovery2Client = secureValueRecovery2Client; this.disconnectionRequestManager = disconnectionRequestManager; @@ -403,7 +405,8 @@ public class AccountsManager extends RedisPubSubAdapter implemen return CompletableFuture.allOf(keysManager.deleteSingleUsePreKeys(aci), keysManager.deleteSingleUsePreKeys(pni), messagesManager.clear(aci), - profilesManager.deleteAll(aci, false)); + profilesManager.deleteAll(aci, false), + changeNumberWaitingPeriodManager.handleAccountCreated(aci, clock.instant())); }) .join(); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java index b9a15cd28..935501165 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java @@ -13,7 +13,6 @@ import io.micrometer.core.instrument.Tags; import jakarta.ws.rs.container.ContainerRequestContext; import java.time.Clock; import java.time.Duration; -import java.time.Instant; import java.util.List; import java.util.Map; import java.util.Optional; @@ -48,7 +47,7 @@ public class ChangeNumberManager { private final AccountsManager accountsManager; private final PhoneVerificationTokenManager phoneVerificationTokenManager; private final RegistrationLockVerificationManager registrationLockVerificationManager; - private final Duration postRegistrationWaitingPeriod; + private final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager; private final RateLimiters rateLimiters; private final Clock clock; @@ -66,7 +65,7 @@ public class ChangeNumberManager { final PhoneVerificationTokenManager phoneVerificationTokenManager, final RegistrationLockVerificationManager registrationLockVerificationManager, final RateLimiters rateLimiters, - final Duration postRegistrationWaitingPeriod, + final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager, final Clock clock) { this.messageSender = messageSender; @@ -74,7 +73,7 @@ public class ChangeNumberManager { this.phoneVerificationTokenManager = phoneVerificationTokenManager; this.registrationLockVerificationManager = registrationLockVerificationManager; this.rateLimiters = rateLimiters; - this.postRegistrationWaitingPeriod = postRegistrationWaitingPeriod; + this.changeNumberWaitingPeriodManager = changeNumberWaitingPeriodManager; this.clock = clock; } @@ -101,11 +100,10 @@ public class ChangeNumberManager { // Only verify and check reglock if there's a data change to be made... if (!account.getNumber().equals(number)) { - final Instant registration = Instant.ofEpochMilli(account.getPrimaryDevice().getCreated()); - final Duration waitingPeriodRemaining = Duration.between(clock.instant().minus(postRegistrationWaitingPeriod), registration); - if (waitingPeriodRemaining.isPositive()) { + final Optional waitingPeriodRemaining = changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(account.getUuid()); + if (waitingPeriodRemaining.isPresent()) { Metrics.counter(POST_REGISTRATION_WAITING_PERIOD_NOT_MET_COUNTER_NAME).increment(); - throw new RateLimitExceededException(waitingPeriodRemaining); + throw new RateLimitExceededException(waitingPeriodRemaining.get()); } rateLimiters.getRegistrationLimiter().validate(number); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManager.java new file mode 100644 index 000000000..83d6a7d8e --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManager.java @@ -0,0 +1,64 @@ +/* + * Copyright 2026 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.storage; + +import com.google.common.annotations.VisibleForTesting; +import io.lettuce.core.SetArgs; +import java.time.Duration; +import java.time.Instant; +import java.util.Optional; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisClusterClient; + +/// Manages post-registration change number waiting period expiration data +public class ChangeNumberWaitingPeriodManager { + + private static final Logger LOGGER = LoggerFactory.getLogger(ChangeNumberWaitingPeriodManager.class); + + private final FaultTolerantRedisClusterClient redisCluster; + private final Duration waitingPeriod; + + public ChangeNumberWaitingPeriodManager(final FaultTolerantRedisClusterClient redisCluster, final Duration waitingPeriod) { + this.redisCluster = redisCluster; + this.waitingPeriod = waitingPeriod; + } + + /// Must be called when an account is created, including re-registration + @VisibleForTesting + public CompletableFuture handleAccountCreated(final UUID aci, final Instant created) { + return redisCluster.withCluster(conn -> conn.async().set(key(aci), "", SetArgs.Builder.exAt(created.plus(waitingPeriod)))) + .toCompletableFuture() + .thenApply(_ -> null); + } + + /// Returns the waiting period duration remaining, if any. If present, {@code duration} will always be positive. + Optional getWaitingPeriodRemaining(final UUID aci) { + final long ttlMillis = redisCluster.withCluster(conn -> conn.sync().ttl(key(aci))); + + if (ttlMillis == -1) { + // key present without TTL. This should never happen. + LOGGER.error("No expiration for {}", aci); + throw new RuntimeException("No expiration for key that must always have a expiration"); + } + + if (ttlMillis == -2) { + // key did not exist + return Optional.empty(); + } + + final Duration remaining = Duration.ofMillis(ttlMillis); + + return remaining.isPositive() ? Optional.of(remaining) : Optional.empty(); + } + + @VisibleForTesting + static String key(final UUID aci) { + return "changeNumberWaiting::{" + aci + "}"; + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java index 5a2b0b405..fed6d47c0 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java @@ -52,6 +52,7 @@ import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecoveryC import org.whispersystems.textsecuregcm.storage.AccountLockManager; import org.whispersystems.textsecuregcm.storage.Accounts; import org.whispersystems.textsecuregcm.storage.AccountsManager; +import org.whispersystems.textsecuregcm.storage.ChangeNumberWaitingPeriodManager; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.storage.DynamoDbRecoveryManager; import org.whispersystems.textsecuregcm.storage.IssuedReceiptsManager; @@ -281,9 +282,11 @@ public record CommandDependencies( configuration.getDynamoDbTables().getDeletedAccountsLock().getTableName()); RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager = new RegistrationRecoveryPasswordsManager(registrationRecoveryPasswords); + final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager = new ChangeNumberWaitingPeriodManager( + rateLimitersCluster, configuration.getChangeNumber().postRegistrationWaitingPeriod()); AccountsManager accountsManager = new AccountsManager(accounts, phoneNumberIdentifiers, cacheCluster, pubsubClient, accountLockManager, keys, messagesManager, profilesManager, - secureStorageClient, secureValueRecovery2Client, disconnectionRequestManager, + changeNumberWaitingPeriodManager, secureStorageClient, secureValueRecovery2Client, disconnectionRequestManager, registrationRecoveryPasswordsManager, accountLockExecutor, messagePollExecutor, retryExecutor, clock, configuration.getLinkDeviceSecretConfiguration().secret().value()); RateLimiters rateLimiters = RateLimiters.create(dynamicConfigurationManager, rateLimitersCluster, retryExecutor); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java index e6f528f84..26f2d9554 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java @@ -140,6 +140,10 @@ 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, @@ -149,6 +153,7 @@ public class AccountCreationDeletionIntegrationTest { keysManager, messagesManager, profilesManager, + changeNumberWaitingPeriodManager, secureStorageClient, svr2Client, disconnectionRequestManager, 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 74ee4ee29..e2cbe3723 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java @@ -138,6 +138,7 @@ class AccountsManagerChangeNumberIntegrationTest { keysManager, messagesManager, profilesManager, + mock(ChangeNumberWaitingPeriodManager.class), secureStorageClient, svr2Client, disconnectionRequestManager, 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 e479dfcc2..ef8d53f2b 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java @@ -119,6 +119,7 @@ class AccountsManagerConcurrentModificationIntegrationTest { mock(KeysManager.class), mock(MessagesManager.class), mock(ProfilesManager.class), + mock(ChangeNumberWaitingPeriodManager.class), mock(SecureStorageClient.class), mock(SecureValueRecoveryClient.class), mock(DisconnectionRequestManager.class), diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerDeviceTransferIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerDeviceTransferIntegrationTest.java index 8b3a6c4c8..79bb037ad 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerDeviceTransferIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerDeviceTransferIntegrationTest.java @@ -64,6 +64,7 @@ public class AccountsManagerDeviceTransferIntegrationTest { mock(KeysManager.class), mock(MessagesManager.class), mock(ProfilesManager.class), + mock(ChangeNumberWaitingPeriodManager.class), mock(SecureStorageClient.class), mock(SecureValueRecoveryClient.class), mock(DisconnectionRequestManager.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 e33bb75a4..be66e52ac 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -147,6 +147,7 @@ class AccountsManagerTest { messagesManager = mock(MessagesManager.class); profilesManager = mock(ProfilesManager.class); disconnectionRequestManager = mock(DisconnectionRequestManager.class); + final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager = mock(ChangeNumberWaitingPeriodManager.class); //noinspection unchecked asyncCommands = mock(RedisAsyncCommands.class); @@ -183,7 +184,7 @@ class AccountsManagerTest { when(phoneNumberIdentifiers.getPhoneNumberIdentifier(anyString())).thenAnswer((Answer>) invocation -> { final String number = invocation.getArgument(0, String.class); - return CompletableFuture.completedFuture(phoneNumberIdentifiersByE164.computeIfAbsent(number, n -> UUID.randomUUID())); + return CompletableFuture.completedFuture(phoneNumberIdentifiersByE164.computeIfAbsent(number, _ -> UUID.randomUUID())); }); final AccountLockManager accountLockManager = mock(AccountLockManager.class); @@ -214,6 +215,8 @@ 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, @@ -224,6 +227,7 @@ class AccountsManagerTest { keysManager, messagesManager, profilesManager, + changeNumberWaitingPeriodManager, storageClient, svr2Client, disconnectionRequestManager, 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 846a092de..8886510ac 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java @@ -134,6 +134,10 @@ 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, @@ -143,6 +147,7 @@ class AccountsManagerUsernameIntegrationTest { keysManager, messageManager, profileManager, + changeNumberWaitingPeriodManager, mock(SecureStorageClient.class), mock(SecureValueRecoveryClient.class), disconnectionRequestManager, @@ -150,7 +155,7 @@ class AccountsManagerUsernameIntegrationTest { Executors.newSingleThreadExecutor(), Executors.newSingleThreadScheduledExecutor(), Executors.newSingleThreadScheduledExecutor(), - mock(Clock.class), + Clock.systemUTC(), "link-device-secret".getBytes(StandardCharsets.UTF_8)); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AddRemoveDeviceIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AddRemoveDeviceIntegrationTest.java index beda11000..ef7eaa002 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AddRemoveDeviceIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AddRemoveDeviceIntegrationTest.java @@ -148,6 +148,7 @@ public class AddRemoveDeviceIntegrationTest { keysManager, messagesManager, profilesManager, + mock(ChangeNumberWaitingPeriodManager.class), secureStorageClient, svr2Client, mock(DisconnectionRequestManager.class), diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java index d0ccabc4c..ce169d2a2 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java @@ -13,6 +13,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; @@ -23,7 +24,6 @@ import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.container.ContainerRequestContext; import java.time.Duration; import java.time.Instant; -import java.time.temporal.ChronoUnit; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -66,6 +66,7 @@ public class ChangeNumberManagerTest { private AccountsManager accountsManager; private PhoneVerificationTokenManager phoneVerificationTokenManager; private RegistrationLockVerificationManager registrationLockVerificationManager; + private ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager; private RateLimiter rateLimiter; private ChangeNumberManager changeNumberManager; @@ -74,21 +75,14 @@ public class ChangeNumberManagerTest { private static final TestClock CLOCK = TestClock.pinned(Instant.now()); - private static final Duration POST_REGISTRATION_WAITING_PERIOD = Duration.ofHours(2); - private static final Device DEFAULT_PRIMARY_DEVICE; - static { - DEFAULT_PRIMARY_DEVICE = new Device(); - DEFAULT_PRIMARY_DEVICE.setId((byte) 1); - DEFAULT_PRIMARY_DEVICE.setCreated(CLOCK.instant().minus(POST_REGISTRATION_WAITING_PERIOD).minusSeconds(1).toEpochMilli()); - } - @BeforeEach void setUp() throws Exception { messageSender = mock(MessageSender.class); accountsManager = mock(AccountsManager.class); registrationLockVerificationManager = mock(RegistrationLockVerificationManager.class); - rateLimiter = mock(RateLimiter.class); phoneVerificationTokenManager = mock(PhoneVerificationTokenManager.class); + changeNumberWaitingPeriodManager = mock(ChangeNumberWaitingPeriodManager.class); + rateLimiter = mock(RateLimiter.class); when(phoneVerificationTokenManager.verify(any(), any(), any(), any())).thenAnswer(invocation -> { final byte[] sessionId = invocation.getArgument(2); @@ -98,12 +92,14 @@ public class ChangeNumberManagerTest { : PhoneVerificationRequest.VerificationType.RECOVERY_PASSWORD; }); + when(changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(any())).thenReturn(Optional.empty()); + final RateLimiters rateLimiters = mock(RateLimiters.class); when(rateLimiters.getRegistrationLimiter()).thenReturn(rateLimiter); changeNumberManager = new ChangeNumberManager(messageSender, accountsManager, phoneVerificationTokenManager, registrationLockVerificationManager, rateLimiters, - POST_REGISTRATION_WAITING_PERIOD, CLOCK); + changeNumberWaitingPeriodManager, CLOCK); updatedPhoneNumberIdentifiersByAccount = new HashMap<>(); @@ -163,7 +159,6 @@ public class ChangeNumberManagerTest { when(account.getIdentifier(IdentityType.ACI)).thenReturn(accountIdentifier); when(account.isIdentifiedBy(any())).thenReturn(false); when(account.isIdentifiedBy(new AciServiceIdentifier(accountIdentifier))).thenReturn(true); - when(account.getPrimaryDevice()).thenReturn(DEFAULT_PRIMARY_DEVICE); when(accountsManager.getAccountsForChangeNumber(eq(accountIdentifier), any())) .thenReturn(new Pair<>(account, Optional.empty())); @@ -206,7 +201,6 @@ public class ChangeNumberManagerTest { when(account.getDevice(primaryDeviceId)).thenReturn(Optional.of(primaryDevice)); when(account.getDevice(linkedDeviceId)).thenReturn(Optional.of(linkedDevice)); when(account.getDevices()).thenReturn(List.of(primaryDevice, linkedDevice)); - when(account.getPrimaryDevice()).thenReturn(DEFAULT_PRIMARY_DEVICE); final ECKeyPair pniIdentityKeyPair = ECKeyPair.generate(); final IdentityKey pniIdentityKey = new IdentityKey(pniIdentityKeyPair.getPublicKey()); @@ -289,7 +283,6 @@ public class ChangeNumberManagerTest { when(account.getIdentifier(IdentityType.ACI)).thenReturn(accountIdentifier); when(account.isIdentifiedBy(any())).thenReturn(false); when(account.isIdentifiedBy(new AciServiceIdentifier(accountIdentifier))).thenReturn(true); - when(account.getPrimaryDevice()).thenReturn(DEFAULT_PRIMARY_DEVICE); when(accountsManager.getAccountsForChangeNumber(eq(accountIdentifier), any())) .thenReturn(new Pair<>(account, Optional.empty())); @@ -329,7 +322,6 @@ public class ChangeNumberManagerTest { when(account.getIdentifier(IdentityType.ACI)).thenReturn(accountIdentifier); when(account.isIdentifiedBy(any())).thenReturn(false); when(account.isIdentifiedBy(new AciServiceIdentifier(accountIdentifier))).thenReturn(true); - when(account.getPrimaryDevice()).thenReturn(DEFAULT_PRIMARY_DEVICE); when(accountsManager.getAccountsForChangeNumber(eq(accountIdentifier), any())) .thenReturn(new Pair<>(account, Optional.empty())); @@ -378,7 +370,6 @@ public class ChangeNumberManagerTest { when(account.getIdentifier(IdentityType.ACI)).thenReturn(accountIdentifier); when(account.isIdentifiedBy(any())).thenReturn(false); when(account.isIdentifiedBy(new AciServiceIdentifier(accountIdentifier))).thenReturn(true); - when(account.getPrimaryDevice()).thenReturn(DEFAULT_PRIMARY_DEVICE); final Account existingAccount = mock(Account.class); when(existingAccount.getNumber()).thenReturn(targetNumber); @@ -407,7 +398,13 @@ public class ChangeNumberManagerTest { @ParameterizedTest @MethodSource - void testRecentRegistration(final boolean expectRateLimited, final boolean sameNumber, final Instant registrationInstant) throws Throwable { + void testRecentRegistration(final boolean expectRateLimited, final boolean sameNumber, final boolean waitingPeriodMet) throws Throwable { + + final Duration waitingPeriod = Duration.ofMinutes(30); + + reset(changeNumberWaitingPeriodManager); + when(changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(any())) + .thenReturn(waitingPeriodMet ? Optional.empty() : Optional.of(waitingPeriod)); final String originalNumber = PhoneNumberUtil.getInstance().format( PhoneNumberUtil.getInstance().getExampleNumber("DE"), PhoneNumberUtil.PhoneNumberFormat.E164); @@ -434,10 +431,6 @@ public class ChangeNumberManagerTest { when(account.isIdentifiedBy(any())).thenReturn(false); when(account.isIdentifiedBy(new AciServiceIdentifier(accountIdentifier))).thenReturn(true); - final Device primaryDevice = mock(Device.class); - when(account.getPrimaryDevice()).thenReturn(primaryDevice); - when(primaryDevice.getCreated()).thenReturn(registrationInstant.toEpochMilli()); - when(accountsManager.getAccountsForChangeNumber(eq(accountIdentifier), any())) .thenReturn(new Pair<>(account, Optional.empty())); @@ -454,8 +447,7 @@ public class ChangeNumberManagerTest { mock(ContainerRequestContext.class)); if (expectRateLimited) { final RateLimitExceededException e = assertThrows(RateLimitExceededException.class, changeNumberOperation); - - assertEquals(Duration.between(CLOCK.instant().minus(POST_REGISTRATION_WAITING_PERIOD), registrationInstant), e.getRetryDuration().orElseThrow()); + assertEquals(waitingPeriod, e.getRetryDuration().orElseThrow()); } else { changeNumberOperation.execute(); verify(accountsManager).changeNumber(accountIdentifier, targetNumber, pniIdentityKey, ecSignedPreKeys, kemLastResortPreKeys, Collections.emptyMap()); @@ -463,16 +455,11 @@ public class ChangeNumberManagerTest { } static Collection testRecentRegistration() { - // truncate to millis because that is the resolution for device.created - final Instant tooRecent = CLOCK.instant().minus(POST_REGISTRATION_WAITING_PERIOD).plusSeconds(1) - .truncatedTo(ChronoUnit.MILLIS); - final Instant outsideWaitingPeriod = CLOCK.instant().minus(POST_REGISTRATION_WAITING_PERIOD).minusSeconds(1) - .truncatedTo(ChronoUnit.MILLIS); return List.of( // expect exception, same number, registration instant - Arguments.argumentSet("waiting period elapsed", false, false, outsideWaitingPeriod), - Arguments.argumentSet("waiting period not elapsed", true, false, tooRecent), - Arguments.argumentSet("waiting period not elapsed; same number", false, true, tooRecent) + Arguments.argumentSet("waiting period elapsed", false, false, true), + Arguments.argumentSet("waiting period not elapsed", true, false, false), + Arguments.argumentSet("waiting period not elapsed; same number", false, true, false) ); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManagerTest.java new file mode 100644 index 000000000..8337421ff --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManagerTest.java @@ -0,0 +1,70 @@ +/* + * Copyright 2026 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.storage; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +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; +import org.whispersystems.textsecuregcm.redis.RedisClusterExtension; + +class ChangeNumberWaitingPeriodManagerTest { + + @RegisterExtension + static final RedisClusterExtension REDIS_CLUSTER_EXTENSION = RedisClusterExtension.builder().build(); + + private static final Duration WAITING_PERIOD = Duration.ofDays(7); + + private ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager; + + @BeforeEach + void setUp() { + changeNumberWaitingPeriodManager = new ChangeNumberWaitingPeriodManager( + REDIS_CLUSTER_EXTENSION.getRedisCluster(), WAITING_PERIOD); + } + + @Test + void testNewAccount() throws Exception { + final UUID aci = UUID.randomUUID(); + + assertTrue(changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(aci).isEmpty()); + + changeNumberWaitingPeriodManager.handleAccountCreated(aci, Instant.now()) + .get(5, TimeUnit.SECONDS); + + assertTrue(changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(aci).isPresent()); + } + + @Test + void testOldAccount() throws Exception { + final UUID aci = UUID.randomUUID(); + + changeNumberWaitingPeriodManager.handleAccountCreated(aci, + Instant.now().minus(WAITING_PERIOD).minus(Duration.ofHours(1))) + .get(5, TimeUnit.SECONDS); + + assertTrue(changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(aci).isEmpty()); + } + + @Test + void testNoTtlException() throws Exception { + final UUID aci = UUID.randomUUID(); + + changeNumberWaitingPeriodManager.handleAccountCreated(aci, Instant.now()).get(5, TimeUnit.SECONDS); + + REDIS_CLUSTER_EXTENSION.getRedisCluster().useCluster(conn -> + conn.sync().persist(ChangeNumberWaitingPeriodManager.key(aci))); + + assertThrows(RuntimeException.class, () -> changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(aci), + "This is an impossible scenario, and it should throw an exception"); + } +}