Skip to content

Commit

Permalink
Remove code that expires old cookie paths (keycloak#26444)
Browse files Browse the repository at this point in the history 
Closes keycloak#26416

Signed-off-by: stianst <[email protected]>
  • Loading branch information
@stianst
stianst authored Jan 24, 2024
1 parent 8609824 commit 85ddac2
Show file tree
Hide file tree
Showing 9 changed files with 141 additions and 237 deletions.
5 changes: 2 additions & 3 deletions services/src/main/java/org/keycloak/forms/login/freemarker/DetachedInfoStateChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,15 @@ public DetachedInfoStateCookie generateAndSetCookie(String messageKey, String me
}

public DetachedInfoStateCookie verifyStateCheckerParameter(String stateCheckerParam) throws VerificationException {
Set<String> cookieVal = CookieHelper.getCookieValue(session, STATE_CHECKER_COOKIE_NAME);
String cookieVal = CookieHelper.getCookieValue(session, STATE_CHECKER_COOKIE_NAME);
if (cookieVal == null || cookieVal.isEmpty()) {
throw new VerificationException("State checker cookie is empty");
}
if (stateCheckerParam == null || stateCheckerParam.isEmpty()) {
throw new VerificationException("State checker parameter is empty");
}

String cookieEncoded = cookieVal.iterator().next();
DetachedInfoStateCookie cookie = session.tokens().decode(cookieEncoded, DetachedInfoStateCookie.class);
DetachedInfoStateCookie cookie = session.tokens().decode(cookieVal, DetachedInfoStateCookie.class);
if (cookie == null) {
throw new VerificationException("Failed to verify DetachedInfoStateCookie");
}
Expand Down
63 changes: 13 additions & 50 deletions services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@
import static org.keycloak.models.light.LightweightUserAdapter.isLightweightUser;
import static org.keycloak.models.UserSessionModel.CORRESPONDING_SESSION_ID;
import static org.keycloak.protocol.oidc.grants.device.DeviceGrantType.isOAuth2DeviceVerificationFlow;
import static org.keycloak.services.util.CookieHelper.getCookie;

/**
* Stateless object that manages authentication
Expand Down Expand Up @@ -211,9 +210,8 @@ public static boolean isOfflineSessionValid(RealmModel realm, UserSessionModel u
public static boolean expireUserSessionCookie(KeycloakSession session, UserSessionModel userSession, RealmModel realm, UriInfo uriInfo, HttpHeaders headers, ClientConnection connection) {
try {
// check to see if any identity cookie is set with the same session and expire it if necessary
Cookie cookie = CookieHelper.getCookie(headers.getCookies(), KEYCLOAK_IDENTITY_COOKIE);
if (cookie == null) return true;
String tokenString = cookie.getValue();
String tokenString = CookieHelper.getCookieValue(session, KEYCLOAK_IDENTITY_COOKIE);
if (tokenString == null) return true;

TokenVerifier<AccessToken> verifier = TokenVerifier.create(tokenString, AccessToken.class)
.realmUrl(Urls.realmIssuer(uriInfo.getBaseUri(), realm.getName()))
Expand Down Expand Up @@ -777,7 +775,7 @@ public static IdentityCookieToken createIdentityToken(KeycloakSession keycloakSe
}

public static void createLoginCookie(KeycloakSession keycloakSession, RealmModel realm, UserModel user, UserSessionModel session, UriInfo uriInfo, ClientConnection connection) {
String cookiePath = getIdentityCookiePath(realm, uriInfo);
String cookiePath = getRealmCookiePath(realm, uriInfo);
String issuer = Urls.realmIssuer(uriInfo.getBaseUri(), realm.getName());
IdentityCookieToken identityCookieToken = createIdentityToken(keycloakSession, realm, user, session, issuer);
String encoded = keycloakSession.tokens().encode(identityCookieToken);
Expand Down Expand Up @@ -805,7 +803,7 @@ public static void createRememberMeCookie(String username, UriInfo uriInfo, Keyc
KeycloakContext context = session.getContext();
RealmModel realm = context.getRealm();
ClientConnection connection = context.getConnection();
String path = getIdentityCookiePath(realm, uriInfo);
String path = getRealmCookiePath(realm, uriInfo);
boolean secureOnly = realm.getSslRequired().isRequired(connection);
// remember me cookie should be persistent (hardcoded to 365 days for now)
//NewCookie cookie = new NewCookie(KEYCLOAK_REMEMBER_ME, "true", path, null, null, realm.getCentralLoginLifespan(), secureOnly);// todo httponly , true);
Expand Down Expand Up @@ -837,66 +835,32 @@ public static String getRememberMeUsername(RealmModel realm, HttpHeaders headers
public static void expireIdentityCookie(RealmModel realm, UriInfo uriInfo, KeycloakSession session) {
ClientConnection connection = session.getContext().getConnection();
logger.debug("Expiring identity cookie");
String path = getIdentityCookiePath(realm, uriInfo);
String path = getRealmCookiePath(realm, uriInfo);
expireCookie(realm, KEYCLOAK_IDENTITY_COOKIE, path, true, connection, SameSiteAttributeValue.NONE, session);
expireCookie(realm, KEYCLOAK_SESSION_COOKIE, path, false, connection, SameSiteAttributeValue.NONE, session);

String oldPath = getOldCookiePath(realm, uriInfo);
expireCookie(realm, KEYCLOAK_IDENTITY_COOKIE, oldPath, true, connection, SameSiteAttributeValue.NONE, session);
expireCookie(realm, KEYCLOAK_SESSION_COOKIE, oldPath, false, connection, SameSiteAttributeValue.NONE, session);
}
public static void expireOldIdentityCookie(RealmModel realm, UriInfo uriInfo, KeycloakSession session) {
ClientConnection connection = session.getContext().getConnection();
logger.debug("Expiring old identity cookie with wrong path");

String oldPath = getOldCookiePath(realm, uriInfo);
expireCookie(realm, KEYCLOAK_IDENTITY_COOKIE, oldPath, true, connection, SameSiteAttributeValue.NONE, session);
expireCookie(realm, KEYCLOAK_SESSION_COOKIE, oldPath, false, connection, SameSiteAttributeValue.NONE, session);
}


public static void expireRememberMeCookie(RealmModel realm, UriInfo uriInfo, KeycloakSession session) {
ClientConnection connection = session.getContext().getConnection();
logger.debug("Expiring remember me cookie");
String path = getIdentityCookiePath(realm, uriInfo);
String path = getRealmCookiePath(realm, uriInfo);
String cookieName = KEYCLOAK_REMEMBER_ME;
expireCookie(realm, cookieName, path, true, connection, null, session);
}

public static void expireOldAuthSessionCookie(RealmModel realm, UriInfo uriInfo, KeycloakSession session) {
logger.debugv("Expire {1} cookie .", AuthenticationSessionManager.AUTH_SESSION_ID);
ClientConnection connection = session.getContext().getConnection();
String oldPath = getOldCookiePath(realm, uriInfo);
expireCookie(realm, AuthenticationSessionManager.AUTH_SESSION_ID, oldPath, true, connection, SameSiteAttributeValue.NONE, session);
}

public static void expireAuthSessionCookie(RealmModel realm, UriInfo uriInfo, KeycloakSession session) {
logger.debugv("Expire {1} cookie .", AuthenticationSessionManager.AUTH_SESSION_ID);
ClientConnection connection = session.getContext().getConnection();
String oldPath = getRealmCookiePath(realm, uriInfo);
expireCookie(realm, AuthenticationSessionManager.AUTH_SESSION_ID, oldPath, true, connection, SameSiteAttributeValue.NONE, session);
}

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

public static String getAccountCookiePath(RealmModel realm, UriInfo uriInfo) {
URI uri = RealmsResource.accountUrl(uriInfo.getBaseUriBuilder()).build(realm.getName());
return uri.getRawPath();
}

public static void expireCookie(RealmModel realm, String cookieName, String path, boolean httpOnly, ClientConnection connection, SameSiteAttributeValue sameSite, KeycloakSession session) {
logger.debugf("Expiring cookie: %s path: %s", cookieName, path);
boolean secureOnly = realm.getSslRequired().isRequired(connection);;
Expand All @@ -908,13 +872,12 @@ public AuthResult authenticateIdentityCookie(KeycloakSession session, RealmModel
}

public static AuthResult authenticateIdentityCookie(KeycloakSession session, RealmModel realm, boolean checkActive) {
Cookie cookie = CookieHelper.getCookie(session.getContext().getRequestHeaders().getCookies(), KEYCLOAK_IDENTITY_COOKIE);
if (cookie == null || "".equals(cookie.getValue())) {
String tokenString = CookieHelper.getCookieValue(session, KEYCLOAK_IDENTITY_COOKIE);
if (tokenString == null || tokenString.isEmpty()) {
logger.debugv("Could not find cookie: {0}", KEYCLOAK_IDENTITY_COOKIE);
return null;
}

String tokenString = cookie.getValue();
AuthResult authResult = verifyIdentityToken(session, realm, session.getContext().getUri(), session.getContext().getConnection(), checkActive, false, null, true, tokenString, session.getContext().getRequestHeaders(), VALIDATE_IDENTITY_COOKIE);
if (authResult == null) {
expireIdentityCookie(realm, session.getContext().getUri(), session);
Expand Down Expand Up @@ -942,10 +905,10 @@ public static Response redirectAfterSuccessfulFlow(KeycloakSession session, Real
ClientSessionContext clientSessionCtx,
HttpRequest request, UriInfo uriInfo, ClientConnection clientConnection,
EventBuilder event, AuthenticationSessionModel authSession, LoginProtocol protocol) {
Cookie sessionCookie = getCookie(request.getHttpHeaders().getCookies(), AuthenticationManager.KEYCLOAK_SESSION_COOKIE);
String sessionCookie = CookieHelper.getCookieValue(session, AuthenticationManager.KEYCLOAK_SESSION_COOKIE);
if (sessionCookie != null) {

String[] split = sessionCookie.getValue().split("/");
String[] split = sessionCookie.split("/");
if (split.length >= 3) {
String oldSessionId = split[2];
if (!oldSessionId.equals(userSession.getId())) {
Expand Down Expand Up @@ -991,13 +954,13 @@ public static Response redirectAfterSuccessfulFlow(KeycloakSession session, Real
}

public static String getSessionIdFromSessionCookie(KeycloakSession session) {
Cookie cookie = getCookie(session.getContext().getRequestHeaders().getCookies(), KEYCLOAK_SESSION_COOKIE);
if (cookie == null || "".equals(cookie.getValue())) {
String cookie = CookieHelper.getCookieValue(session, KEYCLOAK_SESSION_COOKIE);
if (cookie == null || cookie.isEmpty()) {
logger.debugv("Could not find cookie: {0}", KEYCLOAK_SESSION_COOKIE);
return null;
}

String[] parts = cookie.getValue().split("/", 3);
String[] parts = cookie.split("/", 3);
if (parts.length != 3) {
logger.debugv("Cannot parse session value from: {0}", KEYCLOAK_SESSION_COOKIE);
return null;
Expand Down
7 changes: 1 addition & 6 deletions services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,7 @@ void reencodeAuthSessionCookie(String oldEncodedAuthSessionId, AuthSessionId new
* @return list of the values of AUTH_SESSION_ID cookies. It is assumed that values could be encoded with route added (EG. "5e161e00-d426-4ea6-98e9-52eb9844e2d7.node1" )
*/
List<String> getAuthSessionCookies(RealmModel realm) {
Set<String> cookiesVal = CookieHelper.getCookieValue(session, AUTH_SESSION_ID);

if (cookiesVal.size() > 1) {
AuthenticationManager.expireOldAuthSessionCookie(realm, session.getContext().getUri(), session);
}

Set<String> cookiesVal = CookieHelper.getCookieValues(session, AUTH_SESSION_ID);
List<String> authSessionIds = cookiesVal.stream().limit(AUTH_SESSION_COOKIE_LIMIT).collect(Collectors.toList());

if (authSessionIds.isEmpty()) {
Expand Down
25 changes: 11 additions & 14 deletions services/src/main/java/org/keycloak/services/util/CookieHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,17 @@ public static void addCookie(String name, String value, String path, String doma
addCookie(name, value, path, domain, comment, maxAge, secure, httpOnly, null, session);
}

public static String getCookieValue(KeycloakSession session, String name) {
Map<String, Cookie> cookies = session.getContext().getRequestHeaders().getCookies();
Cookie cookie = cookies.get(name);
if (cookie == null) {
String legacy = name + LEGACY_COOKIE;
cookie = cookies.get(legacy);
}
return cookie != null ? cookie.getValue() : null;
}

public static Set<String> getCookieValue(KeycloakSession session, String name) {
public static Set<String> getCookieValues(KeycloakSession session, String name) {
Set<String> ret = getInternalCookieValue(session, name);
if (ret.size() == 0) {
String legacy = name + LEGACY_COOKIE;
Expand Down Expand Up @@ -120,8 +129,7 @@ private static Set<String> getInternalCookieValue(KeycloakSession session, Strin
return cookiesVal;
}


public static Set<String> parseCookie(String header, String name) {
private static Set<String> parseCookie(String header, String name) {
if (header == null || name == null) {
return Collections.emptySet();
}
Expand All @@ -138,15 +146,4 @@ public static Set<String> parseCookie(String header, String name) {
return values;
}

public static Cookie getCookie(Map<String, Cookie> cookies, String name) {
Cookie cookie = cookies.get(name);
if (cookie != null) {
return cookie;
}
else {
String legacy = name + LEGACY_COOKIE;
logger.debugv("Could not find cookie {0}, trying {1}", name, legacy);
return cookies.get(legacy);
}
}
}
35 changes: 0 additions & 35 deletions services/src/test/java/org/keycloak/utils/CookieHelperTest.java
View file

This file was deleted.

5 changes: 1 addition & 4 deletions ...estsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -584,10 +584,7 @@ private AuthDetails repToModel(AuthDetailsRepresentation rep) {
@Path("/get-sso-cookie")
@Produces(MediaType.APPLICATION_JSON)
public String getSSOCookieValue() {
Map<String, Cookie> cookies = request.getHttpHeaders().getCookies();
Cookie cookie = CookieHelper.getCookie(cookies, AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE);
if (cookie == null) return null;
return cookie.getValue();
return CookieHelper.getCookieValue(session, AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE);
}


Expand Down
19 changes: 12 additions & 7 deletions ...uillian/tests/base/src/main/java/org/keycloak/testsuite/client/KeycloakTestingClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,23 @@ public class KeycloakTestingClient implements AutoCloseable {
if (resteasyClient != null) {
client = resteasyClient;
} else {
ResteasyClientBuilder resteasyClientBuilder = (ResteasyClientBuilder) ResteasyClientBuilder.newBuilder();
resteasyClientBuilder.connectionPoolSize(10);
if (serverUrl.startsWith("https")) {
// Disable PKIX path validation errors when running tests using SSL
resteasyClientBuilder.disableTrustManager().hostnameVerification(ResteasyClientBuilder.HostnameVerificationPolicy.ANY);
}
resteasyClientBuilder.httpEngine(AdminClientUtil.getCustomClientHttpEngine(resteasyClientBuilder, 10, null));
ResteasyClientBuilder resteasyClientBuilder = getRestEasyClientBuilder(serverUrl);
client = resteasyClientBuilder.build();
}
target = client.target(serverUrl);
}

public static ResteasyClientBuilder getRestEasyClientBuilder(String serverUrl) {
ResteasyClientBuilder resteasyClientBuilder = (ResteasyClientBuilder) ResteasyClientBuilder.newBuilder();
resteasyClientBuilder.connectionPoolSize(10);
if (serverUrl.startsWith("https")) {
// Disable PKIX path validation errors when running tests using SSL
resteasyClientBuilder.disableTrustManager().hostnameVerification(ResteasyClientBuilder.HostnameVerificationPolicy.ANY);
}
resteasyClientBuilder.httpEngine(AdminClientUtil.getCustomClientHttpEngine(resteasyClientBuilder, 10, null));
return resteasyClientBuilder;
}

public static KeycloakTestingClient getInstance(String serverUrl) {
return new KeycloakTestingClient(serverUrl, null);
}
Expand Down
Loading

0 comments on commit 85ddac2

Please sign in to comment.