Skip to content

Commit 30ce976

Browse files
devondragondevondragon
authored
fix: improve user API message handling and fix detached entity bug (#249)
* fix(api): re-fetch user from database in updatePassword endpoint The user entity from DSUserDetails may be detached from the JPA session, causing issues when attempting to update the password. This fix ensures we have an attached entity by re-fetching from the database. Also adds proper error handling for the case where the user is not found. * feat(i18n): add default message fallbacks for missing translations Improve i18n handling by providing sensible default messages when translations are missing. This prevents raw message keys from being displayed to users when message bundles are incomplete. Changes: - UserAPI: Add default messages to all getMessage() calls - UserActionController: Use getValue() instead of name()/toString() for consistent message key generation from TokenValidationResult - GlobalMessageControllerAdvice: Use messageKey as fallback when no translation is found * fix: address PR feedback for user API improvements - Apply detached entity re-fetch pattern to updateUserAccount for consistency with updatePassword endpoint - Change hardcoded "User not found" error to use i18n getMessage() with fallback for consistency with other error messages - Update test mocks to use 4-argument getMessage() signature that includes default message parameter - Add findUserByEmail mocks to updateUser and updatePassword tests
1 parent d6586e5 commit 30ce976

4 files changed

Lines changed: 33 additions & 18 deletions

File tree

‎src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -155,14 +155,19 @@ public ResponseEntity<JSONResponse> updateUserAccount(@AuthenticationPrincipal D
155155
@Valid @RequestBody UserProfileUpdateDto profileUpdateDto,
156156
HttpServletRequest request, Locale locale) {
157157
validateAuthenticatedUser(userDetails);
158-
User user = userDetails.getUser();
158+
// Re-fetch user from database to ensure we have an attached entity
159+
User user = userService.findUserByEmail(userDetails.getUser().getEmail());
160+
if (user == null) {
161+
log.error("User not found in database: {}", userDetails.getUser().getEmail());
162+
return buildErrorResponse(messages.getMessage("message.user.not-found", null, "User not found", locale), 1, HttpStatus.BAD_REQUEST);
163+
}
159164
user.setFirstName(profileUpdateDto.getFirstName());
160165
user.setLastName(profileUpdateDto.getLastName());
161166
userService.saveRegisteredUser(user);
162167

163168
logAuditEvent("ProfileUpdate", "Success", "User profile updated", user, request);
164169

165-
return buildSuccessResponse(messages.getMessage("message.update-user.success", null, locale), null);
170+
return buildSuccessResponse(messages.getMessage("message.update-user.success", null, "Profile updated successfully", locale), null);
166171
}
167172

168173
/**
@@ -206,7 +211,7 @@ public ResponseEntity<JSONResponse> savePassword(@Valid @RequestBody SavePasswor
206211
// are not a concern. Constant-time comparison is only needed when comparing
207212
// against stored credentials, which is handled by Spring's PasswordEncoder.
208213
if (!savePasswordDto.getNewPassword().equals(savePasswordDto.getConfirmPassword())) {
209-
return buildErrorResponse(messages.getMessage("message.password.mismatch", null, locale), 1,
214+
return buildErrorResponse(messages.getMessage("message.password.mismatch", null, "Passwords do not match", locale), 1,
210215
HttpStatus.BAD_REQUEST);
211216
}
212217

@@ -216,14 +221,14 @@ public ResponseEntity<JSONResponse> savePassword(@Valid @RequestBody SavePasswor
216221

217222
if (tokenResult != UserService.TokenValidationResult.VALID) {
218223
String messageKey = "auth.message." + tokenResult.getValue();
219-
return buildErrorResponse(messages.getMessage(messageKey, null, locale), 2, HttpStatus.BAD_REQUEST);
224+
return buildErrorResponse(messages.getMessage(messageKey, null, "Invalid or expired token", locale), 2, HttpStatus.BAD_REQUEST);
220225
}
221226

222227
// Get user by token
223228
Optional<User> userOptional = userService.getUserByPasswordResetToken(savePasswordDto.getToken());
224229

225230
if (userOptional.isEmpty()) {
226-
return buildErrorResponse(messages.getMessage("auth.message.invalid", null, locale), 3,
231+
return buildErrorResponse(messages.getMessage("auth.message.invalid", null, "Invalid token", locale), 3,
227232
HttpStatus.BAD_REQUEST);
228233
}
229234

@@ -246,7 +251,7 @@ public ResponseEntity<JSONResponse> savePassword(@Valid @RequestBody SavePasswor
246251

247252
logAuditEvent("PasswordReset", "Success", "Password reset completed", user, request);
248253

249-
return buildSuccessResponse(messages.getMessage("message.reset-password.success", null, locale),
254+
return buildSuccessResponse(messages.getMessage("message.reset-password.success", null, "Password has been reset successfully", locale),
250255
"/user/login.html");
251256

252257
} catch (Exception ex) {
@@ -272,7 +277,12 @@ public ResponseEntity<JSONResponse> savePassword(@Valid @RequestBody SavePasswor
272277
public ResponseEntity<JSONResponse> updatePassword(@AuthenticationPrincipal DSUserDetails userDetails,
273278
@Valid @RequestBody PasswordDto passwordDto, HttpServletRequest request, Locale locale) {
274279
validateAuthenticatedUser(userDetails);
275-
User user = userDetails.getUser();
280+
// Re-fetch user from database to ensure we have an attached entity
281+
User user = userService.findUserByEmail(userDetails.getUser().getEmail());
282+
if (user == null) {
283+
log.error("User not found in database: {}", userDetails.getUser().getEmail());
284+
return buildErrorResponse(messages.getMessage("message.user.not-found", null, "User not found", locale), 1, HttpStatus.BAD_REQUEST);
285+
}
276286

277287
try {
278288
// Verify old password is correct
@@ -293,10 +303,10 @@ public ResponseEntity<JSONResponse> updatePassword(@AuthenticationPrincipal DSUs
293303
userService.changeUserPassword(user, passwordDto.getNewPassword());
294304
logAuditEvent("PasswordUpdate", "Success", "User password updated", user, request);
295305

296-
return buildSuccessResponse(messages.getMessage("message.update-password.success", null, locale), null);
306+
return buildSuccessResponse(messages.getMessage("message.update-password.success", null, "Password updated successfully", locale), null);
297307
} catch (InvalidOldPasswordException ex) {
298308
logAuditEvent("PasswordUpdate", "Failure", "Invalid old password", user, request);
299-
return buildErrorResponse(messages.getMessage("message.update-password.invalid-old", null, locale), 1,
309+
return buildErrorResponse(messages.getMessage("message.update-password.invalid-old", null, "Invalid old password", locale), 1,
300310
HttpStatus.BAD_REQUEST);
301311
} catch (Exception ex) {
302312
log.error("Unexpected error during password update.", ex);

‎src/main/java/com/digitalsanctuary/spring/user/controller/UserActionController.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public ModelAndView showChangePasswordPage(final HttpServletRequest request, fin
9292
String redirectString = "redirect:" + forgotPasswordChangeURI;
9393
return new ModelAndView(redirectString, model);
9494
} else {
95-
String messageKey = AUTH_MESSAGE_PREFIX + result.name().toLowerCase();
95+
String messageKey = AUTH_MESSAGE_PREFIX + result.getValue();
9696
model.addAttribute("messageKey", messageKey);
9797
return new ModelAndView("redirect:/index.html", model);
9898
}
@@ -137,7 +137,7 @@ public ModelAndView confirmRegistration(final HttpServletRequest request, final
137137
return new ModelAndView(redirectString, model);
138138
}
139139

140-
model.addAttribute("messageKey", AUTH_MESSAGE_PREFIX + result.toString());
140+
model.addAttribute("messageKey", AUTH_MESSAGE_PREFIX + result.getValue());
141141
model.addAttribute("expired", result == TokenValidationResult.EXPIRED);
142142
model.addAttribute("token", token);
143143
log.debug("UserAPI.confirmRegistration: failed. Token not found or expired.");

‎src/main/java/com/digitalsanctuary/spring/user/web/GlobalMessageControllerAdvice.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ public void addMessageFromKey(WebRequest request, org.springframework.ui.Model m
3939
String messageKey = request.getParameter("messageKey");
4040
if (messageKey != null) {
4141
Locale locale = request.getLocale();
42-
String message = messages.getMessage(messageKey, null, locale);
42+
// Use the messageKey itself as the default if no translation is found
43+
String message = messages.getMessage(messageKey, null, messageKey, locale);
4344
model.addAttribute("message", message);
4445
}
4546
}

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter
400400
.setControllerAdvice(new TestExceptionHandler())
401401
.build();
402402

403-
when(messageSource.getMessage(eq("message.update-password.success"), any(), any(Locale.class)))
403+
when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser);
404+
when(messageSource.getMessage(eq("message.update-password.success"), any(), any(), any(Locale.class)))
404405
.thenReturn("Password updated successfully");
405406
when(userService.checkIfValidOldPassword(any(User.class), eq("oldPassword"))).thenReturn(true);
406407

@@ -443,7 +444,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter
443444
.setControllerAdvice(new TestExceptionHandler())
444445
.build();
445446

446-
when(messageSource.getMessage(eq("message.update-password.invalid-old"), any(), any(Locale.class)))
447+
when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser);
448+
when(messageSource.getMessage(eq("message.update-password.invalid-old"), any(), any(), any(Locale.class)))
447449
.thenReturn("Invalid old password");
448450
when(userService.checkIfValidOldPassword(any(User.class), eq("wrongPassword"))).thenReturn(false);
449451

@@ -509,7 +511,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter
509511
.setControllerAdvice(new TestExceptionHandler())
510512
.build();
511513

512-
when(messageSource.getMessage(eq("message.update-user.success"), any(), any(Locale.class)))
514+
when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser);
515+
when(messageSource.getMessage(eq("message.update-user.success"), any(), any(), any(Locale.class)))
513516
.thenReturn("Profile updated successfully");
514517
when(userService.saveRegisteredUser(any(User.class))).thenReturn(testUser);
515518

@@ -522,8 +525,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter
522525
.andExpect(jsonPath("$.success").value(true))
523526
.andExpect(jsonPath("$.messages[0]").value("Profile updated successfully"));
524527

525-
verify(userService).saveRegisteredUser(argThat(user ->
526-
user.getFirstName().equals("UpdatedFirst") &&
528+
verify(userService).saveRegisteredUser(argThat(user ->
529+
user.getFirstName().equals("UpdatedFirst") &&
527530
user.getLastName().equals("UpdatedLast")
528531
));
529532
}
@@ -741,7 +744,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter
741744
.setControllerAdvice(new TestExceptionHandler())
742745
.build();
743746

744-
when(messageSource.getMessage(eq("message.update-user.success"), any(), any(Locale.class)))
747+
when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser);
748+
when(messageSource.getMessage(eq("message.update-user.success"), any(), any(), any(Locale.class)))
745749
.thenReturn("Profile updated successfully");
746750
when(userService.saveRegisteredUser(any(User.class))).thenReturn(testUser);
747751

0 commit comments

Comments
 (0)