Skip to content

Commit a93fdff

Browse files
devondragondevondragon
committed
test: adapt relocated framework integration tests to 4.4.0 behavior
The integration tests under com.digitalsanctuary.spring.user.* were moved from the library into this demo. 4.4.0's security hardening changed four behaviors they assert against; this updates them to the new, correct behavior (full suite green again: 301 tests, 0 failures). Test isolation (application-test.properties): - Switch the H2 URL to jdbc:h2:mem:testdb-${random.uuid} and ddl-auto to create-drop so each Spring context gets its own database. 4.4.0 made registerNewUserAccount run with Propagation.NOT_SUPPORTED and the LoginAttemptService mutations @transactional, so they COMMIT in their own transactions and survive a test's @transactional rollback. With the old shared db name those committed rows leaked across classes and collided with other classes' deleteAll()/registration (UserAlreadyExistException, FK_VERIFY_USER violations). Mirrors the library's own test-isolation fix. Behavior changes adapted: - Account status enforced on load (LoginHelperService.assertAccountUsable): DSUserDetailsService now throws LockedException/DisabledException for locked/disabled accounts. DSUserDetailsServiceIntegrationTest now asserts those exceptions, and pins accountLockoutDuration so the auto-unlock case is deterministic. - registerNewUserAccount commits outside the test transaction: AccountLockoutIntegrationTest seeds its user via the repository (in-tx, rolls back) instead of the registration service; UserApiIntegrationTestFixed cleans up the committed user in a REQUIRES_NEW transaction before/after each test so registrations don't collide. - Role names are now granted as authorities (for hasRole() checks): AuthorityServiceIntegrationTest expects the ROLE_* names alongside the privileges. - RegistrationListener skips the verification email for already-enabled users: EventSystemIntegrationTest builds its user as unverified so the registration-email events still fire.
1 parent 1c2b910 commit a93fdff

6 files changed

Lines changed: 112 additions & 72 deletions

File tree

‎src/test/java/com/digitalsanctuary/spring/user/api/UserApiIntegrationTestFixed.java‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
66
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
77
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
8+
import org.junit.jupiter.api.AfterEach;
89
import org.junit.jupiter.api.BeforeEach;
910
import org.junit.jupiter.api.DisplayName;
1011
import org.junit.jupiter.api.Test;
@@ -16,12 +17,16 @@
1617
import org.springframework.test.context.bean.override.mockito.MockitoBean;
1718
import org.springframework.test.web.servlet.MockMvc;
1819
import org.springframework.test.web.servlet.MvcResult;
20+
import org.springframework.transaction.PlatformTransactionManager;
21+
import org.springframework.transaction.TransactionDefinition;
1922
import org.springframework.transaction.annotation.Transactional;
23+
import org.springframework.transaction.support.TransactionTemplate;
2024
import com.digitalsanctuary.spring.demo.UserDemoApplication;
2125
import com.digitalsanctuary.spring.user.dto.UserDto;
2226
import com.digitalsanctuary.spring.user.mail.MailService;
2327
import com.digitalsanctuary.spring.user.persistence.model.User;
2428
import com.digitalsanctuary.spring.user.persistence.repository.UserRepository;
29+
import com.digitalsanctuary.spring.user.persistence.repository.VerificationTokenRepository;
2530
import com.digitalsanctuary.spring.user.service.UserService;
2631
import com.fasterxml.jackson.databind.ObjectMapper;
2732
import jakarta.persistence.EntityManager;
@@ -50,24 +55,61 @@ class UserApiIntegrationTestFixed {
5055
@Autowired
5156
private UserService userService;
5257

58+
@Autowired
59+
private VerificationTokenRepository verificationTokenRepository;
60+
61+
@Autowired
62+
private PlatformTransactionManager transactionManager;
63+
5364
@MockitoBean
5465
private MailService mailService;
5566

5667
@PersistenceContext
5768
private EntityManager entityManager;
5869

70+
private static final String TEST_EMAIL = "test@example.com";
71+
5972
private UserDto testUserDto;
6073

6174
@BeforeEach
6275
void setUp() {
76+
// Start from a clean slate. Registration (via the API and via UserService.registerNewUserAccount)
77+
// commits the new user in its own transaction as of 4.4.0 — it does NOT roll back with the test's
78+
// @Transactional. Without a committed cleanup, the user persisted by one test method collides with
79+
// the next (UserAlreadyExistException / 409 Conflict).
80+
deleteTestUserCommitted();
81+
6382
testUserDto = new UserDto();
6483
testUserDto.setFirstName("Test");
6584
testUserDto.setLastName("User");
66-
testUserDto.setEmail("test@example.com");
85+
testUserDto.setEmail(TEST_EMAIL);
6786
testUserDto.setPassword("SecurePass123!");
6887
testUserDto.setMatchingPassword("SecurePass123!");
6988
}
7089

90+
@AfterEach
91+
void tearDown() {
92+
// Remove the user committed by this test so it cannot leak into other tests.
93+
deleteTestUserCommitted();
94+
}
95+
96+
/**
97+
* Deletes the test user (and any verification token) in its own committed transaction. A
98+
* REQUIRES_NEW transaction is required because the class is {@code @Transactional}: a plain delete here
99+
* would roll back with the test and never actually remove the committed registration row.
100+
*/
101+
private void deleteTestUserCommitted() {
102+
TransactionTemplate tx = new TransactionTemplate(transactionManager);
103+
tx.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW);
104+
tx.executeWithoutResult(status -> {
105+
User existing = userRepository.findByEmail(TEST_EMAIL);
106+
if (existing != null) {
107+
verificationTokenRepository.deleteByUser(existing);
108+
userRepository.delete(existing);
109+
}
110+
});
111+
}
112+
71113
@Test
72114
@DisplayName("Should successfully register new user")
73115
void shouldRegisterNewUser() throws Exception {

‎src/test/java/com/digitalsanctuary/spring/user/integration/AuthorityServiceIntegrationTest.java‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,9 @@ void getAuthoritiesFromUser_persistedUser_loadsAuthoritiesWithLazyLoading() {
139139
Collection<? extends GrantedAuthority> authorities = authorityService
140140
.getAuthoritiesFromUser(fetchedUser);
141141

142-
// Then
143-
assertThat(authorities).hasSize(3).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
144-
"LOGIN_PRIVILEGE",
142+
// Then - as of 4.4.0 the role name itself is also granted (needed for hasRole() checks)
143+
assertThat(authorities).hasSize(4).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
144+
"ROLE_USER", "LOGIN_PRIVILEGE",
145145
"UPDATE_OWN_USER_PRIVILEGE", "RESET_OWN_PASSWORD_PRIVILEGE");
146146
}
147147

@@ -159,9 +159,10 @@ void getAuthoritiesFromUser_multipleRoles_combinesAllPrivileges() {
159159
Collection<? extends GrantedAuthority> authorities = authorityService
160160
.getAuthoritiesFromUser(multiRoleUser);
161161

162-
// Then
163-
assertThat(authorities).hasSize(6) // 3 from user role + 3 from manager role
164-
.extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder("LOGIN_PRIVILEGE",
162+
// Then - 6 privileges + the two role names (ROLE_USER, ROLE_MANAGER) granted as of 4.4.0
163+
assertThat(authorities).hasSize(8) // 3 user + 3 manager privileges + 2 role names
164+
.extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder("ROLE_USER",
165+
"ROLE_MANAGER", "LOGIN_PRIVILEGE",
165166
"UPDATE_OWN_USER_PRIVILEGE",
166167
"RESET_OWN_PASSWORD_PRIVILEGE", "ADD_USER_TO_TEAM_PRIVILEGE",
167168
"REMOVE_USER_FROM_TEAM_PRIVILEGE",
@@ -180,9 +181,9 @@ void getAuthoritiesFromUser_adminUser_hasAllAdminPrivileges() {
180181
// When
181182
Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromUser(adminUser);
182183

183-
// Then
184-
assertThat(authorities).hasSize(5).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
185-
"ADMIN_PRIVILEGE",
184+
// Then - 5 admin privileges + the ROLE_ADMIN role name granted as of 4.4.0
185+
assertThat(authorities).hasSize(6).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
186+
"ROLE_ADMIN", "ADMIN_PRIVILEGE",
186187
"INVITE_USER_PRIVILEGE", "READ_USER_PRIVILEGE", "ASSIGN_MANAGER_PRIVILEGE",
187188
"RESET_ANY_USER_PASSWORD_PRIVILEGE");
188189
}
@@ -206,9 +207,9 @@ void getAuthoritiesFromRoles_sharedPrivileges_deduplicatesCorrectly() {
206207
Collection<? extends GrantedAuthority> authorities = authorityService
207208
.getAuthoritiesFromRoles(Arrays.asList(role1, role2));
208209

209-
// Then - Should only have 4 unique privileges (3 from user + 1 shared)
210-
assertThat(authorities).hasSize(4).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
211-
"LOGIN_PRIVILEGE",
210+
// Then - 4 unique privileges (3 from user + 1 shared) + the two role names granted as of 4.4.0
211+
assertThat(authorities).hasSize(6).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
212+
"ROLE_TEST1", "ROLE_TEST2", "LOGIN_PRIVILEGE",
212213
"UPDATE_OWN_USER_PRIVILEGE", "RESET_OWN_PASSWORD_PRIVILEGE", "SHARED_PRIVILEGE");
213214
}
214215

@@ -247,9 +248,11 @@ void getAuthoritiesFromRoles_separatelyLoadedRoles_worksCorrectly() {
247248
Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(
248249
Arrays.asList(loadedUserRole, loadedManagerRole, loadedAdminRole));
249250

250-
// Then - Should have all unique privileges from all three roles
251-
assertThat(authorities).hasSize(11) // 3 + 3 + 5 unique privileges
251+
// Then - all unique privileges from all three roles, plus the three role names granted as of 4.4.0
252+
assertThat(authorities).hasSize(14) // 3 + 3 + 5 privileges + 3 role names
252253
.extracting(GrantedAuthority::getAuthority).contains(
254+
// Role names
255+
"ROLE_USER", "ROLE_MANAGER", "ROLE_ADMIN",
253256
// User privileges
254257
"LOGIN_PRIVILEGE", "UPDATE_OWN_USER_PRIVILEGE",
255258
"RESET_OWN_PASSWORD_PRIVILEGE",

‎src/test/java/com/digitalsanctuary/spring/user/integration/DSUserDetailsServiceIntegrationTest.java‎

Lines changed: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,11 @@
1313
import org.junit.jupiter.api.Test;
1414
import org.junit.jupiter.api.DisplayName;
1515
import org.springframework.beans.factory.annotation.Autowired;
16+
import org.springframework.security.authentication.DisabledException;
17+
import org.springframework.security.authentication.LockedException;
1618
import org.springframework.security.core.GrantedAuthority;
1719
import org.springframework.security.core.userdetails.UsernameNotFoundException;
20+
import org.springframework.test.context.TestPropertySource;
1821
import org.springframework.transaction.annotation.Transactional;
1922

2023
import com.digitalsanctuary.spring.user.persistence.model.Privilege;
@@ -40,6 +43,10 @@
4043
* - Account unlock functionality
4144
*/
4245
@IntegrationTest
46+
// A positive lockout duration makes auto-unlock deterministic: a user locked longer ago than this window
47+
// is eligible to be unlocked on load, while one just locked stays locked. (The framework reads
48+
// user.security.accountLockoutDuration; >= 0 enables time-based auto-unlock.)
49+
@TestPropertySource(properties = "user.security.accountLockoutDuration=30")
4350
@DisplayName("DSUserDetailsService Integration Tests")
4451
class DSUserDetailsServiceIntegrationTest {
4552

@@ -129,6 +136,7 @@ void loadUserByUsername_autoUnlocksEligibleUser() {
129136
.withEmail("autounlock@test.com")
130137
.withLockedDate(oldLockDate)
131138
.withFailedLoginAttempts(5)
139+
.verified() // enabled, so that once auto-unlocked the account is fully usable
132140
.withId(null)
133141
.build();
134142
lockedUser.setRoles(new ArrayList<>(Arrays.asList(userRole)));
@@ -137,14 +145,14 @@ void loadUserByUsername_autoUnlocksEligibleUser() {
137145
// Verify user is initially locked
138146
assertThat(lockedUser.isLocked()).isTrue();
139147

140-
// When
148+
// When - the lock is older than accountLockoutDuration (30m), so loading auto-unlocks the account
141149
DSUserDetails result = dsUserDetailsService.loadUserByUsername("autounlock@test.com");
142150

143-
// Then - Depending on configuration, user might be unlocked
144-
// Note: This behavior depends on accountLockoutDuration configuration
145-
// If configured to auto-unlock after certain time, the user should be unlocked
151+
// Then - the user is unlocked on load and authentication succeeds
146152
assertThat(result).isNotNull();
147153
assertThat(result.getUsername()).isEqualTo("autounlock@test.com");
154+
assertThat(result.isAccountNonLocked()).isTrue();
155+
assertThat(userRepository.findByEmail("autounlock@test.com").isLocked()).isFalse();
148156
}
149157

150158
@Test
@@ -247,8 +255,8 @@ void loadUserByUsername_mapsAllUserDetailsProperties() {
247255

248256
@Test
249257
@Transactional
250-
@DisplayName("Should handle disabled user correctly")
251-
void loadUserByUsername_disabledUser_returnsWithCorrectStatus() {
258+
@DisplayName("Should reject loading a disabled user")
259+
void loadUserByUsername_disabledUser_throwsDisabledException() {
252260
// Given
253261
User disabledUser = UserTestDataBuilder.anUnverifiedUser()
254262
.withEmail("disabled@test.com")
@@ -257,27 +265,24 @@ void loadUserByUsername_disabledUser_returnsWithCorrectStatus() {
257265
disabledUser.setRoles(new ArrayList<>(Arrays.asList(userRole)));
258266
userRepository.save(disabledUser);
259267

260-
// When
261-
DSUserDetails result = dsUserDetailsService.loadUserByUsername("disabled@test.com");
262-
263-
// Then
264-
assertThat(result).isNotNull();
265-
assertThat(result.isEnabled()).isFalse();
266-
assertThat(result.isAccountNonLocked()).isTrue();
267-
assertThat(result.getUsername()).isEqualTo("disabled@test.com");
268+
// When & Then - as of 4.4.0 the login helper enforces account status on every auth path, so loading
269+
// a disabled (unverified) account throws rather than returning a principal with isEnabled()==false.
270+
assertThatThrownBy(() -> dsUserDetailsService.loadUserByUsername("disabled@test.com"))
271+
.isInstanceOf(DisabledException.class);
268272
}
269273

270274
@Test
271275
@Transactional
272-
@DisplayName("Should handle currently locked user correctly")
273-
void loadUserByUsername_currentlyLockedUser_returnsWithLockedStatus() {
274-
// Given - Create a recently locked user (should remain locked)
276+
@DisplayName("Should reject loading a currently locked user")
277+
void loadUserByUsername_currentlyLockedUser_throwsLockedException() {
278+
// Given - a user locked just now: well inside the accountLockoutDuration window, so it must NOT
279+
// auto-unlock and stays locked.
275280
Date recentLockDate = new Date();
276281

277282
User lockedUser = UserTestDataBuilder.aLockedUser()
278283
.withEmail("locked@test.com")
279284
.withLockedDate(recentLockDate)
280-
.verified() // Make the user enabled but locked
285+
.verified() // enabled but locked, to prove lock (not disabled) is what rejects the load
281286
.withId(null)
282287
.build();
283288
lockedUser.setRoles(new ArrayList<>(Arrays.asList(userRole)));
@@ -286,26 +291,9 @@ void loadUserByUsername_currentlyLockedUser_returnsWithLockedStatus() {
286291
// Verify user is initially locked
287292
assertThat(lockedUser.isLocked()).isTrue();
288293

289-
// When
290-
DSUserDetails result = dsUserDetailsService.loadUserByUsername("locked@test.com");
291-
292-
// Then
293-
assertThat(result).isNotNull();
294-
assertThat(result.isEnabled()).isTrue(); // Locked users can still be enabled
295-
296-
// Check the actual lock status - it depends on configuration
297-
// If accountLockoutDuration is 0, the user might be unlocked immediately
298-
// If it's > 0, the user should remain locked since we just locked them
299-
User userAfterLoad = userRepository.findByEmail("locked@test.com");
300-
301-
// The test should verify the actual behavior based on configuration
302-
// If the user is still locked, isAccountNonLocked should be false
303-
// If the user was unlocked by the service, isAccountNonLocked should be true
304-
if (userAfterLoad.isLocked()) {
305-
assertThat(result.isAccountNonLocked()).isFalse();
306-
} else {
307-
// User was unlocked by the service
308-
assertThat(result.isAccountNonLocked()).isTrue();
309-
}
294+
// When & Then - loading a still-locked account throws LockedException (checked before disabled status)
295+
assertThatThrownBy(() -> dsUserDetailsService.loadUserByUsername("locked@test.com"))
296+
.isInstanceOf(LockedException.class);
297+
assertThat(userRepository.findByEmail("locked@test.com").isLocked()).isTrue();
310298
}
311299
}

‎src/test/java/com/digitalsanctuary/spring/user/integration/EventSystemIntegrationTest.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ public TestEventCapture testEventCapture() {
6767

6868
@BeforeEach
6969
void setUp() {
70-
testUser = UserTestDataBuilder.aUser().withId(1L).withEmail("test@example.com").withFirstName("Test").withLastName("User").enabled().build();
70+
// The user must be UNVERIFIED (disabled): as of 4.4.0 RegistrationListener skips sending the
71+
// verification email when the user is already enabled, which is the realistic state for a brand-new
72+
// registration. An enabled user here would make the registration-email assertions never fire.
73+
testUser = UserTestDataBuilder.aUser().withId(1L).withEmail("test@example.com").withFirstName("Test").withLastName("User").unverified().build();
7174
eventCapture.clear();
7275
}
7376

‎src/test/java/com/digitalsanctuary/spring/user/security/AccountLockoutIntegrationTest.java‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
66
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
77
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
8+
import java.util.ArrayList;
89
import java.util.concurrent.CountDownLatch;
910
import java.util.concurrent.ExecutorService;
1011
import java.util.concurrent.Executors;
@@ -25,12 +26,12 @@
2526
import org.springframework.test.web.servlet.MockMvc;
2627
import org.springframework.transaction.annotation.Transactional;
2728
import com.digitalsanctuary.spring.demo.UserDemoApplication;
28-
import com.digitalsanctuary.spring.user.dto.UserDto;
2929
import com.digitalsanctuary.spring.user.persistence.model.User;
3030
import com.digitalsanctuary.spring.user.persistence.repository.UserRepository;
3131
import com.digitalsanctuary.spring.user.service.LoginAttemptService;
3232
import com.digitalsanctuary.spring.user.service.UserService;
3333
import com.digitalsanctuary.spring.user.test.annotations.IntegrationTest;
34+
import com.digitalsanctuary.spring.user.test.builders.UserTestDataBuilder;
3435
import jakarta.persistence.EntityManager;
3536

3637
/**
@@ -79,19 +80,15 @@ void setUp() {
7980
}
8081
entityManager.flush();
8182

82-
// Create test user
83-
UserDto userDto = new UserDto();
84-
userDto.setFirstName("Lockout");
85-
userDto.setLastName("Test");
86-
userDto.setEmail(TEST_EMAIL);
87-
userDto.setPassword(TEST_PASSWORD);
88-
userDto.setMatchingPassword(TEST_PASSWORD);
89-
90-
userService.registerNewUserAccount(userDto);
91-
92-
// Enable the user
93-
entityManager.createNativeQuery("UPDATE user_account SET enabled = true WHERE email = :email").setParameter("email", TEST_EMAIL)
94-
.executeUpdate();
83+
// Create the test user directly via the repository so it lives inside the test's transaction and
84+
// rolls back cleanly. As of 4.4.0, UserService.registerNewUserAccount runs with
85+
// Propagation.NOT_SUPPORTED and commits the new user in its own transaction, which would survive
86+
// the rollback and leak the same email into the next test method (UserAlreadyExistException). The
87+
// lockout tests only need a persisted, enabled user row; they exercise LoginAttemptService directly.
88+
User user = UserTestDataBuilder.aUser().withEmail(TEST_EMAIL).withFirstName("Lockout").withLastName("Test")
89+
.withPassword(TEST_PASSWORD).verified().withId(null).build();
90+
user.setRoles(new ArrayList<>());
91+
userRepository.save(user);
9592
entityManager.flush();
9693
}
9794

0 commit comments

Comments
 (0)