From f429469fc8b80425ac85b0f0562710c0309a86f8 Mon Sep 17 00:00:00 2001 From: Martin Kanis Date: Fri, 6 Apr 2018 09:26:29 +0200 Subject: [PATCH] KEYCLOAK-5270 Realm cookie path for IE<=11 users (#5106) --- ...finispanAuthenticationSessionProvider.java | 4 +- .../AuthenticationSessionProvider.java | 1 + .../protocol/AuthorizationEndpointBase.java | 18 +- .../managers/AuthenticationManager.java | 42 ++- .../AuthenticationSessionManager.java | 116 +++++-- .../managers/UserSessionCrossDCManager.java | 29 +- .../resources/LoginActionsService.java | 7 +- .../services/resources/SessionCodeChecks.java | 30 +- .../keycloak/services/util/CookieHelper.java | 45 ++- .../testsuite/cookies/CookiesPathTest.java | 292 ++++++++++++++++++ 10 files changed, 493 insertions(+), 91 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cookies/CookiesPathTest.java diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanAuthenticationSessionProvider.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanAuthenticationSessionProvider.java index 12019c1b43..dd0ce0675e 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanAuthenticationSessionProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanAuthenticationSessionProvider.java @@ -90,7 +90,7 @@ public class InfinispanAuthenticationSessionProvider implements AuthenticationSe } - private RootAuthenticationSessionEntity getRootAuthenticationSessionEntity(RealmModel realm, String authSessionId) { + private RootAuthenticationSessionEntity getRootAuthenticationSessionEntity(String authSessionId) { // Chance created in this transaction RootAuthenticationSessionEntity entity = tx.get(cache, authSessionId); @@ -181,7 +181,7 @@ public class InfinispanAuthenticationSessionProvider implements AuthenticationSe @Override public RootAuthenticationSessionModel getRootAuthenticationSession(RealmModel realm, String authenticationSessionId) { - RootAuthenticationSessionEntity entity = getRootAuthenticationSessionEntity(realm, authenticationSessionId); + RootAuthenticationSessionEntity entity = getRootAuthenticationSessionEntity(authenticationSessionId); return wrap(realm, entity); } diff --git a/server-spi/src/main/java/org/keycloak/sessions/AuthenticationSessionProvider.java b/server-spi/src/main/java/org/keycloak/sessions/AuthenticationSessionProvider.java index d879b6ee03..bc32430bac 100644 --- a/server-spi/src/main/java/org/keycloak/sessions/AuthenticationSessionProvider.java +++ b/server-spi/src/main/java/org/keycloak/sessions/AuthenticationSessionProvider.java @@ -20,6 +20,7 @@ package org.keycloak.sessions; import org.keycloak.models.ClientModel; import org.keycloak.models.RealmModel; import org.keycloak.provider.Provider; + import java.util.Map; /** diff --git a/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java b/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java index 619a862bba..da2e376a8a 100755 --- a/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java +++ b/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java @@ -35,12 +35,9 @@ import org.keycloak.protocol.LoginProtocol.Error; import org.keycloak.services.ErrorPageException; import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.managers.AuthenticationSessionManager; -import org.keycloak.services.managers.ClientSessionCode; import org.keycloak.services.managers.UserSessionCrossDCManager; import org.keycloak.services.messages.Messages; import org.keycloak.services.resources.LoginActionsService; -import org.keycloak.services.util.CacheControlUtil; -import org.keycloak.services.util.AuthenticationFlowURLHelper; import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.sessions.RootAuthenticationSessionModel; @@ -170,26 +167,25 @@ public abstract class AuthorizationEndpointBase { protected AuthenticationSessionModel createAuthenticationSession(ClientModel client, String requestState) { AuthenticationSessionManager manager = new AuthenticationSessionManager(session); - String authSessionId = manager.getCurrentAuthenticationSessionId(realm); - RootAuthenticationSessionModel rootAuthSession = authSessionId==null ? null : session.authenticationSessions().getRootAuthenticationSession(realm, authSessionId); + RootAuthenticationSessionModel rootAuthSession = manager.getCurrentRootAuthenticationSession(realm); + AuthenticationSessionModel authSession; if (rootAuthSession != null) { - authSession = rootAuthSession.createAuthenticationSession(client); logger.debugf("Sent request to authz endpoint. Root authentication session with ID '%s' exists. Client is '%s' . Created new authentication session with tab ID: %s", rootAuthSession.getId(), client.getClientId(), authSession.getTabId()); - } else { - - UserSessionModel userSession = authSessionId == null ? null : new UserSessionCrossDCManager(session).getUserSessionIfExistsRemotely(realm, authSessionId); + UserSessionCrossDCManager userSessionCrossDCManager = new UserSessionCrossDCManager(session); + UserSessionModel userSession = userSessionCrossDCManager.getUserSessionIfExistsRemotely(manager, realm); if (userSession != null) { - rootAuthSession = session.authenticationSessions().createRootAuthenticationSession(authSessionId, realm); + String userSessionId = userSession.getId(); + rootAuthSession = session.authenticationSessions().createRootAuthenticationSession(userSessionId, realm); authSession = rootAuthSession.createAuthenticationSession(client); logger.debugf("Sent request to authz endpoint. We don't have root authentication session with ID '%s' but we have userSession." + - "Re-created root authentication session with same ID. Client is: %s . New authentication session tab ID: %s", authSessionId, client.getClientId(), authSession.getTabId()); + "Re-created root authentication session with same ID. Client is: %s . New authentication session tab ID: %s", userSessionId, client.getClientId(), authSession.getTabId()); } else { rootAuthSession = manager.createAuthenticationSession(realm, true); authSession = rootAuthSession.createAuthenticationSession(client); diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index 87df91759f..6a1e89b6c3 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -67,6 +67,7 @@ import java.net.URI; import java.security.PublicKey; import java.util.*; import java.util.stream.Collectors; +import java.util.AbstractMap.SimpleEntry; /** * Stateless object that manages authentication @@ -223,20 +224,22 @@ public class AuthenticationManager { // Account management client is used as a placeholder ClientModel client = SystemClientUtil.getSystemClient(realm); - // Try to lookup current authSessionId from browser cookie. If doesn't exists, use the same as current userSession - String authSessionId = null; + String authSessionId; + RootAuthenticationSessionModel rootLogoutSession = null; boolean browserCookiePresent = false; + + // Try to lookup current authSessionId from browser cookie. If doesn't exists, use the same as current userSession if (browserCookie) { - authSessionId = asm.getCurrentAuthenticationSessionId(realm); + rootLogoutSession = asm.getCurrentRootAuthenticationSession(realm); } - if (authSessionId != null) { + if (rootLogoutSession != null) { + authSessionId = rootLogoutSession.getId(); browserCookiePresent = true; } else { authSessionId = userSession.getId(); + rootLogoutSession = session.authenticationSessions().getRootAuthenticationSession(realm, authSessionId); } - // Try to join existing logout session if it exists - RootAuthenticationSessionModel rootLogoutSession = session.authenticationSessions().getRootAuthenticationSession(realm, authSessionId); if (rootLogoutSession == null) { rootLogoutSession = session.authenticationSessions().createRootAuthenticationSession(authSessionId, realm); } @@ -615,7 +618,20 @@ public class AuthenticationManager { String path = getIdentityCookiePath(realm, uriInfo); expireCookie(realm, KEYCLOAK_IDENTITY_COOKIE, path, true, connection); expireCookie(realm, KEYCLOAK_SESSION_COOKIE, path, false, connection); + + String oldPath = getOldCookiePath(realm, uriInfo); + expireCookie(realm, KEYCLOAK_IDENTITY_COOKIE, oldPath, true, connection); + expireCookie(realm, KEYCLOAK_SESSION_COOKIE, oldPath, false, connection); } + public static void expireOldIdentityCookie(RealmModel realm, UriInfo uriInfo, ClientConnection connection) { + logger.debug("Expiring old identity cookie with wrong path"); + + String oldPath = getOldCookiePath(realm, uriInfo); + expireCookie(realm, KEYCLOAK_IDENTITY_COOKIE, oldPath, true, connection); + expireCookie(realm, KEYCLOAK_SESSION_COOKIE, oldPath, false, connection); + } + + public static void expireRememberMeCookie(RealmModel realm, UriInfo uriInfo, ClientConnection connection) { logger.debug("Expiring remember me cookie"); String path = getIdentityCookiePath(realm, uriInfo); @@ -623,11 +639,24 @@ public class AuthenticationManager { expireCookie(realm, cookieName, path, true, connection); } + public static void expireOldAuthSessionCookie(RealmModel realm, UriInfo uriInfo, ClientConnection connection) { + logger.debugv("Expire {1} cookie .", AuthenticationSessionManager.AUTH_SESSION_ID); + + String oldPath = getOldCookiePath(realm, uriInfo); + expireCookie(realm, AuthenticationSessionManager.AUTH_SESSION_ID, oldPath, true, connection); + } + protected static String getIdentityCookiePath(RealmModel realm, UriInfo uriInfo) { return getRealmCookiePath(realm, uriInfo); } public static String getRealmCookiePath(RealmModel realm, UriInfo uriInfo) { + URI uri = RealmsResource.realmBaseUrl(uriInfo).build(realm.getName()); + // KEYCLOAK-5270 + return uri.getRawPath() + "/"; + } + + public static String getOldCookiePath(RealmModel realm, UriInfo uriInfo) { URI uri = RealmsResource.realmBaseUrl(uriInfo).build(realm.getName()); return uri.getRawPath(); } @@ -658,6 +687,7 @@ public class AuthenticationManager { AuthResult authResult = verifyIdentityToken(session, realm, session.getContext().getUri(), session.getContext().getConnection(), checkActive, false, true, tokenString, session.getContext().getRequestHeaders()); if (authResult == null) { expireIdentityCookie(realm, session.getContext().getUri(), session.getContext().getConnection()); + expireOldIdentityCookie(realm, session.getContext().getUri(), session.getContext().getConnection()); return null; } authResult.getSession().setLastSessionRefresh(Time.currentTime()); diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java index 83b23cf377..696315367a 100644 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java @@ -17,8 +17,6 @@ package org.keycloak.services.managers; -import javax.ws.rs.core.UriInfo; - import org.jboss.logging.Logger; import org.keycloak.common.ClientConnection; import org.keycloak.models.ClientModel; @@ -31,6 +29,13 @@ import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.sessions.RootAuthenticationSessionModel; import org.keycloak.sessions.StickySessionEncoderProvider; +import javax.ws.rs.core.UriInfo; +import java.util.AbstractMap.SimpleEntry; +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + /** * @author Marek Posolda */ @@ -38,6 +43,8 @@ public class AuthenticationSessionManager { public static final String AUTH_SESSION_ID = "AUTH_SESSION_ID"; + public static final int AUTH_SESSION_LIMIT = 3; + private static final Logger log = Logger.getLogger(AuthenticationSessionManager.class); private final KeycloakSession session; @@ -64,14 +71,40 @@ public class AuthenticationSessionManager { return rootAuthSession; } + public RootAuthenticationSessionModel getCurrentRootAuthenticationSession(RealmModel realm) { + List authSessionIds = getAuthSessionCookieIds(realm); - /** - * Returns ID of current authentication session if it exists, otherwise returns {@code null}. - * @param realm - * @return - */ - public String getCurrentAuthenticationSessionId(RealmModel realm) { - return getAuthSessionCookieDecoded(realm); + return authSessionIds.stream().map(id -> { + SimpleEntry entry = decodeAuthSessionId(id); + String sessionId = entry.getKey(); + + RootAuthenticationSessionModel rootAuthSession = session.authenticationSessions().getRootAuthenticationSession(realm, sessionId); + + if (rootAuthSession != null) { + reencodeAuthSessionCookie(sessionId, entry.getValue(), realm); + return rootAuthSession; + } + + return null; + }).filter(authSession -> Objects.nonNull(authSession)).findFirst().orElse(null); + } + + public UserSessionModel getUserSessionFromAuthCookie(RealmModel realm) { + List authSessionIds = getAuthSessionCookieIds(realm); + + return authSessionIds.stream().map(id -> { + SimpleEntry entry = decodeAuthSessionId(id); + String sessionId = entry.getKey(); + + UserSessionModel userSession = session.sessions().getUserSession(realm, sessionId); + + if (userSession != null) { + reencodeAuthSessionCookie(sessionId, entry.getValue(), realm); + return userSession; + } + + return null; + }).filter(authSession -> Objects.nonNull(authSession)).findFirst().orElse(null); } @@ -81,13 +114,21 @@ public class AuthenticationSessionManager { * @return */ public AuthenticationSessionModel getCurrentAuthenticationSession(RealmModel realm, ClientModel client, String tabId) { - String authSessionId = getAuthSessionCookieDecoded(realm); + List authSessionIds = getAuthSessionCookieIds(realm); + + return authSessionIds.stream().map(id -> { + SimpleEntry entry = decodeAuthSessionId(id); + String sessionId = entry.getKey(); + + AuthenticationSessionModel authSession = getAuthenticationSessionByIdAndClient(realm, sessionId, client, tabId); + + if (authSession != null) { + reencodeAuthSessionCookie(sessionId, entry.getValue(), realm); + return authSession; + } - if (authSessionId == null) { return null; - } - - return getAuthenticationSessionByIdAndClient(realm, authSessionId, client, tabId); + }).filter(authSession -> Objects.nonNull(authSession)).findFirst().orElse(null); } @@ -105,30 +146,38 @@ public class AuthenticationSessionManager { log.debugf("Set AUTH_SESSION_ID cookie with value %s", encodedAuthSessionId); } + public SimpleEntry decodeAuthSessionId(String authSessionId) { + log.debugf("Found AUTH_SESSION_ID cookie with value %s", authSessionId); + StickySessionEncoderProvider encoder = session.getProvider(StickySessionEncoderProvider.class); + String decodedAuthSessionId = encoder.decodeSessionId(authSessionId); + String reencoded = encoder.encodeSessionId(decodedAuthSessionId); - public String getAuthSessionCookieDecoded(RealmModel realm) { - String cookieVal = CookieHelper.getCookieValue(AUTH_SESSION_ID); + return new SimpleEntry(decodedAuthSessionId, reencoded); + } - if (cookieVal != null) { - log.debugf("Found AUTH_SESSION_ID cookie with value %s", cookieVal); - - StickySessionEncoderProvider encoder = session.getProvider(StickySessionEncoderProvider.class); - String decodedAuthSessionId = encoder.decodeSessionId(cookieVal); - - // Check if owner of this authentication session changed due to re-hashing (usually node failover or addition of new node) - String reencoded = encoder.encodeSessionId(decodedAuthSessionId); - if (!reencoded.equals(cookieVal)) { - log.debugf("Route changed. Will update authentication session cookie"); - setAuthSessionCookie(decodedAuthSessionId, realm); - } - - return decodedAuthSessionId; - } else { - log.debugf("Not found AUTH_SESSION_ID cookie"); - return null; + public void reencodeAuthSessionCookie(String decodedAuthSessionId, String reencodedAuthSessionId, RealmModel realm) { + if (!decodedAuthSessionId.equals(reencodedAuthSessionId)) { + log.debugf("Route changed. Will update authentication session cookie"); + setAuthSessionCookie(decodedAuthSessionId, realm); } } + public List getAuthSessionCookieIds(RealmModel realm) { + Set cookiesVal = CookieHelper.getCookieValue(AUTH_SESSION_ID); + + if (cookiesVal.size() > 1) { + AuthenticationManager.expireOldAuthSessionCookie(realm, session.getContext().getUri(), session.getContext().getConnection()); + } + + List authSessionIds = cookiesVal.stream().limit(AUTH_SESSION_LIMIT).collect(Collectors.toList()); + + if (authSessionIds.isEmpty()) { + log.debugf("Not found AUTH_SESSION_ID cookie"); + } + + return authSessionIds; + } + public void removeAuthenticationSession(RealmModel realm, AuthenticationSessionModel authSession, boolean expireRestartCookie) { RootAuthenticationSessionModel rootAuthSession = authSession.getParentSession(); @@ -156,5 +205,4 @@ public class AuthenticationSessionManager { RootAuthenticationSessionModel rootAuthSession = session.authenticationSessions().getRootAuthenticationSession(realm, authSessionId); return rootAuthSession==null ? null : rootAuthSession.getAuthenticationSession(client, tabId); } - } diff --git a/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java b/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java index 476022c0cd..bd56400809 100644 --- a/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java +++ b/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java @@ -17,14 +17,15 @@ package org.keycloak.services.managers; -import java.util.Map; +import java.util.AbstractMap.SimpleEntry; +import java.util.List; +import java.util.Objects; import org.jboss.logging.Logger; import org.keycloak.models.AuthenticatedClientSessionModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserSessionModel; -import org.keycloak.sessions.CommonClientSessionModel; /** * @author Marek Posolda @@ -60,14 +61,24 @@ public class UserSessionCrossDCManager { // Just check if userSession also exists on remoteCache. It can happen that logout happened on 2nd DC and userSession is already removed on remoteCache and this DC wasn't yet notified - public UserSessionModel getUserSessionIfExistsRemotely(RealmModel realm, String id) { - UserSessionModel userSession = kcSession.sessions().getUserSession(realm, id); + public UserSessionModel getUserSessionIfExistsRemotely(AuthenticationSessionManager asm, RealmModel realm) { + List sessionIds = asm.getAuthSessionCookieIds(realm); - // This will remove userSession "locally" if it doesn't exists on remoteCache - kcSession.sessions().getUserSessionWithPredicate(realm, id, false, (UserSessionModel userSession2) -> { - return userSession2 == null; - }); + return sessionIds.stream().map(id -> { + SimpleEntry entry = asm.decodeAuthSessionId(id); + String sessionId = entry.getKey(); - return kcSession.sessions().getUserSession(realm, id); + // This will remove userSession "locally" if it doesn't exists on remoteCache + kcSession.sessions().getUserSessionWithPredicate(realm, sessionId, false, (UserSessionModel userSession2) -> userSession2 == null); + + UserSessionModel userSession = kcSession.sessions().getUserSession(realm, sessionId); + + if (userSession != null) { + asm.reencodeAuthSessionCookie(sessionId, entry.getValue(), realm); + return userSession; + } + + return null; + }).filter(userSession -> Objects.nonNull(userSession)).findFirst().orElse(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 f517c7d162..1c9ff23b3a 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -322,7 +322,7 @@ public class LoginActionsService { @QueryParam(Constants.TAB_ID) String tabId, @QueryParam(Constants.KEY) String key) { if (key != null) { - return handleActionToken(authSessionId, key, execution, clientId, tabId); + return handleActionToken(key, execution, clientId, tabId); } event.event(EventType.RESET_PASSWORD); @@ -422,10 +422,10 @@ public class LoginActionsService { @QueryParam("execution") String execution, @QueryParam("client_id") String clientId, @QueryParam(Constants.TAB_ID) String tabId) { - return handleActionToken(authSessionId, key, execution, clientId, tabId); + return handleActionToken(key, execution, clientId, tabId); } - protected Response handleActionToken(String authSessionId, String tokenString, String execution, String clientId, String tabId) { + protected Response handleActionToken(String tokenString, String execution, String clientId, String tabId) { T token; ActionTokenHandler handler; ActionTokenContext tokenContext; @@ -442,7 +442,6 @@ public class LoginActionsService { AuthenticationSessionManager authenticationSessionManager = new AuthenticationSessionManager(session); if (client != null) { session.getContext().setClient(client); - authSessionId = authSessionId == null ? authenticationSessionManager.getAuthSessionCookieDecoded(realm) : authSessionId; authSession = authenticationSessionManager.getCurrentAuthenticationSession(realm, client, tabId); } 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 bb4097f459..e61818a1c0 100644 --- a/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java +++ b/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java @@ -18,6 +18,10 @@ package org.keycloak.services.resources; import java.net.URI; +import java.util.AbstractMap.SimpleEntry; +import java.util.List; +import java.util.Objects; +import java.util.Optional; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriBuilder; @@ -73,7 +77,6 @@ public class SessionCodeChecks { private final String flowPath; private final String authSessionId; - public SessionCodeChecks(RealmModel realm, UriInfo uriInfo, HttpRequest request, ClientConnection clientConnection, KeycloakSession session, EventBuilder event, String authSessionId, String code, String execution, String clientId, String tabId, String flowPath) { this.realm = realm; @@ -175,28 +178,22 @@ public class SessionCodeChecks { } // See if we are already authenticated and userSession with same ID exists. - String sessionId = authSessionManager.getCurrentAuthenticationSessionId(realm); - RootAuthenticationSessionModel existingRootAuthSession = null; - if (sessionId != null) { - UserSessionModel userSession = session.sessions().getUserSession(realm, sessionId); - if (userSession != null) { + UserSessionModel userSession = authSessionManager.getUserSessionFromAuthCookie(realm); - LoginFormsProvider loginForm = session.getProvider(LoginFormsProvider.class).setAuthenticationSession(authSession) - .setSuccess(Messages.ALREADY_LOGGED_IN); + if (userSession != null) { + LoginFormsProvider loginForm = session.getProvider(LoginFormsProvider.class).setAuthenticationSession(authSession) + .setSuccess(Messages.ALREADY_LOGGED_IN); - if (client == null) { - loginForm.setAttribute(Constants.SKIP_LINK, true); - } - - response = loginForm.createInfoPage(); - return null; + if (client == null) { + loginForm.setAttribute(Constants.SKIP_LINK, true); } - - existingRootAuthSession = session.authenticationSessions().getRootAuthenticationSession(realm, sessionId); + response = loginForm.createInfoPage(); + return null; } // Otherwise just try to restart from the cookie + RootAuthenticationSessionModel existingRootAuthSession = authSessionManager.getCurrentRootAuthenticationSession(realm); response = restartAuthenticationSessionFromCookie(existingRootAuthSession); return null; } @@ -433,5 +430,4 @@ public class SessionCodeChecks { return new AuthenticationFlowURLHelper(session, realm, uriInfo) .showPageExpired(authSession); } - } diff --git a/services/src/main/java/org/keycloak/services/util/CookieHelper.java b/services/src/main/java/org/keycloak/services/util/CookieHelper.java index 2005695b54..b31f9f782b 100755 --- a/services/src/main/java/org/keycloak/services/util/CookieHelper.java +++ b/services/src/main/java/org/keycloak/services/util/CookieHelper.java @@ -17,19 +17,20 @@ package org.keycloak.services.util; -import java.net.URI; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import org.jboss.logging.Logger; import org.jboss.resteasy.spi.HttpResponse; import org.jboss.resteasy.spi.ResteasyProviderFactory; import org.keycloak.common.util.ServerCookie; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.RealmModel; -import org.keycloak.services.managers.AuthenticationManager; import javax.ws.rs.core.Cookie; import javax.ws.rs.core.HttpHeaders; -import javax.ws.rs.core.UriBuilder; -import javax.ws.rs.core.UriInfo; + /** * @author Bill Burke @@ -37,6 +38,8 @@ import javax.ws.rs.core.UriInfo; */ public class CookieHelper { + private static final Logger logger = Logger.getLogger(CookieHelper.class); + /** * Set a response cookie. This solely exists because JAX-RS 1.1 does not support setting HttpOnly cookies * @@ -58,10 +61,36 @@ public class CookieHelper { } - public static String getCookieValue(String name) { + public static Set getCookieValue(String name) { HttpHeaders headers = ResteasyProviderFactory.getContextData(HttpHeaders.class); + + Set cookiesVal = new HashSet<>(); + + // check for cookies in the request headers + List cookieHeader = headers.getRequestHeaders().get(HttpHeaders.COOKIE); + if (cookieHeader != null) { + logger.debugv("{1} cookie found in the request's header", name); + cookieHeader.stream().map(s -> parseCookie(s, name)).forEach(cookiesVal::addAll); + } + + // get cookies from the cookie field Cookie cookie = headers.getCookies().get(name); - return cookie != null ? cookie.getValue() : null; + if (cookie != null) { + logger.debugv("{1} cookie found in the cookie's field", name); + cookiesVal.add(cookie.getValue()); + } + + + return cookiesVal; } + + public static Set parseCookie(String cookieHeader, String name) { + String parts[] = cookieHeader.split("[;,]"); + + Set cookies = Arrays.stream(parts).filter(part -> part.startsWith(name + "=")).map(part -> + part.substring(part.indexOf('=') + 1)).collect(Collectors.toSet()); + + return cookies; + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cookies/CookiesPathTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cookies/CookiesPathTest.java new file mode 100644 index 0000000000..7cf724bb5b --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cookies/CookiesPathTest.java @@ -0,0 +1,292 @@ +package org.keycloak.testsuite.cookies; + +import org.apache.commons.io.IOUtils; +import org.apache.http.NameValuePair; +import org.apache.http.client.CookieStore; +import org.apache.http.client.entity.UrlEncodedFormEntity; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.client.methods.HttpRequestBase; +import org.apache.http.impl.client.BasicCookieStore; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.client.LaxRedirectStrategy; +import org.apache.http.impl.cookie.BasicClientCookie; +import org.apache.http.message.BasicNameValuePair; +import org.apache.http.protocol.HttpCoreContext; +import org.hamcrest.Matchers; +import org.jboss.arquillian.graphene.page.Page; +import org.junit.Assert; +import org.junit.Test; +import org.keycloak.models.AccountRoles; +import org.keycloak.models.AdminRoles; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.services.managers.AuthenticationManager; +import org.keycloak.services.managers.AuthenticationSessionManager; +import org.keycloak.testsuite.AbstractKeycloakTest; +import org.keycloak.testsuite.ActionURIUtils; +import org.keycloak.testsuite.pages.LoginPage; +import org.keycloak.testsuite.util.OAuthClient; +import org.keycloak.testsuite.util.RealmBuilder; +import org.keycloak.testsuite.util.URLUtils; +import org.keycloak.testsuite.util.UserBuilder; +import org.openqa.selenium.Cookie; + +import java.io.IOException; +import java.util.Calendar; +import java.util.LinkedList; +import java.util.List; +import java.util.Set; + +/** + * @author Martin Kanis + */ +public class CookiesPathTest extends AbstractKeycloakTest { + + @Page + protected LoginPage loginPage; + + public static final String AUTH_SESSION_VALUE = "1869c345-2f90-4724-936d-a1a1ef41dea7"; + + public static final String AUTH_SESSION_VALUE_NODE = "1869c345-2f90-4724-936d-a1a1ef41dea7.host"; + + public static final String OLD_COOKIE_PATH = "/auth/realms/foo"; + + public static final String KC_RESTART = "KC_RESTART"; + + @Test + public void testCookiesPath() { + // navigate to "/realms/foo/account" and remove cookies in the browser for the current path + // first access to the path means there are no cookies being sent + // we are redirected to login page and Keycloak sets cookie's path to "/auth/realms/foo/" + deleteAllCookiesForRealm("foo"); + + Assert.assertTrue("There shouldn't be any cookies sent!", driver.manage().getCookies().isEmpty()); + + // refresh the page and cookies are sent within the request + driver.navigate().refresh(); + + Set cookies = driver.manage().getCookies(); + Assert.assertTrue("There should be cookies sent!", cookies.size() > 0); + // check cookie's path, for some reason IE adds extra slash to the beginning of the path + cookies.stream().forEach(cookie -> Assert.assertThat(cookie.getPath(), Matchers.endsWith("/auth/realms/foo/"))); + + // now navigate to realm which name overlaps the first realm and delete cookies for that realm (foobar) + deleteAllCookiesForRealm("foobar"); + + // cookies shouldn't be sent for the first access to /realms/foobar/account + // At this moment IE would sent cookies for /auth/realms/foo without the fix + cookies = driver.manage().getCookies(); + Assert.assertTrue("There shouldn't be any cookies sent!", cookies.isEmpty()); + + // refresh the page and check if correct cookies were sent + driver.navigate().refresh(); + cookies = driver.manage().getCookies(); + + Assert.assertTrue("There should be cookies sent!", cookies.size() > 0); + // check cookie's path, for some reason IE adds extra slash to the beginning of the path + cookies.stream().forEach(cookie -> Assert.assertThat(cookie.getPath(), Matchers.endsWith("/auth/realms/foobar/"))); + + // lets back to "/realms/foo/account" to test the cookies for "foo" realm are still there and haven't been (correctly) sent to "foobar" + URLUtils.navigateToUri( oauth.AUTH_SERVER_ROOT + "/realms/foo/account", true); + + cookies = driver.manage().getCookies(); + Assert.assertTrue("There should be cookies sent!", cookies.size() > 0); + cookies.stream().forEach(cookie -> Assert.assertThat(cookie.getPath(), Matchers.endsWith("/auth/realms/foo/"))); + } + + @Test + public void testMultipleCookies() throws IOException { + String requestURI = oauth.AUTH_SERVER_ROOT + "/realms/foo/account"; + Calendar calendar = Calendar.getInstance(); + calendar.add(Calendar.DAY_OF_YEAR, 1); + + // create old cookie with wrong path + BasicClientCookie wrongCookie = new BasicClientCookie(AuthenticationSessionManager.AUTH_SESSION_ID, AUTH_SESSION_VALUE); + wrongCookie.setDomain("localhost"); + wrongCookie.setPath(OLD_COOKIE_PATH); + wrongCookie.setExpiryDate(calendar.getTime()); + + // obtain new cookies + CookieStore cookieStore = getCorrectCookies(requestURI); + cookieStore.addCookie(wrongCookie); + + Assert.assertThat(cookieStore.getCookies(), Matchers.hasSize(3)); + + CloseableHttpResponse response = login(requestURI, cookieStore); + response.close(); + + // old cookie has been removed + // now we have AUTH_SESSION_ID, KEYCLOAK_IDENTITY, KEYCLOAK_SESSION, OAuth_Token_Request_State + Assert.assertThat(cookieStore.getCookies(), Matchers.hasSize(4)); + + // does each cookie's path end with "/" + cookieStore.getCookies().stream().filter(c -> !"OAuth_Token_Request_State".equals(c.getName())) + .map(c -> c.getPath()).forEach(path ->Assert.assertThat(path, Matchers.endsWith("/"))); + + // KEYCLOAK_SESSION should end by AUTH_SESSION_ID value + String authSessionId = cookieStore.getCookies().stream().filter(c -> "AUTH_SESSION_ID".equals(c.getName())).findFirst().get().getValue(); + String KCSessionId = cookieStore.getCookies().stream().filter(c -> "KEYCLOAK_SESSION".equals(c.getName())).findFirst().get().getValue(); + String KCSessionSuffix = KCSessionId.split("/")[2]; + Assert.assertThat(authSessionId, Matchers.containsString(KCSessionSuffix)); + } + + @Test + public void testOldCookieWithWrongPath() { + Cookie wrongCookie = new Cookie(AuthenticationSessionManager.AUTH_SESSION_ID, AUTH_SESSION_VALUE, + null, OLD_COOKIE_PATH, null, false, true); + + deleteAllCookiesForRealm("foo"); + + // add old cookie with wrong path + driver.manage().addCookie(wrongCookie); + Set cookies = driver.manage().getCookies(); + Assert.assertThat(cookies, Matchers.hasSize(1)); + + oauth.realm("foo").redirectUri(OAuthClient.AUTH_SERVER_ROOT + "/realms/foo/account").clientId("account").openLoginForm(); + + loginPage.login("foo", "password"); + + // old cookie has been removed and new cookies have been added + cookies = driver.manage().getCookies(); + Assert.assertThat(cookies, Matchers.hasSize(3)); + + // does each cookie's path end with "/" + cookies.stream().map(c -> c.getPath()).forEach(path -> + Assert.assertThat(path, Matchers.endsWith("/"))); + + // KEYCLOAK_SESSION should end by AUTH_SESSION_ID value + String authSessionId = cookies.stream().filter(c -> "AUTH_SESSION_ID".equals(c.getName())).findFirst().get().getValue(); + String KCSessionId = cookies.stream().filter(c -> "KEYCLOAK_SESSION".equals(c.getName())).findFirst().get().getValue(); + String KCSessionSuffix = KCSessionId.split("/")[2]; + Assert.assertThat(authSessionId, Matchers.containsString(KCSessionSuffix)); + } + + @Test + public void testOldCookieWithNodeInValue() throws IOException { + String requestURI = oauth.AUTH_SERVER_ROOT + "/realms/foo/account"; + Calendar calendar = Calendar.getInstance(); + calendar.add(Calendar.DAY_OF_YEAR, 1); + + // create old cookie with wrong path + BasicClientCookie wrongCookie = new BasicClientCookie(AuthenticationSessionManager.AUTH_SESSION_ID, AUTH_SESSION_VALUE_NODE); + wrongCookie.setDomain("localhost"); + wrongCookie.setPath(OLD_COOKIE_PATH); + wrongCookie.setExpiryDate(calendar.getTime()); + + // obtain new cookies + CookieStore cookieStore = getCorrectCookies(requestURI); + cookieStore.addCookie(wrongCookie); + + Assert.assertThat(cookieStore.getCookies(), Matchers.hasSize(3)); + + CloseableHttpResponse response = login(requestURI, cookieStore); + response.close(); + + // old cookie has been removed + // now we have AUTH_SESSION_ID, KEYCLOAK_IDENTITY, KEYCLOAK_SESSION, OAuth_Token_Request_State + Assert.assertThat(cookieStore.getCookies(), Matchers.hasSize(4)); + + // does each cookie's path end with "/" + cookieStore.getCookies().stream().filter(c -> !"OAuth_Token_Request_State".equals(c.getName())) + .map(c -> c.getPath()).forEach(path ->Assert.assertThat(path, Matchers.endsWith("/"))); + + // KEYCLOAK_SESSION should end by AUTH_SESSION_ID value + String authSessionId = cookieStore.getCookies().stream().filter(c -> "AUTH_SESSION_ID".equals(c.getName())).findFirst().get().getValue(); + String KCSessionId = cookieStore.getCookies().stream().filter(c -> "KEYCLOAK_SESSION".equals(c.getName())).findFirst().get().getValue(); + String KCSessionSuffix = KCSessionId.split("/")[2]; + Assert.assertThat(authSessionId, Matchers.containsString(KCSessionSuffix)); + } + + /** + * Add two realms which names are overlapping i.e foo and foobar + * @param testRealms + */ + @Override + public void addTestRealms(List testRealms) { + RealmBuilder foo = RealmBuilder.create().name("foo"); + foo.user(UserBuilder.create().username("foo").password("password").role("account", AdminRoles.ADMIN) + .role("account", AccountRoles.MANAGE_ACCOUNT).role("account", AccountRoles.VIEW_PROFILE).role("account", AccountRoles.MANAGE_ACCOUNT_LINKS)); + testRealms.add(foo.build()); + + RealmBuilder foobar = RealmBuilder.create().name("foobar"); + foo.user(UserBuilder.create().username("foobar").password("password").role("account", AdminRoles.ADMIN) + .role("account", AccountRoles.MANAGE_ACCOUNT).role("account", AccountRoles.VIEW_PROFILE).role("account", AccountRoles.MANAGE_ACCOUNT_LINKS)); + testRealms.add(foobar.build()); + } + + private CloseableHttpResponse sendRequest(HttpRequestBase request, CookieStore cookieStore, HttpCoreContext localContext) throws IOException { + CloseableHttpClient c = HttpClientBuilder.create().setDefaultCookieStore(cookieStore).setRedirectStrategy(new LaxRedirectStrategy()).build(); + + CloseableHttpResponse response = c.execute(request, localContext); + + return response; + } + + private CookieStore getCorrectCookies(String uri) throws IOException { + CookieStore cookieStore = new BasicCookieStore(); + + HttpGet request = new HttpGet(uri); + CloseableHttpResponse response = sendRequest(request, new BasicCookieStore(), new HttpCoreContext()); + + for (org.apache.http.Header h: response.getHeaders("Set-Cookie")) { + if (h.getValue().contains(AuthenticationSessionManager.AUTH_SESSION_ID)) { + cookieStore.addCookie(parseCookie(h.getValue(), AuthenticationSessionManager.AUTH_SESSION_ID)); + } else if (h.getValue().contains(KC_RESTART)) { + cookieStore.addCookie(parseCookie(h.getValue(), KC_RESTART)); + } + } + + response.close(); + + return cookieStore; + } + + private BasicClientCookie parseCookie(String line, String name) { + Calendar calendar = Calendar.getInstance(); + calendar.add(Calendar.DAY_OF_YEAR, 1); + + String path = ""; + String value = ""; + + for (String s: line.split(";")) { + if (s.contains(name)) { + String[] split = s.split("="); + value = split[1]; + } else if (s.contains("Path")) { + String[] split = s.split("="); + path = split[1]; + } + } + + BasicClientCookie c = new BasicClientCookie(name, value); + c.setExpiryDate(calendar.getTime()); + c.setDomain("localhost"); + c.setPath(path); + + return c; + } + + private CloseableHttpResponse login(String requestURI, CookieStore cookieStore) throws IOException { + HttpCoreContext httpContext = new HttpCoreContext(); + HttpGet request = new HttpGet(requestURI); + + // send an initial request, we are redirected to login page + CloseableHttpResponse response = sendRequest(request, cookieStore, httpContext); + String s = IOUtils.toString(response.getEntity().getContent(), "UTF-8"); + response.close(); + String action = ActionURIUtils.getActionURIFromPageSource(s); + + // send credentials to login form + HttpPost post = new HttpPost(action); + List params = new LinkedList<>(); + params.add(new BasicNameValuePair("username", "foo")); + params.add(new BasicNameValuePair("password", "password")); + + post.setHeader("Content-Type", "application/x-www-form-urlencoded"); + post.setEntity(new UrlEncodedFormEntity(params)); + + return sendRequest(post, cookieStore, httpContext); + } +}