Remove support for multiple AUTH_SESSION_ID cookies (#26462)

Closes #26457

Signed-off-by: stianst <stianst@gmail.com>
This commit is contained in:
Stian Thorgersen 2024-01-25 06:58:42 +01:00 committed by GitHub
parent 7f195acc14
commit cbfdae5e75
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 57 additions and 145 deletions

View file

@ -48,8 +48,6 @@ public class AuthenticationSessionManager {
public static final String AUTH_SESSION_ID = "AUTH_SESSION_ID";
public static final int AUTH_SESSION_COOKIE_LIMIT = 3;
private static final Logger log = Logger.getLogger(AuthenticationSessionManager.class);
private final KeycloakSession session;
@ -78,9 +76,11 @@ public class AuthenticationSessionManager {
public RootAuthenticationSessionModel getCurrentRootAuthenticationSession(RealmModel realm) {
List<String> authSessionCookies = getAuthSessionCookies(realm);
String oldEncodedId = getAuthSessionCookies(realm);
if (oldEncodedId == null) {
return null;
}
return authSessionCookies.stream().map(oldEncodedId -> {
AuthSessionId authSessionId = decodeAuthSessionId(oldEncodedId);
String sessionId = authSessionId.getDecodedId();
@ -89,41 +89,22 @@ public class AuthenticationSessionManager {
if (rootAuthSession != null) {
reencodeAuthSessionCookie(oldEncodedId, authSessionId, realm);
return rootAuthSession;
}
} else {
return null;
}).filter(authSession -> Objects.nonNull(authSession)).findFirst().orElse(null);
}
public UserSessionModel getUserSessionFromAuthCookie(RealmModel realm) {
List<String> authSessionCookies = getAuthSessionCookies(realm);
return authSessionCookies.stream().map(oldEncodedId -> {
AuthSessionId authSessionId = decodeAuthSessionId(oldEncodedId);
String sessionId = authSessionId.getDecodedId();
UserSessionModel userSession = session.sessions().getUserSession(realm, sessionId);
if (userSession != null) {
reencodeAuthSessionCookie(oldEncodedId, authSessionId, realm);
return userSession;
}
return null;
}).filter(authSession -> Objects.nonNull(authSession)).findFirst().orElse(null);
}
/**
* Returns current authentication session if it exists, otherwise returns {@code null}.
* @param realm
* @return
*/
public AuthenticationSessionModel getCurrentAuthenticationSession(RealmModel realm, ClientModel client, String tabId) {
List<String> authSessionCookies = getAuthSessionCookies(realm);
String oldEncodedId = getAuthSessionCookies(realm);
if (oldEncodedId == null) {
return null;
}
return authSessionCookies.stream().map(oldEncodedId -> {
AuthSessionId authSessionId = decodeAuthSessionId(oldEncodedId);
String sessionId = authSessionId.getDecodedId();
@ -132,10 +113,9 @@ public class AuthenticationSessionManager {
if (authSession != null) {
reencodeAuthSessionCookie(oldEncodedId, authSessionId, realm);
return authSession;
}
} else {
return null;
}).filter(authSession -> Objects.nonNull(authSession)).findFirst().orElse(null);
}
}
@ -184,28 +164,22 @@ public class AuthenticationSessionManager {
/**
* @param realm
* @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" )
* @return the value of the AUTH_SESSION_ID cookie. 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.getCookieValues(session, AUTH_SESSION_ID);
List<String> authSessionIds = cookiesVal.stream().limit(AUTH_SESSION_COOKIE_LIMIT).collect(Collectors.toList());
if (authSessionIds.isEmpty()) {
log.debugf("Not found AUTH_SESSION_ID cookie");
String getAuthSessionCookies(RealmModel realm) {
String oldEncodedId = CookieHelper.getCookieValue(session, AUTH_SESSION_ID);
if (oldEncodedId == null || oldEncodedId.isEmpty()) {
return null;
}
return authSessionIds.stream().filter(new Predicate<String>() {
@Override
public boolean test(String id) {
StickySessionEncoderProvider encoder = session.getProvider(StickySessionEncoderProvider.class);
// in case the id is encoded with a route when running in a cluster
String decodedId = encoder.decodeSessionId(id);
String decodedId = encoder.decodeSessionId(oldEncodedId);
// we can't blindly trust the cookie and assume it is valid and referencing a valid root auth session
// but make sure the root authentication session actually exists
// without this check there is a risk of resolving user sessions from invalid root authentication sessions as they share the same id
return session.authenticationSessions().getRootAuthenticationSession(realm, decodedId) != null;
}
}).collect(Collectors.toList());
RootAuthenticationSessionModel rootAuthenticationSession = session.authenticationSessions().getRootAuthenticationSession(realm, decodedId);
return rootAuthenticationSession != null ? oldEncodedId : null;
}

View file

@ -64,9 +64,9 @@ 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(AuthenticationSessionManager asm, RealmModel realm) {
List<String> sessionCookies = asm.getAuthSessionCookies(realm);
String oldEncodedId = asm.getAuthSessionCookies(realm);
if (sessionCookies.isEmpty()) {
if (oldEncodedId == null) {
// ideally, we should not rely on auth session id to retrieve user sessions
// in case the auth session was removed, we fall back to the identity cookie
// we are here doing the user session lookup twice, however the second lookup is going to make sure the
@ -74,11 +74,12 @@ public class UserSessionCrossDCManager {
AuthenticationManager.AuthResult authResult = authenticateIdentityCookie(kcSession, realm, true);
if (authResult != null && authResult.getSession() != null) {
sessionCookies = Collections.singletonList(authResult.getSession().getId());
oldEncodedId = authResult.getSession().getId();
} else {
return null;
}
}
return sessionCookies.stream().map(oldEncodedId -> {
AuthSessionId authSessionId = asm.decodeAuthSessionId(oldEncodedId);
String sessionId = authSessionId.getDecodedId();
@ -90,9 +91,8 @@ public class UserSessionCrossDCManager {
if (userSession != null) {
asm.reencodeAuthSessionCookie(oldEncodedId, authSessionId, realm);
return userSession;
}
} else {
return null;
}).filter(userSession -> Objects.nonNull(userSession)).findFirst().orElse(null);
}
}
}

View file

@ -17,18 +17,12 @@
package org.keycloak.services.util;
import org.jboss.logging.Logger;
import jakarta.ws.rs.core.Cookie;
import org.keycloak.http.HttpCookie;
import org.keycloak.http.HttpResponse;
import org.jboss.resteasy.util.CookieParser;
import org.keycloak.models.KeycloakSession;
import jakarta.ws.rs.core.Cookie;
import jakarta.ws.rs.core.HttpHeaders;
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import static org.keycloak.common.util.ServerCookie.SameSiteAttributeValue;
@ -41,8 +35,6 @@ public class CookieHelper {
public static final String LEGACY_COOKIE = "_LEGACY";
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
* @param name
@ -101,49 +93,4 @@ public class CookieHelper {
return cookie != null ? cookie.getValue() : null;
}
public static Set<String> getCookieValues(KeycloakSession session, String name) {
Set<String> ret = getInternalCookieValue(session, name);
if (ret.size() == 0) {
String legacy = name + LEGACY_COOKIE;
logger.debugv("Could not find any cookies with name {0}, trying {1}", name, legacy);
ret = getInternalCookieValue(session, legacy);
}
return ret;
}
private static Set<String> getInternalCookieValue(KeycloakSession session, String name) {
HttpHeaders headers = session.getContext().getHttpRequest().getHttpHeaders();
Set<String> cookiesVal = new HashSet<>();
// check for cookies in the request headers
cookiesVal.addAll(parseCookie(headers.getRequestHeaders().getFirst(HttpHeaders.COOKIE), name));
// get cookies from the cookie field
Cookie cookie = headers.getCookies().get(name);
if (cookie != null) {
logger.debugv("{0} cookie found in the cookie field", name);
cookiesVal.add(cookie.getValue());
}
return cookiesVal;
}
private static Set<String> parseCookie(String header, String name) {
if (header == null || name == null) {
return Collections.emptySet();
}
Set<String> values = new HashSet<>();
for (Cookie cookie : CookieParser.parseCookies(header)) {
if (name.equals(cookie.getName())) {
logger.debugv("{0} cookie found in the request header", name);
values.add(cookie.getValue());
}
}
return values;
}
}

View file

@ -15,7 +15,6 @@ import org.keycloak.testsuite.client.KeycloakTestingClient;
import java.io.IOException;
import java.util.List;
import java.util.Set;
public class CookieHelperTest extends AbstractKeycloakTest {
@ -42,8 +41,8 @@ public class CookieHelperTest extends AbstractKeycloakTest {
filter.setHeader("Cookie", "terms_user=; KC_RESTART=eyJhbGciOiJIUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJhZDUyMjdhMy1iY2ZkLTRjZjAtYTdiNi0zOTk4MzVhMDg1NjYifQ.eyJjaWQiOiJodHRwczovL3Nzby5qYm9zcy5vcmciLCJwdHkiOiJzYW1sIiwicnVyaSI6Imh0dHBzOi8vc3NvLmpib3NzLm9yZy9sb2dpbj9wcm92aWRlcj1SZWRIYXRFeHRlcm5hbFByb3ZpZGVyIiwiYWN0IjoiQVVUSEVOVElDQVRFIiwibm90ZXMiOnsiU0FNTF9SRVFVRVNUX0lEIjoibXBmbXBhYWxkampqa2ZmcG5oYmJoYWdmZmJwam1rbGFqbWVlb2lsaiIsInNhbWxfYmluZGluZyI6InBvc3QifX0.d0QJSOQ6pJGzqcjqDTRwkRpU6fwYeICedL6R9Gqs8CQ; AUTH_SESSION_ID=451ec4be-a0c8-430e-b489-6580f195ccf0; AUTH_SESSION_ID=55000981-8b5e-4c8d-853f-ee4c582c1d0d;AUTH_SESSION_ID=451ec4be-a0c8-430e-b489-6580f195ccf0; AUTH_SESSION_ID=55000981-8b5e-4c8d-853f-ee4c582c1d0d;AUTH_SESSION_ID=451ec4be-a0c8-430e-b489-6580f195ccf0; AUTH_SESSION_ID=55000981-8b5e-4c8d-853f-ee4c582c1d0d4;");
testing.server().run(session -> {
Set<String> authSessionIds = CookieHelper.getCookieValues(session, "AUTH_SESSION_ID");
Assert.assertEquals(3, authSessionIds.size());
String authSessionId = CookieHelper.getCookieValue(session, "AUTH_SESSION_ID");
Assert.assertEquals("55000981-8b5e-4c8d-853f-ee4c582c1d0d4", authSessionId);
});
}
@ -53,20 +52,12 @@ public class CookieHelperTest extends AbstractKeycloakTest {
testing.server().run(session -> {
Assert.assertEquals("new", CookieHelper.getCookieValue(session, "MYCOOKIE"));
Set<String> cookieValues = CookieHelper.getCookieValues(session, "MYCOOKIE");
Assert.assertEquals(1, cookieValues.size());
Assert.assertEquals("new", cookieValues.iterator().next());
});
filter.setHeader("Cookie", "MYCOOKIE_LEGACY=legacy");
testing.server().run(session -> {
Assert.assertEquals("legacy", CookieHelper.getCookieValue(session, "MYCOOKIE"));
Set<String> cookieValues = CookieHelper.getCookieValues(session, "MYCOOKIE");
Assert.assertEquals(1, cookieValues.size());
Assert.assertEquals("legacy", cookieValues.iterator().next());
});
}