From ab72b2b141bce0a6ac2c0020f4994ddef44e13bf Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Thu, 27 Oct 2016 16:04:45 +0200 Subject: [PATCH] KEYCLOAK-3331 Reset password leads to 400 bad request when link is opened in a different browser session --- .../resetcred/ResetCredentialEmail.java | 3 ++ .../resetcred/ResetPassword.java | 7 ++++ .../testsuite/forms/ResetPasswordTest.java | 41 +++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetCredentialEmail.java b/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetCredentialEmail.java index d7e3e7494f..0d41b062c6 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetCredentialEmail.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetCredentialEmail.java @@ -41,6 +41,7 @@ import org.keycloak.models.utils.HmacOTP; import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.services.ServicesLogger; import org.keycloak.services.messages.Messages; +import org.keycloak.services.resources.LoginActionsService; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriBuilder; @@ -60,6 +61,8 @@ public class ResetCredentialEmail implements Authenticator, AuthenticatorFactory @Override public void authenticate(AuthenticationFlowContext context) { + LoginActionsService.createActionCookie(context.getRealm(), context.getUriInfo(), context.getConnection(), context.getClientSession().getId()); + UserModel user = context.getUser(); String username = context.getClientSession().getNote(AbstractUsernameFormAuthenticator.ATTEMPTED_USERNAME); diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetPassword.java b/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetPassword.java index 5ec467d95e..64098fa379 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetPassword.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetPassword.java @@ -20,6 +20,8 @@ package org.keycloak.authentication.authenticators.resetcred; import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; +import org.keycloak.services.managers.AuthenticationManager; +import org.keycloak.services.resources.LoginActionsService; /** * @author Bill Burke @@ -31,6 +33,11 @@ public class ResetPassword extends AbstractSetRequiredActionAuthenticator { @Override public void authenticate(AuthenticationFlowContext context) { + String actionCookie = LoginActionsService.getActionCookie(context.getSession().getContext().getRequestHeaders(), context.getRealm(), context.getUriInfo(), context.getConnection()); + if (actionCookie == null || !actionCookie.equals(context.getClientSession().getId())) { + context.getClientSession().setNote(AuthenticationManager.END_AFTER_REQUIRED_ACTIONS, "true"); + } + if (context.getExecution().isRequired() || (context.getExecution().isOptional() && configuredFor(context))) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java index de89d57f36..e28c2352fe 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java @@ -606,6 +606,47 @@ public class ResetPasswordTest extends TestRealmKeycloakTest { } } + @Test + public void resetPasswordLinkOpenedInNewBrowser() throws IOException, MessagingException { + String username = "login-test"; + String resetUri = oauth.AUTH_SERVER_ROOT + "/realms/test/login-actions/reset-credentials"; + driver.navigate().to(resetUri); + + resetPasswordPage.assertCurrent(); + + resetPasswordPage.changePassword(username); + + loginPage.assertCurrent(); + assertEquals("You should receive an email shortly with further instructions.", loginPage.getSuccessMessage()); + + events.expectRequiredAction(EventType.SEND_RESET_PASSWORD) + .user(userId) + .detail(Details.REDIRECT_URI, oauth.AUTH_SERVER_ROOT + "/realms/test/account/") + .client("account") + .detail(Details.USERNAME, username) + .detail(Details.EMAIL, "login@test.com") + .session((String)null) + .assertEvent(); + + assertEquals(1, greenMail.getReceivedMessages().length); + + MimeMessage message = greenMail.getReceivedMessages()[0]; + + String changePasswordUrl = getPasswordResetEmailLink(message); + + driver.manage().deleteAllCookies(); + driver.navigate().to(changePasswordUrl.trim()); + + System.out.println(driver.getPageSource()); + + updatePasswordPage.assertCurrent(); + + updatePasswordPage.changePassword("resetPassword", "resetPassword"); + + assertTrue(infoPage.isCurrent()); + assertEquals("Your account has been updated.", infoPage.getInfo()); + } + public static String getPasswordResetEmailLink(MimeMessage message) throws IOException, MessagingException { Multipart multipart = (Multipart) message.getContent();