From 0417e4b637536b2bcf18005b6072c08f8d3ab02b Mon Sep 17 00:00:00 2001 From: Cas Date: Sat, 2 Jan 2021 13:19:57 +0100 Subject: [PATCH 1/3] Return correct status codes and messages when a user changes their password --- .../authentication/PasswordChangeDTO.java | 2 +- .../controller/CurrentUserRestController.java | 14 +++++++++++-- .../integration/UserRestIntegrationTest.java | 20 ++++++++++++++++++- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/main/java/ch/wisv/areafiftylan/security/authentication/PasswordChangeDTO.java b/src/main/java/ch/wisv/areafiftylan/security/authentication/PasswordChangeDTO.java index 351c6138f..24d3613a2 100644 --- a/src/main/java/ch/wisv/areafiftylan/security/authentication/PasswordChangeDTO.java +++ b/src/main/java/ch/wisv/areafiftylan/security/authentication/PasswordChangeDTO.java @@ -32,7 +32,7 @@ public class PasswordChangeDTO { String oldPassword = ""; @Getter @Setter - @Length(min = UserServiceImpl.MIN_PASSWORD_LENGTH) + @NotEmpty String newPassword = ""; } diff --git a/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java b/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java index e2d029926..d5714e11c 100644 --- a/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java +++ b/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java @@ -35,6 +35,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.validation.annotation.Validated; @@ -89,8 +90,17 @@ public class CurrentUserRestController { @PostMapping("/password") public ResponseEntity changeCurrentUserPassword(@AuthenticationPrincipal User user, @RequestBody @Validated PasswordChangeDTO passwordChangeDTO) { - userService.changePassword(user.getId(), - passwordChangeDTO.getOldPassword(), passwordChangeDTO.getNewPassword()); + try { + userService.changePassword(user.getId(), + passwordChangeDTO.getOldPassword(), passwordChangeDTO.getNewPassword()); + } catch (Exception e) { + if (e instanceof IllegalArgumentException) { + return createResponseEntity(HttpStatus.NOT_MODIFIED, e.getMessage()); + + } else if (e instanceof AccessDeniedException) { + return createResponseEntity(HttpStatus.FORBIDDEN, e.getMessage()); + } + } return createResponseEntity(HttpStatus.OK, "Password successfully changed"); } diff --git a/src/test/java/ch/wisv/areafiftylan/integration/UserRestIntegrationTest.java b/src/test/java/ch/wisv/areafiftylan/integration/UserRestIntegrationTest.java index 4d26594ef..d3faefb2a 100644 --- a/src/test/java/ch/wisv/areafiftylan/integration/UserRestIntegrationTest.java +++ b/src/test/java/ch/wisv/areafiftylan/integration/UserRestIntegrationTest.java @@ -711,7 +711,7 @@ public void testChangeShortPassword() { contentType(ContentType.JSON). post("/users/current/password"). then(). - statusCode(HttpStatus.SC_BAD_REQUEST); + statusCode(HttpStatus.SC_NOT_MODIFIED); //@formatter:on } @@ -796,6 +796,24 @@ public void testChangePasswordMissingOldPassword() { //@formatter:on } + @Test + public void testChangePasswordShortNewPassword() { + User user = createUser(); + Map passwordDTO = new HashMap<>(); + passwordDTO.put("newPassword", "new"); + + //@formatter:off + given(). + header(getXAuthTokenHeaderForUser(user)). + when(). + body(passwordDTO). + contentType(ContentType.JSON). + post("/users/current/password"). + then(). + statusCode(HttpStatus.SC_BAD_REQUEST); + //@formatter:on + } + //TODO: Move to SchedulerTest /* @Test public void testExpiredUsersNoneExpired() { From c0953536d1cf79cdfcc67e0676e43743b32c3a93 Mon Sep 17 00:00:00 2001 From: Cas Date: Sat, 2 Jan 2021 13:19:57 +0100 Subject: [PATCH 2/3] Return correct status codes and messages when a user changes their password --- .../authentication/PasswordChangeDTO.java | 2 +- .../controller/CurrentUserRestController.java | 14 +++++++++++-- .../integration/UserRestIntegrationTest.java | 20 ++++++++++++++++++- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/main/java/ch/wisv/areafiftylan/security/authentication/PasswordChangeDTO.java b/src/main/java/ch/wisv/areafiftylan/security/authentication/PasswordChangeDTO.java index 351c6138f..24d3613a2 100644 --- a/src/main/java/ch/wisv/areafiftylan/security/authentication/PasswordChangeDTO.java +++ b/src/main/java/ch/wisv/areafiftylan/security/authentication/PasswordChangeDTO.java @@ -32,7 +32,7 @@ public class PasswordChangeDTO { String oldPassword = ""; @Getter @Setter - @Length(min = UserServiceImpl.MIN_PASSWORD_LENGTH) + @NotEmpty String newPassword = ""; } diff --git a/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java b/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java index e2d029926..d5714e11c 100644 --- a/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java +++ b/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java @@ -35,6 +35,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.validation.annotation.Validated; @@ -89,8 +90,17 @@ public class CurrentUserRestController { @PostMapping("/password") public ResponseEntity changeCurrentUserPassword(@AuthenticationPrincipal User user, @RequestBody @Validated PasswordChangeDTO passwordChangeDTO) { - userService.changePassword(user.getId(), - passwordChangeDTO.getOldPassword(), passwordChangeDTO.getNewPassword()); + try { + userService.changePassword(user.getId(), + passwordChangeDTO.getOldPassword(), passwordChangeDTO.getNewPassword()); + } catch (Exception e) { + if (e instanceof IllegalArgumentException) { + return createResponseEntity(HttpStatus.NOT_MODIFIED, e.getMessage()); + + } else if (e instanceof AccessDeniedException) { + return createResponseEntity(HttpStatus.FORBIDDEN, e.getMessage()); + } + } return createResponseEntity(HttpStatus.OK, "Password successfully changed"); } diff --git a/src/test/java/ch/wisv/areafiftylan/integration/UserRestIntegrationTest.java b/src/test/java/ch/wisv/areafiftylan/integration/UserRestIntegrationTest.java index 4d26594ef..d3faefb2a 100644 --- a/src/test/java/ch/wisv/areafiftylan/integration/UserRestIntegrationTest.java +++ b/src/test/java/ch/wisv/areafiftylan/integration/UserRestIntegrationTest.java @@ -711,7 +711,7 @@ public void testChangeShortPassword() { contentType(ContentType.JSON). post("/users/current/password"). then(). - statusCode(HttpStatus.SC_BAD_REQUEST); + statusCode(HttpStatus.SC_NOT_MODIFIED); //@formatter:on } @@ -796,6 +796,24 @@ public void testChangePasswordMissingOldPassword() { //@formatter:on } + @Test + public void testChangePasswordShortNewPassword() { + User user = createUser(); + Map passwordDTO = new HashMap<>(); + passwordDTO.put("newPassword", "new"); + + //@formatter:off + given(). + header(getXAuthTokenHeaderForUser(user)). + when(). + body(passwordDTO). + contentType(ContentType.JSON). + post("/users/current/password"). + then(). + statusCode(HttpStatus.SC_BAD_REQUEST); + //@formatter:on + } + //TODO: Move to SchedulerTest /* @Test public void testExpiredUsersNoneExpired() { From 3ff318e055e9da68fcfcd06668c9f13503f6de7a Mon Sep 17 00:00:00 2001 From: Cas Date: Sat, 30 Jan 2021 16:17:49 +0100 Subject: [PATCH 3/3] Add default case for password change error check --- .../users/controller/CurrentUserRestController.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java b/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java index d5714e11c..9a0e63513 100644 --- a/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java +++ b/src/main/java/ch/wisv/areafiftylan/users/controller/CurrentUserRestController.java @@ -99,6 +99,8 @@ public ResponseEntity changeCurrentUserPassword(@AuthenticationPrincipal User } else if (e instanceof AccessDeniedException) { return createResponseEntity(HttpStatus.FORBIDDEN, e.getMessage()); + } else { + return createResponseEntity(HttpStatus.INTERNAL_SERVER_ERROR, "Something went wrong, please try again!"); } }