diff --git a/src/main/java/uk/gov/hmcts/reform/idam/web/UserController.java b/src/main/java/uk/gov/hmcts/reform/idam/web/UserController.java index 383a92f00..217894847 100644 --- a/src/main/java/uk/gov/hmcts/reform/idam/web/UserController.java +++ b/src/main/java/uk/gov/hmcts/reform/idam/web/UserController.java @@ -16,11 +16,13 @@ import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.ModelAttribute; +import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.HttpServerErrorException; +import org.springframework.web.servlet.ModelAndView; import uk.gov.hmcts.reform.idam.api.internal.model.ActivationResult; import uk.gov.hmcts.reform.idam.api.internal.model.ErrorResponse; import uk.gov.hmcts.reform.idam.api.internal.model.Service; @@ -32,6 +34,7 @@ import uk.gov.hmcts.reform.idam.web.strategic.ValidationService; import java.io.IOException; +import java.util.HashMap; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -46,9 +49,6 @@ import static uk.gov.hmcts.reform.idam.web.helper.MvcKeys.SELF_REGISTER_VIEW; import static uk.gov.hmcts.reform.idam.web.helper.MvcKeys.STATE; -/** - * @author Ivano - */ @Controller @RequestMapping("/users") @Slf4j @@ -83,9 +83,9 @@ public String users(final Map model) { * @should return expiredtoken view and have redirect_uri attribute in model if token expired * @should return useractivation view and no redirect_uri attribute in model if the token is valid * @should return errorpage view error message and no redirect_uri attribute in model if api returns server error - * @should return errorpage view error message for alredy activated account and no redirect_uri attribute in model if api returns status 409 + * @should return errorpage view error message for already activated account and no redirect_uri attribute in model if api returns status 409 */ - @RequestMapping(path = "/register", method = RequestMethod.GET) + @GetMapping("/register") public String userActivation(@RequestParam("token") String token, @RequestParam("code") String code, final Map model) { ValidateRequest validateRequest = new ValidateRequest(); validateRequest.setCode(code); @@ -253,9 +253,10 @@ public String selfRegisterUser(@ModelAttribute("selfRegisterCommand") @Validated * @should return expiredtoken view if HttpClientErrorException occurs and http status is 400 and token is invalid * @should return redirect expiredtoken page if selfRegisterUser service throws HttpClientErrorException and Http code is 404 */ - @RequestMapping(path = "/activate", method = RequestMethod.POST) - public String activateUser(@RequestParam("token") String token, @RequestParam("code") String code, @RequestParam("password1") String password1, @RequestParam("password2") String password2, - final Map model) throws IOException { + @PostMapping("/activate") + public ModelAndView activateUser(@RequestParam("token") String token, @RequestParam("code") String code, + @RequestParam("password1") String password1, @RequestParam("password2") String password2, + final Map model) throws IOException { model.put("token", token); model.put("code", code); try { @@ -263,15 +264,18 @@ public String activateUser(@RequestParam("token") String token, @RequestParam("c String activation = "{\"token\":\"" + token + "\",\"code\":\"" + code + "\",\"password\":\"" + password1 + "\"}"; ResponseEntity response = spiService.activateUser(activation); String redirectUri = getRedirectUri(response.getBody()); + // don't expose parameters other than the url to a GET request + Map successModel = new HashMap<>(); if (redirectUri != null) { - model.put("redirectUri", redirectUri); + successModel.put("redirectUri", redirectUri); } - return "useractivated"; + return new ModelAndView("redirect:useractivated", successModel); } } catch (HttpClientErrorException e) { if (e.getStatusCode() == HttpStatus.NOT_FOUND) { - return "redirect:expiredtoken"; + // don't expose the token in the error page + return new ModelAndView("redirect:expiredtoken", (Map) null); } if (e.getStatusCode() == HttpStatus.BAD_REQUEST) { @@ -281,7 +285,7 @@ public String activateUser(@RequestParam("token") String token, @RequestParam("c "public.common.error.blacklisted.password", "public.common.error.enter.password", model); - return "useractivation"; + return new ModelAndView("useractivation"); } if (validationService.isErrorInResponse(e.getResponseBodyAsString(), ErrorResponse.CodeEnum.PASSWORD_CONTAINS_PERSONAL_INFO)) { @@ -290,11 +294,11 @@ public String activateUser(@RequestParam("token") String token, @RequestParam("c "public.common.error.containspersonalinfo.password", "public.common.error.enter.password", model); - return "useractivation"; + return new ModelAndView("useractivation"); } if (validationService.isErrorInResponse(e.getResponseBodyAsString(), ErrorResponse.CodeEnum.TOKEN_INVALID)) { - return "expiredtoken"; + return new ModelAndView("redirect:expiredtoken", model); } } @@ -305,7 +309,20 @@ public String activateUser(@RequestParam("token") String token, @RequestParam("c model); } - return "useractivation"; + return new ModelAndView("useractivation"); + } + + @GetMapping("/useractivated") + public String userActivated(@RequestParam(required = false) final String redirectUri, final Map model) { + if (redirectUri != null) { + model.put("redirectUri", redirectUri); + } + return "useractivated"; + } + + @GetMapping("/expiredtoken") + public String expiredToken(final Map model) { + return "expiredtoken"; } private String getRedirectUri(String json) throws IOException { @@ -319,4 +336,4 @@ private String getRedirectUri(String json) throws IOException { return null; } } -} \ No newline at end of file +} diff --git a/src/test/java/uk/gov/hmcts/reform/idam/web/AppControllerTest.java b/src/test/java/uk/gov/hmcts/reform/idam/web/AppControllerTest.java index e3df17064..2a7cc54be 100644 --- a/src/test/java/uk/gov/hmcts/reform/idam/web/AppControllerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/idam/web/AppControllerTest.java @@ -1035,7 +1035,7 @@ public void resetPassword_shouldRedirectToExpiredTokenIfHttpClientErrorException .param(TOKEN_PARAMETER, RESET_PASSWORD_TOKEN) .param(CODE_PARAMETER, RESET_PASSWORD_CODE)) .andExpect(status().is3xxRedirection()) - .andExpect(redirectedUrl(EXPIREDTOKEN_VIEW_NAME)); + .andExpect(redirectedUrl(EXPIREDTOKEN_REDIRECTED_VIEW_NAME)); verify(spiService).resetPassword(eq(PASSWORD_ONE), eq(RESET_PASSWORD_TOKEN), eq(RESET_PASSWORD_CODE)); } diff --git a/src/test/java/uk/gov/hmcts/reform/idam/web/UserControllerTest.java b/src/test/java/uk/gov/hmcts/reform/idam/web/UserControllerTest.java index c95e968c1..5460709e9 100644 --- a/src/test/java/uk/gov/hmcts/reform/idam/web/UserControllerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/idam/web/UserControllerTest.java @@ -55,6 +55,7 @@ import static uk.gov.hmcts.reform.idam.web.util.TestConstants.ERROR_MSG; import static uk.gov.hmcts.reform.idam.web.util.TestConstants.ERROR_TITLE; import static uk.gov.hmcts.reform.idam.web.util.TestConstants.ERROR_VIEW_NAME; +import static uk.gov.hmcts.reform.idam.web.util.TestConstants.EXPIREDTOKEN_REDIRECTED_VIEW_NAME; import static uk.gov.hmcts.reform.idam.web.util.TestConstants.EXPIREDTOKEN_VIEW_NAME; import static uk.gov.hmcts.reform.idam.web.util.TestConstants.EXPIRED_ACTIVATION_TOKEN_VIEW_NAME; import static uk.gov.hmcts.reform.idam.web.util.TestConstants.FORM_DATA; @@ -295,7 +296,7 @@ public void activateUser_shouldReturnUseractivatedViewAndRedirectUriInModelIfRet given(spiService.activateUser(eq("{\"token\":\"" + USER_ACTIVATION_TOKEN + "\",\"code\":\"" + USER_ACTIVATION_CODE + "\",\"password\":\"" + USER_PASSWORD + "\"}"))).willReturn(ResponseEntity.ok("{\"redirectUri\":\"" + REDIRECT_URI + "\"}")); mockMvc.perform(getActivateUserPostRequest(USER_ACTIVATION_TOKEN, USER_ACTIVATION_CODE, USER_PASSWORD, USER_PASSWORD)) - .andExpect(status().isOk()) + .andExpect(status().is3xxRedirection()) .andExpect(model().attribute(REDIRECTURI, REDIRECT_URI)) .andExpect(view().name(USER_ACTIVATED_VIEW_NAME)); } @@ -350,7 +351,7 @@ public void activateUser_shouldReturnExpiredtokenViewIfHttpClientErrorExceptionO given(spiService.activateUser(eq("{\"token\":\"" + USER_ACTIVATION_TOKEN + "\",\"code\":\"" + USER_ACTIVATION_CODE + "\",\"password\":\"" + USER_PASSWORD + "\"}"))).willThrow(new HttpClientErrorException(HttpStatus.BAD_REQUEST, "Bad Request", TOKEN_INVALID_RESPONSE.getBytes(), null)); given(validationService.isErrorInResponse(eq(TOKEN_INVALID_RESPONSE), eq(ErrorResponse.CodeEnum.TOKEN_INVALID))).willReturn(true); mockMvc.perform(getActivateUserPostRequest(USER_ACTIVATION_TOKEN, USER_ACTIVATION_CODE, USER_PASSWORD, USER_PASSWORD)) - .andExpect(status().isOk()) + .andExpect(status().is3xxRedirection()) .andExpect(view().name(EXPIREDTOKEN_VIEW_NAME)); } @@ -384,7 +385,7 @@ public void activateUser_shouldReturnRedirectExpiredtokenPageIfSelfRegisterUserS mockMvc.perform(getActivateUserPostRequest(USER_ACTIVATION_TOKEN, USER_ACTIVATION_CODE, USER_PASSWORD, USER_PASSWORD)) .andExpect(status().is3xxRedirection()) - .andExpect(redirectedUrl(EXPIREDTOKEN_VIEW_NAME)); + .andExpect(redirectedUrl(EXPIREDTOKEN_REDIRECTED_VIEW_NAME)); } diff --git a/src/test/java/uk/gov/hmcts/reform/idam/web/util/TestConstants.java b/src/test/java/uk/gov/hmcts/reform/idam/web/util/TestConstants.java index 366e52e09..86a018c52 100644 --- a/src/test/java/uk/gov/hmcts/reform/idam/web/util/TestConstants.java +++ b/src/test/java/uk/gov/hmcts/reform/idam/web/util/TestConstants.java @@ -63,11 +63,12 @@ public class TestConstants { public static final String EXPIRED_PASSWORD_RESET_TOKEN_VIEW_NAME = "expiredPasswordResetLink"; public static final String EXPIRED_ACTIVATION_TOKEN_VIEW_NAME = "expiredActivationLink"; public static final String USER_ACTIVATION_VIEW_NAME = "useractivation"; - public static final String USER_ACTIVATED_VIEW_NAME = "useractivated"; + public static final String USER_ACTIVATED_VIEW_NAME = "redirect:useractivated"; public static final String ERROR_VIEW_NAME = "errorpage"; public static final String USER_CREATED_VIEW_NAME = "usercreated"; public static final String RESETPASSWORD_VIEW_NAME = "resetpassword"; - public static final String EXPIREDTOKEN_VIEW_NAME = "expiredtoken"; + public static final String EXPIREDTOKEN_VIEW_NAME = "redirect:expiredtoken"; + public static final String EXPIREDTOKEN_REDIRECTED_VIEW_NAME = "expiredtoken"; public static final String FORGOT_PASSWORD_VIEW = "forgotpassword"; public static final String FORGOT_PASSWORD_SUCCESS_VIEW = "forgotpasswordsuccess"; public static final String RESET_PASSWORD_SUCCESS_VIEW = "resetpasswordsuccess";