From f3009e37e340eb3eec864cab5e07b22f6adf0819 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Mon, 26 Jan 2026 10:55:26 -0700 Subject: [PATCH 1/3] 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. --- .../java/com/digitalsanctuary/spring/user/api/UserAPI.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java b/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java index 0d1e765..f7656c9 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java +++ b/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java @@ -272,7 +272,12 @@ public ResponseEntity savePassword(@Valid @RequestBody SavePasswor public ResponseEntity updatePassword(@AuthenticationPrincipal DSUserDetails userDetails, @Valid @RequestBody PasswordDto passwordDto, HttpServletRequest request, Locale locale) { validateAuthenticatedUser(userDetails); - User user = userDetails.getUser(); + // Re-fetch user from database to ensure we have an attached entity + User user = userService.findUserByEmail(userDetails.getUser().getEmail()); + if (user == null) { + log.error("User not found in database: {}", userDetails.getUser().getEmail()); + return buildErrorResponse("User not found", 1, HttpStatus.BAD_REQUEST); + } try { // Verify old password is correct From 50e0a03faeeefb3ebf345bec8d96a110712d6600 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Mon, 26 Jan 2026 10:55:36 -0700 Subject: [PATCH 2/3] 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 --- .../digitalsanctuary/spring/user/api/UserAPI.java | 14 +++++++------- .../user/controller/UserActionController.java | 4 ++-- .../user/web/GlobalMessageControllerAdvice.java | 3 ++- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java b/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java index f7656c9..86817ba 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java +++ b/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java @@ -162,7 +162,7 @@ public ResponseEntity updateUserAccount(@AuthenticationPrincipal D logAuditEvent("ProfileUpdate", "Success", "User profile updated", user, request); - return buildSuccessResponse(messages.getMessage("message.update-user.success", null, locale), null); + return buildSuccessResponse(messages.getMessage("message.update-user.success", null, "Profile updated successfully", locale), null); } /** @@ -206,7 +206,7 @@ public ResponseEntity savePassword(@Valid @RequestBody SavePasswor // are not a concern. Constant-time comparison is only needed when comparing // against stored credentials, which is handled by Spring's PasswordEncoder. if (!savePasswordDto.getNewPassword().equals(savePasswordDto.getConfirmPassword())) { - return buildErrorResponse(messages.getMessage("message.password.mismatch", null, locale), 1, + return buildErrorResponse(messages.getMessage("message.password.mismatch", null, "Passwords do not match", locale), 1, HttpStatus.BAD_REQUEST); } @@ -216,14 +216,14 @@ public ResponseEntity savePassword(@Valid @RequestBody SavePasswor if (tokenResult != UserService.TokenValidationResult.VALID) { String messageKey = "auth.message." + tokenResult.getValue(); - return buildErrorResponse(messages.getMessage(messageKey, null, locale), 2, HttpStatus.BAD_REQUEST); + return buildErrorResponse(messages.getMessage(messageKey, null, "Invalid or expired token", locale), 2, HttpStatus.BAD_REQUEST); } // Get user by token Optional userOptional = userService.getUserByPasswordResetToken(savePasswordDto.getToken()); if (userOptional.isEmpty()) { - return buildErrorResponse(messages.getMessage("auth.message.invalid", null, locale), 3, + return buildErrorResponse(messages.getMessage("auth.message.invalid", null, "Invalid token", locale), 3, HttpStatus.BAD_REQUEST); } @@ -246,7 +246,7 @@ public ResponseEntity savePassword(@Valid @RequestBody SavePasswor logAuditEvent("PasswordReset", "Success", "Password reset completed", user, request); - return buildSuccessResponse(messages.getMessage("message.reset-password.success", null, locale), + return buildSuccessResponse(messages.getMessage("message.reset-password.success", null, "Password has been reset successfully", locale), "/user/login.html"); } catch (Exception ex) { @@ -298,10 +298,10 @@ public ResponseEntity updatePassword(@AuthenticationPrincipal DSUs userService.changeUserPassword(user, passwordDto.getNewPassword()); logAuditEvent("PasswordUpdate", "Success", "User password updated", user, request); - return buildSuccessResponse(messages.getMessage("message.update-password.success", null, locale), null); + return buildSuccessResponse(messages.getMessage("message.update-password.success", null, "Password updated successfully", locale), null); } catch (InvalidOldPasswordException ex) { logAuditEvent("PasswordUpdate", "Failure", "Invalid old password", user, request); - return buildErrorResponse(messages.getMessage("message.update-password.invalid-old", null, locale), 1, + return buildErrorResponse(messages.getMessage("message.update-password.invalid-old", null, "Invalid old password", locale), 1, HttpStatus.BAD_REQUEST); } catch (Exception ex) { log.error("Unexpected error during password update.", ex); diff --git a/src/main/java/com/digitalsanctuary/spring/user/controller/UserActionController.java b/src/main/java/com/digitalsanctuary/spring/user/controller/UserActionController.java index e9c03e5..c0bb63f 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/controller/UserActionController.java +++ b/src/main/java/com/digitalsanctuary/spring/user/controller/UserActionController.java @@ -92,7 +92,7 @@ public ModelAndView showChangePasswordPage(final HttpServletRequest request, fin String redirectString = "redirect:" + forgotPasswordChangeURI; return new ModelAndView(redirectString, model); } else { - String messageKey = AUTH_MESSAGE_PREFIX + result.name().toLowerCase(); + String messageKey = AUTH_MESSAGE_PREFIX + result.getValue(); model.addAttribute("messageKey", messageKey); return new ModelAndView("redirect:/index.html", model); } @@ -137,7 +137,7 @@ public ModelAndView confirmRegistration(final HttpServletRequest request, final return new ModelAndView(redirectString, model); } - model.addAttribute("messageKey", AUTH_MESSAGE_PREFIX + result.toString()); + model.addAttribute("messageKey", AUTH_MESSAGE_PREFIX + result.getValue()); model.addAttribute("expired", result == TokenValidationResult.EXPIRED); model.addAttribute("token", token); log.debug("UserAPI.confirmRegistration: failed. Token not found or expired."); diff --git a/src/main/java/com/digitalsanctuary/spring/user/web/GlobalMessageControllerAdvice.java b/src/main/java/com/digitalsanctuary/spring/user/web/GlobalMessageControllerAdvice.java index e328bf7..ee487fa 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/web/GlobalMessageControllerAdvice.java +++ b/src/main/java/com/digitalsanctuary/spring/user/web/GlobalMessageControllerAdvice.java @@ -39,7 +39,8 @@ public void addMessageFromKey(WebRequest request, org.springframework.ui.Model m String messageKey = request.getParameter("messageKey"); if (messageKey != null) { Locale locale = request.getLocale(); - String message = messages.getMessage(messageKey, null, locale); + // Use the messageKey itself as the default if no translation is found + String message = messages.getMessage(messageKey, null, messageKey, locale); model.addAttribute("message", message); } } From 556ecd08634c03dd43883d369fe8e84ca035c424 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Mon, 26 Jan 2026 11:15:15 -0700 Subject: [PATCH 3/3] 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 --- .../spring/user/api/UserAPI.java | 9 +++++++-- .../spring/user/api/UserAPIUnitTest.java | 16 ++++++++++------ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java b/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java index 86817ba..e57cfa5 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java +++ b/src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java @@ -155,7 +155,12 @@ public ResponseEntity updateUserAccount(@AuthenticationPrincipal D @Valid @RequestBody UserProfileUpdateDto profileUpdateDto, HttpServletRequest request, Locale locale) { validateAuthenticatedUser(userDetails); - User user = userDetails.getUser(); + // Re-fetch user from database to ensure we have an attached entity + User user = userService.findUserByEmail(userDetails.getUser().getEmail()); + if (user == null) { + log.error("User not found in database: {}", userDetails.getUser().getEmail()); + return buildErrorResponse(messages.getMessage("message.user.not-found", null, "User not found", locale), 1, HttpStatus.BAD_REQUEST); + } user.setFirstName(profileUpdateDto.getFirstName()); user.setLastName(profileUpdateDto.getLastName()); userService.saveRegisteredUser(user); @@ -276,7 +281,7 @@ public ResponseEntity updatePassword(@AuthenticationPrincipal DSUs User user = userService.findUserByEmail(userDetails.getUser().getEmail()); if (user == null) { log.error("User not found in database: {}", userDetails.getUser().getEmail()); - return buildErrorResponse("User not found", 1, HttpStatus.BAD_REQUEST); + return buildErrorResponse(messages.getMessage("message.user.not-found", null, "User not found", locale), 1, HttpStatus.BAD_REQUEST); } try { diff --git a/src/test/java/com/digitalsanctuary/spring/user/api/UserAPIUnitTest.java b/src/test/java/com/digitalsanctuary/spring/user/api/UserAPIUnitTest.java index c151b95..21358be 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/api/UserAPIUnitTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/api/UserAPIUnitTest.java @@ -400,7 +400,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter .setControllerAdvice(new TestExceptionHandler()) .build(); - when(messageSource.getMessage(eq("message.update-password.success"), any(), any(Locale.class))) + when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser); + when(messageSource.getMessage(eq("message.update-password.success"), any(), any(), any(Locale.class))) .thenReturn("Password updated successfully"); when(userService.checkIfValidOldPassword(any(User.class), eq("oldPassword"))).thenReturn(true); @@ -443,7 +444,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter .setControllerAdvice(new TestExceptionHandler()) .build(); - when(messageSource.getMessage(eq("message.update-password.invalid-old"), any(), any(Locale.class))) + when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser); + when(messageSource.getMessage(eq("message.update-password.invalid-old"), any(), any(), any(Locale.class))) .thenReturn("Invalid old password"); when(userService.checkIfValidOldPassword(any(User.class), eq("wrongPassword"))).thenReturn(false); @@ -509,7 +511,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter .setControllerAdvice(new TestExceptionHandler()) .build(); - when(messageSource.getMessage(eq("message.update-user.success"), any(), any(Locale.class))) + when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser); + when(messageSource.getMessage(eq("message.update-user.success"), any(), any(), any(Locale.class))) .thenReturn("Profile updated successfully"); when(userService.saveRegisteredUser(any(User.class))).thenReturn(testUser); @@ -522,8 +525,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter .andExpect(jsonPath("$.success").value(true)) .andExpect(jsonPath("$.messages[0]").value("Profile updated successfully")); - verify(userService).saveRegisteredUser(argThat(user -> - user.getFirstName().equals("UpdatedFirst") && + verify(userService).saveRegisteredUser(argThat(user -> + user.getFirstName().equals("UpdatedFirst") && user.getLastName().equals("UpdatedLast") )); } @@ -741,7 +744,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter .setControllerAdvice(new TestExceptionHandler()) .build(); - when(messageSource.getMessage(eq("message.update-user.success"), any(), any(Locale.class))) + when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser); + when(messageSource.getMessage(eq("message.update-user.success"), any(), any(), any(Locale.class))) .thenReturn("Profile updated successfully"); when(userService.saveRegisteredUser(any(User.class))).thenReturn(testUser);