AUTH_SESSION_ID cookie has the incorrect route

Fixes #43933

Signed-off-by: Pedro Ruivo <1492066+pruivo@users.noreply.github.com>
Co-authored-by: Pedro Ruivo <1492066+pruivo@users.noreply.github.com>
This commit is contained in:
Pedro Ruivo 2025-11-07 21:32:45 +00:00 committed by GitHub
parent c67b6bc007
commit 80895d7fb4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 182 additions and 152 deletions

View File

@ -98,7 +98,9 @@ public class TopologyInfo {
/**
* True if I am primary owner of the key in case of distributed caches. In case of local caches, always return true
* @deprecated Removed without replacement.
*/
@Deprecated(since = "26.5", forRemoval = true)
public boolean amIOwner(Cache<?, ?> cache, Object key) {
Address myAddress = cache.getCacheManager().getAddress();
Address objectOwnerAddress = getOwnerAddress(cache, key);
@ -110,7 +112,9 @@ public class TopologyInfo {
/**
* Get route to be used as the identifier for sticky session. Return null if I am not able to find the appropriate route (or in case of local mode)
* @deprecated Use {@link org.keycloak.sessions.StickySessionEncoderProvider#sessionIdRoute(String)} instead.
*/
@Deprecated(since = "26.5", forRemoval = true)
public String getRouteName(Cache<?, ?> cache, Object key) {
if (cache.getCacheConfiguration().clustering().cacheMode().isClustered() && isGeneratedNodeName) {
logger.warn("Clustered configuration used, but node name is not properly set. Make sure to start server with jboss.node.name property identifying cluster node");

View File

@ -1,78 +0,0 @@
/*
* Copyright 2016 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.models.sessions.infinispan;
import org.infinispan.Cache;
import org.keycloak.connections.infinispan.InfinispanConnectionProvider;
import org.keycloak.models.KeycloakSession;
import org.keycloak.connections.infinispan.InfinispanUtil;
import org.keycloak.sessions.StickySessionEncoderProvider;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class InfinispanStickySessionEncoderProvider implements StickySessionEncoderProvider {
private final KeycloakSession session;
private final boolean shouldAttachRoute;
public InfinispanStickySessionEncoderProvider(KeycloakSession session, boolean shouldAttachRoute) {
this.session = session;
this.shouldAttachRoute = shouldAttachRoute;
}
@Override
public String encodeSessionId(String sessionId) {
if (!shouldAttachRoute) {
return sessionId;
}
String route = getRoute(sessionId);
if (route != null) {
return sessionId + '.' + route;
} else {
return sessionId;
}
}
@Override
public String decodeSessionId(String encodedSessionId) {
// Try to decode regardless if shouldAttachRoute is true/false. It's possible that some loadbalancers may forward the route information attached by them to the backend keycloak server. We need to remove it then.
int index = encodedSessionId.indexOf('.');
return index == -1 ? encodedSessionId : encodedSessionId.substring(0, index);
}
@Override
public boolean shouldAttachRoute() {
return shouldAttachRoute;
}
@Override
public void close() {
}
private String getRoute(String sessionId) {
InfinispanConnectionProvider ispnProvider = session.getProvider(InfinispanConnectionProvider.class);
Cache cache = ispnProvider.getCache(InfinispanConnectionProvider.AUTHENTICATION_SESSIONS_CACHE_NAME);
return InfinispanUtil.getTopologyInfo(session).getRouteName(cache, sessionId);
}
}

View File

@ -18,30 +18,42 @@
package org.keycloak.models.sessions.infinispan;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import org.infinispan.Cache;
import org.infinispan.remoting.transport.jgroups.JGroupsAddress;
import org.jboss.logging.Logger;
import org.jgroups.util.NameCache;
import org.keycloak.Config;
import org.keycloak.connections.infinispan.InfinispanConnectionProvider;
import org.keycloak.infinispan.util.InfinispanUtils;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.provider.EnvironmentDependentProviderFactory;
import org.keycloak.provider.Provider;
import org.keycloak.provider.ProviderConfigProperty;
import org.keycloak.provider.ProviderConfigurationBuilder;
import org.keycloak.sessions.StickySessionEncoderProvider;
import org.keycloak.sessions.StickySessionEncoderProviderFactory;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.AUTHENTICATION_SESSIONS_CACHE_NAME;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class InfinispanStickySessionEncoderProviderFactory implements StickySessionEncoderProviderFactory, EnvironmentDependentProviderFactory {
public class InfinispanStickySessionEncoderProviderFactory implements StickySessionEncoderProviderFactory, EnvironmentDependentProviderFactory, StickySessionEncoderProvider {
private static final Logger log = Logger.getLogger(InfinispanStickySessionEncoderProviderFactory.class);
private static final char SEPARATOR = '.';
private boolean shouldAttachRoute;
private boolean clustered;
private Cache<String, ?> authenticationCache;
@Override
public StickySessionEncoderProvider create(KeycloakSession session) {
return new InfinispanStickySessionEncoderProvider(session, shouldAttachRoute);
return this;
}
@Override
@ -58,7 +70,11 @@ public class InfinispanStickySessionEncoderProviderFactory implements StickySess
@Override
public void postInit(KeycloakSessionFactory factory) {
try (var session = factory.create()) {
var provider = session.getProvider(InfinispanConnectionProvider.class);
authenticationCache = provider.getCache(AUTHENTICATION_SESSIONS_CACHE_NAME);
clustered = authenticationCache.getCacheConfiguration().clustering().cacheMode().isClustered();
}
}
@Override
@ -76,6 +92,11 @@ public class InfinispanStickySessionEncoderProviderFactory implements StickySess
return InfinispanUtils.PROVIDER_ORDER;
}
@Override
public Set<Class<? extends Provider>> dependsOn() {
return Set.of(InfinispanConnectionProvider.class);
}
@Override
public List<ProviderConfigProperty> getConfigMetadata() {
return ProviderConfigurationBuilder.create()
@ -92,4 +113,45 @@ public class InfinispanStickySessionEncoderProviderFactory implements StickySess
public boolean isSupported(Config.Scope config) {
return InfinispanUtils.isEmbeddedInfinispan();
}
@Override
public String encodeSessionId(String message, String sessionId) {
Objects.requireNonNull(message);
String route = sessionIdRoute(sessionId);
return route == null ? message : message + SEPARATOR + route;
}
@Override
public SessionIdAndRoute decodeSessionIdAndRoute(String encodedSessionId) {
int index = encodedSessionId.indexOf(SEPARATOR);
int length = encodedSessionId.length();
if (index == -1 || index == (length - 1)) {
//route not present
return new SessionIdAndRoute(encodedSessionId, null);
}
return new SessionIdAndRoute(encodedSessionId.substring(0, index), encodedSessionId.substring(index + 1, length));
}
@Override
public boolean shouldAttachRoute() {
return shouldAttachRoute;
}
@Override
public String sessionIdRoute(String sessionId) {
// return null if running in the local mode (start-dev)
return clustered && shouldAttachRoute ? ownerOf(sessionId) : null;
}
private String ownerOf(String sessionId) {
var primaryOwner = authenticationCache.getAdvancedCache()
.getDistributionManager()
.getCacheTopology()
.getDistribution(Objects.requireNonNull(sessionId))
.primary();
// Return null if the logical name is not available yet.
// The following request may be redirected to the wrong instance, but that's ok.
// In a healthy/stable cluster, the name cache is correctly populated.
return primaryOwner instanceof JGroupsAddress jgrpAddr ? NameCache.get(jgrpAddr.getJGroupsAddress()) : null;
}
}

View File

@ -19,42 +19,34 @@ package org.keycloak.models.sessions.infinispan.remote;
import java.lang.invoke.MethodHandles;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import org.jboss.logging.Logger;
import org.keycloak.Config;
import org.keycloak.connections.infinispan.InfinispanConnectionProvider;
import org.keycloak.connections.infinispan.InfinispanUtil;
import org.keycloak.infinispan.util.InfinispanUtils;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.provider.EnvironmentDependentProviderFactory;
import org.keycloak.provider.Provider;
import org.keycloak.provider.ProviderConfigProperty;
import org.keycloak.provider.ProviderConfigurationBuilder;
import org.keycloak.sessions.StickySessionEncoderProvider;
import org.keycloak.sessions.StickySessionEncoderProviderFactory;
public class RemoteStickySessionEncoderProviderFactory implements StickySessionEncoderProviderFactory, EnvironmentDependentProviderFactory {
public class RemoteStickySessionEncoderProviderFactory implements StickySessionEncoderProviderFactory, EnvironmentDependentProviderFactory, StickySessionEncoderProvider {
private static final Logger log = Logger.getLogger(MethodHandles.lookup().lookupClass());
private static final char SEPARATOR = '.';
private static final StickySessionEncoderProvider NO_ROUTER_PROVIDER = new BaseProvider() {
@Override
public String encodeSessionId(String sessionId) {
return sessionId;
}
@Override
public boolean shouldAttachRoute() {
return false;
}
};
private volatile boolean shouldAttachRoute;
private volatile StickySessionEncoderProvider provider;
private volatile String route;
@Override
public StickySessionEncoderProvider create(KeycloakSession session) {
return shouldAttachRoute ? provider : NO_ROUTER_PROVIDER;
return this;
}
@Override
@ -65,7 +57,7 @@ public class RemoteStickySessionEncoderProviderFactory implements StickySessionE
@Override
public void postInit(KeycloakSessionFactory factory) {
try (var session = factory.create()) {
provider = new AttachRouteProvider(getRoute(session));
route = InfinispanUtil.getTopologyInfo(session).getMyNodeName();
}
}
@ -101,48 +93,40 @@ public class RemoteStickySessionEncoderProviderFactory implements StickySessionE
return InfinispanUtils.isRemoteInfinispan();
}
@Override
public Set<Class<? extends Provider>> dependsOn() {
return Set.of(InfinispanConnectionProvider.class);
}
@Override
public void setShouldAttachRoute(boolean shouldAttachRoute) {
this.shouldAttachRoute = shouldAttachRoute;
log.debugf("Should attach route to the sticky session cookie: %b", shouldAttachRoute);
}
private static String getRoute(KeycloakSession session) {
return InfinispanUtil.getTopologyInfo(session).getMyNodeName();
@Override
public String encodeSessionId(String message, String ignored) {
Objects.requireNonNull(message);
return shouldAttachRoute ? message + SEPARATOR + route : message;
}
private static abstract class BaseProvider implements StickySessionEncoderProvider {
@Override
public final String decodeSessionId(String encodedSessionId) {
// Try to decode regardless if shouldAttachRoute is true/false.
// It is possible that some loadbalancers may forward the route information attached by them to the backend keycloak server.
// We need to remove it then.
int index = encodedSessionId.indexOf(SEPARATOR);
return index == -1 ? encodedSessionId : encodedSessionId.substring(0, index);
}
@Override
public final void close() {
@Override
public SessionIdAndRoute decodeSessionIdAndRoute(String encodedSessionId) {
int index = encodedSessionId.indexOf('.');
if (index == -1) {
//route not present
return new SessionIdAndRoute(encodedSessionId, null);
}
return new SessionIdAndRoute(encodedSessionId.substring(0, index), encodedSessionId.substring(index, encodedSessionId.length() - 1));
}
private static class AttachRouteProvider extends BaseProvider {
@Override
public boolean shouldAttachRoute() {
return shouldAttachRoute;
}
private final String route;
private AttachRouteProvider(String route) {
this.route = route;
}
@Override
public String encodeSessionId(String sessionId) {
return sessionId + SEPARATOR + route;
}
@Override
public boolean shouldAttachRoute() {
return true;
}
@Override
public String sessionIdRoute(String ignored) {
return shouldAttachRoute ? route : null;
}
}

View File

@ -17,6 +17,8 @@
package org.keycloak.sessions;
import java.util.Objects;
import org.keycloak.provider.Provider;
/**
@ -24,24 +26,76 @@ import org.keycloak.provider.Provider;
*/
public interface StickySessionEncoderProvider extends Provider {
/**
* @return Encoded value to be used as the value of sticky session cookie (AUTH_SESSION_ID cookie)
* @deprecated Use {@link #encodeSessionId(String, String)} instead.
*/
@Deprecated(since = "26.5", forRemoval = true)
default String encodeSessionId(String sessionId) {
return encodeSessionId(sessionId, sessionId);
}
/**
* @param sessionId
* @return Encoded value to be used as the value of sticky session cookie (AUTH_SESSION_ID cookie)
* Encodes the route into the {@code message}.
* <p>
* The route is computed by the {@code sessionId}, i.e., the Keycloak instance where it is cached.
*
* @param message The message to encode with the route.
* @param sessionId The session ID stored in the cache.
* @return The encoded message with the route information.
* @throws NullPointerException if any parameter is null.
*/
String encodeSessionId(String sessionId);
String encodeSessionId(String message, String sessionId);
/**
* @param encodedSessionId value of the sticky session cookie
* @return decoded value, which represents the actual ID of the {@link AuthenticationSessionModel}
* @deprecated Use {@link #decodeSessionIdAndRoute(String)} instead.
*/
String decodeSessionId(String encodedSessionId);
@Deprecated(since = "26.5", forRemoval = true)
default String decodeSessionId(String encodedSessionId) {
return decodeSessionIdAndRoute(encodedSessionId).sessionId();
}
/**
* @return true if information about route should be attached to the sticky session cookie by Keycloak. Otherwise it may be attached by loadbalancer.
* Decodes the encoded session ID to extract its components, the session ID and the route.
* <p>
* The route component may be {@code null} if the session ID is not correctly encoded, or the sticky session is
* disabled.
*
* @param encodedSessionId The encoded session ID.
* @return The {@link SessionIdAndRoute} with the session ID and the route component. The route may be {@code null}.
* @throws NullPointerException if {@code encodeSessionId} is {@code null}.
*/
SessionIdAndRoute decodeSessionIdAndRoute(String encodedSessionId);
/**
* @return true if information about route should be attached to the sticky session cookie by Keycloak. Otherwise,
* it may be attached by load balancer.
*/
boolean shouldAttachRoute();
/**
* @param sessionId The session ID.
* @return The route for the session ID. It returns {@code null} if sticky session is disabled.
* @throws NullPointerException if {@code sessionId} is {@code null}.
*/
String sessionIdRoute(String sessionId);
/**
* @param sessionId The session ID.
* @param route The router, i.e., the Keycloak instance where the session is cached. It can be {@code null} if
* sticky session is disabled.
*/
record SessionIdAndRoute(String sessionId, String route) {
public SessionIdAndRoute {
Objects.requireNonNull(sessionId);
}
public boolean isSameRoute(String otherRoute) {
return Objects.equals(otherRoute, route);
}
}
}

View File

@ -19,7 +19,6 @@ package org.keycloak.services.managers;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.Objects;
import org.jboss.logging.Logger;
import org.keycloak.common.util.Base64Url;
@ -108,7 +107,7 @@ public class AuthenticationSessionManager {
public void setAuthSessionCookie(String authSessionId) {
StickySessionEncoderProvider encoder = session.getProvider(StickySessionEncoderProvider.class);
String signedAuthSessionId = signAndEncodeToBase64AuthSessionId(authSessionId);
String encodedWithRoute = encoder.encodeSessionId(signedAuthSessionId);
String encodedWithRoute = encoder.encodeSessionId(signedAuthSessionId, authSessionId);
session.getProvider(CookieProvider.class).set(CookieType.AUTH_SESSION_ID, encodedWithRoute);
@ -195,9 +194,9 @@ public class AuthenticationSessionManager {
StickySessionEncoderProvider routeEncoder = session.getProvider(StickySessionEncoderProvider.class);
// in case the id is encoded with a route when running in a cluster
String decodedAuthSessionId = routeEncoder.decodeSessionId(oldEncodedId);
var sessionIdAndRoute = routeEncoder.decodeSessionIdAndRoute(oldEncodedId);
decodedAuthSessionId = decodeBase64AndValidateSignature(decodedAuthSessionId);
String decodedAuthSessionId = decodeBase64AndValidateSignature(sessionIdAndRoute.sessionId());
if(decodedAuthSessionId == null) {
return null;
}
@ -209,10 +208,10 @@ public class AuthenticationSessionManager {
if (rootAuthenticationSession == null) {
return null;
}
String reEncoded = routeEncoder.encodeSessionId(decodedAuthSessionId);
boolean routeChanged = !Objects.equals(oldEncodedId, reEncoded);
String newRoute = routeEncoder.sessionIdRoute(decodedAuthSessionId);
boolean routeChanged = !sessionIdAndRoute.isSameRoute(newRoute);
if (routeChanged) {
log.debugf("Route changed. Will update authentication session cookie. Old: '%s', New: '%s'", oldEncodedId, reEncoded);
log.debugf("Route changed. Will update authentication session cookie. Old: '%s', New: '%s'", sessionIdAndRoute.route(), newRoute);
}
return new AuthSessionCookie(rootAuthenticationSession, routeChanged);
}
@ -289,8 +288,8 @@ public class AuthenticationSessionManager {
return rootAuthSession==null ? null : rootAuthSession.getAuthenticationSession(client, tabId);
}
public AuthenticationSessionModel getAuthenticationSessionByEncodedIdAndClient(RealmModel realm, String encodedAuthSesionId, ClientModel client, String tabId) {
String decodedAuthSessionId = decodeBase64AndValidateSignature(encodedAuthSesionId);
public AuthenticationSessionModel getAuthenticationSessionByEncodedIdAndClient(RealmModel realm, String encodedAuthSessionId, ClientModel client, String tabId) {
String decodedAuthSessionId = decodeBase64AndValidateSignature(encodedAuthSessionId);
return decodedAuthSessionId==null ? null : getAuthenticationSessionByIdAndClient(realm, decodedAuthSessionId, client, tabId);
}

View File

@ -18,13 +18,10 @@
package org.keycloak.testsuite.cluster;
import org.hamcrest.Matchers;
import org.infinispan.Cache;
import org.jboss.arquillian.graphene.page.Page;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.keycloak.connections.infinispan.InfinispanConnectionProvider;
import org.keycloak.connections.infinispan.InfinispanUtil;
import org.keycloak.models.RealmModel;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.services.managers.AuthenticationSessionManager;
@ -43,8 +40,8 @@ import java.util.Set;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.AUTHENTICATION_SESSIONS_CACHE_NAME;
import static org.keycloak.testsuite.AbstractAdminTest.loadJson;
/**
@ -99,11 +96,21 @@ public class AuthenticationSessionClusterTest extends AbstractClusterTest {
String route = authSessionCookie.substring(authSessionCookie.indexOf(".") + 1);
visitedRoutes.add(route);
getTestingClientFor(backendNode(0)).server().run(session -> {
RealmModel realm = session.realms().getRealmByName("test");
session.getContext().setRealm(realm);
StickySessionEncoderProvider provider = session.getProvider(StickySessionEncoderProvider.class);
StickySessionEncoderProvider.SessionIdAndRoute sessionIdAndRoute = provider.decodeSessionIdAndRoute(authSessionCookie);
assertThat(sessionIdAndRoute.route(), Matchers.startsWith("node1"));
String decodedAuthSessionId = new AuthenticationSessionManager(session).decodeBase64AndValidateSignature(sessionIdAndRoute.sessionId());
assertTrue(sessionIdAndRoute.isSameRoute(provider.sessionIdRoute(decodedAuthSessionId)));
});
// Drop all cookies before continue
driver.manage().deleteAllCookies();
}
assertThat(visitedRoutes, Matchers.containsInAnyOrder(Matchers.startsWith("node1"), Matchers.startsWith("node2")));
assertThat(visitedRoutes, Matchers.contains(Matchers.startsWith("node1")));
}
@ -134,10 +141,8 @@ public class AuthenticationSessionClusterTest extends AbstractClusterTest {
getTestingClientFor(backendNode(0)).server().run(session -> {
RealmModel realm = session.realms().getRealmByName("test");
session.getContext().setRealm(realm);
Cache<?, ?> authSessionCache = session.getProvider(InfinispanConnectionProvider.class).getCache(AUTHENTICATION_SESSIONS_CACHE_NAME);
String decodedAuthSessionId = new AuthenticationSessionManager(session).decodeBase64AndValidateSignature(authSessionCookie);
String keyOwner = InfinispanUtil.getTopologyInfo(session).getRouteName(authSessionCache, decodedAuthSessionId);
assertTrue(keyOwner.startsWith("node1"));
assertNull(session.getProvider(StickySessionEncoderProvider.class).sessionIdRoute(decodedAuthSessionId));
});
}