Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make /activate POST request redirect to a GET endpoint that can have … #366

Merged
5 commits merged into from
May 15, 2020
Merged

Make /activate POST request redirect to a GET endpoint that can have … #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
535c1b1
Make /activate POST request redirect to a GET endpoint that can have …
May 1, 2020
8c9ee0e
Restore individual imports.
May 1, 2020
db34f96
Code review changes.
May 15, 2020
edb6d7c
Fix broken functional test.
May 15, 2020
db70823
Empty commit.
May 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 33 additions & 16 deletions src/main/java/uk/gov/hmcts/reform/idam/web/UserController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -83,9 +83,9 @@ public String users(final Map<String, Object> 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<String, Object> model) {
ValidateRequest validateRequest = new ValidateRequest();
validateRequest.setCode(code);
Expand Down Expand Up @@ -253,25 +253,29 @@ 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<String, Object> 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<String, Object> model) throws IOException {
model.put("token", token);
model.put("code", code);
try {
if (validationService.validatePassword(password1, password2, model)) {
String activation = "{\"token\":\"" + token + "\",\"code\":\"" + code + "\",\"password\":\"" + password1 + "\"}";
ResponseEntity<String> response = spiService.activateUser(activation);
String redirectUri = getRedirectUri(response.getBody());
// don't expose parameters other than the url to a GET request
Map<String, Object> 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<String, ?>) null);
}

if (e.getStatusCode() == HttpStatus.BAD_REQUEST) {
Expand All @@ -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)) {
Expand All @@ -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);
}
}

Expand All @@ -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<String, Object> model) {
if (redirectUri != null) {
model.put("redirectUri", redirectUri);
}
return "useractivated";
}

@GetMapping("/expiredtoken")
public String expiredToken(final Map<String, Object> model) {
return "expiredtoken";
}

private String getRedirectUri(String json) throws IOException {
Expand All @@ -319,4 +336,4 @@ private String getRedirectUri(String json) throws IOException {
return null;
}
}
}
}
2 changes: 1 addition & 1 deletion src/test/java/uk/gov/hmcts/reform/idam/web/AppControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
7 changes: 4 additions & 3 deletions src/test/java/uk/gov/hmcts/reform/idam/web/UserControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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));

}

Expand Down
5 changes: 3 additions & 2 deletions src/test/java/uk/gov/hmcts/reform/idam/web/util/TestConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down