Skip to content

Commit

Permalink
Remove support for multiple AUTH_SESSION_ID cookies (keycloak#26462)
Browse files Browse the repository at this point in the history 
Closes keycloak#26457

Signed-off-by: stianst <[email protected]>
  • Loading branch information
@stianst
stianst authored Jan 25, 2024
1 parent 7f195ac commit cbfdae5
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 142 deletions.
98 changes: 36 additions & 62 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 @@ -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;
Expand Down Expand Up @@ -78,64 +76,46 @@ public RootAuthenticationSessionModel createAuthenticationSession(RealmModel rea


public RootAuthenticationSessionModel getCurrentRootAuthenticationSession(RealmModel realm) {
List<String> authSessionCookies = getAuthSessionCookies(realm);

return authSessionCookies.stream().map(oldEncodedId -> {
AuthSessionId authSessionId = decodeAuthSessionId(oldEncodedId);
String sessionId = authSessionId.getDecodedId();

RootAuthenticationSessionModel rootAuthSession = session.authenticationSessions().getRootAuthenticationSession(realm, sessionId);

if (rootAuthSession != null) {
reencodeAuthSessionCookie(oldEncodedId, authSessionId, realm);
return rootAuthSession;
}

String oldEncodedId = getAuthSessionCookies(realm);
if (oldEncodedId == null) {
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);
AuthSessionId authSessionId = decodeAuthSessionId(oldEncodedId);
String sessionId = authSessionId.getDecodedId();

if (userSession != null) {
reencodeAuthSessionCookie(oldEncodedId, authSessionId, realm);
return userSession;
}
RootAuthenticationSessionModel rootAuthSession = session.authenticationSessions().getRootAuthenticationSession(realm, sessionId);

if (rootAuthSession != null) {
reencodeAuthSessionCookie(oldEncodedId, authSessionId, realm);
return rootAuthSession;
} else {
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);

return authSessionCookies.stream().map(oldEncodedId -> {
AuthSessionId authSessionId = decodeAuthSessionId(oldEncodedId);
String sessionId = authSessionId.getDecodedId();
String oldEncodedId = getAuthSessionCookies(realm);
if (oldEncodedId == null) {
return null;
}

AuthenticationSessionModel authSession = getAuthenticationSessionByIdAndClient(realm, sessionId, client, tabId);
AuthSessionId authSessionId = decodeAuthSessionId(oldEncodedId);
String sessionId = authSessionId.getDecodedId();

if (authSession != null) {
reencodeAuthSessionCookie(oldEncodedId, authSessionId, realm);
return authSession;
}
AuthenticationSessionModel authSession = getAuthenticationSessionByIdAndClient(realm, sessionId, client, tabId);

if (authSession != null) {
reencodeAuthSessionCookie(oldEncodedId, authSessionId, realm);
return authSession;
} else {
return null;
}).filter(authSession -> Objects.nonNull(authSession)).findFirst().orElse(null);
}
}


Expand Down Expand Up @@ -184,28 +164,22 @@ void reencodeAuthSessionCookie(String oldEncodedAuthSessionId, AuthSessionId new

/**
* @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);
// 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());
StickySessionEncoderProvider encoder = session.getProvider(StickySessionEncoderProvider.class);
// in case the id is encoded with a route when running in a cluster
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
RootAuthenticationSessionModel rootAuthenticationSession = session.authenticationSessions().getRootAuthenticationSession(realm, decodedId);
return rootAuthenticationSession != null ? oldEncodedId : null;
}


Expand Down
30 changes: 15 additions & 15 deletions services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,35 +64,35 @@ public UserSessionModel getUserSessionWithClient(RealmModel realm, String id, St

// 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
// session exists in remote caches
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();
AuthSessionId authSessionId = asm.decodeAuthSessionId(oldEncodedId);
String sessionId = authSessionId.getDecodedId();

// This will remove userSession "locally" if it doesn't exist on remoteCache
kcSession.sessions().getUserSessionWithPredicate(realm, sessionId, false, (UserSessionModel userSession2) -> userSession2 == null);
// This will remove userSession "locally" if it doesn't exist on remoteCache
kcSession.sessions().getUserSessionWithPredicate(realm, sessionId, false, (UserSessionModel userSession2) -> userSession2 == null);

UserSessionModel userSession = kcSession.sessions().getUserSession(realm, sessionId);

if (userSession != null) {
asm.reencodeAuthSessionCookie(oldEncodedId, authSessionId, realm);
return userSession;
}
UserSessionModel userSession = kcSession.sessions().getUserSession(realm, sessionId);

if (userSession != null) {
asm.reencodeAuthSessionCookie(oldEncodedId, authSessionId, realm);
return userSession;
} else {
return null;
}).filter(userSession -> Objects.nonNull(userSession)).findFirst().orElse(null);
}
}
}
55 changes: 1 addition & 54 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 @@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -101,49 +93,4 @@ public static String getCookieValue(KeycloakSession session, String name) {
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;
}

}
13 changes: 2 additions & 11 deletions ...-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cookies/CookieHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import java.io.IOException;
import java.util.List;
import java.util.Set;

public class CookieHelperTest extends AbstractKeycloakTest {

Expand All @@ -42,8 +41,8 @@ public void testCookieHeaderWithSpaces() {
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);
});
}

Expand All @@ -53,20 +52,12 @@ public void testLegacyCookie() {

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

Expand Down

0 comments on commit cbfdae5

Please sign in to comment.