Merge branch 'master' into buildparam_158_202

Jenkinsfile

@@ -1,5 +1,5 @@
 #!groovy
-def recentLTS = "2.222.4"
+def recentLTS = "2.249.1"
 buildPlugin(configurations: [
   [ platform: "linux", jdk: "8", jenkins: null ],
   [ platform: "windows", jdk: "8", jenkins: recentLTS, javaLevel: "8" ],

pom.xml

@@ -50,14 +50,15 @@
   </scm>
 
   <properties>
-    <revision>2.9</revision>
+    <revision>2.11</revision>
     <changelist>-SNAPSHOT</changelist>
     <java.level>8</java.level>
     <workflow-support.version>3.4</workflow-support.version>
     <jenkins.version>2.138.4</jenkins.version>
     <configuration-as-code.version>1.25</configuration-as-code.version>
     <!-- For compatibility with modern Jenkins LTS -->
     <slf4jVersion>1.7.26</slf4jVersion>
+    <jenkins-test-harness.version>2.67</jenkins-test-harness.version>
   </properties>
 
   <dependencies>

src/main/java/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction.java

@@ -26,6 +26,7 @@
 import org.jenkins.plugins.lockableresources.Messages;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 @Extension
 public class LockableResourcesRootAction implements RootAction {
@@ -84,6 +85,7 @@ public int getNumberOfAllLabels() {
 		return LockableResourcesManager.get().getAllLabels().size();
 	}
 
+	@RequirePOST
 	public void doUnlock(StaplerRequest req, StaplerResponse rsp)
 			throws IOException, ServletException {
 		Jenkins.get().checkPermission(UNLOCK);
@@ -102,6 +104,7 @@ public void doUnlock(StaplerRequest req, StaplerResponse rsp)
 		rsp.forwardToPreviousPage(req);
 	}
 
+	@RequirePOST
 	public void doReserve(StaplerRequest req, StaplerResponse rsp)
 		throws IOException, ServletException {
 		Jenkins.get().checkPermission(RESERVE);
@@ -122,6 +125,7 @@ public void doReserve(StaplerRequest req, StaplerResponse rsp)
 		rsp.forwardToPreviousPage(req);
 	}
 
+	@RequirePOST
 	public void doUnreserve(StaplerRequest req, StaplerResponse rsp)
 		throws IOException, ServletException {
 		Jenkins.get().checkPermission(RESERVE);
@@ -146,6 +150,7 @@ public void doUnreserve(StaplerRequest req, StaplerResponse rsp)
 		rsp.forwardToPreviousPage(req);
 	}
 
+	@RequirePOST
 	public void doReset(StaplerRequest req, StaplerResponse rsp)
 		throws IOException, ServletException {
 		Jenkins.get().checkPermission(UNLOCK);

src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/index.jelly

@@ -1,6 +1,6 @@
 <!--
   Copyright 2013, 6WIND S.A. All rights reserved.
-  Copyright 2019, TobiX
+  Copyright 2019-2020, TobiX
 
   This file is part of the Jenkins Lockable Resources Plugin and is
   published under the MIT license.
@@ -9,18 +9,9 @@
  -->
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout">
-  <script>
-    function find_resource_name(element) {
-      var row = element.up('tr');
-      var resourceName = row.getAttribute('data-resource-name');
-      return resourceName;
-    }
-    function resource_action(button, action) {
-      window.location.assign(action + "?resource=" + find_resource_name(button));
-    }
-  </script>
-
   <l:layout title="${it.displayName}" type="one-column">
+    <script type="text/javascript"
+            src="${resURL}/plugin/lockable-resources/js/lockable-resources.js"></script>
     <l:main-panel>
       <h1>${%Lockable Resources}</h1>
       <table class="pane" style="width: auto;">

src/main/webapp/js/lockable-resources.js

@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2020, Tobias Gruetzmacher
+
+function find_resource_name(element) {
+  var row = element.up('tr');
+  var resourceName = row.getAttribute('data-resource-name');
+  return resourceName;
+}
+
+function resource_action(button, action) {
+  // TODO: Migrate to form:link after Jenkins 2.233 (for button-styled links)
+  var form = document.createElement('form');
+  form.setAttribute('method', 'POST');
+  form.setAttribute('action', action + "?resource=" + find_resource_name(button));
+  crumb.appendToForm(form);
+  document.body.appendChild(form);
+  form.submit();
+}

src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java

@@ -2,6 +2,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasEntry;
+import static org.jenkins.plugins.lockableresources.TestHelpers.clickButton;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -434,7 +435,7 @@ public void unlockButtonWithWaitingRuns() throws Exception {
         j.waitForMessage(
             "[resource1] is locked by " + prevBuild.getFullDisplayName() + ", waiting...", rNext);
         isPaused(rNext, 1, 1);
-        wc.goTo("lockable-resources/unlock?resource=resource1");
+        clickButton(wc, "unlock");
       }
 
       j.waitForMessage("Lock acquired on [resource1]", rNext);
@@ -494,7 +495,7 @@ public void manualUnreserveUnblocksJob() throws Exception {
     LockableResourcesManager.get().createResource("resource1");
     JenkinsRule.WebClient wc = j.createWebClient();
 
-    wc.goTo("lockable-resources/reserve?resource=resource1");
+    clickButton(wc, "reserve");
     LockableResource resource1 = LockableResourcesManager.get().fromName("resource1");
     assertNotNull(resource1);
     resource1.setReservedBy("someone");
@@ -516,7 +517,7 @@ public void manualUnreserveUnblocksJob() throws Exception {
 
     WorkflowRun r = p.scheduleBuild2(0).waitForStart();
     j.waitForMessage("[resource1] is locked, waiting...", r);
-    wc.goTo("lockable-resources/unreserve?resource=resource1");
+    clickButton(wc, "unreserve");
     SemaphoreStep.waitForStart("wait-inside/1", r);
     SemaphoreStep.success("wait-inside/1", null);
     j.assertBuildStatusSuccess(j.waitForCompletion(r));

src/test/java/org/jenkins/plugins/lockableresources/LockableResourceApiTest.java

@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: MIT
+ * Copyright (c) 2020, Tobias Gruetzmacher
+ */
+package org.jenkins.plugins.lockableresources;
+
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import hudson.security.FullControlOnceLoggedInAuthorizationStrategy;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.jenkins.plugins.lockableresources.TestHelpers.clickButton;
+import static org.junit.Assert.assertThrows;
+
+public class LockableResourceApiTest {
+
+  @Rule
+  public JenkinsRule j = new JenkinsRule();
+
+  @Test
+  public void reserveUnreserveApi() throws Exception {
+    LockableResourcesManager.get().createResource("a1");
+
+    j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+    j.jenkins.setAuthorizationStrategy(new FullControlOnceLoggedInAuthorizationStrategy());
+
+    JenkinsRule.WebClient wc = j.createWebClient();
+    wc.login("user");
+    clickButton(wc, "reserve");
+    assertThat(LockableResourcesManager.get().fromName("a1").isReserved(), is(true));
+    clickButton(wc, "unreserve");
+    assertThat(LockableResourcesManager.get().fromName("a1").isReserved(), is(false));
+  }
+
+  @Test
+  @Issue("SECURITY-1958")
+  public void apiUsageHttpGet() {
+    JenkinsRule.WebClient wc = j.createWebClient();
+    FailingHttpStatusCodeException e = assertThrows(FailingHttpStatusCodeException.class,
+      () -> wc.goTo("lockable-resources/reserve?resource=resource1"));
+    assertThat(e.getStatusCode(), is(405));
+  }
+
+}

src/test/java/org/jenkins/plugins/lockableresources/TestHelpers.java

@@ -6,9 +6,13 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
 
+import com.gargoylesoftware.htmlunit.html.HtmlElement;
+import com.gargoylesoftware.htmlunit.html.HtmlElementUtil;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
 import hudson.model.FreeStyleProject;
 import hudson.model.Queue;
 import java.io.IOException;
+import java.util.List;
 import jenkins.model.Jenkins;
 import net.sf.json.JSONArray;
 import net.sf.json.JSONObject;
@@ -65,4 +69,19 @@ public static JSONObject getResourceFromApi(
   public static JSONObject getApiData(JenkinsRule rule) throws IOException {
     return rule.getJSON("plugin/lockable-resources/api/json").getJSONObject();
   }
+
+  // Currently assumes one resource or only clicks the button for the first resource
+  public static void clickButton(JenkinsRule.WebClient wc, String action) throws Exception {
+    HtmlPage htmlPage = wc.goTo("lockable-resources");
+    List<HtmlElement> allButtons = htmlPage.getDocumentElement().getElementsByTagName("button");
+
+    HtmlElement reserveButton = null;
+    for (HtmlElement b : allButtons) {
+      String onClick = b.getAttribute("onClick");
+      if (onClick != null && onClick.contains(action)) {
+        reserveButton = b;
+      }
+    }
+    HtmlElementUtil.click(reserveButton);
+  }
 }