Merge pull request #11 from lacostej/SECURITY-1025

Fix SECURITY-1025

src/main/java/hudson/plugins/batch_task/BatchRun.java

@@ -16,6 +16,7 @@
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.framework.io.LargeText;
+import org.kohsuke.stapler.verb.POST;
 
 import java.io.File;
 import java.io.FileOutputStream;

src/main/java/hudson/plugins/batch_task/BatchTask.java

@@ -39,6 +39,7 @@
 import java.util.regex.Pattern;
 
 import com.thoughtworks.xstream.converters.basic.AbstractSingleValueConverter;
+import org.kohsuke.stapler.verb.POST;
 
 /**
  * A batch task.
@@ -264,6 +265,7 @@ public Object getDynamic(String token, StaplerRequest req, StaplerResponse rsp)
     /**
      * Schedules the execution
      */
+    @POST
     public synchronized void doExecute( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
         getACL().checkPermission(AbstractProject.BUILD);
 
@@ -278,6 +280,7 @@ public synchronized void doExecute( StaplerRequest req, StaplerResponse rsp ) th
     /**
      * Deletes this task.
      */
+    @POST
     public synchronized void doDoDelete(StaplerResponse rsp) throws IOException, ServletException {
         getACL().checkPermission(AbstractProject.DELETE);
 
@@ -319,6 +322,7 @@ private int[] parse(String num) {
 
     private static final Pattern BUILD_NUMBER_PATTERN = Pattern.compile("(\\d+)-(\\d+)");
 
+    @POST
 	public void doCancelQueue(StaplerRequest req, StaplerResponse rsp)
 			throws IOException, ServletException {
         checkAbortPermission();

src/main/resources/hudson/plugins/batch_task/BatchTask/sidepanel.jelly

@@ -15,11 +15,11 @@
       <l:task icon="icon-previous icon-md" href="../../" title="${%Back to Task List}" />
       <j:if test="${it.hasBuildPermission()}">
         <j:if test="${!it.disabled}">
-          <l:task icon="icon-clock icon-md" href="${url}/execute" title="${%Build Now}" />
+          <l:task icon="icon-clock icon-md" href="${url}/execute" title="${%Build Now}" post="true" />
         </j:if>
       </j:if>
       <j:if test="${it.hasDeletePermission()}">
-        <l:task icon="icon-edit-delete icon-md" href="${url}/delete" title="${%Delete Task}" />
+        <l:task icon="icon-edit-delete icon-md" href="${url}/doDelete" post="true" requiresConfirmation="true" title="${%Delete Task}" />
       </j:if>
       <!-- Configure doesn't seem to lead anywhere -->
       <!--j:if test="${it.hasConfigurePermission()}">

src/test/java/hudson/plugins/batch_task/BatchTaskTest.java

@@ -60,7 +60,7 @@ public void testNoBuilds() throws Exception {
         FreeStyleProject p = r.createFreeStyleProject();
         p.addProperty(new BatchTaskProperty(new BatchTask("test", "echo hello")));
         JenkinsRule.WebClient wc = r.createWebClient();
-        HtmlPage page = wc.getPage(p, "batchTasks/task/test/execute");
+        HtmlPage page = TestHelper.assertPost(wc, p.getUrl() + "batchTasks/task/test/execute", "text/html", 200);
         String path = page.getWebResponse().getWebRequest().getUrl().getPath();
         assertTrue("should redirect to noBuilds page: " + path, path.endsWith("/noBuild"));
     }
@@ -72,7 +72,6 @@ public void testNoBuilds() throws Exception {
      */
     @Test
     public void testExecute() throws Exception {
-
         r.jenkins.getGlobalNodeProperties().add(new EnvironmentVariablesNodeProperty(
                 new EnvironmentVariablesNodeProperty.Entry("GLOBAL", "global-property"),
                 new EnvironmentVariablesNodeProperty.Entry("OVERRIDE_ME", "foo")));
@@ -93,7 +92,7 @@ public void testExecute() throws Exception {
         while (freeStyleBuild.isBuilding()) {
             Thread.sleep(100);
         }
-        r.createWebClient().getPage(p, "batchTasks/task/test/execute");
+        TestHelper.assertPost(r.createWebClient(), p.getUrl() + "batchTasks/task/test/execute", "text/html", 200);
         BatchRun run = task.getLastRun();
         assertNotNull("task did not run", run);
         CauseAction ca = run.getAction(CauseAction.class);

src/test/java/hudson/plugins/batch_task/TestHelper.java

@@ -0,0 +1,46 @@
+package hudson.plugins.batch_task;
+
+import org.htmlunit.FailingHttpStatusCodeException;
+import org.htmlunit.HttpMethod;
+import org.htmlunit.WebRequest;
+import org.htmlunit.html.HtmlPage;
+import org.jvnet.hudson.test.JenkinsRule;
+
+import java.io.IOException;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
+
+public class TestHelper {
+    /**
+     * Performs an HTTP POST request to the relative url.
+     *
+     * @param webClient the client
+     * @param relative the url relative to the context path
+     * @param expectedContentType if expecting specific content type or null if not
+     * @param expectedStatus if expecting a http status code or null if not
+     * @throws IOException if so
+     */
+    public static HtmlPage assertPost(JenkinsRule.WebClient webClient, String relative,
+                                      String expectedContentType, Integer expectedStatus) throws IOException {
+        WebRequest request = new WebRequest(webClient.createCrumbedUrl(relative), HttpMethod.POST);
+        try {
+            HtmlPage p = webClient.getPage(request);
+            if (expectedContentType != null) {
+                assertThat(p.getWebResponse().getContentType(), is(expectedContentType));
+            }
+            if (expectedStatus != null) {
+                assertEquals(expectedStatus.intValue(), p.getWebResponse().getStatusCode());
+            }
+            return p;
+        } catch (FailingHttpStatusCodeException e) {
+            if (expectedStatus != null) {
+                assertEquals(expectedStatus.intValue(), e.getStatusCode());
+                return null;
+            } else {
+                throw e;
+            }
+        }
+    }
+}