libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

Merge pull request #4314 from mposolda/KEYCLOAK-5136-browserRefreshImprove

Browse source 
KEYCLOAK-5136 Improve browser refresh button after switch to differen…
This commit is contained in:
Marek Posolda 2017-07-11 14:36:03 +02:00 committed by GitHub
commit 3a13981d3f
6 changed files with 63 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
View file

@ -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 {

8
services/src/main/java/org/keycloak/services/resources/LoginActionsService.java
View file

@ -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);
}
}

14
services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java
View file

@ -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);

27
services/src/main/java/org/keycloak/services/util/BrowserHistoryHelper.java
View file

@ -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;
}

25
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserButtonsTest.java
View file

@ -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();

4
testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java
View file

@ -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");