From bb2de4dc598f2247234496a72ac3c131def26869 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Fri, 31 Oct 2014 13:48:34 +0100 Subject: [PATCH] KEYCLOAK-787 Clicking back to login after sending password reset email fails --- .../java/org/keycloak/events/EventType.java | 2 + .../keycloak/models/ClientSessionModel.java | 1 + .../services/managers/ClientSessionCode.java | 4 -- .../resources/LoginActionsService.java | 13 +++--- .../testsuite/forms/LoginTotpTest.java | 2 +- .../testsuite/forms/ResetPasswordTest.java | 40 +++++++++++++++++++ .../pages/LoginPasswordResetPage.java | 7 ++++ 7 files changed, 58 insertions(+), 11 deletions(-) diff --git a/events/api/src/main/java/org/keycloak/events/EventType.java b/events/api/src/main/java/org/keycloak/events/EventType.java index d41fb8125c..cd624b488f 100755 --- a/events/api/src/main/java/org/keycloak/events/EventType.java +++ b/events/api/src/main/java/org/keycloak/events/EventType.java @@ -40,6 +40,8 @@ public enum EventType { SEND_VERIFY_EMAIL_ERROR, SEND_RESET_PASSWORD, SEND_RESET_PASSWORD_ERROR, + RESET_PASSWORD, + RESET_PASSWORD_ERROR, INVALID_SIGNATURE_ERROR, REGISTER_NODE, diff --git a/model/api/src/main/java/org/keycloak/models/ClientSessionModel.java b/model/api/src/main/java/org/keycloak/models/ClientSessionModel.java index 2c0f9dd8f6..bdbc5c48f6 100755 --- a/model/api/src/main/java/org/keycloak/models/ClientSessionModel.java +++ b/model/api/src/main/java/org/keycloak/models/ClientSessionModel.java @@ -47,6 +47,7 @@ public interface ClientSessionModel { UPDATE_PROFILE, CONFIGURE_TOTP, UPDATE_PASSWORD, + RECOVER_PASSWORD, AUTHENTICATE, SOCIAL_CALLBACK } diff --git a/services/src/main/java/org/keycloak/services/managers/ClientSessionCode.java b/services/src/main/java/org/keycloak/services/managers/ClientSessionCode.java index 526e791769..5574cbb813 100755 --- a/services/src/main/java/org/keycloak/services/managers/ClientSessionCode.java +++ b/services/src/main/java/org/keycloak/services/managers/ClientSessionCode.java @@ -78,10 +78,6 @@ public class ClientSessionCode { return clientSession; } - public boolean isValid(RequiredAction requiredAction) { - return isValid(convertToAction(requiredAction)); - } - public boolean isValid(ClientSessionModel.Action requestedAction) { ClientSessionModel.Action action = clientSession.getAction(); if (action == null) { diff --git a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java index 56bbd9325d..b9704717b1 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -271,9 +271,9 @@ public class LoginActionsService { return Flows.forwardToSecurityFailurePage(session, realm, uriInfo, "Unknown code, please login again through your application."); } ClientSessionModel clientSession = clientCode.getClientSession(); - if (!clientCode.isValid(ClientSessionModel.Action.AUTHENTICATE)) { + if (!(clientCode.isValid(ClientSessionModel.Action.AUTHENTICATE) || clientCode.isValid(ClientSessionModel.Action.RECOVER_PASSWORD))) { clientCode.setAction(ClientSessionModel.Action.AUTHENTICATE); - event.client(clientSession.getClient()).error(Errors.INVALID_USER_CREDENTIALS); + event.client(clientSession.getClient()).error(Errors.INVALID_CODE); return Flows.forms(this.session, realm, clientSession.getClient(), uriInfo).setError(Messages.INVALID_USER) .setClientSessionCode(clientCode.getCode()) .createLogin(); @@ -753,13 +753,14 @@ public class LoginActionsService { @Path("password-reset") @GET public Response passwordReset(@QueryParam("code") String code, @QueryParam("key") String key) { - event.event(EventType.SEND_RESET_PASSWORD); + event.event(EventType.RESET_PASSWORD); if (key != null) { Checks checks = new Checks(); - if (!checks.check(key, ClientSessionModel.Action.UPDATE_PASSWORD)) { + if (!checks.check(key, ClientSessionModel.Action.RECOVER_PASSWORD)) { return checks.response; } ClientSessionCode accessCode = checks.clientCode; + accessCode.setRequiredAction(RequiredAction.UPDATE_PASSWORD); return Flows.forms(session, realm, null, uriInfo) .setClientSessionCode(accessCode.getCode()) .createResponse(RequiredAction.UPDATE_PASSWORD); @@ -820,7 +821,7 @@ public class LoginActionsService { event.session(userSession); TokenManager.attachClientSession(userSession, clientSession); - accessCode.setRequiredAction(RequiredAction.UPDATE_PASSWORD); + accessCode.setAction(ClientSessionModel.Action.RECOVER_PASSWORD); try { UriBuilder builder = Urls.loginPasswordResetBuilder(uriInfo.getBaseUri()); @@ -840,7 +841,7 @@ public class LoginActionsService { } } - return Flows.forms(session, realm, client, uriInfo).setSuccess("emailSent").createPasswordReset(); + return Flows.forms(session, realm, client, uriInfo).setSuccess("emailSent").setClientSessionCode(accessCode.getCode()).createPasswordReset(); } private Response redirectOauth(UserModel user, ClientSessionCode accessCode, ClientSessionModel clientSession, UserSessionModel userSession) { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java index 8a922af7c8..4673165919 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java @@ -166,7 +166,7 @@ public class LoginTotpTest { loginPage.assertCurrent(); Assert.assertEquals("Invalid username or password.", loginPage.getError()); - AssertEvents.ExpectedEvent expectedEvent = events.expectLogin().error("invalid_user_credentials") + AssertEvents.ExpectedEvent expectedEvent = events.expectLogin().error("invalid_code") .user((String)null) .clearDetails() .session((String) null); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java index 22ec3c184c..774d683b64 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java @@ -38,6 +38,7 @@ import org.keycloak.testsuite.MailUtil; import org.keycloak.testsuite.OAuthClient; import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.AppPage.RequestType; +import org.keycloak.testsuite.pages.ErrorPage; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginPasswordResetPage; import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; @@ -96,6 +97,9 @@ public class ResetPasswordTest { @WebResource protected LoginPage loginPage; + @WebResource + protected ErrorPage errorPage; + @WebResource protected LoginPasswordResetPage resetPasswordPage; @@ -110,6 +114,42 @@ public class ResetPasswordTest { resetPassword("login-test"); } + @Test + public void resetPasswordCancel() throws IOException, MessagingException { + loginPage.open(); + loginPage.resetPassword(); + + resetPasswordPage.assertCurrent(); + + resetPasswordPage.changePassword("login-test"); + + resetPasswordPage.assertCurrent(); + + events.expectRequiredAction(EventType.SEND_RESET_PASSWORD).user(userId).detail(Details.USERNAME, "login-test").detail(Details.EMAIL, "login@test.com").assertEvent().getSessionId(); + + resetPasswordPage.backToLogin(); + + Assert.assertTrue(loginPage.isCurrent()); + + loginPage.login("login-test", "password"); + + events.expectLogin().user(userId).detail(Details.USERNAME, "login-test").assertEvent(); + + Assert.assertEquals(1, greenMail.getReceivedMessages().length); + + MimeMessage message = greenMail.getReceivedMessages()[0]; + + String body = (String) message.getContent(); + String changePasswordUrl = MailUtil.getLink(body); + + driver.navigate().to(changePasswordUrl.trim()); + + events.expect(EventType.RESET_PASSWORD_ERROR).client((String) null).user((String) null).error("invalid_code").clearDetails().assertEvent(); + + Assert.assertTrue(errorPage.isCurrent()); + Assert.assertEquals("Unknown code, please login again through your application.", errorPage.getError()); + } + @Test public void resetPasswordByEmail() throws IOException, MessagingException { resetPassword("login@test.com"); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginPasswordResetPage.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginPasswordResetPage.java index 8817c9c6e3..a244c32fcc 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginPasswordResetPage.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginPasswordResetPage.java @@ -41,6 +41,9 @@ public class LoginPasswordResetPage extends AbstractPage { @FindBy(className = "feedback-error") private WebElement emailErrorMessage; + @FindBy(partialLinkText = "Back to Login") + private WebElement backToLogin; + public void changePassword(String username) { usernameInput.sendKeys(username); @@ -63,4 +66,8 @@ public class LoginPasswordResetPage extends AbstractPage { return emailErrorMessage != null ? emailErrorMessage.getText() : null; } + public void backToLogin() { + backToLogin.click(); + } + }