Remove from svrb on account deletion

This commit is contained in:
ravi-signal
2025-07-09 14:10:16 -05:00
parent 656b08f3b6
commit 58b9fa100d
13 changed files with 129 additions and 114 deletions

View File

@@ -18,9 +18,10 @@ import static org.whispersystems.textsecuregcm.util.MockUtils.randomSecretBytes;
import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
import java.security.cert.CertificateException;
import java.util.Arrays;
import java.util.List;
import java.util.UUID;
import java.util.concurrent.CompletionException;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
@@ -28,23 +29,26 @@ import java.util.concurrent.TimeUnit;
import org.apache.commons.lang3.RandomStringUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentials;
import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentialsGenerator;
import org.whispersystems.textsecuregcm.configuration.SecureValueRecoveryConfiguration;
import org.whispersystems.textsecuregcm.util.CompletableFutureTestUtil;
class SecureValueRecovery2ClientTest {
class SecureValueRecoveryClientTest {
private static final List<Integer> ALLOWED_ERRORS = Arrays.asList(567, 568);
private UUID accountUuid;
private ExternalServiceCredentialsGenerator credentialsGenerator;
private ExecutorService httpExecutor;
private ScheduledExecutorService retryExecutor;
private SecureValueRecovery2Client secureValueRecovery2Client;
private SecureValueRecoveryClient secureValueRecoveryClient;
@RegisterExtension
private final WireMockExtension wireMock = WireMockExtension.newInstance()
private static final WireMockExtension wireMock = WireMockExtension.newInstance()
.options(wireMockConfig().dynamicPort().dynamicHttpsPort())
.build();
@@ -107,8 +111,7 @@ class SecureValueRecovery2ClientTest {
"""),
null, null);
secureValueRecovery2Client = new SecureValueRecovery2Client(credentialsGenerator, httpExecutor, retryExecutor,
config);
secureValueRecoveryClient = new SecureValueRecoveryClient(credentialsGenerator, httpExecutor, retryExecutor, config, () -> ALLOWED_ERRORS);
}
@AfterEach
@@ -119,35 +122,31 @@ class SecureValueRecovery2ClientTest {
retryExecutor.awaitTermination(1, TimeUnit.SECONDS);
}
@Test
void deleteStoredData() {
@ParameterizedTest
@CsvSource({
"400, false",
"429, false",
"200, true",
"201, true",
"567, true",
"568, true",
})
void deleteStatus(int status, boolean shouldSucceed) {
final String username = RandomStringUtils.secure().nextAlphabetic(16);
final String password = RandomStringUtils.secure().nextAlphanumeric(32);
when(credentialsGenerator.generateForUuid(accountUuid)).thenReturn(
new ExternalServiceCredentials(username, password));
wireMock.stubFor(delete(urlEqualTo(SecureValueRecovery2Client.DELETE_PATH))
wireMock.stubFor(delete(urlEqualTo(SecureValueRecoveryClient.DELETE_PATH))
.withBasicAuth(username, password)
.willReturn(aResponse().withStatus(202)));
.willReturn(aResponse().withStatus(status)));
assertDoesNotThrow(() -> secureValueRecovery2Client.deleteBackups(accountUuid).join());
}
@Test
void deleteStoredDataFailure() {
final String username = RandomStringUtils.secure().nextAlphabetic(16);
final String password = RandomStringUtils.secure().nextAlphanumeric(32);
when(credentialsGenerator.generateForUuid(accountUuid)).thenReturn(
new ExternalServiceCredentials(username, password));
wireMock.stubFor(delete(urlEqualTo(SecureValueRecovery2Client.DELETE_PATH))
.withBasicAuth(username, password)
.willReturn(aResponse().withStatus(400)));
final CompletionException completionException = assertThrows(CompletionException.class,
() -> secureValueRecovery2Client.deleteBackups(accountUuid).join());
assertTrue(completionException.getCause() instanceof SecureValueRecoveryException);
final CompletableFuture<Void> deleteFuture = secureValueRecoveryClient.removeData(accountUuid);
if (shouldSucceed) {
assertDoesNotThrow(() -> deleteFuture.join());
} else {
CompletableFutureTestUtil.assertFailsWithCause(SecureValueRecoveryException.class, deleteFuture);
}
}
}

View File

@@ -49,7 +49,7 @@ import org.whispersystems.textsecuregcm.identity.IdentityType;
import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisClient;
import org.whispersystems.textsecuregcm.redis.RedisClusterExtension;
import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecovery2Client;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecoveryClient;
import org.whispersystems.textsecuregcm.tests.util.KeysHelper;
import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient;
@@ -134,8 +134,8 @@ public class AccountCreationDeletionIntegrationTest {
final SecureStorageClient secureStorageClient = mock(SecureStorageClient.class);
when(secureStorageClient.deleteStoredData(any())).thenReturn(CompletableFuture.completedFuture(null));
final SecureValueRecovery2Client svr2Client = mock(SecureValueRecovery2Client.class);
when(svr2Client.deleteBackups(any())).thenReturn(CompletableFuture.completedFuture(null));
final SecureValueRecoveryClient svr2Client = mock(SecureValueRecoveryClient.class);
when(svr2Client.removeData(any())).thenReturn(CompletableFuture.completedFuture(null));
final PhoneNumberIdentifiers phoneNumberIdentifiers =
new PhoneNumberIdentifiers(DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient(),
@@ -167,6 +167,7 @@ public class AccountCreationDeletionIntegrationTest {
profilesManager,
secureStorageClient,
svr2Client,
svr2Client,
disconnectionRequestManager,
registrationRecoveryPasswordsManager,
clientPublicKeysManager,

View File

@@ -41,7 +41,7 @@ import org.whispersystems.textsecuregcm.identity.IdentityType;
import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisClient;
import org.whispersystems.textsecuregcm.redis.RedisClusterExtension;
import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecovery2Client;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecoveryClient;
import org.whispersystems.textsecuregcm.storage.DynamoDbExtensionSchema.Tables;
import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
import org.whispersystems.textsecuregcm.tests.util.KeysHelper;
@@ -126,8 +126,8 @@ class AccountsManagerChangeNumberIntegrationTest {
final SecureStorageClient secureStorageClient = mock(SecureStorageClient.class);
when(secureStorageClient.deleteStoredData(any())).thenReturn(CompletableFuture.completedFuture(null));
final SecureValueRecovery2Client svr2Client = mock(SecureValueRecovery2Client.class);
when(svr2Client.deleteBackups(any())).thenReturn(CompletableFuture.completedFuture(null));
final SecureValueRecoveryClient svr2Client = mock(SecureValueRecoveryClient.class);
when(svr2Client.removeData(any())).thenReturn(CompletableFuture.completedFuture(null));
disconnectionRequestManager = mock(DisconnectionRequestManager.class);
@@ -157,6 +157,7 @@ class AccountsManagerChangeNumberIntegrationTest {
profilesManager,
secureStorageClient,
svr2Client,
svr2Client,
disconnectionRequestManager,
registrationRecoveryPasswordsManager,
clientPublicKeysManager,

View File

@@ -54,7 +54,7 @@ import org.whispersystems.textsecuregcm.entities.AccountAttributes;
import org.whispersystems.textsecuregcm.identity.IdentityType;
import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisClient;
import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecovery2Client;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecoveryClient;
import org.whispersystems.textsecuregcm.storage.DynamoDbExtensionSchema.Tables;
import org.whispersystems.textsecuregcm.tests.util.DevicesHelper;
import org.whispersystems.textsecuregcm.tests.util.JsonHelpers;
@@ -135,7 +135,8 @@ class AccountsManagerConcurrentModificationIntegrationTest {
mock(MessagesManager.class),
mock(ProfilesManager.class),
mock(SecureStorageClient.class),
mock(SecureValueRecovery2Client.class),
mock(SecureValueRecoveryClient.class),
mock(SecureValueRecoveryClient.class),
mock(DisconnectionRequestManager.class),
mock(RegistrationRecoveryPasswordsManager.class),
mock(ClientPublicKeysManager.class),

View File

@@ -20,7 +20,7 @@ import org.whispersystems.textsecuregcm.identity.IdentityType;
import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisClusterClient;
import org.whispersystems.textsecuregcm.redis.RedisServerExtension;
import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecovery2Client;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecoveryClient;
import org.whispersystems.textsecuregcm.util.TestRandomUtil;
import java.nio.charset.StandardCharsets;
@@ -68,7 +68,8 @@ public class AccountsManagerDeviceTransferIntegrationTest {
mock(MessagesManager.class),
mock(ProfilesManager.class),
mock(SecureStorageClient.class),
mock(SecureValueRecovery2Client.class),
mock(SecureValueRecoveryClient.class),
mock(SecureValueRecoveryClient.class),
mock(DisconnectionRequestManager.class),
mock(RegistrationRecoveryPasswordsManager.class),
mock(ClientPublicKeysManager.class),

View File

@@ -54,7 +54,6 @@ import java.util.Set;
import java.util.UUID;
import java.util.concurrent.Callable;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.Executor;
import java.util.concurrent.ScheduledExecutorService;
import java.util.function.Consumer;
@@ -90,8 +89,7 @@ import org.whispersystems.textsecuregcm.identity.PniServiceIdentifier;
import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisClient;
import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisClusterClient;
import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecovery2Client;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecoveryException;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecoveryClient;
import org.whispersystems.textsecuregcm.storage.AccountsManager.UsernameReservation;
import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
import org.whispersystems.textsecuregcm.tests.util.DevicesHelper;
@@ -133,7 +131,8 @@ class AccountsManagerTest {
private RedisAdvancedClusterCommands<String, String> clusterCommands;
private RedisAdvancedClusterAsyncCommands<String, String> asyncClusterCommands;
private AccountsManager accountsManager;
private SecureValueRecovery2Client svr2Client;
private SecureValueRecoveryClient svr2Client;
private SecureValueRecoveryClient svrbClient;
private DynamicConfiguration dynamicConfiguration;
private static final Answer<?> ACCOUNT_UPDATE_ANSWER = (answer) -> {
@@ -194,8 +193,11 @@ class AccountsManagerTest {
final SecureStorageClient storageClient = mock(SecureStorageClient.class);
when(storageClient.deleteStoredData(any())).thenReturn(CompletableFuture.completedFuture(null));
svr2Client = mock(SecureValueRecovery2Client.class);
when(svr2Client.deleteBackups(any())).thenReturn(CompletableFuture.completedFuture(null));
svr2Client = mock(SecureValueRecoveryClient.class);
when(svr2Client.removeData(any())).thenReturn(CompletableFuture.completedFuture(null));
svrbClient = mock(SecureValueRecoveryClient.class);
when(svrbClient.removeData(any())).thenReturn(CompletableFuture.completedFuture(null));
final PhoneNumberIdentifiers phoneNumberIdentifiers = mock(PhoneNumberIdentifiers.class);
phoneNumberIdentifiersByE164 = new HashMap<>();
@@ -209,7 +211,6 @@ class AccountsManagerTest {
mock(DynamicConfigurationManager.class);
when(dynamicConfigurationManager.getConfiguration()).thenReturn(dynamicConfiguration);
when(dynamicConfiguration.getSvrStatusCodesToIgnoreForAccountDeletion()).thenReturn(Collections.emptyList());
final AccountLockManager accountLockManager = mock(AccountLockManager.class);
@@ -256,6 +257,7 @@ class AccountsManagerTest {
profilesManager,
storageClient,
svr2Client,
svrbClient,
disconnectionRequestManager,
registrationRecoveryPasswordsManager,
clientPublicKeysManager,
@@ -266,31 +268,6 @@ class AccountsManagerTest {
dynamicConfigurationManager);
}
@ParameterizedTest
@MethodSource
void testDeleteWithSvr2ErrorStatusCodes(final String statusCode, final boolean expectError) throws InterruptedException {
when(svr2Client.deleteBackups(any())).thenReturn(
CompletableFuture.failedFuture(new SecureValueRecoveryException("Failed to delete backup", statusCode)));
when(dynamicConfiguration.getSvrStatusCodesToIgnoreForAccountDeletion()).thenReturn(List.of("500"));
final AccountAttributes attributes = new AccountAttributes(false, 1, 2, null, null, true, null);
final Account createdAccount = createAccount("+18005550123", attributes);
if (expectError) {
assertThrows(CompletionException.class, () -> accountsManager.delete(createdAccount, AccountsManager.DeletionReason.USER_REQUEST).toCompletableFuture().join());
} else {
assertDoesNotThrow(() -> accountsManager.delete(createdAccount, AccountsManager.DeletionReason.USER_REQUEST).toCompletableFuture().join());
}
}
private static Stream<Arguments> testDeleteWithSvr2ErrorStatusCodes() {
return Stream.of(
Arguments.of("500", false),
Arguments.of("429", true)
);
}
@Test
void testGetByServiceIdentifier() {
final UUID aci = UUID.randomUUID();

View File

@@ -42,7 +42,7 @@ import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager;
import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisClient;
import org.whispersystems.textsecuregcm.redis.RedisClusterExtension;
import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecovery2Client;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecoveryClient;
import org.whispersystems.textsecuregcm.storage.DynamoDbExtensionSchema.Tables;
import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
import org.whispersystems.textsecuregcm.util.AttributeValues;
@@ -160,7 +160,8 @@ class AccountsManagerUsernameIntegrationTest {
messageManager,
profileManager,
mock(SecureStorageClient.class),
mock(SecureValueRecovery2Client.class),
mock(SecureValueRecoveryClient.class),
mock(SecureValueRecoveryClient.class),
disconnectionRequestManager,
mock(RegistrationRecoveryPasswordsManager.class),
mock(ClientPublicKeysManager.class),

View File

@@ -41,7 +41,7 @@ import org.whispersystems.textsecuregcm.identity.IdentityType;
import org.whispersystems.textsecuregcm.redis.RedisClusterExtension;
import org.whispersystems.textsecuregcm.redis.RedisServerExtension;
import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecovery2Client;
import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecoveryClient;
import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
import org.whispersystems.textsecuregcm.tests.util.KeysHelper;
import org.whispersystems.textsecuregcm.util.Pair;
@@ -134,8 +134,8 @@ public class AddRemoveDeviceIntegrationTest {
final SecureStorageClient secureStorageClient = mock(SecureStorageClient.class);
when(secureStorageClient.deleteStoredData(any())).thenReturn(CompletableFuture.completedFuture(null));
final SecureValueRecovery2Client svr2Client = mock(SecureValueRecovery2Client.class);
when(svr2Client.deleteBackups(any())).thenReturn(CompletableFuture.completedFuture(null));
final SecureValueRecoveryClient svr2Client = mock(SecureValueRecoveryClient.class);
when(svr2Client.removeData(any())).thenReturn(CompletableFuture.completedFuture(null));
final PhoneNumberIdentifiers phoneNumberIdentifiers =
new PhoneNumberIdentifiers(DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient(),
@@ -169,6 +169,7 @@ public class AddRemoveDeviceIntegrationTest {
profilesManager,
secureStorageClient,
svr2Client,
svr2Client,
mock(DisconnectionRequestManager.class),
mock(RegistrationRecoveryPasswordsManager.class),
clientPublicKeysManager,