From ff1a721d5bbd1d5fa0b202875aa245f431072aed Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Thu, 19 Nov 2020 12:24:38 -0500 Subject: [PATCH] Delete data in the storage service when deleting accounts. --- .../textsecuregcm/WhisperServerService.java | 21 +-- .../securestorage/SecureStorageClient.java | 2 +- .../securestorage/SecureStorageException.java | 13 ++ .../textsecuregcm/storage/AccountCleaner.java | 12 +- .../storage/AccountsManager.java | 29 ++-- .../workers/DeleteUserCommand.java | 8 +- .../SecureStorageClientTest.java | 5 +- .../tests/storage/AccountsManagerTest.java | 144 +++++++++--------- 8 files changed, 141 insertions(+), 93 deletions(-) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/securestorage/SecureStorageException.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 9389328f1..5c9f09eec 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -101,6 +101,7 @@ import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; import org.whispersystems.textsecuregcm.redis.ReplicatedJedisPool; import org.whispersystems.textsecuregcm.s3.PolicySigner; import org.whispersystems.textsecuregcm.s3.PostPolicyGenerator; +import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient; import org.whispersystems.textsecuregcm.sms.SmsSender; import org.whispersystems.textsecuregcm.sms.TwilioSmsSender; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; @@ -292,7 +293,17 @@ public class WhisperServerService extends Application deleteStorageServiceDataFuture = secureStorageClient.deleteStoredData(account.getUuid()); + usernamesManager.delete(account.getUuid()); directoryQueue.deleteAccount(account); directory.remove(account.getNumber()); profilesManager.deleteAll(account.getUuid()); keys.delete(account.getNumber()); messagesManager.clear(account.getNumber(), account.getUuid()); + + deleteStorageServiceDataFuture.join(); + redisDelete(account); databaseDelete(account); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java index 4fe7df015..b757b62c3 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java @@ -17,10 +17,12 @@ import org.jdbi.v3.core.Jdbi; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.WhisperServerConfiguration; +import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentialGenerator; import org.whispersystems.textsecuregcm.metrics.PushLatencyManager; import org.whispersystems.textsecuregcm.providers.RedisClientFactory; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; import org.whispersystems.textsecuregcm.redis.ReplicatedJedisPool; +import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.Accounts; @@ -88,6 +90,9 @@ public class DeleteUserCommand extends EnvironmentCommand account = accountsManager.get(user); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/securestorage/SecureStorageClientTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/securestorage/SecureStorageClientTest.java index e4ceb7001..d432156f7 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/securestorage/SecureStorageClientTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/securestorage/SecureStorageClientTest.java @@ -16,6 +16,7 @@ import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentials; import org.whispersystems.textsecuregcm.configuration.SecureStorageServiceConfiguration; import java.util.UUID; +import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -25,6 +26,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.delete; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -83,6 +85,7 @@ public class SecureStorageClientTest { .withBasicAuth(username, password) .willReturn(aResponse().withStatus(400))); - assertThrows(RuntimeException.class, () -> secureStorageClient.deleteStoredData(accountUuid).join()); + final CompletionException completionException = assertThrows(CompletionException.class, () -> secureStorageClient.deleteStoredData(accountUuid).join()); + assertTrue(completionException.getCause() instanceof SecureStorageException); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java index c259edbbc..0f32e9f36 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java @@ -8,8 +8,8 @@ package org.whispersystems.textsecuregcm.tests.storage; import io.lettuce.core.RedisException; import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; import org.junit.Test; -import org.whispersystems.textsecuregcm.entities.Profile; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; +import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.Accounts; @@ -40,22 +40,23 @@ public class AccountsManagerTest { @Test public void testGetAccountByNumberInCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - DirectoryManager directoryManager = mock(DirectoryManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - Keys keys = mock(Keys.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); + RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); + FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); + Accounts accounts = mock(Accounts.class); + DirectoryManager directoryManager = mock(DirectoryManager.class); + DirectoryQueue directoryQueue = mock(DirectoryQueue.class); + Keys keys = mock(Keys.class); + MessagesManager messagesManager = mock(MessagesManager.class); + UsernamesManager usernamesManager = mock(UsernamesManager.class); + ProfilesManager profilesManager = mock(ProfilesManager.class); + SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); UUID uuid = UUID.randomUUID(); when(commands.get(eq("AccountMap::+14152222222"))).thenReturn(uuid.toString()); when(commands.get(eq("Account3::" + uuid.toString()))).thenReturn("{\"number\": \"+14152222222\", \"name\": \"test\"}"); - AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager); + AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager, secureStorageClient); Optional account = accountsManager.get("+14152222222"); assertTrue(account.isPresent()); @@ -70,21 +71,22 @@ public class AccountsManagerTest { @Test public void testGetAccountByUuidInCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - DirectoryManager directoryManager = mock(DirectoryManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - Keys keys = mock(Keys.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); + RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); + FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); + Accounts accounts = mock(Accounts.class); + DirectoryManager directoryManager = mock(DirectoryManager.class); + DirectoryQueue directoryQueue = mock(DirectoryQueue.class); + Keys keys = mock(Keys.class); + MessagesManager messagesManager = mock(MessagesManager.class); + UsernamesManager usernamesManager = mock(UsernamesManager.class); + ProfilesManager profilesManager = mock(ProfilesManager.class); + SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); UUID uuid = UUID.randomUUID(); when(commands.get(eq("Account3::" + uuid.toString()))).thenReturn("{\"number\": \"+14152222222\", \"name\": \"test\"}"); - AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager); + AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager, secureStorageClient); Optional account = accountsManager.get(uuid); assertTrue(account.isPresent()); @@ -100,22 +102,23 @@ public class AccountsManagerTest { @Test public void testGetAccountByNumberNotInCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - DirectoryManager directoryManager = mock(DirectoryManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - Keys keys = mock(Keys.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - UUID uuid = UUID.randomUUID(); - Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); + RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); + FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); + Accounts accounts = mock(Accounts.class); + DirectoryManager directoryManager = mock(DirectoryManager.class); + DirectoryQueue directoryQueue = mock(DirectoryQueue.class); + Keys keys = mock(Keys.class); + MessagesManager messagesManager = mock(MessagesManager.class); + UsernamesManager usernamesManager = mock(UsernamesManager.class); + ProfilesManager profilesManager = mock(ProfilesManager.class); + UUID uuid = UUID.randomUUID(); + Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); + SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); when(commands.get(eq("AccountMap::+14152222222"))).thenReturn(null); when(accounts.get(eq("+14152222222"))).thenReturn(Optional.of(account)); - AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager); + AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager, secureStorageClient); Optional retrieved = accountsManager.get("+14152222222"); assertTrue(retrieved.isPresent()); @@ -132,22 +135,23 @@ public class AccountsManagerTest { @Test public void testGetAccountByUuidNotInCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - DirectoryManager directoryManager = mock(DirectoryManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - Keys keys = mock(Keys.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - UUID uuid = UUID.randomUUID(); - Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); + RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); + FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); + Accounts accounts = mock(Accounts.class); + DirectoryManager directoryManager = mock(DirectoryManager.class); + DirectoryQueue directoryQueue = mock(DirectoryQueue.class); + Keys keys = mock(Keys.class); + MessagesManager messagesManager = mock(MessagesManager.class); + UsernamesManager usernamesManager = mock(UsernamesManager.class); + ProfilesManager profilesManager = mock(ProfilesManager.class); + UUID uuid = UUID.randomUUID(); + Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); + SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); when(commands.get(eq("Account3::" + uuid))).thenReturn(null); when(accounts.get(eq(uuid))).thenReturn(Optional.of(account)); - AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager); + AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager, secureStorageClient); Optional retrieved = accountsManager.get(uuid); assertTrue(retrieved.isPresent()); @@ -164,22 +168,23 @@ public class AccountsManagerTest { @Test public void testGetAccountByNumberBrokenCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - DirectoryManager directoryManager = mock(DirectoryManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - Keys keys = mock(Keys.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - UUID uuid = UUID.randomUUID(); - Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); + RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); + FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); + Accounts accounts = mock(Accounts.class); + DirectoryManager directoryManager = mock(DirectoryManager.class); + DirectoryQueue directoryQueue = mock(DirectoryQueue.class); + Keys keys = mock(Keys.class); + MessagesManager messagesManager = mock(MessagesManager.class); + UsernamesManager usernamesManager = mock(UsernamesManager.class); + ProfilesManager profilesManager = mock(ProfilesManager.class); + UUID uuid = UUID.randomUUID(); + Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); + SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); when(commands.get(eq("AccountMap::+14152222222"))).thenThrow(new RedisException("Connection lost!")); when(accounts.get(eq("+14152222222"))).thenReturn(Optional.of(account)); - AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager); + AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager, secureStorageClient); Optional retrieved = accountsManager.get("+14152222222"); assertTrue(retrieved.isPresent()); @@ -196,22 +201,23 @@ public class AccountsManagerTest { @Test public void testGetAccountByUuidBrokenCache() { - RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); - FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - Accounts accounts = mock(Accounts.class); - DirectoryManager directoryManager = mock(DirectoryManager.class); - DirectoryQueue directoryQueue = mock(DirectoryQueue.class); - Keys keys = mock(Keys.class); - MessagesManager messagesManager = mock(MessagesManager.class); - UsernamesManager usernamesManager = mock(UsernamesManager.class); - ProfilesManager profilesManager = mock(ProfilesManager.class); - UUID uuid = UUID.randomUUID(); - Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); + RedisAdvancedClusterCommands commands = mock(RedisAdvancedClusterCommands.class); + FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); + Accounts accounts = mock(Accounts.class); + DirectoryManager directoryManager = mock(DirectoryManager.class); + DirectoryQueue directoryQueue = mock(DirectoryQueue.class); + Keys keys = mock(Keys.class); + MessagesManager messagesManager = mock(MessagesManager.class); + UsernamesManager usernamesManager = mock(UsernamesManager.class); + ProfilesManager profilesManager = mock(ProfilesManager.class); + UUID uuid = UUID.randomUUID(); + Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]); + SecureStorageClient secureStorageClient = mock(SecureStorageClient.class); when(commands.get(eq("Account3::" + uuid))).thenThrow(new RedisException("Connection lost!")); when(accounts.get(eq(uuid))).thenReturn(Optional.of(account)); - AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager); + AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheCluster, directoryQueue, keys, messagesManager, usernamesManager, profilesManager, secureStorageClient); Optional retrieved = accountsManager.get(uuid); assertTrue(retrieved.isPresent());