Details
diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
index eed2858..a3983a4 100755
--- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
+++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
@@ -979,7 +979,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
return ParsedCodeContext.response(staleCodeError);
}
- SessionCodeChecks checks = new SessionCodeChecks(realmModel, uriInfo, clientConnection, session, event, code, null, clientId, LoginActionsService.AUTHENTICATE_PATH);
+ SessionCodeChecks checks = new SessionCodeChecks(realmModel, uriInfo, request, clientConnection, session, event, code, null, clientId, LoginActionsService.AUTHENTICATE_PATH);
checks.initialVerify();
if (!checks.verifyActiveAndValidAction(AuthenticationSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
@@ -993,7 +993,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
Response errorResponse = checks.getResponse();
// Remove "code" from browser history
- errorResponse = BrowserHistoryHelper.getInstance().saveResponseAndRedirect(session, authSession, errorResponse, true);
+ errorResponse = BrowserHistoryHelper.getInstance().saveResponseAndRedirect(session, authSession, errorResponse, true, request);
return ParsedCodeContext.response(errorResponse);
}
} else {
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 f3d4769..054cc6e 100755
--- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java
+++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java
@@ -179,7 +179,7 @@ public class LoginActionsService {
}
private SessionCodeChecks checksForCode(String code, String execution, String clientId, String flowPath) {
- SessionCodeChecks res = new SessionCodeChecks(realm, uriInfo, clientConnection, session, event, code, execution, clientId, flowPath);
+ SessionCodeChecks res = new SessionCodeChecks(realm, uriInfo, request, clientConnection, session, event, code, execution, clientId, flowPath);
res.initialVerify();
return res;
}
@@ -200,7 +200,7 @@ public class LoginActionsService {
@GET
public Response restartSession(@QueryParam("client_id") String clientId) {
event.event(EventType.RESTART_AUTHENTICATION);
- SessionCodeChecks checks = new SessionCodeChecks(realm, uriInfo, clientConnection, session, event, null, null, clientId, null);
+ SessionCodeChecks checks = new SessionCodeChecks(realm, uriInfo, request, clientConnection, session, event, null, null, clientId, null);
AuthenticationSessionModel authSession = checks.initialVerifyAuthSession();
if (authSession == null) {
@@ -286,7 +286,7 @@ public class LoginActionsService {
authSession = processor.getAuthenticationSession(); // Could be changed (eg. Forked flow)
}
- return BrowserHistoryHelper.getInstance().saveResponseAndRedirect(session, authSession, response, action);
+ return BrowserHistoryHelper.getInstance().saveResponseAndRedirect(session, authSession, response, action, request);
}
/**
@@ -927,7 +927,7 @@ public class LoginActionsService {
throw new RuntimeException("Unreachable");
}
- return BrowserHistoryHelper.getInstance().saveResponseAndRedirect(session, authSession, response, true);
+ return BrowserHistoryHelper.getInstance().saveResponseAndRedirect(session, authSession, response, true, request);
}
}
diff --git a/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java b/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java
index 0f3ebbe..b5011fb 100644
--- a/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java
+++ b/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java
@@ -24,6 +24,7 @@ import javax.ws.rs.core.UriBuilder;
import javax.ws.rs.core.UriInfo;
import org.jboss.logging.Logger;
+import org.jboss.resteasy.spi.HttpRequest;
import org.keycloak.authentication.AuthenticationProcessor;
import org.keycloak.common.ClientConnection;
import org.keycloak.common.util.ObjectUtil;
@@ -59,6 +60,7 @@ public class SessionCodeChecks {
private final RealmModel realm;
private final UriInfo uriInfo;
+ private final HttpRequest request;
private final ClientConnection clientConnection;
private final KeycloakSession session;
private final EventBuilder event;
@@ -69,9 +71,10 @@ public class SessionCodeChecks {
private final String flowPath;
- public SessionCodeChecks(RealmModel realm, UriInfo uriInfo, ClientConnection clientConnection, KeycloakSession session, EventBuilder event, String code, String execution, String clientId, String flowPath) {
+ public SessionCodeChecks(RealmModel realm, UriInfo uriInfo, HttpRequest request, ClientConnection clientConnection, KeycloakSession session, EventBuilder event, String code, String execution, String clientId, String flowPath) {
this.realm = realm;
this.uriInfo = uriInfo;
+ this.request = request;
this.clientConnection = clientConnection;
this.session = session;
this.event = event;
@@ -220,10 +223,17 @@ public class SessionCodeChecks {
}
}
- if (ObjectUtil.isEqualOrBothNull(execution, lastExecFromSession)) {
+ if (execution == null || execution.equals(lastExecFromSession)) {
// Allow refresh of previous page
clientCode = new ClientSessionCode<>(session, realm, authSession);
actionRequest = false;
+
+ // Allow refresh, but rewrite browser history
+ if (execution == null && lastExecFromSession != null) {
+ logger.debugf("Parameter 'execution' is not in the request, but flow wasn't changed. Will update browser history");
+ request.setAttribute(BrowserHistoryHelper.SHOULD_UPDATE_BROWSER_HISTORY, true);
+ }
+
return true;
} else {
response = showPageExpired(authSession);
diff --git a/services/src/main/java/org/keycloak/services/util/BrowserHistoryHelper.java b/services/src/main/java/org/keycloak/services/util/BrowserHistoryHelper.java
index ef34b16..6832f10 100644
--- a/services/src/main/java/org/keycloak/services/util/BrowserHistoryHelper.java
+++ b/services/src/main/java/org/keycloak/services/util/BrowserHistoryHelper.java
@@ -24,6 +24,7 @@ import java.util.regex.Pattern;
import javax.ws.rs.core.Response;
import org.jboss.logging.Logger;
+import org.jboss.resteasy.spi.HttpRequest;
import org.keycloak.models.KeycloakSession;
import org.keycloak.sessions.AuthenticationSessionModel;
import org.keycloak.theme.BrowserSecurityHeaderSetup;
@@ -44,13 +45,27 @@ import org.keycloak.utils.MediaType;
*/
public abstract class BrowserHistoryHelper {
+ // Request attribute, which specifies if flow was changed in this request (eg. click "register" from the login screen)
+ public static final String SHOULD_UPDATE_BROWSER_HISTORY = "SHOULD_UPDATE_BROWSER_HISTORY";
+
protected static final Logger logger = Logger.getLogger(BrowserHistoryHelper.class);
- public abstract Response saveResponseAndRedirect(KeycloakSession session, AuthenticationSessionModel authSession, Response response, boolean actionRequest);
+ public abstract Response saveResponseAndRedirect(KeycloakSession session, AuthenticationSessionModel authSession, Response response, boolean actionRequest, HttpRequest httpRequest);
public abstract Response loadSavedResponse(KeycloakSession session, AuthenticationSessionModel authSession);
+ protected boolean shouldReplaceBrowserHistory(boolean actionRequest, HttpRequest httpRequest) {
+ if (actionRequest) {
+ return true;
+ }
+
+ Boolean flowChanged = (Boolean) httpRequest.getAttribute(SHOULD_UPDATE_BROWSER_HISTORY);
+ return (flowChanged != null && flowChanged);
+ }
+
+
+
// Always rely on javascript for now
public static BrowserHistoryHelper getInstance() {
return new JavascriptHistoryReplace();
@@ -66,8 +81,8 @@ public abstract class BrowserHistoryHelper {
private static final Pattern HEAD_END_PATTERN = Pattern.compile("</[hH][eE][aA][dD]>");
@Override
- public Response saveResponseAndRedirect(KeycloakSession session, AuthenticationSessionModel authSession, Response response, boolean actionRequest) {
- if (!actionRequest) {
+ public Response saveResponseAndRedirect(KeycloakSession session, AuthenticationSessionModel authSession, Response response, boolean actionRequest, HttpRequest httpRequest) {
+ if (!shouldReplaceBrowserHistory(actionRequest, httpRequest)) {
return response;
}
@@ -129,8 +144,8 @@ public abstract class BrowserHistoryHelper {
private static final String CACHED_RESPONSE = "cached.response";
@Override
- public Response saveResponseAndRedirect(KeycloakSession session, AuthenticationSessionModel authSession, Response response, boolean actionRequest) {
- if (!actionRequest) {
+ public Response saveResponseAndRedirect(KeycloakSession session, AuthenticationSessionModel authSession, Response response, boolean actionRequest, HttpRequest httpRequest) {
+ if (!shouldReplaceBrowserHistory(actionRequest, httpRequest)) {
return response;
}
@@ -179,7 +194,7 @@ public abstract class BrowserHistoryHelper {
private static class NoOpHelper extends BrowserHistoryHelper {
@Override
- public Response saveResponseAndRedirect(KeycloakSession session, AuthenticationSessionModel authSession, Response response, boolean actionRequest) {
+ public Response saveResponseAndRedirect(KeycloakSession session, AuthenticationSessionModel authSession, Response response, boolean actionRequest, HttpRequest httpRequest) {
return response;
}
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java
index acf775c..ca5cb76 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java
@@ -540,16 +540,12 @@ public abstract class AbstractFirstBrokerLoginTest extends AbstractIdentityProvi
driver.navigate().back();
Assert.assertTrue(driver.getPageSource().contains("You are already logged in."));
driver.navigate().forward();
- this.loginExpiredPage.assertCurrent();
- this.loginExpiredPage.clickLoginContinueLink();
this.idpConfirmLinkPage.assertCurrent();
// Click browser 'back' on review profile page
this.idpConfirmLinkPage.clickReviewProfile();
this.updateProfilePage.assertCurrent();
driver.navigate().back();
- this.loginExpiredPage.assertCurrent();
- this.loginExpiredPage.clickLoginContinueLink();
this.updateProfilePage.assertCurrent();
this.updateProfilePage.update("Pedro", "Igor", "psilva@redhat.com");
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserButtonsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserButtonsTest.java
index 08d8265..da54a72 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserButtonsTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserButtonsTest.java
@@ -263,11 +263,12 @@ public class BrowserButtonsTest extends AbstractTestRealmKeycloakTest {
registerPage.clickBackToLogin();
loginPage.assertCurrent();
- // Click browser "back" button. Should be back on register page
+ // Click browser "back" button.
driver.navigate().back();
registerPage.assertCurrent();
}
+
@Test
public void clickBackButtonFromRegisterPage() {
loginPage.open();
@@ -280,6 +281,28 @@ public class BrowserButtonsTest extends AbstractTestRealmKeycloakTest {
}
+ // KEYCLOAK-5136
+ @Test
+ public void clickRefreshButtonOnRegisterPage() {
+ loginPage.open();
+ loginPage.clickRegister();
+ registerPage.assertCurrent();
+
+ // Click browser "refresh" button. Should be still on register page
+ driver.navigate().refresh();
+ registerPage.assertCurrent();
+
+ // Click 'back to login'. Should be on login page
+ registerPage.clickBackToLogin();
+ loginPage.assertCurrent();
+
+ // Click browser 'refresh'. Should be still on login page
+ driver.navigate().refresh();
+ loginPage.assertCurrent();
+
+ }
+
+
@Test
public void backButtonToAuthorizationEndpoint() {
loginPage.open();