diff --git a/service/config/sample.yml b/service/config/sample.yml index 954b2a56b..acc2a2c2c 100644 --- a/service/config/sample.yml +++ b/service/config/sample.yml @@ -138,7 +138,6 @@ dynamoDbTables: tableName: Example_PushNotificationExperimentSamples redeemedReceipts: tableName: Example_RedeemedReceipts - expiration: P30D # Duration of time until rows expire registrationRecovery: tableName: Example_RegistrationRecovery expiration: P300D # Duration of time until rows expire diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 2b8801c79..a2769afdc 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -545,8 +545,7 @@ public class WhisperServerService extends Application attributes = updateItemResponse.attributes(); - final long ddbReceiptExpiration = Long.parseLong(attributes.get(KEY_RECEIPT_EXPIRATION).n()); - final long ddbReceiptLevel = Long.parseLong(attributes.get(KEY_RECEIPT_LEVEL).n()); - final UUID ddbAccountUuid = UUIDUtil.fromByteBuffer(attributes.get(KEY_ACCOUNT_UUID).b().asByteBuffer()); - return ddbReceiptExpiration == receiptExpiration && ddbReceiptLevel == receiptLevel && + final long ddbReceiptExpiration = Long.parseLong(attributes.get(ATTR_RECEIPT_EXPIRATION).n()); + final long ddbReceiptLevel = Long.parseLong(attributes.get(ATTR_RECEIPT_LEVEL).n()); + final UUID ddbAccountUuid = UUIDUtil.fromByteBuffer(attributes.get(ATTR_ACCOUNT_UUID).b().asByteBuffer()); + return ddbReceiptExpiration == receiptExpiration.getEpochSecond() && ddbReceiptLevel == receiptLevel && Objects.equals(ddbAccountUuid, accountUuid); } @@ -119,18 +119,16 @@ public class RedeemedReceiptsManager { .tableName(table) .item(Map.of( KEY_SERIAL, AttributeValues.b(receiptSerial.serialize()), - // NOTE: this differs from the expiration for other receipt types (see RedeemedReceiptsManager#put) which - // sets the expiration to now + a static expiration time. The TTL handling needs to be unified in the future. - KEY_TTL, AttributeValues.n(receiptExpiration.getEpochSecond()), - KEY_RECEIPT_EXPIRATION, AttributeValues.n(receiptExpiration.getEpochSecond()), - KEY_RECEIPT_LEVEL, AttributeValues.n(receiptLevel), - KEY_ACCOUNT_UUID, AttributeValues.b(accountUuid), - KEY_REDEMPTION_TIME, AttributeValues.n(clock.instant().getEpochSecond()))) + ATTR_TTL, rowTtl(receiptExpiration), + ATTR_RECEIPT_EXPIRATION, AttributeValues.n(receiptExpiration.getEpochSecond()), + ATTR_RECEIPT_LEVEL, AttributeValues.n(receiptLevel), + ATTR_ACCOUNT_UUID, AttributeValues.b(accountUuid), + ATTR_REDEMPTION_TIME, AttributeValues.n(clock.instant().getEpochSecond()))) .conditionExpression("attribute_not_exists(#serial) OR (#account_uuid = :account_uuid AND #receipt_level = :receipt_level)") .expressionAttributeNames(Map.of( "#serial", KEY_SERIAL, - "#account_uuid", KEY_ACCOUNT_UUID, - "#receipt_level", KEY_RECEIPT_LEVEL)) + "#account_uuid", ATTR_ACCOUNT_UUID, + "#receipt_level", ATTR_RECEIPT_LEVEL)) .expressionAttributeValues(Map.of( ":account_uuid", AttributeValues.b(accountUuid), ":receipt_level", AttributeValues.n(receiptLevel) @@ -139,4 +137,8 @@ public class RedeemedReceiptsManager { .build()) .build(); } + + private static AttributeValue rowTtl(final Instant receiptExpiration) { + return AttributeValues.n(receiptExpiration.plus(TTL_PADDING).getEpochSecond()); + } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java index c62e6658e..7d44c7e3c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java @@ -263,8 +263,7 @@ public record CommandDependencies( RedeemedReceiptsManager redeemedReceiptsManager = new RedeemedReceiptsManager(clock, configuration.getDynamoDbTables().getRedeemedReceipts().getTableName(), - dynamoDbClient, - configuration.getDynamoDbTables().getRedeemedReceipts().getExpiration()); + dynamoDbClient); Accounts accounts = new Accounts( clock, diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/backup/BackupAuthManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/backup/BackupAuthManagerTest.java index 5b02c62ea..fd0a5ee63 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/backup/BackupAuthManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/backup/BackupAuthManagerTest.java @@ -340,7 +340,7 @@ public class BackupAuthManagerTest { .build(); clock.pin(Instant.EPOCH.plus(Duration.ofDays(1))); when(accountsManager.update(any(Account.class), any())).thenReturn(account); - when(redeemedReceiptsManager.put(any(), eq(expirationTime.getEpochSecond()), eq(201L), eq(aci))) + when(redeemedReceiptsManager.put(any(), eq(expirationTime), eq(201L), eq(aci))) .thenReturn(true); authManager.redeemReceipt(account, receiptPresentation(201, expirationTime)); verify(accountsManager, times(1)).update(any(Account.class), any()); @@ -353,7 +353,7 @@ public class BackupAuthManagerTest { final Account account = new MockAccountBuilder().mediaCredential(Optional.empty()).build(); clock.pin(Instant.EPOCH.plus(Duration.ofDays(1))); - when(redeemedReceiptsManager.put(any(), eq(expirationTime.getEpochSecond()), eq(201L), eq(aci))) + when(redeemedReceiptsManager.put(any(), eq(expirationTime), eq(201L), eq(aci))) .thenReturn(true); assertThatExceptionOfType(BackupMissingIdCommitmentException.class) .isThrownBy(() -> authManager.redeemReceipt(account, receiptPresentation(201, expirationTime))); @@ -374,7 +374,7 @@ public class BackupAuthManagerTest { clock.pin(Instant.EPOCH.plus(Duration.ofDays(1))); when(accountsManager.update(any(Account.class), any())).thenReturn(account); - when(redeemedReceiptsManager.put(any(), eq(newExpirationTime.getEpochSecond()), eq(201L), eq(aci))) + when(redeemedReceiptsManager.put(any(), eq(newExpirationTime), eq(201L), eq(aci))) .thenReturn(true); authManager.redeemReceipt(account, receiptPresentation(201, newExpirationTime)); @@ -429,7 +429,7 @@ public class BackupAuthManagerTest { clock.pin(Instant.EPOCH.plus(Duration.ofDays(1))); when(accountsManager.update(any(Account.class), any())).thenReturn(account); - when(redeemedReceiptsManager.put(any(), eq(expirationTime.getEpochSecond()), eq(201L), eq(aci))) + when(redeemedReceiptsManager.put(any(), eq(expirationTime), eq(201L), eq(aci))) .thenReturn(false); assertThatExceptionOfType(BackupBadReceiptException.class) diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DonationControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DonationControllerTest.java index 0e85a7538..b7962d0f5 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DonationControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DonationControllerTest.java @@ -138,7 +138,7 @@ class DonationControllerTest { when(receiptCredentialPresentation.getReceiptLevel()).thenReturn(receiptLevel); final long receiptExpiration = nowEpochSeconds + 86400 * 30; when(receiptCredentialPresentation.getReceiptExpirationTime()).thenReturn(receiptExpiration); - when(redeemedReceiptsManager.put(same(receiptSerial), eq(receiptExpiration), eq(receiptLevel), eq(AuthHelper.VALID_UUID))).thenReturn(true); + when(redeemedReceiptsManager.put(same(receiptSerial), eq(Instant.ofEpochSecond(receiptExpiration)), eq(receiptLevel), eq(AuthHelper.VALID_UUID))).thenReturn(true); when(accountsManager.getByAccountIdentifier(eq(AuthHelper.VALID_UUID))) .thenReturn(Optional.of(AuthHelper.VALID_ACCOUNT)); @@ -163,7 +163,7 @@ class DonationControllerTest { when(receiptCredentialPresentation.getReceiptLevel()).thenReturn(receiptLevel); final long receiptExpiration = nowEpochSeconds + 86400 * 30; when(receiptCredentialPresentation.getReceiptExpirationTime()).thenReturn(receiptExpiration); - when(redeemedReceiptsManager.put(same(receiptSerial), eq(receiptExpiration), eq(receiptLevel), eq(AuthHelper.VALID_UUID))).thenReturn(false); + when(redeemedReceiptsManager.put(same(receiptSerial), eq(Instant.ofEpochSecond(receiptExpiration)), eq(receiptLevel), eq(AuthHelper.VALID_UUID))).thenReturn(false); when(accountsManager.getByAccountIdentifier(eq(AuthHelper.VALID_UUID))) .thenReturn(Optional.of(AuthHelper.VALID_ACCOUNT)); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/DonationsGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/DonationsGrpcServiceTest.java index bba0c8029..5efd83295 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/DonationsGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/DonationsGrpcServiceTest.java @@ -84,7 +84,7 @@ class DonationsGrpcServiceTest extends SimpleBaseGrpcTest readAccount(final UUID uuid) { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AddRemoveDeviceIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AddRemoveDeviceIntegrationTest.java index f5670998d..c4952d664 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AddRemoveDeviceIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AddRemoveDeviceIntegrationTest.java @@ -102,7 +102,7 @@ public class AddRemoveDeviceIntegrationTest { DYNAMO_DB_EXTENSION.getDynamoDbClient(), DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient(), new RedeemedReceiptsManager(clock, DynamoDbExtensionSchema.Tables.REDEEMED_RECEIPTS.tableName(), - DYNAMO_DB_EXTENSION.getDynamoDbClient(), Duration.ofDays(30)), + DYNAMO_DB_EXTENSION.getDynamoDbClient()), DynamoDbExtensionSchema.Tables.ACCOUNTS.tableName(), DynamoDbExtensionSchema.Tables.NUMBERS.tableName(), DynamoDbExtensionSchema.Tables.PNI_ASSIGNMENTS.tableName(), diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/RedeemedReceiptsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/RedeemedReceiptsManagerTest.java index b2d5c3dc3..d7f044e38 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/RedeemedReceiptsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/RedeemedReceiptsManagerTest.java @@ -6,20 +6,29 @@ package org.whispersystems.textsecuregcm.storage; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.time.Clock; -import java.time.Duration; import java.time.Instant; -import java.util.concurrent.ExecutionException; +import java.util.List; +import java.util.Map; +import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.signal.libsignal.zkgroup.InvalidInputException; import org.signal.libsignal.zkgroup.receipts.ReceiptSerial; import org.whispersystems.textsecuregcm.storage.DynamoDbExtensionSchema.Tables; -import org.whispersystems.textsecuregcm.tests.util.AuthHelper; +import org.whispersystems.textsecuregcm.util.AttributeValues; import org.whispersystems.textsecuregcm.util.TestClock; import org.whispersystems.textsecuregcm.util.TestRandomUtil; +import software.amazon.awssdk.services.dynamodb.model.CancellationReason; +import software.amazon.awssdk.services.dynamodb.model.GetItemRequest; +import software.amazon.awssdk.services.dynamodb.model.GetItemResponse; +import software.amazon.awssdk.services.dynamodb.model.TransactWriteItem; +import software.amazon.awssdk.services.dynamodb.model.TransactWriteItemsRequest; +import software.amazon.awssdk.services.dynamodb.model.TransactionCanceledException; class RedeemedReceiptsManagerTest { @@ -38,30 +47,95 @@ class RedeemedReceiptsManagerTest { redeemedReceiptsManager = new RedeemedReceiptsManager( clock, Tables.REDEEMED_RECEIPTS.tableName(), - DYNAMO_DB_EXTENSION.getDynamoDbClient(), - Duration.ofDays(90)); + DYNAMO_DB_EXTENSION.getDynamoDbClient()); } @Test - void testPut() throws ExecutionException, InterruptedException { - final long receiptExpiration = 42; + void testPut() { + final Instant receiptExpiration = Instant.ofEpochSecond(42); final long receiptLevel = 3; + final UUID uuid1 = UUID.randomUUID(); boolean put; // initial insert should return true - put = redeemedReceiptsManager.put(receiptSerial, receiptExpiration, receiptLevel, AuthHelper.VALID_UUID); + put = redeemedReceiptsManager.put(receiptSerial, receiptExpiration, receiptLevel, uuid1); assertThat(put).isTrue(); // subsequent attempted inserts with modified parameters should return false - put = redeemedReceiptsManager.put(receiptSerial, receiptExpiration + 1, receiptLevel, AuthHelper.VALID_UUID); + put = redeemedReceiptsManager.put(receiptSerial, receiptExpiration.plusSeconds(1), receiptLevel, uuid1); assertThat(put).isFalse(); - put = redeemedReceiptsManager.put(receiptSerial, receiptExpiration, receiptLevel + 1, AuthHelper.VALID_UUID); + put = redeemedReceiptsManager.put(receiptSerial, receiptExpiration, receiptLevel + 1, uuid1); assertThat(put).isFalse(); - put = redeemedReceiptsManager.put(receiptSerial, receiptExpiration, receiptLevel, AuthHelper.VALID_UUID_TWO); + + final UUID uuid2 = UUID.randomUUID(); + put = redeemedReceiptsManager.put(receiptSerial, receiptExpiration, receiptLevel, uuid2); assertThat(put).isFalse(); // repeated insert attempt of the original parameters should return true - put = redeemedReceiptsManager.put(receiptSerial, receiptExpiration, receiptLevel, AuthHelper.VALID_UUID); + put = redeemedReceiptsManager.put(receiptSerial, receiptExpiration, receiptLevel, uuid1); assertThat(put).isTrue(); + + // verify that the TTL is receipt expiration + padding + final GetItemResponse response = DYNAMO_DB_EXTENSION.getDynamoDbClient().getItem(GetItemRequest.builder() + .tableName(Tables.REDEEMED_RECEIPTS.tableName()) + .key(Map.of(RedeemedReceiptsManager.KEY_SERIAL, AttributeValues.b(receiptSerial.serialize()))) + .build()); + assertThat(Long.parseLong(response.item().get(RedeemedReceiptsManager.ATTR_TTL).n())).isEqualTo( + receiptExpiration.plus(RedeemedReceiptsManager.TTL_PADDING).getEpochSecond()); + } + + @Test + void testBuildTransactWriteItemForReceipt() { + final Instant receiptExpiration = Instant.ofEpochSecond(42); + final long receiptLevel1 = 3; + final UUID uuid1 = UUID.randomUUID(); + + final TransactWriteItem writeItem = redeemedReceiptsManager.buildTransactWriteItemForReceipt(receiptSerial, + receiptExpiration, receiptLevel1, uuid1); + + assertThatCode(() -> DYNAMO_DB_EXTENSION.getDynamoDbClient().transactWriteItems(TransactWriteItemsRequest.builder() + .transactItems(writeItem) + .build())).doesNotThrowAnyException(); + + // A subsequent write with the same parameters should be idempotent + assertThatCode(() -> DYNAMO_DB_EXTENSION.getDynamoDbClient().transactWriteItems(TransactWriteItemsRequest.builder() + .transactItems(writeItem) + .build())).doesNotThrowAnyException(); + + // An attempt to insert the same receipt with a different UUID should fail + final UUID uuid2 = UUID.randomUUID(); + final TransactWriteItem writeItem2 = redeemedReceiptsManager.buildTransactWriteItemForReceipt(receiptSerial, + receiptExpiration, receiptLevel1, uuid2); + assertThatThrownBy( + () -> DYNAMO_DB_EXTENSION.getDynamoDbClient().transactWriteItems(TransactWriteItemsRequest.builder() + .transactItems(writeItem2) + .build())) + .isInstanceOfSatisfying(TransactionCanceledException.class, e -> { + final List cancellationReasons = e.cancellationReasons(); + assertThat(cancellationReasons).hasSize(1); + assertThat(cancellationReasons.getFirst().code()).isEqualTo("ConditionalCheckFailed"); + }); + + // An attempt to insert the same receipt with the original UUID but a different receipt level should fail + final long receiptLevel2 = 4; + final TransactWriteItem writeItem3 = redeemedReceiptsManager.buildTransactWriteItemForReceipt(receiptSerial, + receiptExpiration, receiptLevel2, uuid1); + assertThatThrownBy( + () -> DYNAMO_DB_EXTENSION.getDynamoDbClient().transactWriteItems(TransactWriteItemsRequest.builder() + .transactItems(writeItem3) + .build())) + .isInstanceOfSatisfying(TransactionCanceledException.class, e -> { + final List cancellationReasons = e.cancellationReasons(); + assertThat(cancellationReasons).hasSize(1); + assertThat(cancellationReasons.getFirst().code()).isEqualTo("ConditionalCheckFailed"); + }); + + // verify that the TTL is receipt expiration + padding + final GetItemResponse response = DYNAMO_DB_EXTENSION.getDynamoDbClient().getItem(GetItemRequest.builder() + .tableName(Tables.REDEEMED_RECEIPTS.tableName()) + .key(Map.of(RedeemedReceiptsManager.KEY_SERIAL, AttributeValues.b(receiptSerial.serialize()))) + .build()); + assertThat(Long.parseLong(response.item().get(RedeemedReceiptsManager.ATTR_TTL).n())).isEqualTo( + receiptExpiration.plus(RedeemedReceiptsManager.TTL_PADDING).getEpochSecond()); } } diff --git a/service/src/test/resources/config/test.yml b/service/src/test/resources/config/test.yml index 4b17c2e7b..caaedeb14 100644 --- a/service/src/test/resources/config/test.yml +++ b/service/src/test/resources/config/test.yml @@ -144,7 +144,6 @@ dynamoDbTables: tableName: Example_PushNotificationExperimentSamples redeemedReceipts: tableName: redeemed_receipts_test - expiration: P30D # Duration of time until rows expire registrationRecovery: tableName: registration_recovery_passwords_test expiration: P300D # Duration of time until rows expire