[FIXED JENKINS-12585] restrict where sessions are created.

If a resource with 'Set-Cookie' header is cached (either by intermediary like HTTP proxy and reverse proxy, or by the browser), it'll cause identity swap / session mix-up as discussed in this ticket. I suspect this was caused by HttpSessionContextIntegrationFilter2, which is the only code path that attempts to create a session when a request to a static resource is made. So I'm disabling the creation of session in HttpSessionContextIntegrationFilter2. This in turn requires that we have sessions already created when the authentication was successful and people need to login (or else the login will have no effect.) We already do so in layout.jelly, so any request that renders a Jenkins page would have a session, but I've also added it in AuthenticationProcessingFilter2, which ensures that a successful login does have a session.
jenkinsci · May 4, 2012 · 7a4858d · 7a4858d
1 parent d7d9cf4
commit 7a4858d
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 1 deletion.
diff --git a/changelog.html b/changelog.html
@@ -55,6 +55,9 @@
 <!-- Record your changes in the trunk here. -->
 <div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
 <ul class=image>
+  <li class=bug>
+    Don't try to set cookies on cachable requests.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-12585">issue 12585</a>)
   <li class=bug>
     Migrating to Jenkins 1.462 : Bad version number in .class file (unable to load class com.google.common.collect.ImmutableSet).
     Back to guava 11.0.1 .

diff --git a/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java b/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java
@@ -31,6 +31,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.acegisecurity.Authentication;
 import org.acegisecurity.AuthenticationException;
 import org.acegisecurity.ui.webapp.AuthenticationProcessingFilter;
 
@@ -72,6 +73,17 @@ protected String determineFailureUrl(HttpServletRequest request, AuthenticationE
 		return excMap.getProperty(failedClassName, getAuthenticationFailureUrl());
     }
 
+    @Override
+    protected void onSuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response, Authentication authResult) throws IOException {
+        super.onSuccessfulAuthentication(request,response,authResult);
+        // make sure we have a session to store this successful authentication, given that we no longer
+        // let HttpSessionContextIntegrationFilter2 to create sessions.
+        // HttpSessionContextIntegrationFilter stores the updated SecurityContext object into this session later
+        // (either when a redirect is issued, via its HttpResponseWrapper, or when the execution returns to its
+        // doFilter method.
+        request.getSession();
+    }
+
     /**
      * Leave the information about login failure.
      *

diff --git a/core/src/main/resources/lib/layout/layout.jelly b/core/src/main/resources/lib/layout/layout.jelly
@@ -60,7 +60,13 @@ THE SOFTWARE.
 <!-- The path starts with a "/" character but does not end with a "/" character. -->
 <j:set var="rootURL" value="${request.contextPath}" />
 <j:new var="h" className="hudson.Functions" /><!-- instead of JSP functions -->
-<j:set var="_" value="${request.getSession()}"/><!-- depending on what tags are used, we can later end up discovering that we needed a session, but it's too late because the headers are already committed. so ensure we always have a session -->
+<!--
+  depending on what tags are used, we can later end up discovering that we needed a session,
+  but it's too late because the headers are already committed. so ensure we always have a session.
+  this also allows us to configure HttpSessionContextIntegrationFilter not to create sessions,
+  which I suspect can end up creating sessions for wrong resource types (such as static resources.)
+-->
+<j:set var="_" value="${request.getSession()}"/>
 <j:set var="_" value="${h.configureAutoRefresh(request, response, attrs.norefresh!=null)}"/>
 <!--
   load static resources from the path dedicated to a specific version.

diff --git a/war/src/main/webapp/WEB-INF/security/SecurityFilters.groovy b/war/src/main/webapp/WEB-INF/security/SecurityFilters.groovy
@@ -63,6 +63,12 @@ filter(ChainedServletFilter) {
     filters = [
         // this persists the authentication across requests by using session
         bean(HttpSessionContextIntegrationFilter2) {
+            // not allowing filter to create sessions, as it potentially tries to create
+            // sessions for any request (although it usually fails
+            // I suspect this is related to JENKINS-12585, in that
+            // it ends up setting Set-Cookie for image responses.
+            // Instead, we use layout.jelly to create sessions.
+            allowSessionCreation = false;
         },
         bean(ApiTokenFilter),
         // allow clients to submit basic authentication credential