Merge pull request #2198 from daniel-beck/JENKINS-12875

[FIX JENKINS-12875] Change default crumb name to Jenkins-Crumb
jenkinsci · Apr 1, 2016 · a87c38c · a87c38c
2 parents 4649e04 + 8b6d38d
commit a87c38c
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 15 deletions.
diff --git a/core/src/main/java/hudson/security/csrf/CrumbFilter.java b/core/src/main/java/hudson/security/csrf/CrumbFilter.java
@@ -62,17 +62,11 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
             String crumbFieldName = crumbIssuer.getDescriptor().getCrumbRequestField();
             String crumbSalt = crumbIssuer.getDescriptor().getCrumbSalt();
 
-            String crumb = httpRequest.getHeader(crumbFieldName);
             boolean valid = false;
+            String crumb = extractCrumbFromRequest(httpRequest, crumbFieldName);
             if (crumb == null) {
-                Enumeration<?> paramNames = request.getParameterNames();
+                // compatibility for clients that hard-code the default crumb name up to Jenkins 1.TODO
-                while (paramNames.hasMoreElements()) {
+                extractCrumbFromRequest(httpRequest, ".crumb");
-                    String paramName = (String) paramNames.nextElement();
-                    if (crumbFieldName.equals(paramName)) {
-                        crumb = request.getParameter(paramName);
-                        break;
-                    }
-                }
             }
             if (crumb != null) {
                 if (crumbIssuer.validateCrumb(httpRequest, crumbSalt, crumb)) {
@@ -93,6 +87,21 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
         }
     }
 
+    private String extractCrumbFromRequest(HttpServletRequest httpRequest, String crumbFieldName) {
+        String crumb = httpRequest.getHeader(crumbFieldName);
+        if (crumb == null) {
+            Enumeration<?> paramNames = httpRequest.getParameterNames();
+            while (paramNames.hasMoreElements()) {
+                String paramName = (String) paramNames.nextElement();
+                if (crumbFieldName.equals(paramName)) {
+                    crumb = httpRequest.getParameter(paramName);
+                    break;
+                }
+            }
+        }
+        return crumb;
+    }
+
     protected static boolean isMultipart(HttpServletRequest request) {
         if (request == null) {
             return false;

diff --git a/core/src/main/java/hudson/security/csrf/CrumbIssuer.java b/core/src/main/java/hudson/security/csrf/CrumbIssuer.java
@@ -44,6 +44,9 @@ public abstract class CrumbIssuer implements Describable<CrumbIssuer>, Extension
 
     private static final String CRUMB_ATTRIBUTE = CrumbIssuer.class.getName() + "_crumb";
 
+    @Restricted(NoExternalUse.class)
+    public static final String DEFAULT_CRUMB_NAME = "Jenkins-Crumb";
+
     /**
      * Get the name of the request parameter the crumb will be stored in. Exposed
      * here for the remote API.
@@ -199,7 +202,7 @@ public static class RestrictedApi extends Api {
             } else if ("concat(//crumbRequestField,\":\",//crumb)".equals(xpath)) { // new FullDuplexHttpStream; Main
                 text = ci.getCrumbRequestField() + ':' + ci.getCrumb();
             } else if ("concat(//crumbRequestField,'=',//crumb)".equals(xpath)) { // NetBeans
-                if (ci.getCrumbRequestField().startsWith(".")) {
+                if (ci.getCrumbRequestField().startsWith(".") || ci.getCrumbRequestField().contains("-")) {
                     text = ci.getCrumbRequestField() + '=' + ci.getCrumb();
                 } else {
                     text = null;

diff --git a/core/src/main/java/hudson/security/csrf/CrumbIssuerDescriptor.java b/core/src/main/java/hudson/security/csrf/CrumbIssuerDescriptor.java
@@ -64,7 +64,7 @@ public String getCrumbRequestField() {
      */
     public void setCrumbRequestField(String requestField) {
         if (Util.fixEmptyAndTrim(requestField) == null) {
-            crumbRequestField = ".crumb";
+            crumbRequestField = CrumbIssuer.DEFAULT_CRUMB_NAME;
         } else {
             crumbRequestField = requestField;
         }

diff --git a/core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java b/core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java
@@ -124,7 +124,7 @@ public static final class DescriptorImpl extends CrumbIssuerDescriptor<DefaultCr
         private final static HexStringConfidentialKey CRUMB_SALT = new HexStringConfidentialKey(Jenkins.class,"crumbSalt",16);
 
         public DescriptorImpl() {
-            super(CRUMB_SALT.get(), System.getProperty("hudson.security.csrf.requestfield", ".crumb"));
+            super(CRUMB_SALT.get(), System.getProperty("hudson.security.csrf.requestfield", CrumbIssuer.DEFAULT_CRUMB_NAME));
             load();
         }
 

diff --git a/test/src/test/groovy/hudson/model/SlaveTest.groovy b/test/src/test/groovy/hudson/model/SlaveTest.groovy
@@ -28,6 +28,7 @@ import static hudson.util.FormValidation.Kind.WARNING
 import static org.junit.Assert.assertNotNull
 import static org.junit.Assert.assertEquals
 
+import hudson.security.csrf.CrumbIssuer
 import hudson.slaves.DumbSlave
 import hudson.slaves.JNLPLauncher
 import hudson.util.FormValidation
@@ -84,7 +85,7 @@ class SlaveTest {
         HttpURLConnection con = new URL(j.getURL(),url).openConnection();
         con.requestMethod = "POST"
         con.setRequestProperty("Content-Type","application/xml;charset=UTF-8")
-        con.setRequestProperty(".crumb","test")
+        con.setRequestProperty(CrumbIssuer.DEFAULT_CRUMB_NAME,"test")
         con.doOutput = true;
         con.outputStream.write(xml.getBytes("UTF-8"))
         con.outputStream.close();

diff --git a/test/src/test/java/lib/form/RowSetTest.java b/test/src/test/java/lib/form/RowSetTest.java
@@ -2,6 +2,7 @@
 
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
 import hudson.model.RootAction;
+import hudson.security.csrf.CrumbIssuer;
 import junit.framework.Assert;
 import net.sf.json.JSONObject;
 import org.junit.Rule;
@@ -27,12 +28,12 @@ public void json() throws Exception {
     public static class Subject implements RootAction {
         public void doSubmitTest1(StaplerRequest req) throws Exception {
             JSONObject json = req.getSubmittedForm();
-            json.remove("crumb");
+            json.remove(CrumbIssuer.DEFAULT_CRUMB_NAME);
             System.out.println(json);
 
             JSONObject expected = JSONObject.fromObject(
                     "{'a':'aaa','b':'bbb','c':{'c1':'ccc1','c2':'ccc2'},'d':{'d1':'d1','d2':'d2'}}");
-            Assert.assertEquals(json,expected);
+            Assert.assertEquals(expected, json);
         }
 
         @Override