diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 3c427977f..4549d7d27 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -156,6 +156,7 @@ import org.whispersystems.textsecuregcm.storage.Accounts; import org.whispersystems.textsecuregcm.storage.AccountsDynamoDb; import org.whispersystems.textsecuregcm.storage.AccountsDynamoDbMigrator; import org.whispersystems.textsecuregcm.storage.AccountsManager; +import org.whispersystems.textsecuregcm.storage.ContactDiscoveryWriter; import org.whispersystems.textsecuregcm.storage.DeletedAccounts; import org.whispersystems.textsecuregcm.storage.DeletedAccountsDirectoryReconciler; import org.whispersystems.textsecuregcm.storage.DeletedAccountsManager; @@ -482,6 +483,7 @@ public class WhisperServerService extends Application getRelay() { requireNotStale(); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDb.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDb.java index fee196cca..abc8d790f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDb.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDb.java @@ -55,6 +55,8 @@ public class AccountsDynamoDb extends AbstractDynamoDbStore implements AccountSt static final String ATTR_ACCOUNT_DATA = "D"; // internal version for optimistic locking static final String ATTR_VERSION = "V"; + // canonically discoverable + static final String ATTR_CANONICALLY_DISCOVERABLE = "C"; private final DynamoDbClient client; private final DynamoDbAsyncClient asyncClient; @@ -160,7 +162,8 @@ public class AccountsDynamoDb extends AbstractDynamoDbStore implements AccountSt KEY_ACCOUNT_UUID, AttributeValues.fromUUID(uuid), ATTR_ACCOUNT_E164, AttributeValues.fromString(account.getNumber()), ATTR_ACCOUNT_DATA, AttributeValues.fromByteArray(SystemMapper.getMapper().writeValueAsBytes(account)), - ATTR_VERSION, AttributeValues.fromInt(account.getVersion()))) + ATTR_VERSION, AttributeValues.fromInt(account.getVersion()), + ATTR_CANONICALLY_DISCOVERABLE, AttributeValues.fromBool(account.shouldBeVisibleInDirectory()))) .build()) .build(); } @@ -193,13 +196,15 @@ public class AccountsDynamoDb extends AbstractDynamoDbStore implements AccountSt updateItemRequest = UpdateItemRequest.builder() .tableName(accountsTableName) .key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()))) - .updateExpression("SET #data = :data ADD #version :version_increment") + .updateExpression("SET #data = :data, #cds = :cds ADD #version :version_increment") .conditionExpression("attribute_exists(#number) AND #version = :version") .expressionAttributeNames(Map.of("#number", ATTR_ACCOUNT_E164, "#data", ATTR_ACCOUNT_DATA, + "#cds", ATTR_CANONICALLY_DISCOVERABLE, "#version", ATTR_VERSION)) .expressionAttributeValues(Map.of( ":data", AttributeValues.fromByteArray(SystemMapper.getMapper().writeValueAsBytes(account)), + ":cds", AttributeValues.fromBool(account.shouldBeVisibleInDirectory()), ":version", AttributeValues.fromInt(account.getVersion()), ":version_increment", AttributeValues.fromInt(1))) .returnValues(ReturnValue.UPDATED_NEW) @@ -428,6 +433,7 @@ public class AccountsDynamoDb extends AbstractDynamoDbStore implements AccountSt static Account fromItem(Map item) { if (!item.containsKey(ATTR_ACCOUNT_DATA) || !item.containsKey(ATTR_ACCOUNT_E164) || + // TODO: eventually require ATTR_CANONICALLY_DISCOVERABLE !item.containsKey(KEY_ACCOUNT_UUID)) { throw new RuntimeException("item missing values"); } @@ -436,6 +442,7 @@ public class AccountsDynamoDb extends AbstractDynamoDbStore implements AccountSt account.setNumber(item.get(ATTR_ACCOUNT_E164).s()); account.setUuid(UUIDUtil.fromByteBuffer(item.get(KEY_ACCOUNT_UUID).b().asByteBuffer())); account.setVersion(Integer.parseInt(item.get(ATTR_VERSION).n())); + account.setCanonicallyDiscoverable(Optional.ofNullable(item.get(ATTR_CANONICALLY_DISCOVERABLE)).map(av -> av.bool()).orElse(false)); return account; diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java new file mode 100644 index 000000000..2453ef16c --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java @@ -0,0 +1,34 @@ +package org.whispersystems.textsecuregcm.storage; + +import java.util.List; +import java.util.Optional; +import java.util.UUID; + +public class ContactDiscoveryWriter extends AccountDatabaseCrawlerListener { + + private final AccountStore accounts; + + public ContactDiscoveryWriter(final AccountStore accounts) { + this.accounts = accounts; + } + + @Override + public void onCrawlStart() { + // nothing + } + + @Override + public void onCrawlEnd(final Optional fromUuid) { + // nothing + } + + @Override + protected void onCrawlChunk(final Optional fromUuid, final List chunkAccounts) + throws AccountDatabaseCrawlerRestartException { + for (Account account : chunkAccounts) { + if (account.isCanonicallyDiscoverable() != account.shouldBeVisibleInDirectory()) { + accounts.update(account); + } + } + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/AttributeValues.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/AttributeValues.java index 5f6253c46..20464bd7c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/AttributeValues.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/AttributeValues.java @@ -23,6 +23,8 @@ public class AttributeValues { return AttributeValue.builder().n(Long.toString(value)).build(); } + public static AttributeValue fromBool(boolean value) { return AttributeValue.builder().bool(value).build(); } + public static AttributeValue fromInt(int value) { return AttributeValue.builder().n(Integer.toString(value)).build(); } @@ -43,6 +45,10 @@ public class AttributeValues { return AttributeValue.builder().b(value).build(); } + private static boolean toBool(AttributeValue av) { + return av.bool(); + } + private static int toInt(AttributeValue av) { return Integer.parseInt(av.n()); } @@ -67,6 +73,10 @@ public class AttributeValues { return Optional.ofNullable(item.get(key)); } + public static boolean getBool(Map item, String key, boolean defaultValue) { + return AttributeValues.get(item, key).map(AttributeValues::toBool).orElse(defaultValue); + } + public static int getInt(Map item, String key, int defaultValue) { return AttributeValues.get(item, key).map(AttributeValues::toInt).orElse(defaultValue); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDbTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDbTest.java index 68014a845..bb118e479 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDbTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDbTest.java @@ -143,11 +143,11 @@ class AccountsDynamoDbTest { boolean freshUser = accountsDynamoDb.create(account); assertThat(freshUser).isTrue(); - verifyStoredState("+14151112222", account.getUuid(), account); + verifyStoredState("+14151112222", account.getUuid(), account, true); freshUser = accountsDynamoDb.create(account); assertThat(freshUser).isTrue(); - verifyStoredState("+14151112222", account.getUuid(), account); + verifyStoredState("+14151112222", account.getUuid(), account, true); } @@ -161,7 +161,7 @@ class AccountsDynamoDbTest { accountsDynamoDb.create(account); - verifyStoredState("+14151112222", account.getUuid(), account); + verifyStoredState("+14151112222", account.getUuid(), account, true); } @Test @@ -210,7 +210,7 @@ class AccountsDynamoDbTest { accountsDynamoDb.create(account); - verifyStoredState("+14151112222", account.getUuid(), account); + verifyStoredState("+14151112222", account.getUuid(), account, true); account.setProfileName("name"); @@ -223,7 +223,7 @@ class AccountsDynamoDbTest { final boolean freshUser = accountsDynamoDb.create(account); assertThat(freshUser).isFalse(); - verifyStoredState("+14151112222", firstUuid, account); + verifyStoredState("+14151112222", firstUuid, account, true); device = generateDevice(1); Account invalidAccount = generateAccount("+14151113333", firstUuid, Collections.singleton(device)); @@ -250,7 +250,7 @@ class AccountsDynamoDbTest { retrieved = accountsDynamoDb.get(account.getUuid()); assertThat(retrieved.isPresent()).isTrue(); - verifyStoredState("+14151112222", account.getUuid(), account); + verifyStoredState("+14151112222", account.getUuid(), account, true); device = generateDevice(1); Account unknownAccount = generateAccount("+14151113333", UUID.randomUUID(), Collections.singleton(device)); @@ -263,7 +263,7 @@ class AccountsDynamoDbTest { assertThat(account.getVersion()).isEqualTo(2); - verifyStoredState("+14151112222", account.getUuid(), account); + verifyStoredState("+14151112222", account.getUuid(), account, true); account.setVersion(1); @@ -274,7 +274,7 @@ class AccountsDynamoDbTest { accountsDynamoDb.update(account); - verifyStoredState("+14151112222", account.getUuid(), account); + verifyStoredState("+14151112222", account.getUuid(), account, true); } @Test @@ -481,13 +481,13 @@ class AccountsDynamoDbTest { assertThat(migrated).isTrue(); - verifyStoredState("+14151112222", account.getUuid(), account); + verifyStoredState("+14151112222", account.getUuid(), account, true); migrated = accountsDynamoDb.migrate(account).get(); assertThat(migrated).isFalse(); - verifyStoredState("+14151112222", account.getUuid(), account); + verifyStoredState("+14151112222", account.getUuid(), account, true); UUID secondUuid = UUID.randomUUID(); @@ -497,7 +497,7 @@ class AccountsDynamoDbTest { migrated = accountsDynamoDb.migrate(accountRemigrationWithDifferentUuid).get(); assertThat(migrated).isFalse(); - verifyStoredState("+14151112222", firstUuid, account); + verifyStoredState("+14151112222", firstUuid, account, true); account.setVersion(account.getVersion() + 1); @@ -506,6 +506,39 @@ class AccountsDynamoDbTest { assertThat(migrated).isTrue(); } + @Test + void testCanonicallyDiscoverableSet() { + Device device = generateDevice(1); + UUID uuid = UUID.randomUUID(); + Account account = generateAccount("+14151112222", uuid, Collections.singleton(device)); + account.setDiscoverableByPhoneNumber(false); + accountsDynamoDb.create(account); + verifyStoredState("+14151112222", account.getUuid(), account, false); + account.setDiscoverableByPhoneNumber(true); + accountsDynamoDb.update(account); + verifyStoredState("+14151112222", account.getUuid(), account, true); + account.setDiscoverableByPhoneNumber(false); + accountsDynamoDb.update(account); + verifyStoredState("+14151112222", account.getUuid(), account, false); + } + + @Test + void testContactDiscoveryWriter() throws Exception { + Device device = generateDevice(1); + UUID uuid = UUID.randomUUID(); + Account account = generateAccount("+14151112222", uuid, Collections.singleton(device)); + accountsDynamoDb.create(account); + verifyStoredState("+14151112222", account.getUuid(), account, true); + ContactDiscoveryWriter writer = new ContactDiscoveryWriter(accountsDynamoDb); + account.setCanonicallyDiscoverable(false); + writer.onCrawlChunk(null, List.of(account)); + verifyStoredState("+14151112222", account.getUuid(), account, true); + account.setCanonicallyDiscoverable(true); + account.setDiscoverableByPhoneNumber(false); + writer.onCrawlChunk(null, List.of(account)); + verifyStoredState("+14151112222", account.getUuid(), account, false); + } + private Device generateDevice(long id) { Random random = new Random(System.currentTimeMillis()); SignedPreKey signedPreKey = new SignedPreKey(random.nextInt(), "testPublicKey-" + random.nextInt(), "testSignature-" + random.nextInt()); @@ -527,7 +560,7 @@ class AccountsDynamoDbTest { return new Account(number, uuid, devices, unidentifiedAccessKey); } - private void verifyStoredState(String number, UUID uuid, Account expecting) { + private void verifyStoredState(String number, UUID uuid, Account expecting, boolean canonicallyDiscoverable) { final DynamoDbClient db = dynamoDbExtension.getDynamoDbClient(); final GetItemResponse get = db.getItem(GetItemRequest.builder() @@ -543,6 +576,8 @@ class AccountsDynamoDbTest { assertThat(AttributeValues.getInt(get.item(), AccountsDynamoDb.ATTR_VERSION, -1)) .isEqualTo(expecting.getVersion()); + assertThat(AttributeValues.getBool(get.item(), AccountsDynamoDb.ATTR_CANONICALLY_DISCOVERABLE, !canonicallyDiscoverable)).isEqualTo(canonicallyDiscoverable); + Account result = AccountsDynamoDb.fromItem(get.item()); verifyStoredState(number, uuid, result, expecting); } else {