[JENKINS-22834] Added help texts and input validations.

jenkinsci · Aug 2, 2014 · c00383e · c00383e
1 parent e78155e
commit c00383e
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 16 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/AdditionalClasspath.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/AdditionalClasspath.java
@@ -24,21 +24,30 @@
 
 package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy;
 
+import java.io.File;
+import java.io.IOException;
+
+import jenkins.model.Jenkins;
+
+import org.apache.commons.lang.StringUtils;
+import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
 
 import hudson.Extension;
 import hudson.model.AbstractDescribableImpl;
 import hudson.model.Descriptor;
+import hudson.util.FormValidation;
 
 /**
- *
+ * A classpath used for a groovy script.
  */
 public class AdditionalClasspath extends AbstractDescribableImpl<AdditionalClasspath> {
     private final String path;
 
     @DataBoundConstructor
     public AdditionalClasspath(String path) {
-        this.path = path;
+        this.path = StringUtils.trim(path);
     }
 
     public String getPath() {
@@ -67,8 +76,30 @@ public boolean equals(Object obj) {
     public static class DescriptorImpl extends Descriptor<AdditionalClasspath> {
         @Override
         public String getDisplayName() {
-            // TODO
-            return "classpath";
+            return Messages.AdditionalClasspath_DisplayName();
+        }
+
+        public FormValidation doCheckPath(@QueryParameter String path) {
+            if (StringUtils.isBlank(path)) {
+                return FormValidation.ok();
+            }
+            File file = new File(path);
+            if (!file.isAbsolute()) {
+                return FormValidation.error(Messages.AdditionalClasspath_path_notAbsolute());
+            }
+            if (!file.exists()) {
+                return FormValidation.error(Messages.AdditionalClasspath_path_notExists());
+            }
+            if (Jenkins.getInstance().isUseSecurity() && !Jenkins.getInstance().hasPermission(Jenkins.RUN_SCRIPTS)) {
+                try {
+                    if (!ScriptApproval.get().isClasspathApproved(path)) {
+                        return FormValidation.error(Messages.AdditionalClasspath_path_notApproved());
+                    }
+                } catch(IOException e) {
+                    return FormValidation.error(Messages.AdditionalClasspath_path_notApproved(), e);
+                }
+            }
+            return FormValidation.ok();
         }
     }
 }
diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java
@@ -45,6 +45,7 @@
 import java.util.concurrent.Callable;
 import javax.annotation.CheckForNull;
 import jenkins.model.Jenkins;
+import org.apache.commons.lang.StringUtils;
 import org.codehaus.groovy.control.CompilationFailedException;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist;
@@ -154,6 +155,9 @@ public Object evaluate(BuildListener listener, ClassLoader loader, Binding bindi
             List<URL> urlList = new ArrayList<URL>(getAdditionalClasspathList().size());
 
             for (AdditionalClasspath classpath: getAdditionalClasspathList()) {
+                if (StringUtils.isBlank(classpath.getPath())) {
+                    continue;
+                }
                 File file = new File(classpath.getPath());
                 if (!file.isAbsolute()) {
                     listener.getLogger().println(String.format("%s: classpath should be absolute. Not added to class loader", file));

diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java
@@ -24,6 +24,10 @@
 
 package org.jenkinsci.plugins.scriptsecurity.scripts;
 
+import net.sf.json.JSONArray;
+import net.sf.json.JSONObject;
+import net.sf.json.JsonConfig;
+import net.sf.json.processors.JsonValueProcessor;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.AclAwareWhitelist;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.ProxyWhitelist;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException;
@@ -106,7 +110,6 @@ public static ScriptApproval get() {
      * It is treated only with the hash,
      * but additional information is provided for convenience.
      */
-    @Restricted(NoExternalUse.class) // for use from Jelly
     public static class ApprovedClasspath {
         private final String hash;
         private final String path;
@@ -256,7 +259,6 @@ public String getHash() {
      * They are distinguished only with hashes,
      * but other additional information is provided for users.
      */
-    @Restricted(NoExternalUse.class) // for use from Jelly
     public static final class PendingClasspath extends PendingThing {
         private final String path;
         private final String hash;
@@ -284,6 +286,16 @@ public String getPath() {
         @Override public boolean equals(Object obj) {
             return obj instanceof PendingClasspath && ((PendingClasspath) obj).getHash().equals(getHash());
         }
+
+        /**
+         * Prepared as context causes cyclic reference.
+         * @return a JSON object.
+         */
+        protected JSONObject toJSON() {
+            return new JSONObject()
+                .element("hash", getHash())
+                .element("path", getPath());
+        }
     }
 
     private final Set<PendingScript> pendingScripts = new LinkedHashSet<PendingScript>();
@@ -513,12 +525,7 @@ public synchronized void configureingClasspath(@Nonnull String path, @Nonnull Ap
     public synchronized boolean isClasspathApproved(@Nonnull String path) throws IOException {
         String hash = hashClasspath(path);
 
-        ApprovedClasspath approvedPath = getApprovedClasspath(hash);
-        if (approvedPath != null) {
-            LOG.fine(String.format("%s (%s) has been approved as %s.", path, hash, approvedPath.getPath()));
-        }
-
-        return (approvedPath != null);
+        return hasApprovedClasspath(hash);
     }
 
     private static Item currentExecutingItem() {
@@ -761,23 +768,33 @@ public Set<PendingSignature> getPendingSignatures() {
         return Jenkins.getInstance().getExtensionList(Whitelist.class).get(ApprovedWhitelist.class).reconfigure();
     }
 
-    @Restricted(NoExternalUse.class) // for use from Jelly
     public List<ApprovedClasspath> getApprovedClasspaths() {
         return new ArrayList<ApprovedClasspath>(getApprovedClasspathMap().values());
     }
 
-    @Restricted(NoExternalUse.class) // for use from Jelly
     public List<PendingClasspath> getPendingClasspaths() {
         return new ArrayList<PendingClasspath>(getPendingClasspathMap().values());
     }
 
     @Restricted(NoExternalUse.class) // for use from Ajax
     @JavaScriptMethod
     public Object getClasspathRenderInfo() {
-        return new Object[]{
+        JsonConfig config = new JsonConfig();
+        config.registerJsonValueProcessor(PendingClasspath.class, new JsonValueProcessor() {
+            @Override
+            public Object processObjectValue(String key, Object obj, JsonConfig config) {
+                return ((PendingClasspath)obj).toJSON();
+            }
+
+            @Override
+            public Object processArrayValue(Object obj, JsonConfig config) {
+                return ((PendingClasspath)obj).toJSON();
+            }
+        });
+        return JSONArray.fromObject(new Object[]{
                 getPendingClasspaths(),
                 getApprovedClasspaths(),
-        };
+        }, config);
     }
 
     @Restricted(NoExternalUse.class) // for use from AJAX

diff --git a/...es/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/AdditionalClasspath/help-path.html b/...es/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/AdditionalClasspath/help-path.html
@@ -0,0 +1,6 @@
+<div>
+A path to a jar file or a directory containing class files.
+Must be an absolute path.
+This path should be approved by an administrator or a user with the RUN_SCRIPT permission, or the script fails.
+If the file or files are once approved, they are treated approved even located in another path.
+</div>
diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/Messages.properties b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/Messages.properties
@@ -0,0 +1,4 @@
+AdditionalClasspath.DisplayName=classpath
+AdditionalClasspath.path.notAbsolute=Classpath must be absolute
+AdditionalClasspath.path.notExists=Specified path does not exist
+AdditionalClasspath.path.notApproved=This classpath is not approved. Require an approval before execution.
diff --git a/...lugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/help-additionalClasspathList.html b/...lugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/help-additionalClasspathList.html
@@ -0,0 +1,3 @@
+<div>
+Additional classpaths accessible from the groovy script.
+</div>