[SECURITY-402,SECURITY-403] Squashed commit of the following:

commit 0ad3bf0c4a82a47ee183bec085cf1e6537619d1a Merge: 51c33ef 0c80b27 Author: rsandell <[email protected]> Date: Tue Feb 20 16:38:01 2018 +0100 Merge remote-tracking branch 'origin' into SECURITY-403 commit 51c33ef34d46f5fc859d1273eb309a447b821bc7 Author: rsandell <[email protected]> Date: Fri Feb 16 17:50:24 2018 +0100 [SECURITY-403] Fixing test after csrf commit f8c49b7ba45344559e16815f6d5a8ed69856c1f8 Author: rsandell <[email protected]> Date: Thu Feb 15 13:38:00 2018 +0100 [SECURITY-403] Also try to prevent CSRF commit aaccb2517cf32accfbc67c1228e19c5cd5cad0dc Author: rsandell <[email protected]> Date: Wed Feb 14 18:32:23 2018 +0100 [SECURITY-403] Fixing the test commit 8663e69ed5db3d6fb5f0d9e550b47de011570434 Author: rsandell <[email protected]> Date: Thu Feb 1 13:36:32 2018 +0100 [SECURITY-402,SECURITY-403] More Admin checks One test failing that I haven't figured out why yet
jenkinsci · Feb 21, 2018 · a222f2d · a222f2d
1 parent 0c80b27
commit a222f2d
Show file tree

Hide file tree

Showing 6 changed files with 216 additions and 3 deletions.
diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritManagement.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritManagement.java
@@ -41,6 +41,7 @@
 import hudson.model.Hudson;
 import hudson.model.ManagementLink;
 import hudson.model.Saveable;
+import hudson.security.Permission;
 import hudson.util.FormValidation;
 import jenkins.model.Jenkins;
 import jenkins.model.ModelObjectWithContextMenu;
@@ -52,6 +53,7 @@
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.bind.JavaScriptMethod;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -112,6 +114,7 @@ public DescriptorImpl getDescriptor() {
 
     @Override
     public ContextMenu doContextMenu(StaplerRequest request, StaplerResponse response) throws Exception {
+        checkPermission();
         Jenkins jenkins = Jenkins.getInstance();
         assert jenkins != null;
         ContextMenu menu = new ContextMenu();
@@ -189,6 +192,7 @@ public AutoCompletionCandidates doAutoCompleteCopyNewItemFrom(@QueryParameter fi
      */
     @SuppressWarnings("unused") //Called from Jelly
     public List<GerritServer> getServers() {
+        checkPermission();
         PluginImpl plugin = PluginImpl.getInstance();
         if (plugin == null) {
             return Collections.emptyList();
@@ -204,6 +208,7 @@ public List<GerritServer> getServers() {
      */
     @SuppressWarnings("unused") //Called from Jelly
     public GerritServer getServer(String encodedServerName) {
+        checkPermission();
         String serverName;
         try {
             serverName = URLDecoder.decode(encodedServerName, CharEncoding.UTF_8);
@@ -259,7 +264,9 @@ public JSONObject getServerStatuses() {
      * @return the new GerritServer
      * @throws IOException when error sending redirect back to the list of servers
      */
+    @RequirePOST
     public GerritServer doAddNewServer(StaplerRequest req, StaplerResponse rsp) throws IOException {
+        checkPermission();
         String serverName = req.getParameter("name");
         PluginImpl plugin = PluginImpl.getInstance();
         assert plugin != null;
@@ -384,6 +391,7 @@ public List<String> getServerNames() {
      * @return ok or error.
      */
     public FormValidation doNameFreeCheck(@QueryParameter("value") final String value) {
+        checkPermission();
         PluginImpl plugin = PluginImpl.getInstance();
         assert plugin != null;
         if (plugin.containsServer(value)) {
@@ -404,9 +412,11 @@ public FormValidation doNameFreeCheck(@QueryParameter("value") final String valu
      * @throws IOException          if something unfortunate happens.
      * @throws InterruptedException if something unfortunate happens.
      */
+    @RequirePOST
     public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws ServletException,
             IOException,
             InterruptedException {
+        checkPermission();
         if (logger.isDebugEnabled()) {
             logger.debug("submit {}", req.toString());
         }
@@ -422,4 +432,26 @@ public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws Servl
         rsp.sendRedirect(".");
     }
 
+    /**
+     * Checks that the current user has {@link #getRequiredPermission()} permission.
+     * If Jenkins is currently active.
+     */
+    private void checkPermission() {
+        final Jenkins jenkins = Jenkins.getInstance();
+        if (jenkins != null) {
+            jenkins.checkPermission(getRequiredPermission());
+        }
+    }
+
+    /**
+     * {@link Jenkins#ADMINISTER} permission.
+     * Also used by Jelly
+     *
+     * @return the permission
+     */
+    @Override
+    public Permission getRequiredPermission() {
+        return Jenkins.ADMINISTER;
+    }
+
 }
diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java
@@ -39,6 +39,7 @@
 import hudson.model.Failure;
 import hudson.model.Descriptor;
 import hudson.model.Hudson;
+import hudson.security.Permission;
 import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
 import hudson.util.Secret;
@@ -88,6 +89,7 @@
 import org.kohsuke.stapler.bind.JavaScriptMethod;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -235,6 +237,7 @@ public IGerritHudsonTriggerConfig getConfig() {
      * @param config the config.
      */
     public void setConfig(IGerritHudsonTriggerConfig config) {
+        checkPermission();
         this.config = config;
     }
 
@@ -394,11 +397,33 @@ public boolean isLastServer() {
         return PluginImpl.getServers_().size() == 1;
     }
 
+    /**
+     * Checks that the current user has {@link #getRequiredPermission()} permission.
+     * If Jenkins is currently active.
+     */
+    private void checkPermission() {
+        final Jenkins jenkins = Jenkins.getInstance();
+        if (jenkins != null) {
+            jenkins.checkPermission(getRequiredPermission());
+        }
+    }
+
+    /**
+     * {@link Jenkins#ADMINISTER} permission for viewing and changing the configuration.
+     * Also used by Jelly
+     *
+     * @return the permission
+     */
+    public final Permission getRequiredPermission() {
+        return Jenkins.ADMINISTER;
+    }
+
     /**
      * Starts the server's project list updater, send command queue and event manager.
      *
      */
     public void start() {
+        checkPermission();
         logger.info("Starting GerritServer: " + name);
 
         //do not try to connect to gerrit unless there is a URL or a hostname in the text fields
@@ -445,6 +470,7 @@ private void initializeConnectionListener() {
      *
      */
     public void stop() {
+        checkPermission();
         logger.info("Stopping GerritServer " + name);
 
         if (projectListUpdater != null) {
@@ -521,6 +547,7 @@ public GerritConnectionListener getGerritConnectionListener() {
      *
      */
     public synchronized void startConnection() {
+        checkPermission();
         if (!config.hasDefaultValues()) {
             if (gerritConnection == null) {
                 logger.debug("Starting Gerrit connection...");
@@ -545,6 +572,7 @@ public synchronized void startConnection() {
      *
      */
     public synchronized void stopConnection() {
+        checkPermission();
         if (gerritConnection != null) {
             gerritConnection.shutdown(true);
             gerritConnection.removeListener(gerritConnectionListener);
@@ -868,9 +896,11 @@ public static Map<Notify, String> notificationLevelTextsById() {
      * @throws IOException if something unfortunate happens.
      * @throws InterruptedException if something unfortunate happens.
      */
+   @RequirePOST
     public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws ServletException,
     IOException,
     InterruptedException {
+        checkPermission();
         if (logger.isDebugEnabled()) {
             logger.debug("submit {}", req.toString());
         }
@@ -908,6 +938,7 @@ public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws Servl
      * @param newName the new name
      */
     private void rename(String newName) {
+        checkPermission();
         if (isConnected()) {
             stopConnection();
             stop();
@@ -1002,7 +1033,9 @@ private void removeGerritTriggerInJobs() {
      *
      * @return connection status.
      */
+    @RequirePOST
     public JSONObject doWakeup() {
+        checkPermission();
         Timer timer = new Timer();
         try {
             startConnection();
@@ -1047,7 +1080,9 @@ public void run() {
      *
      * @return connection status.
      */
+    @RequirePOST
     public JSONObject doSleep() {
+        checkPermission();
         Timer timer = new Timer();
         try {
             stopConnection();
@@ -1207,10 +1242,12 @@ private void setConnectionResponse(String response) {
      * @throws IOException if something unfortunate happens.
      * @throws InterruptedException if something unfortunate happens.
      */
+    @RequirePOST
     public void doRemoveConfirm(StaplerRequest req, StaplerResponse rsp) throws ServletException,
     IOException,
     InterruptedException {
 
+        checkPermission();
         stopConnection();
         stop();
         PluginImpl plugin = PluginImpl.getInstance();
@@ -1404,6 +1441,7 @@ public FormValidation doValidTimeCheck(
     public FormValidation doNameFreeCheck(
             @QueryParameter("value")
             final String value) {
+        checkPermission();
         if (!value.equals(name)) {
             if (PluginImpl.containsServer_(value)) {
                 return FormValidation.error("The server name " + value + " is already in use!");

diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/PluginImpl.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/PluginImpl.java
@@ -282,6 +282,7 @@ public static GerritServer getFirstServer_() {
      * @param servers the list to be set.
      */
     public void setServers(List<GerritServer> servers) {
+        checkAdmin();
         if (this.servers != servers) {
             this.servers.clear();
             this.servers.addAll(servers);
@@ -295,6 +296,7 @@ public void setServers(List<GerritServer> servers) {
      * @return the list after adding the server.
      */
     public List<GerritServer> addServer(GerritServer s) {
+        checkAdmin();
         servers.add(s);
         return servers;
     }
@@ -306,10 +308,23 @@ public List<GerritServer> addServer(GerritServer s) {
      * @return the list after removing the server.
      */
     public List<GerritServer> removeServer(GerritServer s) {
+        checkAdmin();
         servers.remove(s);
         return servers;
     }
 
+    /**
+     * Checks that the current user has {@link Jenkins#ADMINISTER} permission.
+     * If Jenkins is currently active.
+     */
+    private void checkAdmin() {
+        final Jenkins jenkins = Jenkins.getInstance(); //Hoping this method doesn't change meaning again
+        if (jenkins != null) {
+            //If Jenkins is not alive then we are not started, so no unauthorised user might do anything
+            jenkins.checkPermission(Jenkins.ADMINISTER);
+        }
+    }
+
 
     /**
      * Check whether the list of servers contains a GerritServer object of a specific name.

diff --git a/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/remove.jelly b/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/remove.jelly
@@ -1,7 +1,7 @@
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout"
          xmlns:t="/lib/hudson" xmlns:f="/lib/form" xmlns:i="jelly:fmt">
-    <l:layout title="${%Remove Server}" norefresh="true">
+    <l:layout title="${%Remove Server}" norefresh="true" permission="${it.requiredPermission}">
         <l:side-panel>
             <l:tasks>
                 <l:task icon="images/24x24/up.gif" href="${rootURL}/" title="${%Back to Dashboard}"/>

diff --git a/src/main/webapp/js/server-table.js b/src/main/webapp/js/server-table.js
@@ -22,6 +22,8 @@
  *  THE SOFTWARE.
  */
 
+/*global crumb*/
+
 function serverTable() {
     'use strict';
     // private funcs
@@ -158,17 +160,21 @@ function serverTable() {
     dataTable.subscribe("buttonClickEvent", function(oArgs) {
         var elButton = oArgs.target;
         var oRecord = this.getRecord(elButton);
+        var crumbStr = "";
+        if (crumb.fieldName !== null) {
+            crumbStr = crumb.fieldName + "=" + crumb.value;
+        }
 
         if (elButton.name === "server") {
             // for BtnServer
             if (oRecord.getData("status") === "up") {
                 YAHOO.log("Stop connection.");
                 elButton.firstElementChild.src = urlSysImg(24, "blue_anime.gif");
-                YAHOO.util.Connect.asyncRequest('GET', oRecord.getData("serverUrl") + "/sleep", connectionCallBack);
+                YAHOO.util.Connect.asyncRequest('POST', oRecord.getData("serverUrl") + "/sleep", connectionCallBack, crumbStr);
             } else if (oRecord.getData("status") === "down") {
                 YAHOO.log("Start connection.");
                 elButton.firstElementChild.src = urlSysImg(24, "red_anime.gif");
-                YAHOO.util.Connect.asyncRequest('GET', oRecord.getData("serverUrl") + "/wakeup", connectionCallBack);
+                YAHOO.util.Connect.asyncRequest('POST', oRecord.getData("serverUrl") + "/wakeup", connectionCallBack, crumbStr);
             }
         } else if (elButton.name === "edit") {
             // for btnEdit