CVE-2020-2114

@@ -27,15 +27,18 @@
 import hudson.util.CopyOnWriteList;
 import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
+import hudson.util.Secret;
 import jenkins.model.Jenkins;
 import jenkins.tasks.SimpleBuildStep;
 import net.sf.json.JSONArray;
 import net.sf.json.JSONObject;
 import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.Symbol;
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 import javax.annotation.Nonnull;
 import java.io.IOException;
@@ -508,36 +511,38 @@ public Result[] getPluginFailureResultConstraints() {
         }
 
         @SuppressWarnings("unused")
-        public FormValidation doLoginCheck(final StaplerRequest req, StaplerResponse rsp) {
-            final String name = Util.fixNull(req.getParameter("name"));
-            final String accessKey = Util.fixNull(req.getParameter("accessKey"));
-            final String secretKey = Util.fixNull(req.getParameter("secretKey"));
-            final String useIAMCredential = Util.fixNull(req.getParameter("useRole"));
+        @RequirePOST
+        public FormValidation doLoginCheck(@QueryParameter String name, @QueryParameter String accessKey,
+                                           @QueryParameter Secret secretKey, @QueryParameter boolean useRole) {
+            Jenkins.get().checkPermission(Jenkins.ADMINISTER);
 
-            final boolean couldBeValidated = !name.isEmpty() && !accessKey.isEmpty() && !secretKey.isEmpty();
-            final boolean useRole = Boolean.parseBoolean(useIAMCredential);
+            final String checkedName = Util.fixNull(name);
+            final String checkedAccessKey = Util.fixNull(accessKey);
+            final String checkedSecretKey = secretKey != null ? secretKey.getPlainText() : "";
+
+            final boolean couldBeValidated = !checkedName.isEmpty() && !checkedAccessKey.isEmpty() && !checkedSecretKey.isEmpty();
 
             if (!couldBeValidated) {
-                if (name.isEmpty()) {
+                if (checkedName.isEmpty()) {
                     return FormValidation.ok("Please, enter name");
                 }
 
                 if (useRole) {
                     return FormValidation.ok();
                 }
 
-                if (accessKey.isEmpty()) {
+                if (checkedAccessKey.isEmpty()) {
                     return FormValidation.ok("Please, enter accessKey");
                 }
 
-                if (secretKey.isEmpty()) {
+                if (checkedSecretKey.isEmpty()) {
                     return FormValidation.ok("Please, enter secretKey");
                 }
             }
 
             final String defaultRegion = ClientHelper.DEFAULT_AMAZON_S3_REGION_NAME;
             final AmazonS3Client client = ClientHelper.createClient(
-                    accessKey, secretKey, useRole, defaultRegion, Jenkins.get().proxy);
+                    checkedAccessKey, checkedSecretKey, useRole, defaultRegion, Jenkins.get().proxy);
 
             try {
                 client.listBuckets();

@@ -6,23 +6,20 @@
       <f:repeatable var="profile" items="${descriptor.profiles}">
         <table width="100%">
           <f:entry title="Profile name" help="/plugin/s3/help-profile.html">
-            <f:textbox name="s3.name" value="${profile.name}" onchange="Form.findMatchingInput(this,'s3.accessKey').onchange()"/>
+            <f:textbox name="name" value="${profile.name}" />
           </f:entry>
           <f:entry title="Use IAM Role" help="/plugin/s3/help-role.html">
-            <f:checkbox name="s3.useRole" value="${profile.useRole}" checked="${profile.useRole}"/>
+            <f:checkbox name="useRole" value="${profile.useRole}" />
           </f:entry>
           <f:entry title="Access key" help="/plugin/s3/help-accesskey.html">
-            <f:textbox name="s3.accessKey" value="${profile.accessKey}"
-                    checkMethod="post"
-                    checkUrl="'${rootURL}/publisher/S3BucketPublisher/loginCheck?name='+encodeURIComponent(Form.findMatchingInput(this,'s3.name').value)+'&amp;secretKey='+encodeURIComponent(Form.findMatchingInput(this,'s3.secretKey').value)+'&amp;accessKey='+encodeURIComponent(this.value)+'&amp;useRole='+encodeURIComponent(Form.findMatchingInput(this,'s3.useRole').value)"
-            />
+            <f:textbox name="accessKey" value="${profile.accessKey}" />
           </f:entry>
           <f:entry title="Secret key" help="/plugin/s3/help-secretkey.html">
-            <input class="setting-input" name="s3.secretKey"
-                    type="password" value="${profile.secretKey}"
-                    onchange="Form.findMatchingInput(this,'s3.accessKey').onchange()"
-            />
+            <f:password name="secretKey" value="${profile.secretKey}" />
           </f:entry>
+          <f:validateButton
+             title="${%Test Connection}" progress="${%Testing...}"
+             method="loginCheck" with="name,useRole,accessKey,secretKey" />
           <f:advanced>
             <f:entry title="Max upload retries">
                 <f:number name="s3.maxUploadRetries" value="${profile.maxUploadRetries}"/>

@@ -0,0 +1,42 @@
+package hudson.plugins.s3;
+
+import com.gargoylesoftware.htmlunit.HttpMethod;
+import com.gargoylesoftware.htmlunit.WebRequest;
+import com.gargoylesoftware.htmlunit.util.UrlUtils;
+import hudson.model.Item;
+import hudson.security.SecurityRealm;
+import jenkins.model.Jenkins;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+
+public class S3BucketPublisherTest {
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    @Test
+    public void testConfigExists() throws Exception {
+        SecurityRealm securityRealm = j.createDummySecurityRealm();
+        j.getInstance().setSecurityRealm(securityRealm);
+        j.getInstance().setAuthorizationStrategy(
+                new MockAuthorizationStrategy().grant(Item.READ, Item.DISCOVER).everywhere().toAuthenticated()
+                        .grant(Jenkins.READ, Item.DISCOVER).everywhere().toEveryone()
+                        .grant(Item.CONFIGURE).everywhere().to("bob")
+                        .grant(Jenkins.ADMINISTER).everywhere().to("alice"));
+        j.jenkins.setCrumbIssuer(null);
+
+        JenkinsRule.WebClient webClient = j.createWebClient();
+        webClient.getOptions().setThrowExceptionOnFailingStatusCode(false);
+        WebRequest request = new WebRequest(
+                UrlUtils.toUrlUnsafe(webClient.getContextPath() + "publisher/S3BucketPublisher/loginCheck?name=myname&accessKey=myAccess&secretKey=mykey&useRole=false"),
+                HttpMethod.POST);
+
+        webClient.login("bob", "bob");
+        Assert.assertEquals(403, webClient.getPage(request).getWebResponse().getStatusCode());
+
+        webClient = j.createWebClient().login("alice", "alice");
+        Assert.assertEquals(200, webClient.getPage(request).getWebResponse().getStatusCode());
+    }
+}