From 99b1f48e0e84f341ca103662816c15e172305582 Mon Sep 17 00:00:00 2001 From: Ehren Kret Date: Thu, 4 Nov 2021 14:33:18 -0500 Subject: [PATCH] Copy badges from existing account on re-reg --- .../textsecuregcm/WhisperServerService.java | 7 +++---- .../controllers/AccountController.java | 11 ++++++----- .../storage/AccountsManager.java | 12 ++++++++++-- .../workers/DeleteUserCommand.java | 4 +++- .../SetUserDiscoverabilityCommand.java | 4 +++- ...ntsManagerChangeNumberIntegrationTest.java | 19 +++++++++++-------- ...ConcurrentModificationIntegrationTest.java | 7 +++++-- .../controllers/AccountControllerTest.java | 9 ++++++--- .../tests/storage/AccountsManagerTest.java | 15 +++++++++------ 9 files changed, 56 insertions(+), 32 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 770c8907c..23ed1a5bb 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -53,7 +53,6 @@ import java.util.concurrent.TimeUnit; import javax.servlet.DispatcherType; import javax.servlet.FilterRegistration; import javax.servlet.ServletRegistration; -import javax.ws.rs.container.DynamicFeature; import org.eclipse.jetty.servlets.CrossOriginFilter; import org.glassfish.jersey.server.ServerProperties; import org.jdbi.v3.core.Jdbi; @@ -458,9 +457,9 @@ public class WhisperServerService extends Application existingAccount = accounts.get(number); + Optional existingAccount = accounts.get(number); - if (existingAccount.isPresent()) { - verifyRegistrationLock(existingAccount.get(), accountAttributes.getRegistrationLock()); - } + if (existingAccount.isPresent()) { + verifyRegistrationLock(existingAccount.get(), accountAttributes.getRegistrationLock()); + } if (availableForTransfer.orElse(false) && existingAccount.map(Account::isTransferSupported).orElse(false)) { throw new WebApplicationException(Response.status(409).build()); @@ -365,7 +365,8 @@ public class AccountController { rateLimiters.getVerifyLimiter().clear(number); - Account account = accounts.create(number, password, signalAgent, accountAttributes); + Account account = accounts.create(number, password, signalAgent, accountAttributes, + existingAccount.map(Account::getBadges).orElseGet(ArrayList::new)); { metricRegistry.meter(name(AccountController.class, "verify", Util.getCountryCode(number))).mark(); 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 826aca953..430bd062b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -17,6 +17,9 @@ import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; import io.micrometer.core.instrument.Metrics; import io.micrometer.core.instrument.Tags; import java.io.IOException; +import java.time.Clock; +import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.UUID; import java.util.concurrent.CompletableFuture; @@ -72,6 +75,7 @@ public class AccountsManager { private final SecureBackupClient secureBackupClient; private final ClientPresenceManager clientPresenceManager; private final ObjectMapper mapper; + private final Clock clock; public enum DeletionReason { ADMIN_DELETED("admin"), @@ -94,7 +98,8 @@ public class AccountsManager { final StoredVerificationCodeManager pendingAccounts, final SecureStorageClient secureStorageClient, final SecureBackupClient secureBackupClient, - final ClientPresenceManager clientPresenceManager) { + final ClientPresenceManager clientPresenceManager, + final Clock clock) { this.accounts = accounts; this.cacheCluster = cacheCluster; this.deletedAccountsManager = deletedAccountsManager; @@ -108,12 +113,14 @@ public class AccountsManager { this.secureBackupClient = secureBackupClient; this.clientPresenceManager = clientPresenceManager; this.mapper = SystemMapper.getMapper(); + this.clock = Objects.requireNonNull(clock); } public Account create(final String number, final String password, final String signalAgent, - final AccountAttributes accountAttributes) throws InterruptedException { + final AccountAttributes accountAttributes, + final List accountBadges) throws InterruptedException { try (Timer.Context ignored = createTimer.time()) { final Account account = new Account(); @@ -137,6 +144,7 @@ public class AccountsManager { account.setUnidentifiedAccessKey(accountAttributes.getUnidentifiedAccessKey()); account.setUnrestrictedUnidentifiedAccess(accountAttributes.isUnrestrictedUnidentifiedAccess()); account.setDiscoverableByPhoneNumber(accountAttributes.isDiscoverableByPhoneNumber()); + account.setBadges(clock, accountBadges); final UUID originalUuid = account.getUuid(); 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 f896212d9..b80b9a2aa 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java @@ -18,6 +18,7 @@ import io.dropwizard.jdbi3.JdbiFactory; import io.dropwizard.setup.Environment; import io.lettuce.core.resource.ClientResources; import io.micrometer.core.instrument.Metrics; +import java.time.Clock; import java.util.Optional; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -88,6 +89,7 @@ public class DeleteUserCommand extends EnvironmentCommand account = accountsManager.get(user); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/SetUserDiscoverabilityCommand.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/SetUserDiscoverabilityCommand.java index 03517adb2..96d88a299 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/SetUserDiscoverabilityCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/SetUserDiscoverabilityCommand.java @@ -18,6 +18,7 @@ import io.dropwizard.jdbi3.JdbiFactory; import io.dropwizard.setup.Environment; import io.lettuce.core.resource.ClientResources; import io.micrometer.core.instrument.Metrics; +import java.time.Clock; import java.util.Optional; import java.util.UUID; import java.util.concurrent.ExecutorService; @@ -91,6 +92,7 @@ public class SetUserDiscoverabilityCommand extends EnvironmentCommand maybeAccount; 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 1c6b5ee89..9e14e1087 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerChangeNumberIntegrationTest.java @@ -15,6 +15,8 @@ import static org.mockito.Mockito.when; import com.opentable.db.postgres.embedded.LiquibasePreparer; import com.opentable.db.postgres.junit5.EmbeddedPostgresExtension; import com.opentable.db.postgres.junit5.PreparedDbExtension; +import java.time.Clock; +import java.util.ArrayList; import java.util.Optional; import java.util.UUID; import java.util.concurrent.CompletableFuture; @@ -158,7 +160,8 @@ class AccountsManagerChangeNumberIntegrationTest { mock(StoredVerificationCodeManager.class), secureStorageClient, secureBackupClient, - clientPresenceManager); + clientPresenceManager, + mock(Clock.class)); } } @@ -167,7 +170,7 @@ class AccountsManagerChangeNumberIntegrationTest { final String originalNumber = "+18005551111"; final String secondNumber = "+18005552222"; - final Account account = accountsManager.create(originalNumber, "password", null, new AccountAttributes()); + final Account account = accountsManager.create(originalNumber, "password", null, new AccountAttributes(), new ArrayList<>()); final UUID originalUuid = account.getUuid(); accountsManager.changeNumber(account, secondNumber); @@ -188,7 +191,7 @@ class AccountsManagerChangeNumberIntegrationTest { final String originalNumber = "+18005551111"; final String secondNumber = "+18005552222"; - Account account = accountsManager.create(originalNumber, "password", null, new AccountAttributes()); + Account account = accountsManager.create(originalNumber, "password", null, new AccountAttributes(), new ArrayList<>()); final UUID originalUuid = account.getUuid(); account = accountsManager.changeNumber(account, secondNumber); @@ -210,10 +213,10 @@ class AccountsManagerChangeNumberIntegrationTest { final String originalNumber = "+18005551111"; final String secondNumber = "+18005552222"; - final Account account = accountsManager.create(originalNumber, "password", null, new AccountAttributes()); + final Account account = accountsManager.create(originalNumber, "password", null, new AccountAttributes(), new ArrayList<>()); final UUID originalUuid = account.getUuid(); - final Account existingAccount = accountsManager.create(secondNumber, "password", null, new AccountAttributes()); + final Account existingAccount = accountsManager.create(secondNumber, "password", null, new AccountAttributes(), new ArrayList<>()); final UUID existingAccountUuid = existingAccount.getUuid(); accountsManager.update(existingAccount, a -> a.addDevice(new Device(Device.MASTER_ID, "test", "token", "salt", null, null, null, true, 1, null, 0, 0, null, 0, new DeviceCapabilities()))); @@ -238,15 +241,15 @@ class AccountsManagerChangeNumberIntegrationTest { final String originalNumber = "+18005551111"; final String secondNumber = "+18005552222"; - final Account account = accountsManager.create(originalNumber, "password", null, new AccountAttributes()); + final Account account = accountsManager.create(originalNumber, "password", null, new AccountAttributes(), new ArrayList<>()); final UUID originalUuid = account.getUuid(); - final Account existingAccount = accountsManager.create(secondNumber, "password", null, new AccountAttributes()); + final Account existingAccount = accountsManager.create(secondNumber, "password", null, new AccountAttributes(), new ArrayList<>()); final UUID existingAccountUuid = existingAccount.getUuid(); accountsManager.changeNumber(account, secondNumber); - final Account reRegisteredAccount = accountsManager.create(originalNumber, "password", null, new AccountAttributes()); + final Account reRegisteredAccount = accountsManager.create(originalNumber, "password", null, new AccountAttributes(), new ArrayList<>()); assertEquals(existingAccountUuid, reRegisteredAccount.getUuid()); 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 9693dbf09..a653f11ce 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java @@ -18,7 +18,9 @@ import static org.mockito.Mockito.verify; import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; import java.io.IOException; +import java.time.Clock; import java.time.Instant; +import java.util.ArrayList; import java.util.Optional; import java.util.Random; import java.util.UUID; @@ -124,7 +126,8 @@ class AccountsManagerConcurrentModificationIntegrationTest { mock(StoredVerificationCodeManager.class), mock(SecureStorageClient.class), mock(SecureBackupClient.class), - mock(ClientPresenceManager.class) + mock(ClientPresenceManager.class), + mock(Clock.class) ); } } @@ -135,7 +138,7 @@ class AccountsManagerConcurrentModificationIntegrationTest { final UUID uuid; { final Account account = accountsManager.update( - accountsManager.create("+14155551212", "password", null, new AccountAttributes()), + accountsManager.create("+14155551212", "password", null, new AccountAttributes(), new ArrayList<>()), a -> { a.setUnidentifiedAccessKey(new byte[16]); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java index 2c992f31f..7f71bed3f 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -31,6 +31,7 @@ import java.security.SecureRandom; import java.time.Duration; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Optional; import java.util.Set; @@ -232,10 +233,11 @@ class AccountControllerTest { when(accountsManager.get(eq(SENDER_HAS_STORAGE))).thenReturn(Optional.of(senderHasStorage)); when(accountsManager.get(eq(SENDER_TRANSFER))).thenReturn(Optional.of(senderTransfer)); - when(accountsManager.create(any(), any(), any(), any())).thenAnswer((Answer) invocation -> { + when(accountsManager.create(any(), any(), any(), any(), any())).thenAnswer((Answer) invocation -> { final Account account = mock(Account.class); when(account.getUuid()).thenReturn(UUID.randomUUID()); when(account.getNumber()).thenReturn(invocation.getArgument(0, String.class)); + when(account.getBadges()).thenReturn(invocation.getArgument(4, List.class)); return account; }); @@ -1017,7 +1019,7 @@ class AccountControllerTest { .header("Authorization", AuthHelper.getProvisioningAuthHeader(SENDER, "bar")) .put(Entity.entity(new AccountAttributes(), MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); - verify(accountsManager).create(eq(SENDER), eq("bar"), any(), any()); + verify(accountsManager).create(eq(SENDER), eq("bar"), any(), any(), anyList()); if (enrolledInVerifyExperiment) { verify(smsSender).reportVerificationSucceeded("VerificationSid"); @@ -1095,7 +1097,8 @@ class AccountControllerTest { verify(pinLimiter).validate(eq(SENDER_REG_LOCK)); verify(accountsManager).create(eq(SENDER_REG_LOCK), eq("bar"), any(), argThat( - attributes -> Hex.toStringCondensed(registration_lock_key).equals(attributes.getRegistrationLock()))); + attributes -> Hex.toStringCondensed(registration_lock_key).equals(attributes.getRegistrationLock())), + argThat(List::isEmpty)); } @Test 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 a9627bbd3..b981466fc 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 @@ -25,6 +25,8 @@ import static org.mockito.Mockito.when; import io.lettuce.core.RedisException; import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; +import java.time.Clock; +import java.util.ArrayList; import java.util.HashSet; import java.util.Optional; import java.util.UUID; @@ -124,7 +126,8 @@ class AccountsManagerTest { mock(StoredVerificationCodeManager.class), storageClient, backupClient, - mock(ClientPresenceManager.class)); + mock(ClientPresenceManager.class), + mock(Clock.class)); } @Test @@ -340,7 +343,7 @@ class AccountsManagerTest { final String e164 = "+18005550123"; final AccountAttributes attributes = new AccountAttributes(false, 0, null, null, true, null); - accountsManager.create(e164, "password", null, attributes); + accountsManager.create(e164, "password", null, attributes, new ArrayList<>()); verify(accounts).create(argThat(account -> e164.equals(account.getNumber()))); verifyNoInteractions(keys); @@ -359,7 +362,7 @@ class AccountsManagerTest { final String e164 = "+18005550123"; final AccountAttributes attributes = new AccountAttributes(false, 0, null, null, true, null); - accountsManager.create(e164, "password", null, attributes); + accountsManager.create(e164, "password", null, attributes, new ArrayList<>()); verify(accounts).create( argThat(account -> e164.equals(account.getNumber()) && existingUuid.equals(account.getUuid()))); @@ -382,7 +385,7 @@ class AccountsManagerTest { final String e164 = "+18005550123"; final AccountAttributes attributes = new AccountAttributes(false, 0, null, null, true, null); - accountsManager.create(e164, "password", null, attributes); + accountsManager.create(e164, "password", null, attributes, new ArrayList<>()); verify(accounts).create( argThat(account -> e164.equals(account.getNumber()) && recentlyDeletedUuid.equals(account.getUuid()))); @@ -395,7 +398,7 @@ class AccountsManagerTest { @ValueSource(booleans = {true, false}) void testCreateWithDiscoverability(final boolean discoverable) throws InterruptedException { final AccountAttributes attributes = new AccountAttributes(false, 0, null, null, discoverable, null); - final Account account = accountsManager.create("+18005550123", "password", null, attributes); + final Account account = accountsManager.create("+18005550123", "password", null, attributes, new ArrayList<>()); assertEquals(discoverable, account.isDiscoverableByPhoneNumber()); @@ -410,7 +413,7 @@ class AccountsManagerTest { final AccountAttributes attributes = new AccountAttributes(false, 0, null, null, true, new DeviceCapabilities(false, false, false, hasStorage, false, false, false, false, false)); - final Account account = accountsManager.create("+18005550123", "password", null, attributes); + final Account account = accountsManager.create("+18005550123", "password", null, attributes, new ArrayList<>()); assertEquals(hasStorage, account.isStorageSupported()); }