From 936efe872ad1b8fc728f1c6b6bf4f83b6b063be7 Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 10 Jul 2017 22:52:44 +0200 Subject: [PATCH] KEYCLOAK-5061 Process correct initial flow when action expired --- .../oidc/endpoints/AuthorizationEndpoint.java | 2 +- .../resources/LoginActionsService.java | 26 +++++++-- .../RequiredActionEmailVerificationTest.java | 2 +- .../testsuite/forms/ResetPasswordTest.java | 55 ++++++++++++++++++- .../login/messages/messages_en.properties | 2 +- 5 files changed, 77 insertions(+), 10 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java index 402be4cb03..38dbc8f5e4 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java @@ -433,7 +433,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { String flowId = flow.getId(); AuthenticationProcessor processor = createProcessor(authenticationSession, flowId, LoginActionsService.RESET_CREDENTIALS_PATH); - authenticationSession.setClientNote(APP_INITIATED_FLOW, LoginActionsService.REGISTRATION_PATH); + authenticationSession.setClientNote(APP_INITIATED_FLOW, LoginActionsService.RESET_CREDENTIALS_PATH); return processor.authenticate(); } 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 61bdf259e9..f3d4769786 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -342,7 +342,7 @@ public class LoginActionsService { } authSession = createAuthenticationSessionForClient(); - return processResetCredentials(false, null, authSession); + return processResetCredentials(false, null, authSession, null); } event.event(EventType.RESET_PASSWORD); @@ -386,7 +386,7 @@ public class LoginActionsService { } - return processResetCredentials(checks.isActionRequest(), execution, authSession); + return processResetCredentials(checks.isActionRequest(), execution, authSession, null); } /** @@ -469,7 +469,9 @@ public class LoginActionsService { flowPath = AUTHENTICATE_PATH; } AuthenticationProcessor.resetFlow(authSession, flowPath); - return processAuthentication(false, null, authSession, Messages.EXPIRED_ACTION_TOKEN_SESSION_EXISTS); + + // Process correct flow + return processFlowFromPath(flowPath, authSession, Messages.EXPIRED_ACTION_TOKEN_SESSION_EXISTS); } return handleActionTokenVerificationException(null, ex, Errors.EXPIRED_CODE, Messages.EXPIRED_ACTION_TOKEN_NO_SESSION); @@ -539,6 +541,20 @@ public class LoginActionsService { } } + + private Response processFlowFromPath(String flowPath, AuthenticationSessionModel authSession, String errorMessage) { + if (AUTHENTICATE_PATH.equals(flowPath)) { + return processAuthentication(false, null, authSession, errorMessage); + } else if (REGISTRATION_PATH.equals(flowPath)) { + return processRegistration(false, null, authSession, errorMessage); + } else if (RESET_CREDENTIALS_PATH.equals(flowPath)) { + return processResetCredentials(false, null, authSession, errorMessage); + } else { + return ErrorPage.error(session, errorMessage == null ? Messages.INVALID_REQUEST : errorMessage); + } + } + + private ActionTokenHandler resolveActionTokenHandler(String actionId) throws VerificationException { if (actionId == null) { throw new VerificationException("Action token operation not set"); @@ -562,10 +578,10 @@ public class LoginActionsService { return ErrorPage.error(session, errorMessage == null ? Messages.INVALID_CODE : errorMessage); } - protected Response processResetCredentials(boolean actionRequest, String execution, AuthenticationSessionModel authSession) { + protected Response processResetCredentials(boolean actionRequest, String execution, AuthenticationSessionModel authSession, String errorMessage) { AuthenticationProcessor authProcessor = new ResetCredentialsActionTokenHandler.ResetCredsAuthenticationProcessor(); - return processFlow(actionRequest, execution, authSession, RESET_CREDENTIALS_PATH, realm.getResetCredentialsFlow(), null, authProcessor); + return processFlow(actionRequest, execution, authSession, RESET_CREDENTIALS_PATH, realm.getResetCredentialsFlow(), errorMessage, authProcessor); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java index 041ee0edf8..e366ef5dad 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java @@ -424,7 +424,7 @@ public class RequiredActionEmailVerificationTest extends AbstractTestRealmKeyclo driver.navigate().to(verificationUrl.trim()); loginPage.assertCurrent(); - assertEquals("Action expired. Please login again.", loginPage.getError()); + assertEquals("Action expired. Please start again.", loginPage.getError()); events.expectRequiredAction(EventType.EXECUTE_ACTION_TOKEN_ERROR) .error(Errors.EXPIRED_CODE) 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 32adba72e9..c5147b9d70 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 @@ -371,7 +371,7 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest { loginPage.assertCurrent(); - assertEquals("Action expired. Please login again.", loginPage.getError()); + assertEquals("Action expired. Please start again.", loginPage.getError()); events.expectRequiredAction(EventType.EXECUTE_ACTION_TOKEN_ERROR).error("expired_code").client((String) null).user(userId).session((String) null).clearDetails().detail(Details.ACTION, ResetCredentialsActionToken.TOKEN_TYPE).assertEvent(); } finally { @@ -407,7 +407,7 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest { loginPage.assertCurrent(); - assertEquals("Action expired. Please login again.", loginPage.getError()); + assertEquals("Action expired. Please start again.", loginPage.getError()); events.expectRequiredAction(EventType.EXECUTE_ACTION_TOKEN_ERROR).error("expired_code").client((String) null).user(userId).session((String) null).clearDetails().detail(Details.ACTION, ResetCredentialsActionToken.TOKEN_TYPE).assertEvent(); } finally { @@ -463,6 +463,57 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest { } } + + // KEYCLOAK-5061 + @Test + public void resetPasswordExpiredCodeForgotPasswordFlow() throws IOException, MessagingException, InterruptedException { + final AtomicInteger originalValue = new AtomicInteger(); + + RealmRepresentation realmRep = testRealm().toRepresentation(); + originalValue.set(realmRep.getActionTokenGeneratedByUserLifespan()); + realmRep.setActionTokenGeneratedByUserLifespan(60); + testRealm().update(realmRep); + + try { + // Redirect directly to KC "forgot password" endpoint instead of "authenticate" endpoint + String loginUrl = oauth.getLoginFormUrl(); + String forgotPasswordUrl = loginUrl.replace("/auth?", "/forgot-credentials?"); // Workaround, but works + + driver.navigate().to(forgotPasswordUrl); + resetPasswordPage.assertCurrent(); + resetPasswordPage.changePassword("login-test"); + + loginPage.assertCurrent(); + assertEquals("You should receive an email shortly with further instructions.", loginPage.getSuccessMessage()); + expectedMessagesCount++; + + events.expectRequiredAction(EventType.SEND_RESET_PASSWORD) + .session((String)null) + .user(userId).detail(Details.USERNAME, "login-test").detail(Details.EMAIL, "login@test.com").assertEvent(); + + assertEquals(1, greenMail.getReceivedMessages().length); + + MimeMessage message = greenMail.getReceivedMessages()[0]; + + String changePasswordUrl = getPasswordResetEmailLink(message); + + setTimeOffset(70); + + driver.navigate().to(changePasswordUrl.trim()); + + resetPasswordPage.assertCurrent(); + + assertEquals("Action expired. Please start again.", loginPage.getError()); + + events.expectRequiredAction(EventType.EXECUTE_ACTION_TOKEN_ERROR).error("expired_code").client((String) null).user(userId).session((String) null).clearDetails().detail(Details.ACTION, ResetCredentialsActionToken.TOKEN_TYPE).assertEvent(); + } finally { + setTimeOffset(0); + + realmRep.setActionTokenGeneratedByUserLifespan(originalValue.get()); + testRealm().update(realmRep); + } + } + @Test public void resetPasswordDisabledUser() throws IOException, MessagingException, InterruptedException { UserRepresentation user = findUser("login-test"); diff --git a/themes/src/main/resources/theme/base/login/messages/messages_en.properties b/themes/src/main/resources/theme/base/login/messages/messages_en.properties index d319f5b552..947b64d95a 100755 --- a/themes/src/main/resources/theme/base/login/messages/messages_en.properties +++ b/themes/src/main/resources/theme/base/login/messages/messages_en.properties @@ -131,7 +131,7 @@ accountTemporarilyDisabledMessage=Account is temporarily disabled, contact admin expiredCodeMessage=Login timeout. Please login again. expiredActionMessage=Action expired. Please continue with login now. expiredActionTokenNoSessionMessage=Action expired. -expiredActionTokenSessionExistsMessage=Action expired. Please login again. +expiredActionTokenSessionExistsMessage=Action expired. Please start again. missingFirstNameMessage=Please specify first name. missingLastNameMessage=Please specify last name.