Clear username links in the same transaction when clearing username hashes

This commit is contained in:
Jon Chambers 2023-10-18 12:49:32 -04:00 committed by Jon Chambers
parent ac0c8b1e9a
commit 6441d5838d
3 changed files with 14 additions and 3 deletions

View File

@ -274,7 +274,6 @@ public class AccountController {
@ApiResponse(responseCode = "204", description = "Username successfully deleted.", useReturnTypeSchema = true) @ApiResponse(responseCode = "204", description = "Username successfully deleted.", useReturnTypeSchema = true)
@ApiResponse(responseCode = "401", description = "Account authentication check failed.") @ApiResponse(responseCode = "401", description = "Account authentication check failed.")
public CompletableFuture<Void> deleteUsernameHash(@Auth final AuthenticatedAccount auth) { public CompletableFuture<Void> deleteUsernameHash(@Auth final AuthenticatedAccount auth) {
clearUsernameLink(auth.getAccount());
return accounts.clearUsernameHash(auth.getAccount()) return accounts.clearUsernameHash(auth.getAccount())
.thenRun(Util.NOOP); .thenRun(Util.NOOP);
} }

View File

@ -537,22 +537,27 @@ public class Accounts extends AbstractDynamoDbStore {
return account.getUsernameHash().map(usernameHash -> { return account.getUsernameHash().map(usernameHash -> {
final Timer.Sample sample = Timer.start(); final Timer.Sample sample = Timer.start();
@Nullable final UUID originalLinkHandle = account.getUsernameLinkHandle();
@Nullable final byte[] originalEncryptedUsername = account.getEncryptedUsername().orElse(null);
final TransactWriteItemsRequest request; final TransactWriteItemsRequest request;
try { try {
final List<TransactWriteItem> writeItems = new ArrayList<>(); final List<TransactWriteItem> writeItems = new ArrayList<>();
account.setUsernameHash(null); account.setUsernameHash(null);
account.setUsernameLinkDetails(null, null);
writeItems.add( writeItems.add(
TransactWriteItem.builder() TransactWriteItem.builder()
.update(Update.builder() .update(Update.builder()
.tableName(accountsTableName) .tableName(accountsTableName)
.key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()))) .key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid())))
.updateExpression("SET #data = :data REMOVE #username_hash ADD #version :version_increment") .updateExpression("SET #data = :data REMOVE #username_hash, #username_link ADD #version :version_increment")
.conditionExpression("#version = :version") .conditionExpression("#version = :version")
.expressionAttributeNames(Map.of("#data", ATTR_ACCOUNT_DATA, .expressionAttributeNames(Map.of("#data", ATTR_ACCOUNT_DATA,
"#username_hash", ATTR_USERNAME_HASH, "#username_hash", ATTR_USERNAME_HASH,
"#username_link", ATTR_USERNAME_LINK_UUID,
"#version", ATTR_VERSION)) "#version", ATTR_VERSION))
.expressionAttributeValues(Map.of( .expressionAttributeValues(Map.of(
":data", accountDataAttributeValue(account), ":data", accountDataAttributeValue(account),
@ -570,11 +575,14 @@ public class Accounts extends AbstractDynamoDbStore {
throw new IllegalArgumentException(e); throw new IllegalArgumentException(e);
} finally { } finally {
account.setUsernameHash(usernameHash); account.setUsernameHash(usernameHash);
account.setUsernameLinkDetails(originalLinkHandle, originalEncryptedUsername);
} }
return asyncClient.transactWriteItems(request) return asyncClient.transactWriteItems(request)
.thenAccept(ignored -> { .thenAccept(ignored -> {
account.setUsernameHash(null); account.setUsernameHash(null);
account.setUsernameLinkDetails(null, null);
account.setVersion(account.getVersion() + 1); account.setVersion(account.getVersion() + 1);
}) })
.exceptionally(throwable -> { .exceptionally(throwable -> {

View File

@ -837,7 +837,11 @@ class AccountsTest {
assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty(); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty();
assertThat(accounts.getByAccountIdentifier(account.getUuid())) assertThat(accounts.getByAccountIdentifier(account.getUuid()))
.hasValueSatisfying(clearedAccount -> assertThat(clearedAccount.getUsernameHash()).isEmpty()); .hasValueSatisfying(clearedAccount -> {
assertThat(clearedAccount.getUsernameHash()).isEmpty();
assertThat(clearedAccount.getUsernameLinkHandle()).isNull();
assertThat(clearedAccount.getEncryptedUsername().isEmpty());
});
} }
@Test @Test