diff --git a/core/pom.xml b/core/pom.xml index 04824df881..cea6984233 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -231,7 +231,7 @@ org.kohsuke.stapler stapler - 1.88 + 1.89 org.jvnet.localizer diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java index 0d4db4a1b1..9d9737d24c 100644 --- a/core/src/main/java/hudson/Util.java +++ b/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, 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) { diff --git a/core/src/main/java/hudson/scm/SubversionSCM.java b/core/src/main/java/hudson/scm/SubversionSCM.java index 65d878be7a..9f97669bcc 100644 --- a/core/src/main/java/hudson/scm/SubversionSCM.java +++ b/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")); diff --git a/core/src/main/java/hudson/scm/browsers/FishEyeCVS.java b/core/src/main/java/hudson/scm/browsers/FishEyeCVS.java index 0eddfe4074..491eb3f75b 100644 --- a/core/src/main/java/hudson/scm/browsers/FishEyeCVS.java +++ b/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 .../browse/foobar/"); 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(); diff --git a/core/src/main/java/hudson/scm/browsers/FishEyeSVN.java b/core/src/main/java/hudson/scm/browsers/FishEyeSVN.java index 801555d793..6ad87886a1 100644 --- a/core/src/main/java/hudson/scm/browsers/FishEyeSVN.java +++ b/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(); diff --git a/core/src/main/java/hudson/scm/browsers/Sventon.java b/core/src/main/java/hudson/scm/browsers/Sventon.java index 99b1534d4e..9dae0ed96e 100644 --- a/core/src/main/java/hudson/scm/browsers/Sventon.java +++ b/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")); diff --git a/core/src/main/java/hudson/security/AccessControlled.java b/core/src/main/java/hudson/security/AccessControlled.java index 734e2d664e..bbee527514 100644 --- a/core/src/main/java/hudson/security/AccessControlled.java +++ b/core/src/main/java/hudson/security/AccessControlled.java @@ -1,5 +1,7 @@ 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)} diff --git a/core/src/main/java/hudson/tasks/BuildTrigger.java b/core/src/main/java/hudson/tasks/BuildTrigger.java index 6b4b7ec4b3..e178a1a679 100644 --- a/core/src/main/java/hudson/tasks/BuildTrigger.java +++ b/core/src/main/java/hudson/tasks/BuildTrigger.java @@ -1,6 +1,7 @@ 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"); diff --git a/core/src/main/java/hudson/tasks/JavadocArchiver.java b/core/src/main/java/hudson/tasks/JavadocArchiver.java index 0c4e43d7f6..459b10a325 100644 --- a/core/src/main/java/hudson/tasks/JavadocArchiver.java +++ b/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(); } diff --git a/core/src/main/java/hudson/tasks/Shell.java b/core/src/main/java/hudson/tasks/Shell.java index 596cad18ce..cbb1717b67 100644 --- a/core/src/main/java/hudson/tasks/Shell.java +++ b/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(); } } diff --git a/core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java b/core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java index 56dc7e8d06..1ab00108c4 100644 --- a/core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java +++ b/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(); diff --git a/core/src/main/java/hudson/triggers/TimerTrigger.java b/core/src/main/java/hudson/triggers/TimerTrigger.java index 78ad6a45ba..2e640f0888 100644 --- a/core/src/main/java/hudson/triggers/TimerTrigger.java +++ b/core/src/main/java/hudson/triggers/TimerTrigger.java @@ -59,7 +59,9 @@ public class TimerTrigger extends Trigger { * 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(); diff --git a/core/src/main/java/hudson/util/FormFieldValidator.java b/core/src/main/java/hudson/util/FormFieldValidator.java index 03c34279eb..0792f5d11b 100644 --- a/core/src/main/java/hudson/util/FormFieldValidator.java +++ b/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); } diff --git a/core/src/main/resources/hudson/scm/browsers/Sventon/config.jelly b/core/src/main/resources/hudson/scm/browsers/Sventon/config.jelly index 4642e23818..362a3d4b07 100644 --- a/core/src/main/resources/hudson/scm/browsers/Sventon/config.jelly +++ b/core/src/main/resources/hudson/scm/browsers/Sventon/config.jelly @@ -1,7 +1,12 @@ - + + + + + + + diff --git a/core/src/main/resources/hudson/tasks/ArtifactArchiver/config.jelly b/core/src/main/resources/hudson/tasks/ArtifactArchiver/config.jelly index 932b7a0890..053c87d799 100644 --- a/core/src/main/resources/hudson/tasks/ArtifactArchiver/config.jelly +++ b/core/src/main/resources/hudson/tasks/ArtifactArchiver/config.jelly @@ -1,7 +1,7 @@ + checkUrl="'descriptorByName/ArtifactArchiver/check?value='+escape(this.value)" /> diff --git a/core/src/main/resources/hudson/tasks/BuildTrigger/config.jelly b/core/src/main/resources/hudson/tasks/BuildTrigger/config.jelly index 82856a2746..c82104fd26 100644 --- a/core/src/main/resources/hudson/tasks/BuildTrigger/config.jelly +++ b/core/src/main/resources/hudson/tasks/BuildTrigger/config.jelly @@ -1,7 +1,7 @@ + checkUrl="'descriptorByName/BuildTrigger/check?value='+escape(this.value)"/> diff --git a/core/src/main/resources/hudson/tasks/Fingerprinter/config.jelly b/core/src/main/resources/hudson/tasks/Fingerprinter/config.jelly index 020332f591..416f817ac7 100644 --- a/core/src/main/resources/hudson/tasks/Fingerprinter/config.jelly +++ b/core/src/main/resources/hudson/tasks/Fingerprinter/config.jelly @@ -2,7 +2,7 @@ + checkUrl="'descriptorByName/Fingerprinter/check?value='+escape(this.value)" /> diff --git a/core/src/main/resources/hudson/tasks/JavadocArchiver/config.jelly b/core/src/main/resources/hudson/tasks/JavadocArchiver/config.jelly index 7dfbde14ed..1602c26b71 100644 --- a/core/src/main/resources/hudson/tasks/JavadocArchiver/config.jelly +++ b/core/src/main/resources/hudson/tasks/JavadocArchiver/config.jelly @@ -2,7 +2,7 @@ + checkUrl="'descriptorByName/JavadocArchiver/check?value='+escape(this.value)"/> diff --git a/core/src/main/resources/hudson/tasks/junit/JUnitResultArchiver/config.jelly b/core/src/main/resources/hudson/tasks/junit/JUnitResultArchiver/config.jelly index 8ce98b8aa4..aaede3692e 100644 --- a/core/src/main/resources/hudson/tasks/junit/JUnitResultArchiver/config.jelly +++ b/core/src/main/resources/hudson/tasks/junit/JUnitResultArchiver/config.jelly @@ -2,6 +2,6 @@ + checkUrl="'descriptorByName/JUnitResultArchiver/check?value='+escape(this.value)"/> \ No newline at end of file diff --git a/core/src/main/resources/hudson/tasks/test/AggregatedTestResultPublisher/config.jelly b/core/src/main/resources/hudson/tasks/test/AggregatedTestResultPublisher/config.jelly index 45bc7ab2c5..1b41f9cfb3 100644 --- a/core/src/main/resources/hudson/tasks/test/AggregatedTestResultPublisher/config.jelly +++ b/core/src/main/resources/hudson/tasks/test/AggregatedTestResultPublisher/config.jelly @@ -8,7 +8,7 @@ + checkUrl="'descriptorByName/AggregatedTestResultPublisher/check?value='+escape(this.value)"/> diff --git a/core/src/main/resources/lib/hudson/project/config-upstream-pseudo-trigger.jelly b/core/src/main/resources/lib/hudson/project/config-upstream-pseudo-trigger.jelly index 49bb9a1356..070bf7f0ba 100644 --- a/core/src/main/resources/lib/hudson/project/config-upstream-pseudo-trigger.jelly +++ b/core/src/main/resources/lib/hudson/project/config-upstream-pseudo-trigger.jelly @@ -13,7 +13,7 @@