Skip to content

Commit 0f4ca43

Browse files
devondragonclaude
andcommitted
fix: address PR review feedback for password history and tests
- Fix password history off-by-one error in cleanUpPasswordHistory() - Keep historyCount + 1 entries to ensure proper password reuse prevention - With historyCount=3, now keeps [current, prev1, prev2, prev3] - Prevents immediate reuse of the Nth previous password - Fix UserAPIUnitTest NPE from unstubbed PasswordPolicyService mock - Add stub to return empty list (no validation errors) for all registration tests - Add Collections import for the stub return value - Ensures tests don't fail with NullPointerException on .isEmpty() call These changes address critical issues identified in code review: 1. Password history was only preventing reuse of historyCount-1 passwords 2. Unit tests were failing due to unstubbed mock returning null 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 90c8feb commit 0f4ca43

2 files changed

Lines changed: 14 additions & 2 deletions

File tree

src/main/java/com/digitalsanctuary/spring/user/service/UserService.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -308,8 +308,11 @@ private void cleanUpPasswordHistory(User user) {
308308
}
309309

310310
List<PasswordHistoryEntry> entries = passwordHistoryRepository.findByUserOrderByEntryDateDesc(user);
311-
if (entries.size() > historyCount) {
312-
List<PasswordHistoryEntry> toDelete = entries.subList(historyCount, entries.size());
311+
// Keep historyCount + 1 entries: the current password plus historyCount previous passwords
312+
// This ensures we actually prevent reuse of the last historyCount passwords
313+
int maxEntries = historyCount + 1;
314+
if (entries.size() > maxEntries) {
315+
List<PasswordHistoryEntry> toDelete = entries.subList(maxEntries, entries.size());
313316
passwordHistoryRepository.deleteAll(toDelete);
314317
log.debug("Cleaned up {} old password history entries for user: {}", toDelete.size(), user.getEmail());
315318
}

src/test/java/com/digitalsanctuary/spring/user/api/UserAPIUnitTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.springframework.web.bind.annotation.RestControllerAdvice;
4747
import org.springframework.http.HttpStatus;
4848

49+
import java.util.Collections;
4950
import java.util.Locale;
5051

5152
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
@@ -143,6 +144,8 @@ void registerUserAccount_success_withVerificationEmail() throws Exception {
143144
.build();
144145

145146
when(userService.registerNewUserAccount(any(UserDto.class))).thenReturn(newUser);
147+
when(passwordPolicyService.validate(any(), anyString(), anyString(), any(Locale.class)))
148+
.thenReturn(Collections.emptyList());
146149

147150
// When & Then
148151
MvcResult result = mockMvc.perform(post("/user/registration")
@@ -182,6 +185,8 @@ void registerUserAccount_success_withAutoLogin() throws Exception {
182185
.build();
183186

184187
when(userService.registerNewUserAccount(any(UserDto.class))).thenReturn(newUser);
188+
when(passwordPolicyService.validate(any(), anyString(), anyString(), any(Locale.class)))
189+
.thenReturn(Collections.emptyList());
185190

186191
// When & Then
187192
mockMvc.perform(post("/user/registration")
@@ -202,6 +207,8 @@ void registerUserAccount_userAlreadyExists() throws Exception {
202207
// Given
203208
when(userService.registerNewUserAccount(any(UserDto.class)))
204209
.thenThrow(new UserAlreadyExistException("User already exists"));
210+
when(passwordPolicyService.validate(any(), anyString(), anyString(), any(Locale.class)))
211+
.thenReturn(Collections.emptyList());
205212

206213
// When & Then
207214
mockMvc.perform(post("/user/registration")
@@ -252,6 +259,8 @@ void registerUserAccount_unexpectedError() throws Exception {
252259
// Given
253260
when(userService.registerNewUserAccount(any(UserDto.class)))
254261
.thenThrow(new RuntimeException("Database error"));
262+
when(passwordPolicyService.validate(any(), anyString(), anyString(), any(Locale.class)))
263+
.thenReturn(Collections.emptyList());
255264

256265
// When & Then
257266
mockMvc.perform(post("/user/registration")

0 commit comments

Comments
 (0)