All sensitive requests now require POST

jenkinsci · Jan 26, 2020 · 0c376d4 · 0c376d4
1 parent 30f2726
commit 0c376d4
Show file tree

Hide file tree

Showing 9 changed files with 97 additions and 27 deletions.
diff --git a/src/main/java/net/hurstfrost/hudson/sounds/HudsonSoundsNotifier.java b/src/main/java/net/hurstfrost/hudson/sounds/HudsonSoundsNotifier.java
@@ -19,6 +19,7 @@
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -370,7 +371,10 @@ public HudsonSoundsNotifier newInstance(@Nullable StaplerRequest req, @Nonnull J
 			return m;
 		}
 
+		@RequirePOST
 		public FormValidation doTestSound(@QueryParameter String selectedSound, @QueryParameter String soundArchive, @QueryParameter String playMethod, @QueryParameter String systemCommand, @QueryParameter int pipeTimeoutSecs) {
+			Jenkins.get().checkPermission(SoundsAgentAction.PERMISSION);
+
 			if (StringUtils.isEmpty(selectedSound)) {
 				return FormValidation.error("Please choose a sound to test.");
 			}
@@ -414,6 +418,7 @@ public FormValidation doCheckSystemCommand(@QueryParameter final String systemCo
          * @param value the value parameter from the request
          * @return a FormValidation
          */
+        @RequirePOST
 	    public FormValidation doCheckSoundArchive(@QueryParameter final String value) {
 			Jenkins.get().checkPermission(PERMISSION);
 

diff --git a/src/main/java/net/hurstfrost/hudson/sounds/SoundsAgentAction.java b/src/main/java/net/hurstfrost/hudson/sounds/SoundsAgentAction.java
@@ -17,6 +17,7 @@
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.*;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 import javax.servlet.ServletException;
 import javax.servlet.http.Cookie;
@@ -196,7 +197,8 @@ public FormValidation validateUrl(String soundUrl) {
 
 			return FormValidation.ok();
     	}
-
+
+    	@RequirePOST
 		public FormValidation doTestSound(@QueryParameter String selectedSound) {
 			Jenkins.get().checkPermission(PERMISSION);
 
@@ -207,7 +209,8 @@ public FormValidation doTestSound(@QueryParameter String selectedSound) {
 				return FormValidation.error(String.format("Sound failed : " + e));
 			}
 		}
-
+
+        @RequirePOST
 		public FormValidation doTestUrl(@QueryParameter String soundUrl) {
 			Jenkins.get().checkPermission(PERMISSION);
 

diff --git a/src/main/java/net/hurstfrost/hudson/sounds/SoundsBuildTask.java b/src/main/java/net/hurstfrost/hudson/sounds/SoundsBuildTask.java
@@ -17,6 +17,7 @@
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 import java.io.IOException;
 import java.net.MalformedURLException;
@@ -135,7 +136,8 @@ protected DescriptorImpl(Class<? extends SoundsBuildTask> clazz) {
         public boolean isApplicable(Class<? extends AbstractProject> jobType) {
             return true;
         }
-
+
+        @RequirePOST
         public FormValidation doCheckSoundUrl(@QueryParameter String soundUrl) {
 			Jenkins.get().checkPermission(SoundsAgentAction.PERMISSION);
 
@@ -184,6 +186,7 @@ public List<SoundBite> getSounds() {
         	return hudsonSoundsDescriptor.getSounds();
 		}
 
+		@RequirePOST
 		public FormValidation doTestSound(@QueryParameter String selectedSound) {
             Jenkins.get().checkPermission(SoundsAgentAction.PERMISSION);
 
@@ -194,7 +197,8 @@ public FormValidation doTestSound(@QueryParameter String selectedSound) {
 				return FormValidation.error(String.format("Sound failed : " + e));
 			}
 		}
-
+
+		@RequirePOST
 		public FormValidation doTestUrl(@QueryParameter String soundUrl) {
             Jenkins.get().checkPermission(SoundsAgentAction.PERMISSION);
 

diff --git a/src/main/resources/net/hurstfrost/hudson/sounds/HudsonSoundsNotifier/global.jelly b/src/main/resources/net/hurstfrost/hudson/sounds/HudsonSoundsNotifier/global.jelly
@@ -4,7 +4,7 @@
 	xmlns:f="/lib/form">
 	<f:section title="Jenkins Sounds">
 		<f:entry title="${%Sound archive location}" field="soundArchive" help="/plugin/sounds/help-globalConfig-archive.html">
-		     <f:textbox />
+		     <f:textbox checkMethod="post" />
 		</f:entry>
 		<f:radioBlock name="playMethod" value="BROWSER" title="Play through HTML5 Audio enabled browser." checked="${descriptor.playMethod=='BROWSER'}" help="/plugin/sounds/help-globalConfig-browser.html">
 		</f:radioBlock>

diff --git a/src/main/resources/net/hurstfrost/hudson/sounds/SoundsBuildTask/config.jelly b/src/main/resources/net/hurstfrost/hudson/sounds/SoundsBuildTask/config.jelly
@@ -14,7 +14,7 @@
 	</f:radioBlock>
 	<f:radioBlock name="soundSource" value="URL" title="from a URL" checked="${instance.sourceType=='URL'}" help="/plugin/sounds/help-soundsTask-url.html">
 		<f:entry title="URL" field="soundUrl">
-		     <f:textbox />
+		     <f:textbox checkMethod="post" />
 		</f:entry>
   		<f:validateButton method="testUrl" title="Test" with="soundUrl"/>
 	</f:radioBlock>

diff --git a/src/test/java/net/hurstfrost/hudson/sounds/HudsonSoundsNotifierTest.java b/src/test/java/net/hurstfrost/hudson/sounds/HudsonSoundsNotifierTest.java
@@ -20,7 +20,7 @@
 
 import static org.junit.Assert.*;
 
-public class HudsonSoundsNotifierTest {
+public class HudsonSoundsNotifierTest extends TestWithTools {
 	@Rule
 	public JenkinsRule j = new JenkinsRule();
 
@@ -177,14 +177,25 @@ public void directHttpDescriptorAccessWithPermission() throws Exception {
 
 		HtmlPage page;
 
-		page = webClient.goTo("descriptorByName/net.hurstfrost.hudson.sounds.HudsonSoundsNotifier/checkSoundArchive?value=file://nonexistantfile");
+		page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient, "descriptorByName/net.hurstfrost.hudson.sounds.HudsonSoundsNotifier/checkSoundArchive?value=file://nonexistantfile");
 		assertEquals("Resource not found at 'file://nonexistantfile'", page.asText());
 	}
 
 	@Test
 	public void directHttpDescriptorAccessWithoutPermission() throws Exception {
 		JenkinsRule.WebClient webClient = j.createWebClient();
+		HtmlPage page;
+
+		page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient, "descriptorByName/net.hurstfrost.hudson.sounds.HudsonSoundsNotifier/checkSoundArchive?value=file://nonexistantfile");
+		assertEquals(page.getWebResponse().getStatusCode(), HttpURLConnection.HTTP_FORBIDDEN);
+	}
+
+	@Test
+	public void directHttpDescriptorAccessWithGetDisallowed() throws Exception {
+		JenkinsRule.WebClient webClient = j.createWebClient();
+
+		webClient.login("configure");
 
-		webClient.assertFails("descriptorByName/net.hurstfrost.hudson.sounds.HudsonSoundsNotifier/checkSoundArchive?value=file://nonexistantfile", HttpURLConnection.HTTP_FORBIDDEN);
+		webClient.assertFails("descriptorByName/net.hurstfrost.hudson.sounds.HudsonSoundsNotifier/checkSoundArchive?value=file://nonexistantfile", HttpURLConnection.HTTP_BAD_METHOD);
 	}
 }
diff --git a/src/test/java/net/hurstfrost/hudson/sounds/SoundsAgentActionTest.java b/src/test/java/net/hurstfrost/hudson/sounds/SoundsAgentActionTest.java
@@ -26,7 +26,7 @@
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
-public class SoundsAgentActionTest {
+public class SoundsAgentActionTest extends TestWithTools {
     @Rule
     public JenkinsRule j = new JenkinsRule();
 
@@ -98,21 +98,35 @@ public void directHttpDescriptorAccessWithPermission() throws Exception {
 
         webClient.login("configure");
 
-        HtmlPage page = webClient.goTo("descriptorByName/net.hurstfrost.hudson.sounds.SoundsAgentAction/testUrl?soundUrl=http://localhost:8080/");
+        HtmlPage page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient,"descriptorByName/net.hurstfrost.hudson.sounds.SoundsAgentAction/testUrl?soundUrl=http://localhost:8080/");
         assertEquals("Sound played successfully", page.asText());
 
-        page = webClient.goTo("descriptorByName/net.hurstfrost.hudson.sounds.SoundsAgentAction/testSound?selectedSound=NO_SUCH_SOUND");
+        page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient, "descriptorByName/net.hurstfrost.hudson.sounds.SoundsAgentAction/testSound?selectedSound=NO_SUCH_SOUND");
         assertEquals("Sound failed : net.hurstfrost.hudson.sounds.UnplayableSoundBiteException : No such sound.", page.asText());
     }
 
     @Test
     public void directHttpDescriptorAccessWithoutPermission() throws Exception {
         JenkinsRule.WebClient webClient = j.createWebClient();
+        HtmlPage page;
 
         webClient.login("noconfigure");
 
-        webClient.assertFails("descriptorByName/net.hurstfrost.hudson.sounds.SoundsAgentAction/testUrl?soundUrl=http://localhost:8080/", HttpURLConnection.HTTP_FORBIDDEN);
-        webClient.assertFails("descriptorByName/net.hurstfrost.hudson.sounds.SoundsAgentAction/testSound?selectedSound=EXPLODE", HttpURLConnection.HTTP_FORBIDDEN);
+        page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient, "descriptorByName/net.hurstfrost.hudson.sounds.SoundsAgentAction/testUrl?soundUrl=http://localhost:8080/");
+        assertEquals(page.getWebResponse().getStatusCode(), HttpURLConnection.HTTP_FORBIDDEN);
+
+        page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient, "descriptorByName/net.hurstfrost.hudson.sounds.SoundsAgentAction/testSound?selectedSound=EXPLODE");
+        assertEquals(page.getWebResponse().getStatusCode(), HttpURLConnection.HTTP_FORBIDDEN);
+    }
+
+    @Test
+    public void directHttpDescriptorAccessWithGetDisallowed() throws Exception {
+        JenkinsRule.WebClient webClient = j.createWebClient();
+
+        webClient.login("configure");
+
+        webClient.assertFails("descriptorByName/net.hurstfrost.hudson.sounds.SoundsAgentAction/testUrl?soundUrl=http://localhost:8080/", HttpURLConnection.HTTP_BAD_METHOD);
+        webClient.assertFails("descriptorByName/net.hurstfrost.hudson.sounds.SoundsAgentAction/testSound?selectedSound=EXPLODE", HttpURLConnection.HTTP_BAD_METHOD);
     }
 
     @Test
@@ -359,7 +373,7 @@ public void soundsAreRemovedAfter5s() throws Exception {
         assertEquals(12, descriptor.version);
     }
 
-	/*
+    /*
 	@Test
 	public void testAudioFormats() throws Exception {
 		String[]	urls = new String[] {

diff --git a/src/test/java/net/hurstfrost/hudson/sounds/SoundsBuildTaskTest.java b/src/test/java/net/hurstfrost/hudson/sounds/SoundsBuildTaskTest.java
@@ -11,7 +11,7 @@
 
 import static org.junit.Assert.assertEquals;
 
-public class SoundsBuildTaskTest {
+public class SoundsBuildTaskTest extends TestWithTools {
     @Rule
     public JenkinsRule j = new JenkinsRule();
 
@@ -27,30 +27,44 @@ public void before() throws Exception {
     @Test
     public void directHttpDescriptorAccessWithPermission() throws Exception {
         JenkinsRule.WebClient webClient = j.createWebClient();
+        HtmlPage page;
 
         webClient.login("user");
 
-        HtmlPage page;
-
-        page = webClient.goTo("job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/testUrl?soundUrl=http://localhost:8080/");
+        page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient, "job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/testUrl?soundUrl=http://localhost:8080/");
         assertEquals("Sound played successfully", page.asText());
 
-        page = webClient.goTo("job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/testSound?selectedSound=NO_SUCH_SOUND");
+        page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient, "job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/testSound?selectedSound=NO_SUCH_SOUND");
         assertEquals("Sound failed : net.hurstfrost.hudson.sounds.UnplayableSoundBiteException : No such sound.", page.asText());
 
-        page = webClient.goTo("job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/checkSoundUrl?soundUrl=file://nonexistantfile");
+        page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient, "job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/checkSoundUrl?soundUrl=file://nonexistantfile");
         assertEquals("Resource not found or not readable", page.asText());
-
-        page = webClient.goTo("job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/checkSoundUrl?soundUrl=http://localhost:8080/");
-        assertEquals("", page.asText());
     }
 
     @Test
     public void directHttpDescriptorAccessWithoutPermission() throws Exception {
         JenkinsRule.WebClient webClient = j.createWebClient();
+        HtmlPage page;
+
+        page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient, "job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/testUrl?soundUrl=http://localhost:8080/");
+        assertEquals(page.getWebResponse().getStatusCode(), HttpURLConnection.HTTP_FORBIDDEN);
+
+        page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient, "job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/testSound?selectedSound=EXPLODE");
+        assertEquals(page.getWebResponse().getStatusCode(), HttpURLConnection.HTTP_FORBIDDEN);
+
+        page = whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(webClient, "job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/checkSoundUrl?soundUrl=file://nonexistantfile");
+        assertEquals(page.getWebResponse().getStatusCode(), HttpURLConnection.HTTP_FORBIDDEN);
+    }
+
+    @Test
+    public void directHttpDescriptorAccessWithGetDisallowed() throws Exception {
+        JenkinsRule.WebClient webClient = j.createWebClient();
+
+        webClient.login("user");
+
+        webClient.assertFails("descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/testUrl?soundUrl=http://localhost:8080/", HttpURLConnection.HTTP_BAD_METHOD);
+        webClient.assertFails("descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/testSound?selectedSound=EXPLODE", HttpURLConnection.HTTP_BAD_METHOD);
 
-        webClient.assertFails("job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/testUrl?soundUrl=http://localhost:8080/", HttpURLConnection.HTTP_FORBIDDEN);
-        webClient.assertFails("job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/testSound?selectedSound=EXPLODE", HttpURLConnection.HTTP_FORBIDDEN);
-        webClient.assertFails("job/freestyle/descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/checkSoundUrl?soundUrl=file://nonexistantfile", HttpURLConnection.HTTP_FORBIDDEN);
+        webClient.assertFails("descriptorByName/net.hurstfrost.hudson.sounds.SoundsBuildTask/checkSoundUrl?soundUrl=file://nonexistantfile", HttpURLConnection.HTTP_BAD_METHOD);
     }
 }
diff --git a/src/test/java/net/hurstfrost/hudson/sounds/TestWithTools.java b/src/test/java/net/hurstfrost/hudson/sounds/TestWithTools.java
@@ -0,0 +1,19 @@
+package net.hurstfrost.hudson.sounds;
+
+import com.gargoylesoftware.htmlunit.HttpMethod;
+import com.gargoylesoftware.htmlunit.Page;
+import com.gargoylesoftware.htmlunit.WebRequest;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import org.jvnet.hudson.test.JenkinsRule;
+
+import java.io.IOException;
+import java.net.URL;
+
+public class TestWithTools {
+    protected HtmlPage whyDoesntJenkinsRuleWebClientLetMeDoPostForPage(JenkinsRule.WebClient webClient, String relative) throws IOException {
+        WebRequest webRequest = new WebRequest(new URL(webClient.getContextPath() + relative), HttpMethod.POST);
+        webClient.addCrumb(webRequest);
+        Page page = webClient.loadWebResponseInto(webClient.loadWebResponse(webRequest), webClient.getCurrentWindow());
+        return (HtmlPage) page;
+    }
+}