From fc421d3f219ead4cbb1d629c6ab38bff35ac1d7a Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Thu, 17 Jun 2021 17:15:24 -0400 Subject: [PATCH] Introduce a common interface for verification code stores. --- .../storage/PendingAccounts.java | 22 +++---- .../storage/PendingAccountsManager.java | 5 +- .../textsecuregcm/storage/PendingDevices.java | 13 ++-- .../storage/PendingDevicesManager.java | 4 +- .../storage/VerificationCodeStore.java | 18 ++++++ .../tests/storage/PendingAccountsTest.java | 64 +++++++++---------- .../tests/storage/PendingDevicesTest.java | 26 ++++---- 7 files changed, 85 insertions(+), 67 deletions(-) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStore.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccounts.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccounts.java index 83815a2b8..b616cc82d 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccounts.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccounts.java @@ -15,7 +15,7 @@ import org.whispersystems.textsecuregcm.auth.StoredVerificationCode; import org.whispersystems.textsecuregcm.storage.mappers.StoredVerificationCodeRowMapper; import org.whispersystems.textsecuregcm.util.Constants; -public class PendingAccounts { +public class PendingAccounts implements VerificationCodeStore { private final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); private final Timer insertTimer = metricRegistry.timer(name(PendingAccounts.class, "insert" )); @@ -30,29 +30,26 @@ public class PendingAccounts { this.database.getDatabase().registerRowMapper(new StoredVerificationCodeRowMapper()); } - @VisibleForTesting - public void insert (String number, String verificationCode, long timestamp, String pushCode) { - insert(number, verificationCode, timestamp, pushCode, null); - } - - public void insert(String number, String verificationCode, long timestamp, String pushCode, String twilioVerificationSid) { + @Override + public void insert(final String number, final StoredVerificationCode storedVerificationCode) { database.use(jdbi -> jdbi.useHandle(handle -> { try (Timer.Context ignored = insertTimer.time()) { handle.createUpdate("INSERT INTO pending_accounts (number, verification_code, timestamp, push_code, twilio_verification_sid) " + "VALUES (:number, :verification_code, :timestamp, :push_code, :twilio_verification_sid) " + "ON CONFLICT(number) DO UPDATE " + "SET verification_code = EXCLUDED.verification_code, timestamp = EXCLUDED.timestamp, push_code = EXCLUDED.push_code, twilio_verification_sid = EXCLUDED.twilio_verification_sid") - .bind("verification_code", verificationCode) - .bind("timestamp", timestamp) + .bind("verification_code", storedVerificationCode.getCode()) + .bind("timestamp", storedVerificationCode.getTimestamp()) .bind("number", number) - .bind("push_code", pushCode) - .bind("twilio_verification_sid", twilioVerificationSid) + .bind("push_code", storedVerificationCode.getPushCode()) + .bind("twilio_verification_sid", storedVerificationCode.getTwilioVerificationSid().orElse(null)) .execute(); } })); } - public Optional getCodeForNumber(String number) { + @Override + public Optional findForNumber(String number) { return database.with(jdbi ->jdbi.withHandle(handle -> { try (Timer.Context ignored = getCodeForNumberTimer.time()) { return handle.createQuery("SELECT verification_code, timestamp, push_code, twilio_verification_sid FROM pending_accounts WHERE number = :number") @@ -63,6 +60,7 @@ public class PendingAccounts { })); } + @Override public void remove(String number) { database.use(jdbi-> jdbi.useHandle(handle -> { try (Timer.Context ignored = removeTimer.time()) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccountsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccountsManager.java index 5ad2692e8..7238dca40 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingAccountsManager.java @@ -16,8 +16,7 @@ public class PendingAccountsManager { } public void store(String number, StoredVerificationCode code) { - pendingAccounts.insert(number, code.getCode(), code.getTimestamp(), code.getPushCode(), - code.getTwilioVerificationSid().orElse(null)); + pendingAccounts.insert(number, code); } public void remove(String number) { @@ -25,6 +24,6 @@ public class PendingAccountsManager { } public Optional getCodeForNumber(String number) { - return pendingAccounts.getCodeForNumber(number); + return pendingAccounts.findForNumber(number); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevices.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevices.java index 3aa455cf3..1ffd8b4e1 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevices.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevices.java @@ -14,7 +14,7 @@ import org.whispersystems.textsecuregcm.auth.StoredVerificationCode; import org.whispersystems.textsecuregcm.storage.mappers.StoredVerificationCodeRowMapper; import org.whispersystems.textsecuregcm.util.Constants; -public class PendingDevices { +public class PendingDevices implements VerificationCodeStore { private final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); private final Timer insertTimer = metricRegistry.timer(name(PendingDevices.class, "insert" )); @@ -28,20 +28,22 @@ public class PendingDevices { this.database.getDatabase().registerRowMapper(new StoredVerificationCodeRowMapper()); } - public void insert(String number, String verificationCode, long timestamp) { + @Override + public void insert(final String number, final StoredVerificationCode storedVerificationCode) { database.use(jdbi ->jdbi.useHandle(handle -> { try (Timer.Context timer = insertTimer.time()) { handle.createUpdate("WITH upsert AS (UPDATE pending_devices SET verification_code = :verification_code, timestamp = :timestamp WHERE number = :number RETURNING *) " + "INSERT INTO pending_devices (number, verification_code, timestamp) SELECT :number, :verification_code, :timestamp WHERE NOT EXISTS (SELECT * FROM upsert)") .bind("number", number) - .bind("verification_code", verificationCode) - .bind("timestamp", timestamp) + .bind("verification_code", storedVerificationCode.getCode()) + .bind("timestamp", storedVerificationCode.getTimestamp()) .execute(); } })); } - public Optional getCodeForNumber(String number) { + @Override + public Optional findForNumber(String number) { return database.with(jdbi -> jdbi.withHandle(handle -> { try (Timer.Context timer = getCodeForNumberTimer.time()) { return handle.createQuery("SELECT verification_code, timestamp, NULL as push_code, NULL as twilio_verification_sid FROM pending_devices WHERE number = :number") @@ -52,6 +54,7 @@ public class PendingDevices { })); } + @Override public void remove(String number) { database.use(jdbi -> jdbi.useHandle(handle -> { try (Timer.Context timer = removeTimer.time()) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevicesManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevicesManager.java index 377f1deeb..35ced2d55 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevicesManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PendingDevicesManager.java @@ -16,7 +16,7 @@ public class PendingDevicesManager { } public void store(String number, StoredVerificationCode code) { - pendingDevices.insert(number, code.getCode(), code.getTimestamp()); + pendingDevices.insert(number, code); } public void remove(String number) { @@ -24,6 +24,6 @@ public class PendingDevicesManager { } public Optional getCodeForNumber(String number) { - return pendingDevices.getCodeForNumber(number); + return pendingDevices.findForNumber(number); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStore.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStore.java new file mode 100644 index 000000000..38b3fd532 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationCodeStore.java @@ -0,0 +1,18 @@ +/* + * Copyright 2013-2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.storage; + +import org.whispersystems.textsecuregcm.auth.StoredVerificationCode; +import java.util.Optional; + +public interface VerificationCodeStore { + + void insert(String number, StoredVerificationCode verificationCode); + + Optional findForNumber(String number); + + void remove(String number); +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingAccountsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingAccountsTest.java index 2cf30ddcc..37f310772 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingAccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingAccountsTest.java @@ -37,7 +37,7 @@ class PendingAccountsTest { @Test void testStore() throws SQLException { - pendingAccounts.insert("+14151112222", "1234", 1111, null); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("1234", 1111, null, null)); PreparedStatement statement = db.getTestDatabase().getConnection().prepareStatement("SELECT * FROM pending_accounts WHERE number = ?"); statement.setString(1, "+14151112222"); @@ -57,7 +57,7 @@ class PendingAccountsTest { @Test void testStoreWithPushChallenge() throws SQLException { - pendingAccounts.insert("+14151112222", null, 1111, "112233"); + pendingAccounts.insert("+14151112222", new StoredVerificationCode(null, 1111, "112233", null)); PreparedStatement statement = db.getTestDatabase().getConnection().prepareStatement("SELECT * FROM pending_accounts WHERE number = ?"); statement.setString(1, "+14151112222"); @@ -77,7 +77,7 @@ class PendingAccountsTest { @Test void testStoreWithTwilioVerificationSid() throws SQLException { - pendingAccounts.insert("+14151112222", null, 1111, null, "id1"); + pendingAccounts.insert("+14151112222", new StoredVerificationCode(null, 1111, null, "id1")); PreparedStatement statement = db.getTestDatabase().getConnection() .prepareStatement("SELECT * FROM pending_accounts WHERE number = ?"); @@ -99,41 +99,41 @@ class PendingAccountsTest { @Test void testRetrieve() throws Exception { - pendingAccounts.insert("+14151112222", "4321", 2222, null); - pendingAccounts.insert("+14151113333", "1212", 5555, null); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4321", 2222, null, null)); + pendingAccounts.insert("+14151113333", new StoredVerificationCode("1212", 5555, null, null)); - Optional verificationCode = pendingAccounts.getCodeForNumber("+14151112222"); + Optional verificationCode = pendingAccounts.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("4321"); assertThat(verificationCode.get().getTimestamp()).isEqualTo(2222); - Optional missingCode = pendingAccounts.getCodeForNumber("+11111111111"); + Optional missingCode = pendingAccounts.findForNumber("+11111111111"); assertThat(missingCode.isPresent()).isFalse(); } @Test void testRetrieveWithPushChallenge() throws Exception { - pendingAccounts.insert("+14151112222", "4321", 2222, "bar"); - pendingAccounts.insert("+14151113333", "1212", 5555, "bang"); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4321", 2222, "bar", null)); + pendingAccounts.insert("+14151113333", new StoredVerificationCode("1212", 5555, "bang", null)); - Optional verificationCode = pendingAccounts.getCodeForNumber("+14151112222"); + Optional verificationCode = pendingAccounts.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("4321"); assertThat(verificationCode.get().getTimestamp()).isEqualTo(2222); assertThat(verificationCode.get().getPushCode()).isEqualTo("bar"); - Optional missingCode = pendingAccounts.getCodeForNumber("+11111111111"); + Optional missingCode = pendingAccounts.findForNumber("+11111111111"); assertThat(missingCode.isPresent()).isFalse(); } @Test void testRetrieveWithTwilioVerificationSid() throws Exception { - pendingAccounts.insert("+14151112222", "4321", 2222, "bar", "id1"); - pendingAccounts.insert("+14151113333", "1212", 5555, "bang", "id2"); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4321", 2222, "bar", "id1")); + pendingAccounts.insert("+14151113333", new StoredVerificationCode("1212", 5555, "bang", "id2")); - Optional verificationCode = pendingAccounts.getCodeForNumber("+14151112222"); + Optional verificationCode = pendingAccounts.findForNumber("+14151112222"); assertThat(verificationCode).isPresent(); assertThat(verificationCode.get().getCode()).isEqualTo("4321"); @@ -141,16 +141,16 @@ class PendingAccountsTest { assertThat(verificationCode.get().getPushCode()).isEqualTo("bar"); assertThat(verificationCode.get().getTwilioVerificationSid()).contains("id1"); - Optional missingCode = pendingAccounts.getCodeForNumber("+11111111111"); + Optional missingCode = pendingAccounts.findForNumber("+11111111111"); assertThat(missingCode).isNotPresent(); } @Test void testOverwrite() throws Exception { - pendingAccounts.insert("+14151112222", "4321", 2222, null); - pendingAccounts.insert("+14151112222", "4444", 3333, null); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4321", 2222, null, null)); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4444", 3333, null, null)); - Optional verificationCode = pendingAccounts.getCodeForNumber("+14151112222"); + Optional verificationCode = pendingAccounts.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("4444"); @@ -159,10 +159,10 @@ class PendingAccountsTest { @Test void testOverwriteWithPushToken() throws Exception { - pendingAccounts.insert("+14151112222", "4321", 2222, "bar"); - pendingAccounts.insert("+14151112222", "4444", 3333, "bang"); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4321", 2222, "bar", null)); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4444", 3333, "bang", null)); - Optional verificationCode = pendingAccounts.getCodeForNumber("+14151112222"); + Optional verificationCode = pendingAccounts.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("4444"); @@ -172,10 +172,10 @@ class PendingAccountsTest { @Test void testOverwriteWithTwilioVerificationSid() throws Exception { - pendingAccounts.insert("+14151112222", "4321", 2222, "bar", "id1"); - pendingAccounts.insert("+14151112222", "4444", 3333, "bang", "id2"); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4321", 2222, "bar", "id1")); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4444", 3333, "bang", "id2")); - Optional verificationCode = pendingAccounts.getCodeForNumber("+14151112222"); + Optional verificationCode = pendingAccounts.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("4444"); @@ -186,11 +186,11 @@ class PendingAccountsTest { @Test void testVacuum() { - pendingAccounts.insert("+14151112222", "4321", 2222, null); - pendingAccounts.insert("+14151112222", "4444", 3333, null); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4321", 2222, null, null)); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4444", 3333, null, null)); pendingAccounts.vacuum(); - Optional verificationCode = pendingAccounts.getCodeForNumber("+14151112222"); + Optional verificationCode = pendingAccounts.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("4444"); @@ -199,10 +199,10 @@ class PendingAccountsTest { @Test void testRemove() { - pendingAccounts.insert("+14151112222", "4321", 2222, "bar"); - pendingAccounts.insert("+14151113333", "1212", 5555, null); + pendingAccounts.insert("+14151112222", new StoredVerificationCode("4321", 2222, "bar", null)); + pendingAccounts.insert("+14151113333", new StoredVerificationCode("1212", 5555, null, null)); - Optional verificationCode = pendingAccounts.getCodeForNumber("+14151112222"); + Optional verificationCode = pendingAccounts.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("4321"); @@ -210,10 +210,10 @@ class PendingAccountsTest { pendingAccounts.remove("+14151112222"); - verificationCode = pendingAccounts.getCodeForNumber("+14151112222"); + verificationCode = pendingAccounts.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isFalse(); - verificationCode = pendingAccounts.getCodeForNumber("+14151113333"); + verificationCode = pendingAccounts.findForNumber("+14151113333"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("1212"); assertThat(verificationCode.get().getTimestamp()).isEqualTo(5555); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingDevicesTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingDevicesTest.java index 34047174c..c925f2412 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingDevicesTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PendingDevicesTest.java @@ -38,7 +38,7 @@ public class PendingDevicesTest { @Test public void testStore() throws SQLException { - pendingDevices.insert("+14151112222", "1234", 1111); + pendingDevices.insert("+14151112222", new StoredVerificationCode("1234", 1111, null, null)); PreparedStatement statement = db.getTestDatabase().getConnection().prepareStatement("SELECT * FROM pending_devices WHERE number = ?"); statement.setString(1, "+14151112222"); @@ -57,25 +57,25 @@ public class PendingDevicesTest { @Test public void testRetrieve() throws Exception { - pendingDevices.insert("+14151112222", "4321", 2222); - pendingDevices.insert("+14151113333", "1212", 5555); + pendingDevices.insert("+14151112222", new StoredVerificationCode("4321", 2222, null, null)); + pendingDevices.insert("+14151113333", new StoredVerificationCode("1212", 5555, null, null)); - Optional verificationCode = pendingDevices.getCodeForNumber("+14151112222"); + Optional verificationCode = pendingDevices.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("4321"); assertThat(verificationCode.get().getTimestamp()).isEqualTo(2222); - Optional missingCode = pendingDevices.getCodeForNumber("+11111111111"); + Optional missingCode = pendingDevices.findForNumber("+11111111111"); assertThat(missingCode.isPresent()).isFalse(); } @Test public void testOverwrite() throws Exception { - pendingDevices.insert("+14151112222", "4321", 2222); - pendingDevices.insert("+14151112222", "4444", 3333); + pendingDevices.insert("+14151112222", new StoredVerificationCode("4321", 2222, null, null)); + pendingDevices.insert("+14151112222", new StoredVerificationCode("4444", 3333, null, null)); - Optional verificationCode = pendingDevices.getCodeForNumber("+14151112222"); + Optional verificationCode = pendingDevices.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("4444"); @@ -84,10 +84,10 @@ public class PendingDevicesTest { @Test public void testRemove() { - pendingDevices.insert("+14151112222", "4321", 2222); - pendingDevices.insert("+14151113333", "1212", 5555); + pendingDevices.insert("+14151112222", new StoredVerificationCode("4321", 2222, null, null)); + pendingDevices.insert("+14151113333", new StoredVerificationCode("1212", 5555, null, null)); - Optional verificationCode = pendingDevices.getCodeForNumber("+14151112222"); + Optional verificationCode = pendingDevices.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("4321"); @@ -95,10 +95,10 @@ public class PendingDevicesTest { pendingDevices.remove("+14151112222"); - verificationCode = pendingDevices.getCodeForNumber("+14151112222"); + verificationCode = pendingDevices.findForNumber("+14151112222"); assertThat(verificationCode.isPresent()).isFalse(); - verificationCode = pendingDevices.getCodeForNumber("+14151113333"); + verificationCode = pendingDevices.findForNumber("+14151113333"); assertThat(verificationCode.isPresent()).isTrue(); assertThat(verificationCode.get().getCode()).isEqualTo("1212"); assertThat(verificationCode.get().getTimestamp()).isEqualTo(5555);