[FIXED HUDSON-2715] Field validators in configure screen shouldn't require the...

[FIXED HUDSON-2715] Field validators in configure screen shouldn't require the admin permission where possible.

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@13838 71c3de6d-444a-0410-be80-ed276b4c234a

core/pom.xml

@@ -231,7 +231,7 @@
     <dependency>
       <groupId>org.kohsuke.stapler</groupId>
       <artifactId>stapler</artifactId>
-      <version>1.88</version>
+      <version>1.89</version>
     </dependency>
     <dependency>
       <groupId>org.jvnet.localizer</groupId>

core/src/main/java/hudson/Util.java

@@ -148,7 +148,7 @@ public class Util {
         if(!logfile.exists())
             return "";
 
-        StringBuffer str = new StringBuffer((int)logfile.length());
+        StringBuilder str = new StringBuilder((int)logfile.length());
 
         BufferedReader r = new BufferedReader(new InputStreamReader(new FileInputStream(logfile),charset));
         char[] buf = new char[1024];
@@ -433,7 +433,7 @@ public class Util {
     }
 
     public static String toHexString(byte[] data, int start, int len) {
-        StringBuffer buf = new StringBuffer();
+        StringBuilder buf = new StringBuilder();
         for( int i=0; i<len; i++ ) {
             int b = data[start+i]&0xFF;
             if(b<16)    buf.append('0');
@@ -548,7 +548,7 @@ public class Util {
         try {
             boolean escaped = false;
 
-            StringBuffer out = new StringBuffer(s.length());
+            StringBuilder out = new StringBuilder(s.length());
 
             ByteArrayOutputStream buf = new ByteArrayOutputStream();
             OutputStreamWriter w = new OutputStreamWriter(buf,"UTF-8");
@@ -581,7 +581,7 @@ public class Util {
      * Escapes HTML unsafe characters like &lt;, &amp;to the respective character entities.
      */
     public static String escape(String text) {
-        StringBuffer buf = new StringBuffer(text.length()+64);
+        StringBuilder buf = new StringBuilder(text.length()+64);
         for( int i=0; i<text.length(); i++ ) {
             char ch = text.charAt(i);
             if(ch=='\n')
@@ -607,7 +607,7 @@ public class Util {
     }
 
     public static String xmlEscape(String text) {
-        StringBuffer buf = new StringBuffer(text.length()+64);
+        StringBuilder buf = new StringBuilder(text.length()+64);
         for( int i=0; i<text.length(); i++ ) {
             char ch = text.charAt(i);
             if(ch=='<')

core/src/main/java/hudson/model/AbstractItem.java

@@ -196,6 +196,27 @@ public abstract class AbstractItem extends Actionable implements Item, HttpDelet
         return Items.getConfigFile(this);
     }
 
+    /**
+     * Exposes {@link Descriptor} by its name to URL.
+     *
+     * This allows descriptor to perform a job-specific check easily. 
+     *
+     * @since 1.270.
+     *
+     * @param className
+     *      Either fully qualified class name (recommended) or the short name.
+     */
+    public Descriptor getDescriptorByName(String className) {
+        for( Descriptor d : Descriptor.ALL ) {
+            String name = d.clazz.getName();
+            if(name.equals(className))
+                return d;
+            if(name.substring(name.lastIndexOf('.')+1).equals(className))
+                return d;
+        }
+        return null;
+    }
+
     /**
      * Accepts the new description.
      */

core/src/main/java/hudson/model/Hudson.java

@@ -2306,7 +2306,7 @@ public final class Hudson extends View implements ItemGroup<TopLevelItem>, Node,
     public void doItemExistsCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
         // this method can be used to check if a file exists anywhere in the file system,
         // so it should be protected.
-        new FormFieldValidator(req,rsp,true) {
+        new FormFieldValidator(req,rsp,Item.CREATE) {
             protected void check() throws IOException, ServletException {
                 String job = fixEmpty(request.getParameter("value"));
                 if(job==null) {

core/src/main/java/hudson/scm/SubversionSCM.java

@@ -1258,8 +1258,8 @@ public class SubversionSCM extends SCM implements Serializable {
          * validate the value for a remote (repository) location.
          */
         public void doSvnRemoteLocationCheck(final StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
-            // this can be used to hit any accessible URL, so limit that to admins
-            new FormFieldValidator(req, rsp, true) {
+            // false==No permisison needed for basic check
+            new FormFieldValidator(req, rsp, false) {
                 protected void check() throws IOException, ServletException {
                     // syntax check first
                     String url = Util.nullify(request.getParameter("value"));
@@ -1277,8 +1277,10 @@ public class SubversionSCM extends SCM implements Serializable {
                         return;
                     }
 
-                    // test the connection
-                    try {
+                    // Test the connection only if we have admin permission
+                    if (!Hudson.getInstance().hasPermission(Hudson.ADMINISTER)) {
+                        ok();
+                    } else try {
                         SVNURL repoURL = SVNURL.parseURIDecoded(url);
                         if (checkRepositoryPath(repoURL)==SVNNodeKind.NONE) {
                             SVNRepository repository = null;
@@ -1370,6 +1372,7 @@ public class SubversionSCM extends SCM implements Serializable {
          * validate the value for a local location (local checkout directory).
          */
         public void doSvnLocalLocationCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
+            // false==No permission needed for this syntax check
             new FormFieldValidator(req, rsp, false) {
                 protected void check() throws IOException, ServletException {
                     String v = Util.nullify(request.getParameter("value"));

core/src/main/java/hudson/scm/browsers/FishEyeCVS.java

@@ -2,6 +2,7 @@ package hudson.scm.browsers;
 
 import hudson.Util;
 import hudson.model.Descriptor;
+import hudson.model.Hudson;
 import hudson.scm.CVSChangeLogSet;
 import hudson.scm.CVSChangeLogSet.File;
 import hudson.scm.CVSChangeLogSet.Revision;
@@ -70,7 +71,8 @@ public final class FishEyeCVS extends CVSRepositoryBrowser {
         }
 
         public void doCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
-            new FormFieldValidator.URLCheck(req,rsp) {
+            // false==No permission needed for basic check
+            new FormFieldValidator(req,rsp,false) {
                 @Override
                 protected void check() throws IOException, ServletException {
                     String value = Util.fixEmpty(request.getParameter("value"));
@@ -85,14 +87,25 @@ public final class FishEyeCVS extends CVSRepositoryBrowser {
                         errorWithMarkup("The URL should end like <tt>.../browse/foobar/</tt>");
                         return;
                     }
-                    try {
-                        if (findText(open(new URL(value)), "FishEye")) {
-                            ok();
-                        } else {
-                            error("This is a valid URL but it doesn't look like FishEye");
-                        }
-                    } catch (IOException e) {
-                        handleIOException(value, e);
+                    // Connect to URL and check content only if we have admin permission
+                    if (Hudson.getInstance().hasPermission(Hudson.ADMINISTER)) {
+                        final String finalValue = value;
+                        new FormFieldValidator.URLCheck(request,response) {
+                            @Override
+                            protected void check() throws IOException, ServletException {
+                                try {
+                                    if (findText(open(new URL(finalValue)), "FishEye")) {
+                                        ok();
+                                    } else {
+                                        error("This is a valid URL but it doesn't look like FishEye");
+                                    }
+                                } catch (IOException e) {
+                                    handleIOException(finalValue, e);
+                                }
+                            }
+                        }.process();
+                    } else {
+                        ok();
                     }
                 }
             }.process();

core/src/main/java/hudson/scm/browsers/FishEyeSVN.java

@@ -2,6 +2,7 @@ package hudson.scm.browsers;
 
 import static hudson.Util.fixEmpty;
 import hudson.model.Descriptor;
+import hudson.model.Hudson;
 import hudson.scm.RepositoryBrowser;
 import hudson.scm.SubversionChangeLogSet.LogEntry;
 import hudson.scm.SubversionChangeLogSet.Path;
@@ -116,7 +117,9 @@ public class FishEyeSVN extends SubversionRepositoryBrowser {
          * Performs on-the-fly validation of the URL.
          */
         public void doCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
-            new FormFieldValidator.URLCheck(req,rsp) {
+            // false==No permission needed for basic check
+            new FormFieldValidator(req,rsp,false) {
+                @Override
                 protected void check() throws IOException, ServletException {
                     String value = fixEmpty(request.getParameter("value"));
                     if(value==null) {// nothing entered yet
@@ -130,14 +133,25 @@ public class FishEyeSVN extends SubversionRepositoryBrowser {
                         return;
                     }
 
-                    try {
-                        if(findText(open(new URL(value)),"FishEye")) {
-                            ok();
-                        } else {
-                            error("This is a valid URL but it doesn't look like FishEye");
-                        }
-                    } catch (IOException e) {
-                        handleIOException(value,e);
+                    // Connect to URL and check content only if we have admin permission
+                    if (Hudson.getInstance().hasPermission(Hudson.ADMINISTER)) {
+                        final String finalValue = value;
+                        new FormFieldValidator.URLCheck(request,response) {
+                            @Override
+                            protected void check() throws IOException, ServletException {
+                                try {
+                                    if(findText(open(new URL(finalValue)),"FishEye")) {
+                                        ok();
+                                    } else {
+                                        error("This is a valid URL but it doesn't look like FishEye");
+                                    }
+                                } catch (IOException e) {
+                                    handleIOException(finalValue,e);
+                                }
+                            }
+                        }.process();
+                    } else {
+                        ok();
                     }
                 }
             }.process();

core/src/main/java/hudson/scm/browsers/Sventon.java

@@ -118,6 +118,7 @@ public class Sventon extends SubversionRepositoryBrowser {
          * Performs on-the-fly validation of the URL.
          */
         public void doCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
+            // URLCheck requires Admin permission
             new FormFieldValidator.URLCheck(req,rsp) {
                 protected void check() throws IOException, ServletException {
                     String value = fixEmpty(request.getParameter("value"));

core/src/main/java/hudson/security/AccessControlled.java

 package hudson.security;
 
+import org.acegisecurity.AccessDeniedException;
+
 /**
  * Object that has an {@link ACL}
  *
@@ -17,7 +19,7 @@ public interface AccessControlled {
     /**
      * Convenient short-cut for {@code getACL().checkPermission(permission)}
      */
-    void checkPermission(Permission permission);
+    void checkPermission(Permission permission) throws AccessDeniedException;
 
     /**
      * Convenient short-cut for {@code getACL().hasPermission(permission)}

core/src/main/java/hudson/tasks/BuildTrigger.java

 package hudson.tasks;
 
 import hudson.Launcher;
+import hudson.security.AccessControlled;
 import hudson.matrix.MatrixAggregatable;
 import hudson.matrix.MatrixAggregator;
 import hudson.matrix.MatrixBuild;
@@ -272,7 +273,9 @@ public class BuildTrigger extends Publisher implements DependecyDeclarer, Matrix
          * Form validation method.
          */
         public void doCheck( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
-            new FormFieldValidator(req,rsp,true) {
+            // Require CONFIGURE permission on this project
+            AccessControlled anc = (AccessControlled) req.findAncestor(AccessControlled.class).getObject();
+            new FormFieldValidator(req,rsp,anc,Item.CONFIGURE) {
                 protected void check() throws IOException, ServletException {
                     String list = request.getParameter("value");
 

core/src/main/java/hudson/tasks/JavadocArchiver.java

@@ -187,6 +187,7 @@ public class JavadocArchiver extends Publisher {
          * Performs on-the-fly validation on the file mask wildcard.
          */
         public void doCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
+            // WorkspaceDirectory requires CONFIGURE permission on this project
             new FormFieldValidator.WorkspaceDirectory(req,rsp).process();
         }
 

core/src/main/java/hudson/tasks/Shell.java

@@ -127,6 +127,7 @@ public class Shell extends CommandInterpreter {
          * Check the existence of sh in the given location.
          */
         public void doCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
+            // Executable requires admin permission
             new FormFieldValidator.Executable(req,rsp).process();
         }
     }

core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java

@@ -7,6 +7,7 @@ import hudson.model.AbstractProject;
 import hudson.model.BuildListener;
 import hudson.model.Fingerprint.RangeSet;
 import hudson.model.Hudson;
+import hudson.model.Item;
 import hudson.model.Result;
 import hudson.model.Run;
 import hudson.model.TaskListener;
@@ -226,7 +227,10 @@ public class AggregatedTestResultPublisher extends Publisher {
         }
 
         public void doCheck(StaplerRequest req, StaplerResponse rsp, @QueryParameter final String value) throws IOException, ServletException {
-            new FormFieldValidator(req,rsp,false) {
+            // Require CONFIGURE permission on this project
+            AbstractProject project = (AbstractProject) req.findAncestor(AbstractProject.class).getObject();
+
+            new FormFieldValidator(req,rsp,project,Item.CONFIGURE) {
                 protected void check() throws IOException, ServletException {
                     for (String name : Util.tokenize(value, ",")) {
                         name = name.trim();

core/src/main/java/hudson/triggers/TimerTrigger.java

@@ -59,7 +59,9 @@ public class TimerTrigger extends Trigger<BuildableItem> {
          * Performs syntax check.
          */
         public void doCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
-            new FormFieldValidator(req,rsp,true) {
+            // false==No permission needed for this syntax check
+            new FormFieldValidator(req,rsp,false) {
+                @Override
                 protected void check() throws IOException, ServletException {
                     try {
                         String msg = CronTabList.create(fixNull(request.getParameter("value"))).checkSanity();

core/src/main/java/hudson/util/FormFieldValidator.java

@@ -6,6 +6,8 @@ import hudson.FilePath;
 import hudson.Util;
 import hudson.model.AbstractProject;
 import hudson.model.Hudson;
+import hudson.model.Item;
+import hudson.model.Job;
 import hudson.security.Permission;
 import hudson.security.AccessControlled;
 
@@ -21,6 +23,7 @@ import javax.servlet.ServletException;
 
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
+import org.acegisecurity.AccessDeniedException;
 
 /**
  * Base class that provides the framework for doing on-the-fly form field validation.
@@ -39,12 +42,13 @@ public abstract class FormFieldValidator {
     /**
      * Permission to check, or null if this check doesn't require any permission.
      */
-    private final Permission permission;
+    protected final Permission permission;
 
     /**
      * The object to which the permission is checked against.
+     * If {@link #permission} is non-null, must be non-null.
      */
-    private final AccessControlled subject;
+    protected final AccessControlled subject;
 
     /**
      * @param adminOnly
@@ -72,7 +76,14 @@ public abstract class FormFieldValidator {
      */
     public final void process() throws IOException, ServletException {
         if(permission!=null)
-            subject.checkPermission(permission);
+            try {
+                subject.checkPermission(permission);
+            } catch (AccessDeniedException e) {
+                // if the user has hudson-wisde admin permission, all checks are allowed
+                // this is to protect Hudson administrator from broken ACL/SecurityRealm implementation/configuration.
+                if(!Hudson.getInstance().hasPermission(Hudson.ADMINISTER))
+                    throw e;
+            }
 
         check();
     }
@@ -271,13 +282,14 @@ public abstract class FormFieldValidator {
         }
 
         public WorkspaceFileMask(StaplerRequest request, StaplerResponse response, boolean errorIfNotExist) {
-            super(request, response, false);
+            // Require CONFIGURE permission on the job
+            super(request, response, (AbstractProject) request.findAncestor(AbstractProject.class).getObject(), Item.CONFIGURE);
             this.errorIfNotExist = errorIfNotExist;
         }
 
         protected void check() throws IOException, ServletException {
             String value = fixEmpty(request.getParameter("value"));
-            AbstractProject<?,?> p = Hudson.getInstance().getItemByFullName(request.getParameter("job"),AbstractProject.class);
+            AbstractProject<?,?> p = (AbstractProject<?,?>)subject;
 
             if(value==null || p==null) {
                 ok(); // none entered yet, or something is seriously wrong
@@ -333,14 +345,15 @@ public abstract class FormFieldValidator {
         private final boolean expectingFile;
 
         public WorkspaceFilePath(StaplerRequest request, StaplerResponse response, boolean errorIfNotExist, boolean expectingFile) {
-            super(request, response, false);
+            // Require CONFIGURE permission on this job
+            super(request, response, (AbstractProject) request.findAncestor(AbstractProject.class).getObject(), Item.CONFIGURE);
             this.errorIfNotExist = errorIfNotExist;
             this.expectingFile = expectingFile;
         }
 
         protected void check() throws IOException, ServletException {
             String value = fixEmpty(request.getParameter("value"));
-            AbstractProject<?, ?> p = getProject();
+            AbstractProject<?,?> p = (AbstractProject<?,?>)subject;
 
             if(value==null || p==null) {
                 ok(); // none entered yet, or something is seriously wrong
@@ -394,10 +407,6 @@ public abstract class FormFieldValidator {
         protected FilePath getBaseDirectory(AbstractProject<?,?> p) {
             return p.getWorkspace();
         }
-
-        protected AbstractProject<?,?> getProject() {
-            return Hudson.getInstance().getItemByFullName(request.getParameter("job"),AbstractProject.class);
-        }
     }
 
     /**
@@ -416,6 +425,7 @@ public abstract class FormFieldValidator {
     public static class Executable extends FormFieldValidator {
 
         public Executable(StaplerRequest request, StaplerResponse response) {
+            // Require admin permission
             super(request, response, true);
         }
 

core/src/main/resources/hudson/scm/browsers/Sventon/config.jelly

 <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">
   <f:entry title="${%URL}" help="/help/scm-browsers/sventon/url.html">
-    <f:textbox name="sventon.svn.url" value="${browser.url}"
-        checkUrl="'${rootURL}/repositoryBrowser/Sventon/check?value='+escape(this.value)"/>
+    <j:choose>
+      <j:when test="${h.hasPermission(app.ADMINISTER)}">
+        <j:set var="sventonCheck" value="'${rootURL}/repositoryBrowser/Sventon/check?value='+escape(this.value)" />
+      </j:when>
+      <j:otherwise><j:set var="sventonCheck" value="" /></j:otherwise>
+    </j:choose>
+    <f:textbox name="sventon.svn.url" value="${browser.url}" checkUrl="${sventonCheck}"/>
   </f:entry>
   <f:entry title="${%Repository Instance}" help="/help/scm-browsers/sventon/repository-instance.html">
     <f:textbox name="sventon.svn.repositoryInstance" value="${browser.repositoryInstance}" />

core/src/main/resources/hudson/tasks/ArtifactArchiver/config.jelly

 <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">
 	<f:entry title="${%Files to archive}" help="/help/tasks/artifactArchiver/includes.html">
 	  <f:textbox name="artifacts.artifacts" value="${instance.artifacts}"
-               checkUrl="'${rootURL}/publisher/ArtifactArchiver/check?job=${it.fullName}&amp;value='+escape(this.value)" />
+               checkUrl="'descriptorByName/ArtifactArchiver/check?value='+escape(this.value)" />
 	</f:entry>
   <f:advanced>
     <f:entry title="${%Excludes}" help="/help/tasks/artifactArchiver/excludes.html">

core/src/main/resources/hudson/tasks/BuildTrigger/config.jelly

 <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">
 	<f:entry title="${%Projects to build}">
 	  <f:textbox name="buildTrigger.childProjects" value="${instance.childProjectsValue}"
-      checkUrl="'${rootURL}/publisher/BuildTrigger/check?value='+escape(this.value)"/>
+               checkUrl="'descriptorByName/BuildTrigger/check?value='+escape(this.value)"/>
 	</f:entry>
   <j:if test="${descriptor.showEvenIfUnstableOption(targetType)}">
     <f:entry title="">

core/src/main/resources/hudson/tasks/Fingerprinter/config.jelly

@@ -2,7 +2,7 @@
   <f:entry title="${%Files to fingerprint}"
     description="${%description}">
     <f:textbox name="fingerprint_targets" value="${instance.targets}"
-               checkUrl="'${rootURL}/publisher/Fingerprinter/check?job=${it.fullName}&amp;value='+escape(this.value)" />
+               checkUrl="'descriptorByName/Fingerprinter/check?value='+escape(this.value)" />
   </f:entry>
   <f:entry title="">
     <f:checkbox name="fingerprint_artifacts" checked="${instance.recordBuildArtifacts}" />

core/src/main/resources/hudson/tasks/JavadocArchiver/config.jelly

@@ -2,7 +2,7 @@
   <f:entry title="${%Javadoc directory}"
     description="${%description}">
     <f:textbox name="javadoc_dir" value="${instance.javadocDir}"
-        checkUrl="'${rootURL}/publisher/JavadocArchiver/check?job=${it.fullName}&amp;value='+escape(this.value)"/>
+        checkUrl="'descriptorByName/JavadocArchiver/check?value='+escape(this.value)"/>
   </f:entry>
   <f:entry help="/help/project-config/javadoc-keep-all.html">
     <f:checkbox name="keep_all" checked="${instance.keepAll}" />

core/src/main/resources/hudson/tasks/junit/JUnitResultArchiver/config.jelly

@@ -2,6 +2,6 @@
   <f:entry title="${%Test report XMLs}"
     description="${%description}">
     <f:textbox name="junitreport_includes" value="${instance.testResults}"
-               checkUrl="'${rootURL}/publisher/JUnitResultArchiver/check?job=${it.fullName}&amp;value='+escape(this.value)"/>
+               checkUrl="'descriptorByName/JUnitResultArchiver/check?value='+escape(this.value)"/>
   </f:entry>
 </j:jelly>
\ No newline at end of file

core/src/main/resources/hudson/tasks/test/AggregatedTestResultPublisher/config.jelly

@@ -8,7 +8,7 @@
         <f:entry title="${%Jobs to aggregate}"
                  help="/help/tasks/aggregate-test/manual-list.html">
           <f:textbox name="aggragatedTestResult.jobs" value="${instance.jobs}"
-                     checkUrl="'${rootURL}/publisher/AggregatedTestResultPublisher/check?value='+escape(this.value)"/>
+                     checkUrl="'descriptorByName/AggregatedTestResultPublisher/check?value='+escape(this.value)"/>
         </f:entry>        
       </f:optionalBlock>
     </table>

core/src/main/resources/lib/hudson/project/config-upstream-pseudo-trigger.jelly

@@ -13,7 +13,7 @@
     <f:entry title="${%Projects names}"
              description="${%Multiple projects can be specified like 'abc, def'}">
       <input class="setting-input validated" name="upstreamProjects"
-        checkUrl="'${rootURL}/publisher/BuildTrigger/check?value='+escape(this.value)"
+        checkUrl="'descriptorByName/BuildTrigger/check?value='+escape(this.value)"
         type="text" value="${h.getProjectListString(up)}"/>
     </f:entry>
   </f:optionalBlock>