[SECURITY-2786] Fixes for CSRF vulnerability and missing permission c…

…hecks (#189) * Use POST for form validation methods and check for ADMINISTER permissions * Wrap validate buttons in isAdmin checks
jenkinsci · Feb 15, 2025 · c456eb0 · c456eb0
1 parent f96106c
commit c456eb0
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 21 deletions.
diff --git a/src/main/java/org/thoughtslive/jenkins/plugins/jira/Site.java b/src/main/java/org/thoughtslive/jenkins/plugins/jira/Site.java
@@ -36,6 +36,7 @@
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.DataBoundSetter;
 import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.verb.POST;
 import org.thoughtslive.jenkins.plugins.jira.api.ResponseData;
 import org.thoughtslive.jenkins.plugins.jira.service.JiraService;
 
@@ -235,10 +236,15 @@ protected Authentication getAuthentication(Item item) {
       return item instanceof Queue.Task ? Tasks.getAuthenticationOf((Queue.Task) item) : ACL.SYSTEM;
     }
 
+    @POST
     public FormValidation doValidateCredentials(@QueryParameter String url,
         @QueryParameter String credentialsId,
         @QueryParameter Integer timeout,
         @QueryParameter Integer readTimeout) throws IOException {
+      if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) {
+        return FormValidation.warning("Insufficient permissions");
+      }
+
       FormValidation validation = doCheckUrl(url);
       if (validation.kind != Kind.OK) {
         return FormValidation.error(Messages.Site_emptyURL());
@@ -278,12 +284,17 @@ public FormValidation doValidateCredentials(@QueryParameter String url,
      * Checks if the details required for the basic login is valid. TODO: This validation can be
      * moved to JiraStepsConfig so that we can also verify the name is valid.
      */
+    @POST
     public FormValidation doValidateBasic(@QueryParameter String name, @QueryParameter String url,
         @QueryParameter String loginType, @QueryParameter String timeout,
         @QueryParameter String readTimeout,
         @QueryParameter String userName, @QueryParameter String password,
         @QueryParameter String consumerKey, @QueryParameter String privateKey,
         @QueryParameter String secret, @QueryParameter String token) throws IOException {
+      if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) {
+        return FormValidation.warning("Insufficient permissions");
+      }
+
       url = Util.fixEmpty(url);
       name = Util.fixEmpty(name);
       userName = Util.fixEmpty(userName);
@@ -352,12 +363,17 @@ public FormValidation doValidateBasic(@QueryParameter String name, @QueryParamet
 
     // This is stupid but no choice as I couldn't find the way to get the
     // value loginType (radioBlock as a @QueryParameter)
+    @POST
     public FormValidation doValidateOAuth(@QueryParameter String name, @QueryParameter String url,
         @QueryParameter String loginType, @QueryParameter String timeout,
         @QueryParameter String readTimeout,
         @QueryParameter String userName, @QueryParameter String password,
         @QueryParameter String consumerKey, @QueryParameter String privateKey,
         @QueryParameter String secret, @QueryParameter String token) throws IOException {
+      if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) {
+        return FormValidation.warning("Insufficient permissions");
+      }
+
       url = Util.fixEmpty(url);
       name = Util.fixEmpty(name);
       consumerKey = Util.fixEmpty(consumerKey);

diff --git a/src/main/resources/org/thoughtslive/jenkins/plugins/jira/Site/config.jelly b/src/main/resources/org/thoughtslive/jenkins/plugins/jira/Site/config.jelly
@@ -4,6 +4,7 @@
   xmlns:f="/lib/form"
   xmlns:j="jelly:core"
   xmlns:c="/lib/credentials"
+  xmlns:l="/lib/layout"
 >
   <f:entry field="name" title="Name">
     <f:textbox/>
@@ -27,13 +28,15 @@
         <f:entry field="password" title="Password">
           <f:password/>
         </f:entry>
-        <f:entry>
-          <f:validateButton inline="true"
-            method="validateBasic"
-            progress="Checking..."
-            title="${%Test Connection}"
-            with="name,url,loginType,timeout,readTimeout,userName,password,consumerKey,privateKey,secret,token"/>
-        </f:entry>
+        <l:isAdmin>
+          <f:entry>
+            <f:validateButton inline="true"
+              method="validateBasic"
+              progress="Checking..."
+              title="${%Test Connection}"
+              with="name,url,loginType,timeout,readTimeout,userName,password,consumerKey,privateKey,secret,token"/>
+          </f:entry>
+        </l:isAdmin>
       </f:nested>
     </f:radioBlock>
     <f:radioBlock checked="${instance.isLoginType('OAUTH')}" field="loginType" inline="true"
@@ -51,27 +54,31 @@
         <f:entry field="token" title="Token">
           <f:password/>
         </f:entry>
-        <f:entry>
-          <f:validateButton inline="true"
-            method="validateOAuth"
-            progress="Checking..."
-            title="${%Test Connection}"
-            with="name,url,loginType,timeout,readTimeout,userName,password,consumerKey,privateKey,secret,token"/>
-        </f:entry>
+        <l:isAdmin>
+          <f:entry>
+            <f:validateButton inline="true"
+              method="validateOAuth"
+              progress="Checking..."
+              title="${%Test Connection}"
+              with="name,url,loginType,timeout,readTimeout,userName,password,consumerKey,privateKey,secret,token"/>
+          </f:entry>
+        </l:isAdmin>
       </f:nested>
     </f:radioBlock>
     <f:radioBlock checked="${instance.isLoginType('CREDENTIAL')}" field="loginType" inline="true" name="loginType" title="${%Credentials}" value="CREDENTIAL">
       <f:nested>
         <f:entry field="credentialsId" title="${%Credentials}">
           <c:select />
         </f:entry>
-        <f:entry>
-          <f:validateButton inline="true"
-            method="validateCredentials"
-            progress="Checking..."
-            title="${%Test Connection}"
-            with="url,credentialsId,timeout,readTimeout"/>
-        </f:entry>
+        <l:isAdmin>
+          <f:entry>
+            <f:validateButton inline="true"
+              method="validateCredentials"
+              progress="Checking..."
+              title="${%Test Connection}"
+              with="url,credentialsId,timeout,readTimeout"/>
+          </f:entry>
+        </l:isAdmin>
       </f:nested>
     </f:radioBlock>
   </f:section>