This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new dee994384c Fix BZ 69839 - Ensure session ID changes are promulgated to
SSO Valve
dee994384c is described below
commit dee994384c9da80b8c2db4e0f5c64baeb4c4d853
Author: Mark Thomas <[email protected]>
AuthorDate: Thu Oct 9 15:25:42 2025 +0100
Fix BZ 69839 - Ensure session ID changes are promulgated to SSO Valve
Patch provided by Kim Johan Andersson.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69839
---
java/org/apache/catalina/Session.java | 4 +
.../catalina/authenticator/LocalStrings.properties | 1 +
.../catalina/authenticator/SingleSignOn.java | 23 ++
.../catalina/authenticator/SingleSignOnEntry.java | 12 +
.../authenticator/SingleSignOnListener.java | 14 +-
.../authenticator/SingleSignOnSessionKey.java | 7 +
.../apache/catalina/session/StandardSession.java | 3 +
.../authenticator/TestSSOChangeSessionId.java | 268 +++++++++++++++++++++
webapps/docs/changelog.xml | 6 +
9 files changed, 336 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/catalina/Session.java
b/java/org/apache/catalina/Session.java
index cd652ae35c..ed37b1e158 100644
--- a/java/org/apache/catalina/Session.java
+++ b/java/org/apache/catalina/Session.java
@@ -56,6 +56,10 @@ public interface Session {
*/
String SESSION_PASSIVATED_EVENT = "passivateSession";
+ /**
+ * The SessionEvent event type when a session changes its sessionId.
+ */
+ String SESSION_CHANGED_ID_EVENT = "changeSessionId";
// ------------------------------------------------------------- Properties
diff --git a/java/org/apache/catalina/authenticator/LocalStrings.properties
b/java/org/apache/catalina/authenticator/LocalStrings.properties
index 126507b5db..a9764fb975 100644
--- a/java/org/apache/catalina/authenticator/LocalStrings.properties
+++ b/java/org/apache/catalina/authenticator/LocalStrings.properties
@@ -76,6 +76,7 @@ singleSignOn.debug.principalFound=SSO found cached Principal
[{0}] with authenti
singleSignOn.debug.principalNotFound=SSO did not find a cached Principal.
Erasing SSO cookie for session [{0}]
singleSignOn.debug.register=SSO registering SSO session [{0}] for user [{1}]
with authentication type [{2}]
singleSignOn.debug.removeSession=SSO removing application session [{0}] from
SSO session [{1}]
+singleSignOn.debug.sessionChangedId=SSO changing sessionID in session [{0},
oldSessionId {1}] from SSO session [{2}]
singleSignOn.debug.sessionLogout=SSO processing a log out for SSO session
[{0}] and application session [{1}]
singleSignOn.debug.sessionTimeout=SSO processing a time out for SSO session
[{0}] and application session [{1}]
singleSignOn.debug.update=SSO updating SSO session [{0}] to authentication
type [{1}]
diff --git a/java/org/apache/catalina/authenticator/SingleSignOn.java
b/java/org/apache/catalina/authenticator/SingleSignOn.java
index 1d31889965..c1ace9da25 100644
--- a/java/org/apache/catalina/authenticator/SingleSignOn.java
+++ b/java/org/apache/catalina/authenticator/SingleSignOn.java
@@ -613,4 +613,27 @@ public class SingleSignOn extends ValveBase {
super.stopInternal();
engine = null;
}
+
+ protected void sessionChangedId(String ssoId, Session session, String
oldSessionId) {
+ if (containerLog.isDebugEnabled()) {
+
containerLog.debug(sm.getString("singleSignOn.debug.sessionChangedId", session,
oldSessionId, ssoId));
+ }
+
+ SingleSignOnEntry entry = cache.get(ssoId);
+ if (entry == null) {
+ return;
+ }
+
+ /*
+ * Associate the new sessionId with this SingleSignOnEntry. A
SessionListener will be registered for the new
+ * sessionID. If not, then we would not notice any subsequent
Session.SESSION_DESTROYED_EVENT for the session.
+ */
+ entry.addSession(this, ssoId, session);
+
+ /*
+ * Remove the obsolete sessionId from the SingleSignOnEntry. The
sessionId part of the SingleSignOnSessionKey is
+ * final.
+ */
+ entry.removeSession(session, oldSessionId);
+ }
}
diff --git a/java/org/apache/catalina/authenticator/SingleSignOnEntry.java
b/java/org/apache/catalina/authenticator/SingleSignOnEntry.java
index 58367cfc16..a25ff15ea5 100644
--- a/java/org/apache/catalina/authenticator/SingleSignOnEntry.java
+++ b/java/org/apache/catalina/authenticator/SingleSignOnEntry.java
@@ -99,6 +99,18 @@ public class SingleSignOnEntry implements Serializable {
sessionKeys.remove(key);
}
+ /**
+ * Removes the given <code>Session</code> from the list of those
associated with this SSO, using the previous
+ * sessionId
+ *
+ * @param session the <code>Session</code> to remove.
+ * @param oldSessionId the previous sessionId of the <code>Session</code>
to remove.
+ */
+ public void removeSession(Session session, String oldSessionId) {
+ SingleSignOnSessionKey key = new SingleSignOnSessionKey(session,
oldSessionId);
+ sessionKeys.remove(key);
+ }
+
/**
* Returns the HTTP Session identifiers associated with this SSO.
*
diff --git a/java/org/apache/catalina/authenticator/SingleSignOnListener.java
b/java/org/apache/catalina/authenticator/SingleSignOnListener.java
index 15e88e0d79..2ce7be484c 100644
--- a/java/org/apache/catalina/authenticator/SingleSignOnListener.java
+++ b/java/org/apache/catalina/authenticator/SingleSignOnListener.java
@@ -38,7 +38,8 @@ public class SingleSignOnListener implements SessionListener,
Serializable {
@Override
public void sessionEvent(SessionEvent event) {
- if (!Session.SESSION_DESTROYED_EVENT.equals(event.getType())) {
+ final String type = event.getType();
+ if (!(Session.SESSION_DESTROYED_EVENT.equals(type) ||
Session.SESSION_CHANGED_ID_EVENT.equals(type))) {
return;
}
@@ -56,6 +57,15 @@ public class SingleSignOnListener implements
SessionListener, Serializable {
if (sso == null) {
return;
}
- sso.sessionDestroyed(ssoId, session);
+
+ switch (type) {
+ case Session.SESSION_CHANGED_ID_EVENT:
+ sso.sessionChangedId(ssoId, session, (String) event.getData());
+ break;
+
+ case Session.SESSION_DESTROYED_EVENT:
+ sso.sessionDestroyed(ssoId, session);
+ break;
+ }
}
}
diff --git a/java/org/apache/catalina/authenticator/SingleSignOnSessionKey.java
b/java/org/apache/catalina/authenticator/SingleSignOnSessionKey.java
index d2d0da2ddf..b57186ad8f 100644
--- a/java/org/apache/catalina/authenticator/SingleSignOnSessionKey.java
+++ b/java/org/apache/catalina/authenticator/SingleSignOnSessionKey.java
@@ -41,6 +41,13 @@ public class SingleSignOnSessionKey implements Serializable {
this.hostName = context.getParent().getName();
}
+ public SingleSignOnSessionKey(Session session, String sessionId) {
+ this.sessionId = sessionId;
+ Context context = session.getManager().getContext();
+ this.contextName = context.getName();
+ this.hostName = context.getParent().getName();
+ }
+
public String getSessionId() {
return sessionId;
}
diff --git a/java/org/apache/catalina/session/StandardSession.java
b/java/org/apache/catalina/session/StandardSession.java
index d937d69d05..661548ee55 100644
--- a/java/org/apache/catalina/session/StandardSession.java
+++ b/java/org/apache/catalina/session/StandardSession.java
@@ -338,6 +338,9 @@ public class StandardSession implements HttpSession,
Session, Serializable {
@Override
public void tellChangedSessionId(String newId, String oldId, boolean
notifySessionListeners,
boolean notifyContainerListeners) {
+ // Notify interested session event listeners
+ fireSessionEvent(SESSION_CHANGED_ID_EVENT, oldId);
+
Context context = manager.getContext();
// notify ContainerListeners
if (notifyContainerListeners) {
diff --git a/test/org/apache/catalina/authenticator/TestSSOChangeSessionId.java
b/test/org/apache/catalina/authenticator/TestSSOChangeSessionId.java
new file mode 100644
index 0000000000..6919cd1e8d
--- /dev/null
+++ b/test/org/apache/catalina/authenticator/TestSSOChangeSessionId.java
@@ -0,0 +1,268 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.catalina.authenticator;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServlet;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.Manager;
+import org.apache.catalina.Session;
+import org.apache.catalina.connector.Request;
+import org.apache.catalina.connector.Response;
+import org.apache.catalina.startup.TesterServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.catalina.valves.ValveBase;
+import org.apache.tomcat.util.buf.ByteChunk;
+import org.apache.tomcat.util.descriptor.web.LoginConfig;
+import org.apache.tomcat.util.descriptor.web.SecurityCollection;
+import org.apache.tomcat.util.descriptor.web.SecurityConstraint;
+
+public class TestSSOChangeSessionId extends TomcatBaseTest {
+
+ protected static final boolean USE_COOKIES = true;
+
+ private static final String NOTSORANDOMID = "NOTSORANNDOMID";
+ private static final String USER = "user";
+ private static final String PWD = "pwd";
+ private static final String ROLE = "role";
+
+ private static final String HTTP_PREFIX = "http://localhost:";;
+ private static final String CONTEXT_PATH = "/test";
+ private static final String URI_PROTECTED = "/protected";
+ private static final String URI_AUTHENTICATION = "/authentication";
+
+ private Context testContext;
+ private SingleSignOn singleSignOn;
+
+ private static final String SERVER_COOKIE_HEADER = "Set-Cookie";
+ private static final String CLIENT_COOKIE_HEADER = "Cookie";
+
+ private List<String> cookies;
+
+ /*
+ * setup two webapps for every test
+ *
+ * note: the super class tearDown method will stop tomcat
+ */
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+
+ // create a tomcat server using the default in-memory Realm
+ final Tomcat tomcat = getTomcatInstance();
+
+ // Must have a real docBase for webapps - just use temp
+ testContext = tomcat.addContext(CONTEXT_PATH,
System.getProperty("java.io.tmpdir"));
+
+ singleSignOn = new SingleSignOn();
+ tomcat.getHost().getPipeline().addValve(singleSignOn);
+
+ testContext.getPipeline().addValve(new SessionManipulationFilter());
+
+ // Add protected servlet to the context
+ Tomcat.addServlet(testContext, "TesterServlet1", new TesterServlet());
+ testContext.addServletMappingDecoded(URI_PROTECTED, "TesterServlet1");
+
+ SecurityCollection collection1 = new SecurityCollection();
+ collection1.addPatternDecoded(URI_PROTECTED);
+ SecurityConstraint sc1 = new SecurityConstraint();
+ sc1.addAuthRole(ROLE);
+ sc1.addCollection(collection1);
+ testContext.addConstraint(sc1);
+
+ // Add Authenticator
+ Tomcat.addServlet(testContext, "LoginServlet", new LoginServlet());
+ testContext.addServletMappingDecoded(URI_AUTHENTICATION,
"LoginServlet");
+
+ SecurityCollection collection2 = new SecurityCollection();
+ collection2.addPatternDecoded(URI_AUTHENTICATION);
+ SecurityConstraint sc2 = new SecurityConstraint();
+ // do not add a role - which signals access permitted without one
+ sc2.addCollection(collection2);
+ testContext.addConstraint(sc2);
+
+ // Configure the authenticator and inherit the Realm from Engine
+ LoginConfig lc = new LoginConfig();
+ lc.setAuthMethod("NONE");
+ testContext.setLoginConfig(lc);
+ AuthenticatorBase nonloginAuthenticator = new NonLoginAuthenticator();
+ testContext.getPipeline().addValve(nonloginAuthenticator);
+
+ // add the test user and role to the Realm
+ tomcat.addUser(USER, PWD);
+ tomcat.addRole(USER, ROLE);
+
+ tomcat.start();
+ }
+
+ private static class SessionManipulationFilter extends ValveBase {
+ @Override
+ public void invoke(Request request, Response response) throws
IOException, ServletException {
+
+ if (request.getPrincipal() != null) {
+ final Manager manager = request.getContext().getManager();
+ final Session session =
manager.findSession(request.getSession().getId());
+ manager.changeSessionId(session, NOTSORANDOMID);
+ }
+
+ getNext().invoke(request, response);
+ }
+ }
+
+ private static class LoginServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest request, HttpServletResponse
response)
+ throws ServletException, IOException {
+
+ if (request.getUserPrincipal() == null) {
+ request.login(USER, PWD);
+
+ response.setContentType("text/plain");
+ PrintWriter out = response.getWriter();
+ out.print("OK");
+
+ } else {
+ response.setStatus(HttpServletResponse.SC_CONFLICT);
+ }
+ }
+ }
+
+ @Test
+ public void testChangeSessionId() throws Exception {
+ final Map<String,SingleSignOnEntry> cache = singleSignOn.cache;
+ Assert.assertTrue("No SSO entries should be present at startup",
cache.isEmpty());
+
+ // Authenticate a session
+ doTest(CONTEXT_PATH + URI_AUTHENTICATION, USE_COOKIES,
HttpServletResponse.SC_OK);
+
+ Assert.assertFalse("SSO must now be present", cache.isEmpty());
+
+ Assert.assertEquals("Only one SSO entry must be present", 1,
cache.size());
+ final Collection<SingleSignOnEntry> ssoEntries = cache.values();
+ Assert.assertEquals("Only one session should be present, as there is
only one context in the test", 1,
+ ssoEntries.size());
+
+ final SingleSignOnEntry singleSignOnEntry =
ssoEntries.iterator().next();
+ final Set<SingleSignOnSessionKey> sessions =
singleSignOnEntry.findSessions();
+
+ Assert.assertEquals(1, sessions.size());
+
+ final SingleSignOnSessionKey singleSignOnSessionKey1 =
sessions.iterator().next();
+ Assert.assertNotEquals("A random SessionId is expected",
NOTSORANDOMID, singleSignOnSessionKey1.getSessionId());
+
+ // Perform the request that changes the session id:
+ doTest(CONTEXT_PATH + URI_PROTECTED, USE_COOKIES,
HttpServletResponse.SC_OK);
+
+ Assert.assertEquals("2 sessions means we have a zombie
SingleSignOnSessionKey", 1, sessions.size());
+
+ final SingleSignOnSessionKey singleSignOnSessionKey2 =
sessions.iterator().next();
+ Assert.assertEquals(NOTSORANDOMID,
singleSignOnSessionKey2.getSessionId());
+
+ final Session session =
testContext.getManager().findSession(NOTSORANDOMID);
+ Assert.assertNotNull("We need a session in order to test expiry",
session);
+
+ session.expire();
+
+ Assert.assertTrue("No SSO entries should be present after expiration
of the session", cache.isEmpty());
+
+ // Shouldn't this return 401?
+ doTest(CONTEXT_PATH + URI_PROTECTED, USE_COOKIES,
HttpServletResponse.SC_FORBIDDEN);
+ }
+
+ @After
+ @Override
+ public void tearDown() throws Exception {
+ super.tearDown();
+ }
+
+ public void doTest(String uri, boolean useCookie, int expectedRC) throws
Exception {
+
+ Map<String,List<String>> reqHeaders = new HashMap<>();
+ Map<String,List<String>> respHeaders = new HashMap<>();
+
+ if (useCookie && (cookies != null)) {
+ addCookies(reqHeaders);
+ }
+
+ ByteChunk bc = new ByteChunk();
+ int rc = getUrl(HTTP_PREFIX + getPort() + uri, bc, reqHeaders,
respHeaders);
+ if (expectedRC != HttpServletResponse.SC_OK) {
+ Assert.assertEquals(expectedRC, rc);
+ Assert.assertTrue(bc.getLength() > 0);
+ } else {
+ Assert.assertEquals("OK", bc.toString());
+ saveCookies(respHeaders);
+ }
+ }
+
+ /*
+ * add all saved cookies to the outgoing request
+ */
+ protected void addCookies(Map<String,List<String>> reqHeaders) {
+ if ((cookies != null) && (cookies.size() > 0)) {
+ StringBuilder cookieHeader = new StringBuilder();
+ boolean first = true;
+ for (String cookie : cookies) {
+ if (!first) {
+ cookieHeader.append(';');
+ } else {
+ first = false;
+ }
+ cookieHeader.append(cookie);
+ }
+ List<String> cookieHeaderList = new ArrayList<>(1);
+ cookieHeaderList.add(cookieHeader.toString());
+ reqHeaders.put(CLIENT_COOKIE_HEADER, cookieHeaderList);
+ }
+ }
+
+ /*
+ * extract and save the server cookies from the incoming response
+ */
+ protected void saveCookies(Map<String,List<String>> respHeaders) {
+ // we only save the Cookie values, not header prefix
+ List<String> cookieHeaders = respHeaders.get(SERVER_COOKIE_HEADER);
+ if (cookieHeaders == null) {
+ cookies = null;
+ } else {
+ cookies = new ArrayList<>(cookieHeaders.size());
+ for (String cookieHeader : cookieHeaders) {
+ cookies.add(cookieHeader.substring(0,
cookieHeader.indexOf(';')));
+ }
+ }
+ }
+}
\ No newline at end of file
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ff4227ef7b..19ccb21bc3 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -123,6 +123,12 @@
<fix>
Reject requests that map to invalid Windows file names earlier. (markt)
</fix>
+ <fix>
+ <bug>69839</bug>: Ensure that changes to session IDs (typically after
+ authentication) are promulgated to the SSO Valve to ensure that SSO
+ entries are fully clean-up on session expiration. Patch provided by Kim
+ Johan Andersson. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="WebSocket">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
