From 209f8155d19617ce1ce1855cb2dfbe2fc01e4019 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Wed, 2 Nov 2016 11:53:07 +0100 Subject: [PATCH] KEYCLOAK-3835 Remove redirect on flow and return not modified if page is refreshed --- .../AuthenticationProcessor.java | 2 +- .../resources/LoginActionsService.java | 34 ++++++++++++++----- .../testsuite/forms/LoginTotpTest.java | 20 +++++++++++ 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index 3b409a04c5..2f23cb82a4 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -802,7 +802,7 @@ public class AuthenticationProcessor { * */ public void setActionSuccessful() { - oneActionWasSuccessful = true; +// oneActionWasSuccessful = true; } public Response checkWasSuccessfulBrowserAction() { 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 7e0e11274b..2bdb1297da 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -96,6 +96,7 @@ public class LoginActionsService { public static final String REQUIRED_ACTION = "required-action"; public static final String FIRST_BROKER_LOGIN_PATH = "first-broker-login"; public static final String POST_BROKER_LOGIN_PATH = "post-broker-login"; + public static final String LAST_PROCESSED_CODE = "last_processed_code"; private RealmModel realm; @@ -323,14 +324,22 @@ public class LoginActionsService { public Response authenticate(@QueryParam("code") String code, @QueryParam("execution") String execution) { event.event(EventType.LOGIN); - Checks checks = new Checks(); - if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { - return checks.response; - } - event.detail(Details.CODE_ID, code); - ClientSessionCode clientSessionCode = checks.clientCode; - ClientSessionModel clientSession = clientSessionCode.getClientSession(); + ClientSessionModel clientSession = ClientSessionCode.getClientSession(code, session, realm); + if (clientSession != null && code.equals(clientSession.getNote(LAST_PROCESSED_CODE))) { + // Allow refresh of previous page + } else { + Checks checks = new Checks(); + if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { + return checks.response; + } + + ClientSessionCode clientSessionCode = checks.clientCode; + clientSession = clientSessionCode.getClientSession(); + } + + event.detail(Details.CODE_ID, code); + clientSession.setNote(LAST_PROCESSED_CODE, code); return processAuthentication(execution, clientSession, null); } @@ -373,12 +382,21 @@ public class LoginActionsService { public Response authenticateForm(@QueryParam("code") String code, @QueryParam("execution") String execution) { event.event(EventType.LOGIN); + + ClientSessionModel clientSession = ClientSessionCode.getClientSession(code, session, realm); + if (clientSession != null && code.equals(clientSession.getNote(LAST_PROCESSED_CODE))) { + // Post already processed (refresh) - ignore form post and return next form + request.getFormParameters().clear(); + return authenticate(code, null); + } + Checks checks = new Checks(); if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { return checks.response; } final ClientSessionCode clientCode = checks.clientCode; - final ClientSessionModel clientSession = clientCode.getClientSession(); + clientSession = clientCode.getClientSession(); + clientSession.setNote(LAST_PROCESSED_CODE, code); return processAuthentication(execution, clientSession, null); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java index 06ebacd883..15a8b6b9b4 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java @@ -127,6 +127,26 @@ public class LoginTotpTest extends TestRealmKeycloakTest { events.expectLogin().assertEvent(); } + // KEYCLOAK-3835 + @Test + public void loginWithTotpRefreshTotpPage() throws Exception { + loginPage.open(); + loginPage.login("test-user@localhost", "password"); + + Assert.assertTrue(loginTotpPage.isCurrent()); + + // Refresh TOTP page + driver.navigate().refresh(); + + System.out.println(driver.getPageSource()); + + loginTotpPage.login(totp.generateTOTP("totpSecret")); + + Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + + events.expectLogin().assertEvent(); + } + @Test public void loginWithTotpCancel() throws Exception { loginPage.open();