test(security): integration tests proving token-replay guard and GDPR after-commit fix

- AbstractConcurrentTokenConsumeTest (+ PostgreSQL/MariaDB Testcontainers subclasses):
  two threads race to consume the same password-reset and verification token; assert
  exactly one wins and the token is gone. Proves the conditional-DELETE single-use
  guard holds under real concurrency on production-grade databases (not just H2).
- GdprDeletionAfterCommitIntegrationTest: proves executeUserDeletion now runs through
  the transactional proxy — the UserDeletedEvent is delivered from the afterCommit
  synchronization (synchronization active, user already deleted), and on a contributor
  failure the deletion rolls back atomically and NO event is published.

These are the integration-level proofs the third-pass review requested for findings
P1 (token replay) and P2 (GDPR atomicity).

Author: devondragon

src/test/java/com/digitalsanctuary/spring/user/gdpr/GdprDeletionAfterCommitIntegrationTest.java

@@ -0,0 +1,195 @@
+package com.digitalsanctuary.spring.user.gdpr;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.context.TestConfiguration;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Import;
+import org.springframework.context.event.EventListener;
+import org.springframework.test.context.ActiveProfiles;
+import org.springframework.transaction.support.TransactionSynchronizationManager;
+import com.digitalsanctuary.spring.user.event.UserDeletedEvent;
+import com.digitalsanctuary.spring.user.persistence.model.User;
+import com.digitalsanctuary.spring.user.persistence.repository.UserRepository;
+import com.digitalsanctuary.spring.user.test.app.TestApplication;
+import com.digitalsanctuary.spring.user.test.builders.UserTestDataBuilder;
+
+/**
+ * Integration test proving that {@link GdprDeletionService} actually runs the deletion inside a transaction and
+ * publishes {@link UserDeletedEvent} only <em>after that transaction commits</em>. This is the integration-level
+ * proof for the self-invocation fix: {@code deleteUser} now invokes {@code executeUserDeletion} through the Spring
+ * proxy ({@code self}), so the {@code @Transactional} boundary is honored. A unit test cannot verify this — the
+ * transactional proxy only exists in a real context.
+ *
+ * <p>
+ * This test is deliberately <strong>not</strong> {@code @Transactional}: the service must run and commit (or roll
+ * back) its own transaction so the after-commit synchronization fires for real. Two behaviors are asserted:
+ * </p>
+ * <ul>
+ * <li><b>Happy path:</b> the user row is committed (gone) by the time the event listener runs, and no transaction is
+ * active during event delivery — i.e. the event is genuinely after-commit, not mid-transaction.</li>
+ * <li><b>Rollback path:</b> when a data contributor throws, the whole deletion rolls back (the user still exists) and
+ * <em>no</em> {@link UserDeletedEvent} is published — proving the event is not emitted on a failed/partial delete.</li>
+ * </ul>
+ */
+@SpringBootTest(classes = TestApplication.class)
+@ActiveProfiles("test")
+@Import(GdprDeletionAfterCommitIntegrationTest.TestBeans.class)
+@DisplayName("GdprDeletionService after-commit / rollback integration")
+class GdprDeletionAfterCommitIntegrationTest {
+
+    @Autowired
+    private GdprDeletionService gdprDeletionService;
+
+    @Autowired
+    private UserRepository userRepository;
+
+    @Autowired
+    private DeletedEventRecorder recorder;
+
+    @Autowired
+    private TogglableFailingContributor failingContributor;
+
+    @AfterEach
+    void cleanUp() {
+        // No test-managed transaction here, so clean up committed rows and reset shared test state.
+        userRepository.deleteAll();
+        recorder.reset();
+        failingContributor.disarm();
+    }
+
+    @Test
+    @DisplayName("publishes UserDeletedEvent AFTER the deletion transaction commits")
+    void publishesEventAfterCommit() {
+        User user = userRepository.save(UserTestDataBuilder.aUser()
+                .withId(null) // let the DB assign the id so save() performs an INSERT, not a merge
+                .withEmail("after-commit-" + System.nanoTime() + "@test.com")
+                .withFirstName("After").withLastName("Commit").enabled().build());
+        Long userId = user.getId();
+
+        GdprDeletionService.DeletionResult result = gdprDeletionService.deleteUser(user, false);
+
+        assertThat(result.isSuccess()).as("deletion should succeed").isTrue();
+        assertThat(recorder.isReceived()).as("UserDeletedEvent should be published").isTrue();
+        // The whole point of the fix: by the time the event fires, the delete has COMMITTED.
+        assertThat(recorder.wasUserAbsentAtEventTime())
+                .as("user row must already be deleted when the event is delivered").isTrue();
+        // Distinguishes the fix from the bug: the event is delivered from within the transaction's afterCommit
+        // synchronization (synchronization active). In the broken self-invocation version there was no transaction,
+        // so the event published immediately with NO synchronization active and this would be false.
+        assertThat(recorder.wasSynchronizationActiveAtEventTime())
+                .as("event must be delivered inside the transaction's afterCommit synchronization").isTrue();
+        assertThat(userRepository.findById(userId)).as("user is deleted").isEmpty();
+    }
+
+    @Test
+    @DisplayName("rolls back the deletion and publishes NO event when a contributor fails")
+    void rollsBackAndPublishesNoEventOnFailure() {
+        failingContributor.arm();
+        User user = userRepository.save(UserTestDataBuilder.aUser()
+                .withId(null) // let the DB assign the id so save() performs an INSERT, not a merge
+                .withEmail("rollback-" + System.nanoTime() + "@test.com")
+                .withFirstName("Roll").withLastName("Back").enabled().build());
+        Long userId = user.getId();
+
+        GdprDeletionService.DeletionResult result = gdprDeletionService.deleteUser(user, false);
+
+        assertThat(result.isSuccess()).as("deletion should fail when a contributor throws").isFalse();
+        assertThat(recorder.isReceived())
+                .as("no UserDeletedEvent may be published when the transaction rolled back").isFalse();
+        assertThat(userRepository.findById(userId))
+                .as("the user must still exist — the deletion transaction rolled back atomically").isPresent();
+    }
+
+    @TestConfiguration
+    static class TestBeans {
+        @Bean
+        DeletedEventRecorder deletedEventRecorder(UserRepository userRepository) {
+            return new DeletedEventRecorder(userRepository);
+        }
+
+        @Bean
+        TogglableFailingContributor togglableFailingContributor() {
+            return new TogglableFailingContributor();
+        }
+    }
+
+    /**
+     * Records receipt of {@link UserDeletedEvent} and captures, at event-delivery time, whether the user row is
+     * already gone (committed) and whether a transaction is still active. Used to prove after-commit semantics.
+     */
+    static class DeletedEventRecorder {
+        private final UserRepository userRepository;
+        private volatile boolean received;
+        private volatile boolean userAbsentAtEventTime;
+        private volatile boolean synchronizationActiveAtEventTime;
+
+        DeletedEventRecorder(UserRepository userRepository) {
+            this.userRepository = userRepository;
+        }
+
+        @EventListener
+        void onUserDeleted(UserDeletedEvent event) {
+            received = true;
+            synchronizationActiveAtEventTime = TransactionSynchronizationManager.isSynchronizationActive();
+            userAbsentAtEventTime = userRepository.findById(event.getUserId()).isEmpty();
+        }
+
+        boolean isReceived() {
+            return received;
+        }
+
+        boolean wasUserAbsentAtEventTime() {
+            return userAbsentAtEventTime;
+        }
+
+        boolean wasSynchronizationActiveAtEventTime() {
+            return synchronizationActiveAtEventTime;
+        }
+
+        void reset() {
+            received = false;
+            userAbsentAtEventTime = false;
+            synchronizationActiveAtEventTime = false;
+        }
+    }
+
+    /**
+     * A {@link GdprDataContributor} that throws during {@code prepareForDeletion} only when armed, used to force a
+     * transaction rollback in the middle of the deletion.
+     */
+    static class TogglableFailingContributor implements GdprDataContributor {
+        private final AtomicBoolean armed = new AtomicBoolean(false);
+
+        void arm() {
+            armed.set(true);
+        }
+
+        void disarm() {
+            armed.set(false);
+        }
+
+        @Override
+        public String getDataKey() {
+            return "test-failing-contributor";
+        }
+
+        @Override
+        public Map<String, Object> exportUserData(User user) {
+            return Map.of();
+        }
+
+        @Override
+        public void prepareForDeletion(User user) {
+            if (armed.get()) {
+                throw new RuntimeException("Simulated contributor failure to force rollback");
+            }
+        }
+    }
+}

src/test/java/com/digitalsanctuary/spring/user/service/AbstractConcurrentTokenConsumeTest.java

@@ -0,0 +1,163 @@
+package com.digitalsanctuary.spring.user.service;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import com.digitalsanctuary.spring.user.persistence.model.PasswordResetToken;
+import com.digitalsanctuary.spring.user.persistence.model.User;
+import com.digitalsanctuary.spring.user.persistence.model.VerificationToken;
+import com.digitalsanctuary.spring.user.persistence.repository.PasswordResetTokenRepository;
+import com.digitalsanctuary.spring.user.persistence.repository.UserRepository;
+import com.digitalsanctuary.spring.user.persistence.repository.VerificationTokenRepository;
+import com.digitalsanctuary.spring.user.test.app.TestApplication;
+import com.digitalsanctuary.spring.user.test.builders.UserTestDataBuilder;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.RepeatedTest;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.ActiveProfiles;
+
+/**
+ * Validates that single-use token consumption is truly atomic under concurrency on a real, production-grade database
+ * (not just H2). The fix makes the conditional {@code DELETE} (which returns the affected row count) the atomicity
+ * guard: the row lock serializes concurrent deletes, so exactly one caller observes a count of {@code 1} (and applies
+ * the effect) while the rest observe {@code 0} (and are rejected). A plain read-check-delete would let two requests
+ * both read the token under READ_COMMITTED and both succeed — the replay this test guards against.
+ *
+ * <p>
+ * Two threads race to consume the SAME token at the same instant (released together via a {@link CountDownLatch}).
+ * Exactly one must win. This is asserted for both the password-reset consume path
+ * ({@link UserService#validateAndConsumePasswordResetToken(String)}) and the email-verification consume path
+ * ({@link UserVerificationService#validateVerificationToken(String)}).
+ * </p>
+ *
+ * <p>
+ * Subclasses provide a real database via Testcontainers. The class is deliberately NOT {@code @Transactional}: each
+ * consume must run in its own service-managed transaction on its own thread, and a test-managed transaction would
+ * defeat the race.
+ * </p>
+ */
+@SpringBootTest(classes = TestApplication.class)
+@ActiveProfiles("test")
+abstract class AbstractConcurrentTokenConsumeTest {
+
+	@Autowired
+	private UserService userService;
+
+	@Autowired
+	private UserVerificationService userVerificationService;
+
+	@Autowired
+	private UserRepository userRepository;
+
+	@Autowired
+	private PasswordResetTokenRepository passwordResetTokenRepository;
+
+	@Autowired
+	private VerificationTokenRepository verificationTokenRepository;
+
+	@Autowired
+	private TokenHasher tokenHasher;
+
+	@AfterEach
+	void cleanUp() {
+		// The threads commit their own transactions, so clean up explicitly.
+		passwordResetTokenRepository.deleteAll();
+		verificationTokenRepository.deleteAll();
+		userRepository.deleteAll();
+	}
+
+	@RepeatedTest(value = 3, name = "{displayName} [run {currentRepetition}/{totalRepetitions}]")
+	@DisplayName("password-reset token is consumed by exactly one of two racing threads")
+	void shouldConsumePasswordResetTokenExactlyOnceUnderConcurrency() throws InterruptedException {
+		final User user = userRepository.save(UserTestDataBuilder.aUser()
+				.withId(null) // let the DB assign the id so save() performs an INSERT, not a merge
+				.withEmail("pwd-race-" + System.nanoTime() + "@test.com")
+				.withFirstName("Pwd").withLastName("Race").enabled().build());
+		final String rawToken = "pwd-reset-" + System.nanoTime();
+		passwordResetTokenRepository.save(new PasswordResetToken(tokenHasher.hash(rawToken), user, 60));
+
+		final List<Object> outcomes = raceTwoThreads(() -> userService.validateAndConsumePasswordResetToken(rawToken));
+
+		final long wins = outcomes.stream().filter(o -> o instanceof User).count();
+		assertThat(wins).as("exactly one thread may consume the password-reset token").isEqualTo(1);
+		assertThat(passwordResetTokenRepository.findByToken(tokenHasher.hash(rawToken)))
+				.as("the token must be gone after consumption").isNull();
+	}
+
+	@RepeatedTest(value = 3, name = "{displayName} [run {currentRepetition}/{totalRepetitions}]")
+	@DisplayName("verification token is consumed by exactly one of two racing threads")
+	void shouldConsumeVerificationTokenExactlyOnceUnderConcurrency() throws InterruptedException {
+		final User user = userRepository.save(UserTestDataBuilder.aUser()
+				.withId(null) // let the DB assign the id so save() performs an INSERT, not a merge
+				.withEmail("verify-race-" + System.nanoTime() + "@test.com")
+				.withFirstName("Verify").withLastName("Race").unverified().build());
+		final String rawToken = "verify-" + System.nanoTime();
+		verificationTokenRepository.save(new VerificationToken(tokenHasher.hash(rawToken), user, 60));
+
+		final List<Object> outcomes =
+				raceTwoThreads(() -> userVerificationService.validateVerificationToken(rawToken));
+
+		final long valid = outcomes.stream()
+				.filter(o -> o == UserService.TokenValidationResult.VALID).count();
+		assertThat(valid).as("exactly one thread may validate (consume) the verification token").isEqualTo(1);
+		assertThat(verificationTokenRepository.findByToken(tokenHasher.hash(rawToken)))
+				.as("the token must be gone after consumption").isNull();
+		assertThat(userRepository.findById(user.getId()))
+				.get().extracting(User::isEnabled).as("the user is enabled exactly once").isEqualTo(true);
+	}
+
+	/**
+	 * Runs the given consume action on two threads released simultaneously and returns both outcomes.
+	 *
+	 * @param consume the consume action under test
+	 * @return the two outcomes (a result object, or {@code null} for the losing/rejected call)
+	 */
+	private List<Object> raceTwoThreads(final Callable<Object> consume) throws InterruptedException {
+		final int threadCount = 2;
+		final CountDownLatch readyLatch = new CountDownLatch(threadCount);
+		final CountDownLatch startLatch = new CountDownLatch(1);
+		final ExecutorService executor = Executors.newFixedThreadPool(threadCount);
+		try {
+			final List<Future<Object>> futures = new ArrayList<>();
+			for (int i = 0; i < threadCount; i++) {
+				futures.add(executor.submit(() -> {
+					readyLatch.countDown();
+					if (!startLatch.await(30, TimeUnit.SECONDS)) {
+						throw new IllegalStateException("start gate was never opened");
+					}
+					return consume.call();
+				}));
+			}
+
+			assertThat(readyLatch.await(30, TimeUnit.SECONDS))
+					.as("both consume threads should reach the start gate").isTrue();
+			startLatch.countDown();
+
+			final AtomicInteger unexpected = new AtomicInteger();
+			final List<Object> outcomes = new ArrayList<>();
+			for (Future<Object> future : futures) {
+				try {
+					outcomes.add(future.get(60, TimeUnit.SECONDS));
+				} catch (ExecutionException e) {
+					unexpected.incrementAndGet();
+				} catch (Exception e) {
+					unexpected.incrementAndGet();
+				}
+			}
+			assertThat(unexpected.get()).as("neither consume call should throw").isZero();
+			return outcomes;
+		} finally {
+			executor.shutdownNow();
+		}
+	}
+}

src/test/java/com/digitalsanctuary/spring/user/service/MariaDBConcurrentTokenConsumeTest.java

@@ -0,0 +1,34 @@
+package com.digitalsanctuary.spring.user.service;
+
+import org.junit.jupiter.api.DisplayName;
+import org.springframework.test.context.DynamicPropertyRegistry;
+import org.springframework.test.context.DynamicPropertySource;
+import org.testcontainers.containers.MariaDBContainer;
+import org.testcontainers.junit.jupiter.Container;
+import org.testcontainers.junit.jupiter.Testcontainers;
+
+/**
+ * Runs the concurrent single-use token-consume race against a real MariaDB container, proving the conditional DELETE
+ * guard prevents token replay under InnoDB's default isolation.
+ */
+@Testcontainers
+@DisplayName("MariaDB Concurrent Token Consume Tests")
+class MariaDBConcurrentTokenConsumeTest extends AbstractConcurrentTokenConsumeTest {
+
+	@Container
+	static final MariaDBContainer<?> MARIADB = new MariaDBContainer<>("mariadb:11.4")
+			.withDatabaseName("testdb")
+			.withUsername("test")
+			.withPassword("test");
+
+	@DynamicPropertySource
+	static void configureProperties(DynamicPropertyRegistry registry) {
+		registry.add("spring.datasource.url", MARIADB::getJdbcUrl);
+		registry.add("spring.datasource.username", MARIADB::getUsername);
+		registry.add("spring.datasource.password", MARIADB::getPassword);
+		registry.add("spring.datasource.driver-class-name", () -> "org.mariadb.jdbc.Driver");
+		registry.add("spring.jpa.hibernate.ddl-auto", () -> "create");
+		registry.add("spring.jpa.database-platform", () -> "org.hibernate.dialect.MariaDBDialect");
+		registry.add("spring.jpa.properties.hibernate.dialect", () -> "org.hibernate.dialect.MariaDBDialect");
+	}
+}