From 7a4858d65f2431396c2f4dadbc3d654712bc02a8 Mon Sep 17 00:00:00 2001
From: Kohsuke Kawaguchi <kk@kohsuke.org>
Date: Thu, 3 May 2012 17:47:58 -0700
Subject: [PATCH] [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.
---
 changelog.html                                       |  3 +++
 .../security/AuthenticationProcessingFilter2.java    | 12 ++++++++++++
 core/src/main/resources/lib/layout/layout.jelly      |  8 +++++++-
 .../webapp/WEB-INF/security/SecurityFilters.groovy   |  6 ++++++
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/changelog.html b/changelog.html
index 027282dea2..67efbdc753 100644
--- a/changelog.html
+++ b/changelog.html
@@ -55,6 +55,9 @@ Upcoming changes</a>
 <!-- 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
index 0013687d4a..faef1690a6 100644
--- a/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java
+++ b/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java
@@ -31,6 +31,7 @@ import java.io.IOException;
 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 @@ public class AuthenticationProcessingFilter2 extends AuthenticationProcessingFil
 		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
index 7f51e9148d..6e6d7143cf 100644
--- 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
index 36a8a18d96..3db1b75962 100644
--- 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
-- 
GitLab