From d84705ffd678346e8a9c229a8e3b78d5bd188756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20Gond=C5=BEa?= Date: Thu, 18 Apr 2019 11:52:58 +0200 Subject: [PATCH 01/54] Do not suppress domain exception in favor of follow-up exception --- core/src/main/java/hudson/util/AtomicFileWriter.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/hudson/util/AtomicFileWriter.java b/core/src/main/java/hudson/util/AtomicFileWriter.java index c3da305885..ca689c2bbe 100644 --- a/core/src/main/java/hudson/util/AtomicFileWriter.java +++ b/core/src/main/java/hudson/util/AtomicFileWriter.java @@ -205,18 +205,18 @@ public class AtomicFileWriter extends Writer { try { Files.move(tmpPath, destPath, StandardCopyOption.REPLACE_EXISTING); } catch (IOException e1) { - e1.addSuppressed(e); + e.addSuppressed(e1); LOGGER.log(Level.WARNING, "Unable to move {0} to {1}. Attempting to delete {0} and abandoning.", new Path[]{tmpPath, destPath}); try { Files.deleteIfExists(tmpPath); } catch (IOException e2) { - e2.addSuppressed(e1); + e1.addSuppressed(e2); LOGGER.log(Level.WARNING, "Unable to delete {0}, good bye then!", tmpPath); - throw e2; + throw e; } - throw e1; + throw e; } } } From c76ee598101acbf442ff43c9e45808c64ca8a7bd Mon Sep 17 00:00:00 2001 From: Jeff Thompson Date: Mon, 2 Dec 2019 13:39:55 -0700 Subject: [PATCH 02/54] Add findsecbugs plugin to spotbugs. And suppress existing warnings. We should clean some of them up, but that's for different PRs. --- cli/src/main/java/hudson/cli/CLI.java | 3 +++ .../cli/DiagnosedStreamCorruptionException.java | 3 +++ .../main/java/hudson/cli/FullDuplexHttpStream.java | 4 ++++ .../main/java/hudson/cli/NoCheckTrustManager.java | 3 +++ cli/src/main/java/hudson/cli/SSHCLI.java | 2 +- .../main/java/hudson/ClassicPluginStrategy.java | 1 + core/src/main/java/hudson/Proc.java | 2 ++ core/src/main/java/hudson/Util.java | 2 ++ core/src/main/java/hudson/cli/Connection.java | 3 +++ .../java/hudson/console/AnnotatedLargeText.java | 2 ++ core/src/main/java/hudson/console/ConsoleNote.java | 2 ++ core/src/main/java/hudson/scheduler/Hash.java | 5 ++++- .../hudson/security/BasicAuthenticationFilter.java | 2 ++ .../security/HudsonAuthenticationEntryPoint.java | 2 ++ core/src/main/java/hudson/util/ConsistentHash.java | 2 ++ .../main/java/hudson/util/DOSToUnixPathHelper.java | 3 +++ .../main/java/hudson/util/FormFieldValidator.java | 4 ++++ .../src/main/java/jenkins/model/RunIdMigrator.java | 2 ++ .../jenkins/model/identity/IdentityRootAction.java | 2 ++ .../jenkins/slaves/JnlpSlaveAgentProtocol4.java | 2 ++ pom.xml | 14 ++++++++++++++ 21 files changed, 63 insertions(+), 2 deletions(-) diff --git a/cli/src/main/java/hudson/cli/CLI.java b/cli/src/main/java/hudson/cli/CLI.java index 1b9b4856ad..4cf98e85c1 100644 --- a/cli/src/main/java/hudson/cli/CLI.java +++ b/cli/src/main/java/hudson/cli/CLI.java @@ -23,6 +23,7 @@ */ package hudson.cli; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.cli.client.Messages; import java.io.DataInputStream; import javax.net.ssl.HostnameVerifier; @@ -104,6 +105,7 @@ public class CLI { } private enum Mode {HTTP, SSH, WEB_SOCKET} + @SuppressFBWarnings(value = {"PATH_TRAVERSAL_IN", "URLCONNECTION_SSRF_FD"}, justification = "User provided values for running the program.") public static int _main(String[] _args) throws Exception { List args = Arrays.asList(_args); PrivateKeyProvider provider = new PrivateKeyProvider(); @@ -175,6 +177,7 @@ public class CLI { HttpsURLConnection.setDefaultSSLSocketFactory(context.getSocketFactory()); // bypass host name check, too. HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() { + @SuppressFBWarnings(value = "WEAK_HOSTNAME_VERIFIER", justification = "User set parameter to skip verifier.") public boolean verify(String s, SSLSession sslSession) { return true; } diff --git a/cli/src/main/java/hudson/cli/DiagnosedStreamCorruptionException.java b/cli/src/main/java/hudson/cli/DiagnosedStreamCorruptionException.java index 4708b425db..c7d3963f3b 100644 --- a/cli/src/main/java/hudson/cli/DiagnosedStreamCorruptionException.java +++ b/cli/src/main/java/hudson/cli/DiagnosedStreamCorruptionException.java @@ -1,5 +1,7 @@ package hudson.cli; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import java.io.PrintWriter; import java.io.StreamCorruptedException; import java.io.StringWriter; @@ -11,6 +13,7 @@ import java.io.StringWriter; * * @author Kohsuke Kawaguchi */ +@SuppressFBWarnings(value = "INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE") class DiagnosedStreamCorruptionException extends StreamCorruptedException { private final Exception diagnoseFailure; private final byte[] readBack; diff --git a/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java b/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java index 723e97f16c..1f066a620d 100644 --- a/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java +++ b/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java @@ -1,5 +1,7 @@ package hudson.cli; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -44,6 +46,7 @@ public class FullDuplexHttpStream { * @param authorization * The value of the authorization header, if non-null. */ + @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Client-side code doesn't involve SSRF.") public FullDuplexHttpStream(URL base, String relativeTarget, String authorization) throws IOException { if (!base.toString().endsWith("/")) { throw new IllegalArgumentException(base.toString()); @@ -93,6 +96,7 @@ public class FullDuplexHttpStream { } // As this transport mode is using POST, it is necessary to resolve possible redirections using GET first. + @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Client-side code doesn't involve SSRF.") private URL tryToResolveRedirects(URL base, String authorization) { try { HttpURLConnection con = (HttpURLConnection) base.openConnection(); diff --git a/cli/src/main/java/hudson/cli/NoCheckTrustManager.java b/cli/src/main/java/hudson/cli/NoCheckTrustManager.java index c69b3330c2..5a8346cb6e 100644 --- a/cli/src/main/java/hudson/cli/NoCheckTrustManager.java +++ b/cli/src/main/java/hudson/cli/NoCheckTrustManager.java @@ -1,5 +1,7 @@ package hudson.cli; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import javax.net.ssl.X509TrustManager; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; @@ -7,6 +9,7 @@ import java.security.cert.X509Certificate; /** * @author Kohsuke Kawaguchi */ +@SuppressFBWarnings(value = "WEAK_TRUST_MANAGER", justification = "User set parameter to skip verifier.") public class NoCheckTrustManager implements X509TrustManager { public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { } diff --git a/cli/src/main/java/hudson/cli/SSHCLI.java b/cli/src/main/java/hudson/cli/SSHCLI.java index 39dc9210a3..cbfeea16b8 100644 --- a/cli/src/main/java/hudson/cli/SSHCLI.java +++ b/cli/src/main/java/hudson/cli/SSHCLI.java @@ -61,7 +61,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; */ class SSHCLI { - @SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE", justification = "Due to whatever reason FindBugs reports it fot try-with-resources") + @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Client-side code doesn't involve SSRF.") static int sshConnection(String jenkinsUrl, String user, List args, PrivateKeyProvider provider, final boolean strictHostKey) throws IOException { Logger.getLogger(SecurityUtils.class.getName()).setLevel(Level.WARNING); // suppress: BouncyCastle not registered, using the default JCE provider URL url = new URL(jenkinsUrl + "login"); diff --git a/core/src/main/java/hudson/ClassicPluginStrategy.java b/core/src/main/java/hudson/ClassicPluginStrategy.java index c5369614de..d426d36226 100644 --- a/core/src/main/java/hudson/ClassicPluginStrategy.java +++ b/core/src/main/java/hudson/ClassicPluginStrategy.java @@ -78,6 +78,7 @@ import java.util.logging.Logger; import static org.apache.commons.io.FilenameUtils.getBaseName; +@SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "Admistrator action installing a plugin, which could do far worse.") public class ClassicPluginStrategy implements PluginStrategy { private static final Logger LOGGER = Logger.getLogger(ClassicPluginStrategy.class.getName()); diff --git a/core/src/main/java/hudson/Proc.java b/core/src/main/java/hudson/Proc.java index 6acb1ceeba..d134d0b8b7 100644 --- a/core/src/main/java/hudson/Proc.java +++ b/core/src/main/java/hudson/Proc.java @@ -23,6 +23,7 @@ */ package hudson; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.Launcher.ProcStarter; import hudson.model.TaskListener; import hudson.remoting.Channel; @@ -215,6 +216,7 @@ public abstract class Proc { * @param err * null to redirect stderr to stdout. */ + @SuppressFBWarnings("COMMAND_INJECTION") public LocalProc(String[] cmd,String[] env,InputStream in,OutputStream out,OutputStream err,File workDir) throws IOException { this( calcName(cmd), stderr(environment(new ProcessBuilder(cmd),env).directory(workDir), err==null || err== SELFPUMP_OUTPUT), diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java index 7775b4ca6a..de29e9a013 100644 --- a/core/src/main/java/hudson/Util.java +++ b/core/src/main/java/hudson/Util.java @@ -596,6 +596,8 @@ public class Util { * @see DigestUtils#md5Hex(InputStream) */ @Nonnull + @SuppressFBWarnings(value = "WEAK_MESSAGE_DIGEST_MD5", justification = + "This method should only be used for non-security applications where their weakness is not a problem.") public static String getDigestOf(@Nonnull InputStream source) throws IOException { try { MessageDigest md5 = MessageDigest.getInstance("MD5"); diff --git a/core/src/main/java/hudson/cli/Connection.java b/core/src/main/java/hudson/cli/Connection.java index 1961447778..a181c5d208 100644 --- a/core/src/main/java/hudson/cli/Connection.java +++ b/core/src/main/java/hudson/cli/Connection.java @@ -23,6 +23,7 @@ */ package hudson.cli; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.remoting.ClassFilter; import hudson.remoting.ObjectInputStreamEx; import hudson.remoting.SocketChannelStream; @@ -113,6 +114,7 @@ public class Connection { /** * Receives an object sent by {@link #writeObject(Object)} */ + @SuppressFBWarnings(value = "OBJECT_DESERIALIZATION", justification = "Not used. We should just remove it.") public T readObject() throws IOException, ClassNotFoundException { ObjectInputStream ois = new ObjectInputStreamEx(in, getClass().getClassLoader(), ClassFilter.DEFAULT); @@ -192,6 +194,7 @@ public class Connection { * @return * A new {@link Connection} object that includes the transport encryption. */ + @SuppressFBWarnings(value = "STATIC_IV", justification = "Not used. We should just remove it.") public Connection encryptConnection(SecretKey sessionKey, String algorithm) throws IOException, GeneralSecurityException { Cipher cout = Cipher.getInstance(algorithm); cout.init(Cipher.ENCRYPT_MODE, sessionKey, new IvParameterSpec(sessionKey.getEncoded())); diff --git a/core/src/main/java/hudson/console/AnnotatedLargeText.java b/core/src/main/java/hudson/console/AnnotatedLargeText.java index c862d0c61b..278c32c9a1 100644 --- a/core/src/main/java/hudson/console/AnnotatedLargeText.java +++ b/core/src/main/java/hudson/console/AnnotatedLargeText.java @@ -25,6 +25,7 @@ */ package hudson.console; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import jenkins.model.Jenkins; import hudson.remoting.ObjectInputStreamEx; import java.util.concurrent.TimeUnit; @@ -116,6 +117,7 @@ public class AnnotatedLargeText extends LargeText { rsp.setContentType(isHtml() ? "text/html;charset=UTF-8" : "text/plain;charset=UTF-8"); } + @SuppressFBWarnings(value = "OBJECT_DESERIALIZATION") private ConsoleAnnotator createAnnotator(StaplerRequest req) throws IOException { try { String base64 = req!=null ? req.getHeader("X-ConsoleAnnotator") : null; diff --git a/core/src/main/java/hudson/console/ConsoleNote.java b/core/src/main/java/hudson/console/ConsoleNote.java index 4d2e2c2530..c8dd1675fe 100644 --- a/core/src/main/java/hudson/console/ConsoleNote.java +++ b/core/src/main/java/hudson/console/ConsoleNote.java @@ -23,6 +23,7 @@ */ package hudson.console; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.ExtensionPoint; import hudson.Functions; import hudson.MarkupText; @@ -232,6 +233,7 @@ public abstract class ConsoleNote implements Serializable, Describable { /** * All the items in the hash, to their replication factors. diff --git a/core/src/main/java/hudson/util/DOSToUnixPathHelper.java b/core/src/main/java/hudson/util/DOSToUnixPathHelper.java index afe949c8db..ab49acb693 100644 --- a/core/src/main/java/hudson/util/DOSToUnixPathHelper.java +++ b/core/src/main/java/hudson/util/DOSToUnixPathHelper.java @@ -1,5 +1,6 @@ package hudson.util; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.EnvVars; import hudson.Util; import org.kohsuke.accmod.Restricted; @@ -16,6 +17,7 @@ class DOSToUnixPathHelper { void error(String string); void validate(File fexe); } + @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN") static private boolean checkPrefix(String prefix, Helper helper) { File f = new File(prefix); if(f.exists()) { @@ -30,6 +32,7 @@ class DOSToUnixPathHelper { } return false; } + @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN") static void iteratePath(String exe, Helper helper) { exe = fixEmpty(exe); if(exe==null) { diff --git a/core/src/main/java/hudson/util/FormFieldValidator.java b/core/src/main/java/hudson/util/FormFieldValidator.java index 292d5d1141..09f9955655 100644 --- a/core/src/main/java/hudson/util/FormFieldValidator.java +++ b/core/src/main/java/hudson/util/FormFieldValidator.java @@ -23,6 +23,7 @@ */ package hudson.util; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.FilePath; import hudson.ProxyConfiguration; import hudson.Util; @@ -145,6 +146,7 @@ public abstract class FormFieldValidator { /** * Gets the parameter as a file. */ + @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "Not used.") protected final File getFileParameter(String paramName) { return new File(Util.fixNull(request.getParameter(paramName))); } @@ -318,6 +320,7 @@ public abstract class FormFieldValidator { super(request, response); } + @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD") protected void check() throws IOException, ServletException { String value = fixEmpty(request.getParameter("value")); if(value==null) {// nothing entered yet @@ -513,6 +516,7 @@ public abstract class FormFieldValidator { super(request, response, true); } + @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "Not used.") protected void check() throws IOException, ServletException { String exe = fixEmpty(request.getParameter("value")); FormFieldValidator.Executable self = this; diff --git a/core/src/main/java/jenkins/model/RunIdMigrator.java b/core/src/main/java/jenkins/model/RunIdMigrator.java index 4af582072e..69406c0292 100644 --- a/core/src/main/java/jenkins/model/RunIdMigrator.java +++ b/core/src/main/java/jenkins/model/RunIdMigrator.java @@ -24,6 +24,7 @@ package jenkins.model; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.Extension; import hudson.Util; import hudson.model.Job; @@ -309,6 +310,7 @@ public final class RunIdMigrator { * Reverses the migration, in case you want to revert to the older format. * @param args one parameter, {@code $JENKINS_HOME} */ + @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "Only used as an command-line process.") public static void main(String... args) throws Exception { if (args.length != 1) { throw new Exception("pass one parameter, $JENKINS_HOME"); diff --git a/core/src/main/java/jenkins/model/identity/IdentityRootAction.java b/core/src/main/java/jenkins/model/identity/IdentityRootAction.java index 1ea465ec07..d83cb88ef7 100644 --- a/core/src/main/java/jenkins/model/identity/IdentityRootAction.java +++ b/core/src/main/java/jenkins/model/identity/IdentityRootAction.java @@ -1,5 +1,6 @@ package jenkins.model.identity; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.Extension; import hudson.model.UnprotectedRootAction; import java.security.MessageDigest; @@ -61,6 +62,7 @@ public class IdentityRootAction implements UnprotectedRootAction { * * @return the fingerprint of the public key. */ + @SuppressFBWarnings(value = "WEAK_MESSAGE_DIGEST_MD5", justification = "Not used for security. ") public String getFingerprint() { RSAPublicKey key = InstanceIdentityProvider.RSA.getPublicKey(); if (key == null) { diff --git a/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol4.java b/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol4.java index e1d85d782b..8e3f595999 100644 --- a/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol4.java +++ b/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol4.java @@ -23,6 +23,7 @@ */ package jenkins.slaves; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.Extension; import hudson.ExtensionList; import hudson.model.Computer; @@ -98,6 +99,7 @@ public class JnlpSlaveAgentProtocol4 extends AgentProtocol { * @throws KeyManagementException if things go wrong. * @throws IOException if things go wrong. */ + @SuppressFBWarnings("HARD_CODE_PASSWORD") public JnlpSlaveAgentProtocol4() throws KeyStoreException, KeyManagementException, IOException { // prepare our local identity and certificate X509Certificate identityCertificate = InstanceIdentityProvider.RSA.getCertificate(); diff --git a/pom.xml b/pom.xml index b26f01f57d..24a7fdfac5 100755 --- a/pom.xml +++ b/pom.xml @@ -407,6 +407,20 @@ THE SOFTWARE. org.codehaus.mojo animal-sniffer-maven-plugin + + com.github.spotbugs + spotbugs-maven-plugin + + + + com.h3xstream.findsecbugs + findsecbugs-plugin + 1.10.1 + + + 768 + + From ced25e51fd201ec3ae7ae9d3e9262e573cf11e27 Mon Sep 17 00:00:00 2001 From: Jeff Thompson Date: Mon, 2 Dec 2019 15:29:45 -0700 Subject: [PATCH 03/54] Make sure everything has a justification. And move a couple of items to the exclude file. If we ever get to a lower threshold, these will show up all over the place. --- .../hudson/cli/DiagnosedStreamCorruptionException.java | 3 --- core/src/main/java/hudson/Proc.java | 2 +- core/src/main/java/hudson/console/AnnotatedLargeText.java | 2 +- core/src/main/java/hudson/console/ConsoleNote.java | 2 +- .../java/hudson/security/BasicAuthenticationFilter.java | 2 +- .../hudson/security/HudsonAuthenticationEntryPoint.java | 2 +- core/src/main/java/hudson/util/DOSToUnixPathHelper.java | 4 ++-- core/src/main/java/hudson/util/FormFieldValidator.java | 2 +- .../main/java/jenkins/slaves/JnlpSlaveAgentProtocol4.java | 2 +- src/findbugs/findbugs-excludes.xml | 8 ++++++++ 10 files changed, 17 insertions(+), 12 deletions(-) diff --git a/cli/src/main/java/hudson/cli/DiagnosedStreamCorruptionException.java b/cli/src/main/java/hudson/cli/DiagnosedStreamCorruptionException.java index c7d3963f3b..4708b425db 100644 --- a/cli/src/main/java/hudson/cli/DiagnosedStreamCorruptionException.java +++ b/cli/src/main/java/hudson/cli/DiagnosedStreamCorruptionException.java @@ -1,7 +1,5 @@ package hudson.cli; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - import java.io.PrintWriter; import java.io.StreamCorruptedException; import java.io.StringWriter; @@ -13,7 +11,6 @@ import java.io.StringWriter; * * @author Kohsuke Kawaguchi */ -@SuppressFBWarnings(value = "INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE") class DiagnosedStreamCorruptionException extends StreamCorruptedException { private final Exception diagnoseFailure; private final byte[] readBack; diff --git a/core/src/main/java/hudson/Proc.java b/core/src/main/java/hudson/Proc.java index d134d0b8b7..f387d6abcc 100644 --- a/core/src/main/java/hudson/Proc.java +++ b/core/src/main/java/hudson/Proc.java @@ -216,7 +216,7 @@ public abstract class Proc { * @param err * null to redirect stderr to stdout. */ - @SuppressFBWarnings("COMMAND_INJECTION") + @SuppressFBWarnings(value = "COMMAND_INJECTION", justification = "Command injection is the point of this old, barely used class.") public LocalProc(String[] cmd,String[] env,InputStream in,OutputStream out,OutputStream err,File workDir) throws IOException { this( calcName(cmd), stderr(environment(new ProcessBuilder(cmd),env).directory(workDir), err==null || err== SELFPUMP_OUTPUT), diff --git a/core/src/main/java/hudson/console/AnnotatedLargeText.java b/core/src/main/java/hudson/console/AnnotatedLargeText.java index 278c32c9a1..9625a8609f 100644 --- a/core/src/main/java/hudson/console/AnnotatedLargeText.java +++ b/core/src/main/java/hudson/console/AnnotatedLargeText.java @@ -117,7 +117,7 @@ public class AnnotatedLargeText extends LargeText { rsp.setContentType(isHtml() ? "text/html;charset=UTF-8" : "text/plain;charset=UTF-8"); } - @SuppressFBWarnings(value = "OBJECT_DESERIALIZATION") + @SuppressFBWarnings(value = "OBJECT_DESERIALIZATION", justification = "Deserialization is protected by logic.") private ConsoleAnnotator createAnnotator(StaplerRequest req) throws IOException { try { String base64 = req!=null ? req.getHeader("X-ConsoleAnnotator") : null; diff --git a/core/src/main/java/hudson/console/ConsoleNote.java b/core/src/main/java/hudson/console/ConsoleNote.java index c8dd1675fe..82ca8365e7 100644 --- a/core/src/main/java/hudson/console/ConsoleNote.java +++ b/core/src/main/java/hudson/console/ConsoleNote.java @@ -233,7 +233,7 @@ public abstract class ConsoleNote implements Serializable, Describable + + + + + + + + From 37fc33f92dc697c6fb50b0500b24159eb2b203f7 Mon Sep 17 00:00:00 2001 From: Jeff Thompson <37345299+jeffret-b@users.noreply.github.com> Date: Tue, 3 Dec 2019 08:55:46 -0700 Subject: [PATCH 04/54] Correct spelling --- core/src/main/java/hudson/ClassicPluginStrategy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/ClassicPluginStrategy.java b/core/src/main/java/hudson/ClassicPluginStrategy.java index d426d36226..a3903fbb1b 100644 --- a/core/src/main/java/hudson/ClassicPluginStrategy.java +++ b/core/src/main/java/hudson/ClassicPluginStrategy.java @@ -78,7 +78,7 @@ import java.util.logging.Logger; import static org.apache.commons.io.FilenameUtils.getBaseName; -@SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "Admistrator action installing a plugin, which could do far worse.") +@SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "Administrator action installing a plugin, which could do far worse.") public class ClassicPluginStrategy implements PluginStrategy { private static final Logger LOGGER = Logger.getLogger(ClassicPluginStrategy.class.getName()); From 2241771c9a9cd1a95532894a78a650ee006e19e1 Mon Sep 17 00:00:00 2001 From: Jeff Thompson <37345299+jeffret-b@users.noreply.github.com> Date: Fri, 20 Dec 2019 12:48:54 -0700 Subject: [PATCH 05/54] Correct spelling Co-Authored-By: Wadeck Follonier --- .../java/hudson/security/HudsonAuthenticationEntryPoint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/security/HudsonAuthenticationEntryPoint.java b/core/src/main/java/hudson/security/HudsonAuthenticationEntryPoint.java index 4c1ab5a98d..8c8959b63c 100644 --- a/core/src/main/java/hudson/security/HudsonAuthenticationEntryPoint.java +++ b/core/src/main/java/hudson/security/HudsonAuthenticationEntryPoint.java @@ -61,7 +61,7 @@ import java.text.MessageFormat; */ public class HudsonAuthenticationEntryPoint extends AuthenticationProcessingFilterEntryPoint { @Override - @SuppressFBWarnings(value = "XSS_SERVLET", justification = "Interemediate step for redirecting users to login page.") + @SuppressFBWarnings(value = "XSS_SERVLET", justification = "Intermediate step for redirecting users to login page.") public void commence(ServletRequest request, ServletResponse response, AuthenticationException reason) throws IOException, ServletException { HttpServletRequest req = (HttpServletRequest) request; HttpServletResponse rsp = (HttpServletResponse) response; From 348a26f77eec159261d5567b5228300ff140996d Mon Sep 17 00:00:00 2001 From: Jeff Thompson Date: Fri, 20 Dec 2019 15:01:47 -0700 Subject: [PATCH 06/54] Narrow the suppression. And add TODO connected with JENKINS tickets for relevant issues. --- cli/src/main/java/hudson/cli/CLI.java | 14 +++++++++-- .../java/hudson/cli/FullDuplexHttpStream.java | 13 ++++++---- .../java/hudson/cli/NoCheckTrustManager.java | 4 +++- cli/src/main/java/hudson/cli/SSHCLI.java | 8 +++++-- .../java/hudson/ClassicPluginStrategy.java | 2 +- core/src/main/java/hudson/Util.java | 13 +++++++--- core/src/main/java/hudson/cli/Connection.java | 12 ++++++---- .../hudson/console/AnnotatedLargeText.java | 8 +++++-- .../main/java/hudson/console/ConsoleNote.java | 8 +++++-- core/src/main/java/hudson/scheduler/Hash.java | 9 +++++-- .../security/BasicAuthenticationFilter.java | 10 +++++--- .../HudsonAuthenticationEntryPoint.java | 24 +++++++++++-------- .../main/java/hudson/util/ConsistentHash.java | 10 ++++++-- .../java/hudson/util/DOSToUnixPathHelper.java | 8 ++++--- .../java/hudson/util/FormFieldValidator.java | 9 ++++--- .../java/jenkins/model/RunIdMigrator.java | 9 +++++-- .../model/identity/IdentityRootAction.java | 8 ++++++- .../slaves/JnlpSlaveAgentProtocol4.java | 10 ++++---- 18 files changed, 127 insertions(+), 52 deletions(-) diff --git a/cli/src/main/java/hudson/cli/CLI.java b/cli/src/main/java/hudson/cli/CLI.java index 4cf98e85c1..8cbc3882de 100644 --- a/cli/src/main/java/hudson/cli/CLI.java +++ b/cli/src/main/java/hudson/cli/CLI.java @@ -191,7 +191,7 @@ public class CLI { continue; } if(head.equals("-i") && args.size()>=2) { - File f = new File(args.get(1)); + File f = getFileFromArguments(args); if (!f.exists()) { printUsage(Messages.CLI_NoSuchFileExists(f)); return -1; @@ -293,7 +293,7 @@ public class CLI { if (userInfo != null) { factory = factory.basicAuth(userInfo); } else if (auth != null) { - factory = factory.basicAuth(auth.startsWith("@") ? FileUtils.readFileToString(new File(auth.substring(1)), Charset.defaultCharset()).trim() : auth); + factory = factory.basicAuth(auth.startsWith("@") ? readAuthFromFile(auth).trim() : auth); } if (mode == Mode.HTTP) { @@ -307,6 +307,16 @@ public class CLI { throw new AssertionError(); } + @SuppressFBWarnings(value = {"PATH_TRAVERSAL_IN", "URLCONNECTION_SSRF_FD"}, justification = "User provided values for running the program.") + private static String readAuthFromFile(String auth) throws IOException { + return FileUtils.readFileToString(new File(auth.substring(1)), Charset.defaultCharset()); + } + + @SuppressFBWarnings(value = {"PATH_TRAVERSAL_IN", "URLCONNECTION_SSRF_FD"}, justification = "User provided values for running the program.") + private static File getFileFromArguments(List args) { + return new File(args.get(1)); + } + private static int webSocketConnection(String url, List args, CLIConnectionFactory factory) throws Exception { LOGGER.fine(() -> "Trying to connect to " + url + " via plain protocol over WebSocket"); class CLIEndpoint extends Endpoint { diff --git a/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java b/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java index 1f066a620d..fe8db45116 100644 --- a/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java +++ b/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java @@ -46,7 +46,6 @@ public class FullDuplexHttpStream { * @param authorization * The value of the authorization header, if non-null. */ - @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Client-side code doesn't involve SSRF.") public FullDuplexHttpStream(URL base, String relativeTarget, String authorization) throws IOException { if (!base.toString().endsWith("/")) { throw new IllegalArgumentException(base.toString()); @@ -63,7 +62,7 @@ public class FullDuplexHttpStream { // server->client LOGGER.fine("establishing download side"); - HttpURLConnection con = (HttpURLConnection) target.openConnection(); + HttpURLConnection con = openHttpConnection(target); con.setDoOutput(true); // request POST to avoid caching con.setRequestMethod("POST"); con.addRequestProperty("Session", uuid.toString()); @@ -81,7 +80,7 @@ public class FullDuplexHttpStream { // client->server uses chunked encoded POST for unlimited capacity. LOGGER.fine("establishing upload side"); - con = (HttpURLConnection) target.openConnection(); + con = openHttpConnection(target); con.setDoOutput(true); // request POST con.setRequestMethod("POST"); con.setChunkedStreamingMode(0); @@ -95,11 +94,15 @@ public class FullDuplexHttpStream { LOGGER.fine("established upload side"); } - // As this transport mode is using POST, it is necessary to resolve possible redirections using GET first. @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Client-side code doesn't involve SSRF.") + private HttpURLConnection openHttpConnection(URL target) throws IOException { + return (HttpURLConnection) target.openConnection(); + } + + // As this transport mode is using POST, it is necessary to resolve possible redirections using GET first. @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Client-side code doesn't involve SSRF.") private URL tryToResolveRedirects(URL base, String authorization) { try { - HttpURLConnection con = (HttpURLConnection) base.openConnection(); + HttpURLConnection con = openHttpConnection(base); if (authorization != null) { con.addRequestProperty("Authorization", authorization); } diff --git a/cli/src/main/java/hudson/cli/NoCheckTrustManager.java b/cli/src/main/java/hudson/cli/NoCheckTrustManager.java index 5a8346cb6e..cceacf2b21 100644 --- a/cli/src/main/java/hudson/cli/NoCheckTrustManager.java +++ b/cli/src/main/java/hudson/cli/NoCheckTrustManager.java @@ -9,14 +9,16 @@ import java.security.cert.X509Certificate; /** * @author Kohsuke Kawaguchi */ -@SuppressFBWarnings(value = "WEAK_TRUST_MANAGER", justification = "User set parameter to skip verifier.") public class NoCheckTrustManager implements X509TrustManager { + @SuppressFBWarnings(value = "WEAK_TRUST_MANAGER", justification = "User set parameter to skip verifier.") public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { } + @SuppressFBWarnings(value = "WEAK_TRUST_MANAGER", justification = "User set parameter to skip verifier.") public void checkServerTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { } + @SuppressFBWarnings(value = "WEAK_TRUST_MANAGER", justification = "User set parameter to skip verifier.") public X509Certificate[] getAcceptedIssuers() { return new X509Certificate[0]; } diff --git a/cli/src/main/java/hudson/cli/SSHCLI.java b/cli/src/main/java/hudson/cli/SSHCLI.java index cbfeea16b8..5321536c5c 100644 --- a/cli/src/main/java/hudson/cli/SSHCLI.java +++ b/cli/src/main/java/hudson/cli/SSHCLI.java @@ -61,11 +61,10 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; */ class SSHCLI { - @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Client-side code doesn't involve SSRF.") static int sshConnection(String jenkinsUrl, String user, List args, PrivateKeyProvider provider, final boolean strictHostKey) throws IOException { Logger.getLogger(SecurityUtils.class.getName()).setLevel(Level.WARNING); // suppress: BouncyCastle not registered, using the default JCE provider URL url = new URL(jenkinsUrl + "login"); - URLConnection conn = url.openConnection(); + URLConnection conn = openConnection(url); CLI.verifyJenkinsConnection(conn); String endpointDescription = conn.getHeaderField("X-SSH-Endpoint"); @@ -131,6 +130,11 @@ class SSHCLI { } } + @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Client-side code doesn't involve SSRF.") + private static URLConnection openConnection(URL url) throws IOException { + return url.openConnection(); + } + private SSHCLI() {} } diff --git a/core/src/main/java/hudson/ClassicPluginStrategy.java b/core/src/main/java/hudson/ClassicPluginStrategy.java index a3903fbb1b..e3af9fc07c 100644 --- a/core/src/main/java/hudson/ClassicPluginStrategy.java +++ b/core/src/main/java/hudson/ClassicPluginStrategy.java @@ -78,7 +78,6 @@ import java.util.logging.Logger; import static org.apache.commons.io.FilenameUtils.getBaseName; -@SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "Administrator action installing a plugin, which could do far worse.") public class ClassicPluginStrategy implements PluginStrategy { private static final Logger LOGGER = Logger.getLogger(ClassicPluginStrategy.class.getName()); @@ -432,6 +431,7 @@ public class ClassicPluginStrategy implements PluginStrategy { return null; } + @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "Administrator action installing a plugin, which could do far worse.") private static File resolve(File base, String relative) { File rel = new File(relative); if(rel.isAbsolute()) diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java index de29e9a013..7c9f7c5fe7 100644 --- a/core/src/main/java/hudson/Util.java +++ b/core/src/main/java/hudson/Util.java @@ -589,6 +589,8 @@ public class Util { /** * Computes MD5 digest of the given input stream. * + * This method should only be used for non-security applications where the MD5 weakness is not a problem. + * * @param source * The stream will be closed by this method at the end of this method. * @return @@ -596,11 +598,9 @@ public class Util { * @see DigestUtils#md5Hex(InputStream) */ @Nonnull - @SuppressFBWarnings(value = "WEAK_MESSAGE_DIGEST_MD5", justification = - "This method should only be used for non-security applications where their weakness is not a problem.") public static String getDigestOf(@Nonnull InputStream source) throws IOException { try { - MessageDigest md5 = MessageDigest.getInstance("MD5"); + MessageDigest md5 = getMd5(); DigestInputStream in = new DigestInputStream(source, md5); // Note: IOUtils.copy() buffers the input internally, so there is no // need to use a BufferedInputStream. @@ -620,6 +620,13 @@ public class Util { */ } + // TODO JENKINS-60563 remove MD5 from all usages in Jenkins + @SuppressFBWarnings(value = "WEAK_MESSAGE_DIGEST_MD5", justification = + "This method should only be used for non-security applications where the MD5 weakness is not a problem.") + private static MessageDigest getMd5() throws NoSuchAlgorithmException { + return MessageDigest.getInstance("MD5"); + } + @Nonnull public static String getDigestOf(@Nonnull String text) { try { diff --git a/core/src/main/java/hudson/cli/Connection.java b/core/src/main/java/hudson/cli/Connection.java index a181c5d208..bdeb192fd3 100644 --- a/core/src/main/java/hudson/cli/Connection.java +++ b/core/src/main/java/hudson/cli/Connection.java @@ -114,7 +114,8 @@ public class Connection { /** * Receives an object sent by {@link #writeObject(Object)} */ - @SuppressFBWarnings(value = "OBJECT_DESERIALIZATION", justification = "Not used. We should just remove it.") + // TODO JENKINS-60562 remove this class + @SuppressFBWarnings(value = "OBJECT_DESERIALIZATION", justification = "Not used. We should just remove it. Class is deprecated.") public T readObject() throws IOException, ClassNotFoundException { ObjectInputStream ois = new ObjectInputStreamEx(in, getClass().getClassLoader(), ClassFilter.DEFAULT); @@ -194,19 +195,22 @@ public class Connection { * @return * A new {@link Connection} object that includes the transport encryption. */ - @SuppressFBWarnings(value = "STATIC_IV", justification = "Not used. We should just remove it.") public Connection encryptConnection(SecretKey sessionKey, String algorithm) throws IOException, GeneralSecurityException { Cipher cout = Cipher.getInstance(algorithm); - cout.init(Cipher.ENCRYPT_MODE, sessionKey, new IvParameterSpec(sessionKey.getEncoded())); + cout.init(Cipher.ENCRYPT_MODE, sessionKey, createIv(sessionKey)); CipherOutputStream o = new CipherOutputStream(out, cout); Cipher cin = Cipher.getInstance(algorithm); - cin.init(Cipher.DECRYPT_MODE, sessionKey, new IvParameterSpec(sessionKey.getEncoded())); + cin.init(Cipher.DECRYPT_MODE, sessionKey, createIv(sessionKey)); CipherInputStream i = new CipherInputStream(in, cin); return new Connection(i,o); } + private IvParameterSpec createIv(SecretKey sessionKey) { + return new IvParameterSpec(sessionKey.getEncoded()); + } + /** * Given a byte array that contains arbitrary number of bytes, digests or expands those bits into the specified * number of bytes without loss of entropy. diff --git a/core/src/main/java/hudson/console/AnnotatedLargeText.java b/core/src/main/java/hudson/console/AnnotatedLargeText.java index 9625a8609f..36d2664e92 100644 --- a/core/src/main/java/hudson/console/AnnotatedLargeText.java +++ b/core/src/main/java/hudson/console/AnnotatedLargeText.java @@ -117,7 +117,6 @@ public class AnnotatedLargeText extends LargeText { rsp.setContentType(isHtml() ? "text/html;charset=UTF-8" : "text/plain;charset=UTF-8"); } - @SuppressFBWarnings(value = "OBJECT_DESERIALIZATION", justification = "Deserialization is protected by logic.") private ConsoleAnnotator createAnnotator(StaplerRequest req) throws IOException { try { String base64 = req!=null ? req.getHeader("X-ConsoleAnnotator") : null; @@ -130,7 +129,7 @@ public class AnnotatedLargeText extends LargeText { long timestamp = ois.readLong(); if (TimeUnit.HOURS.toMillis(1) > abs(System.currentTimeMillis()-timestamp)) // don't deserialize something too old to prevent a replay attack - return (ConsoleAnnotator) ois.readObject(); + return getConsoleAnnotator(ois); } catch (RuntimeException ex) { throw new IOException("Could not decode input", ex); } @@ -142,6 +141,11 @@ public class AnnotatedLargeText extends LargeText { return ConsoleAnnotator.initial(context); } + @SuppressFBWarnings(value = "OBJECT_DESERIALIZATION", justification = "Deserialization is protected by logic.") + private ConsoleAnnotator getConsoleAnnotator(ObjectInputStream ois) throws IOException, ClassNotFoundException { + return (ConsoleAnnotator) ois.readObject(); + } + @CheckReturnValue @Override public long writeLogTo(long start, Writer w) throws IOException { diff --git a/core/src/main/java/hudson/console/ConsoleNote.java b/core/src/main/java/hudson/console/ConsoleNote.java index 82ca8365e7..e55f7c2629 100644 --- a/core/src/main/java/hudson/console/ConsoleNote.java +++ b/core/src/main/java/hudson/console/ConsoleNote.java @@ -233,7 +233,6 @@ public abstract class ConsoleNote implements Serializable, Describable implements Serializable, Describable implements Serializable, Describable" + - "" + - "" + - "" + - "%n" + - "%n%n"+ - "Authentication required%n"+ - " - + + + + Context where the page is loaded. + + + Title of the HTML page. Rendered in the page content inside a <h1>. + + + The method to call from within the HudsonPrivateSecurityRealm. + + + Determines if the tag will include the captcha. + + + The wrapper for the data provided by the user and the error message(s) if any. Is null on first form display. + + +

${title}

-
+
-
+
${data.errorMessage}
-
+
${error.value}
- + @@ -55,17 +73,17 @@ THE SOFTWARE. - - - - + + + + diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/_entryFormPage.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/_entryFormPage.jelly index 8009ad6518..cb30e27bdf 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/_entryFormPage.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/_entryFormPage.jelly @@ -22,28 +22,36 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. --> - + - + + + + Corresponds to the "it" of Jelly, meaning the context where the page is loaded. + + + Title of the HTML page. + Rendered into <title> tag, in the page content inside a <h1> and as the label of the submit button. + + + The method to call from within the HudsonPrivateSecurityRealm. + + + Determines if the tag will include the captcha. + + + The wrapper for the data provided by the user and the error message(s) if any. Is null on first form display. + + - - - + -
- + + - diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/_entryFormPage/resources.css b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/_entryFormPage/resources.css new file mode 100644 index 0000000000..18e0744642 --- /dev/null +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/_entryFormPage/resources.css @@ -0,0 +1,12 @@ +input { + /* match width with captcha image */ + width:200px; +} + +.form-content { + margin: 2em; +} + +.form-content .error { + margin-bottom: 1em; +} diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/addUser.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/addUser.jelly index daacc17bfc..b9b3f246c7 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/addUser.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/addUser.jelly @@ -26,6 +26,6 @@ THE SOFTWARE. Page for admin to create a new user --> - - - \ No newline at end of file + + + diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly index 9757152168..a36f9c14f0 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly @@ -23,8 +23,7 @@ THE SOFTWARE. --> - + diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/firstUser.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/firstUser.jelly index 6ebd369ae0..b3568d1a6e 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/firstUser.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/firstUser.jelly @@ -26,6 +26,6 @@ THE SOFTWARE. This is used to create the first user. --> - - - \ No newline at end of file + + + diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/index.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/index.jelly index a7cad84726..29d3ffc4a2 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/index.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/index.jelly @@ -23,7 +23,7 @@ THE SOFTWARE. --> - + @@ -32,20 +32,20 @@ THE SOFTWARE.
${%Username}:
${%Password}:
${%E-mail address}:
${%E-mail address}:
${%Enter text as shown}:
- [${%captcha}] + [${%captcha}]
- - - + diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/loginLink.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/loginLink.jelly index c2ef8305fa..ba75e740a8 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/loginLink.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/loginLink.jelly @@ -23,10 +23,10 @@ THE SOFTWARE. --> - + | ${%sign up} - \ No newline at end of file + diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly index 8d864d01e9..577f963096 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly @@ -23,13 +23,13 @@ THE SOFTWARE. --> - + - - - + + + - \ No newline at end of file + diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/signupWithFederatedIdentity.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/signupWithFederatedIdentity.jelly index 8daec8d381..83f9eef9ec 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/signupWithFederatedIdentity.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/signupWithFederatedIdentity.jelly @@ -26,6 +26,6 @@ THE SOFTWARE. User self sign up page. --> - - - \ No newline at end of file + + + diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/success.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/success.jelly index 59363c6d1c..a6f137e1a2 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/success.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/success.jelly @@ -23,7 +23,7 @@ THE SOFTWARE. --> - + diff --git a/core/src/main/resources/hudson/security/SecurityRealm/signup.jelly b/core/src/main/resources/hudson/security/SecurityRealm/signup.jelly index 9f054b876c..3609d38405 100644 --- a/core/src/main/resources/hudson/security/SecurityRealm/signup.jelly +++ b/core/src/main/resources/hudson/security/SecurityRealm/signup.jelly @@ -22,7 +22,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. --> - + diff --git a/core/src/main/resources/lib/layout/hasPermission.jelly b/core/src/main/resources/lib/layout/hasPermission.jelly index d42a5f9689..6371acc382 100644 --- a/core/src/main/resources/lib/layout/hasPermission.jelly +++ b/core/src/main/resources/lib/layout/hasPermission.jelly @@ -26,7 +26,13 @@ THE SOFTWARE. Renders the body only if the current user has the specified permission - + + By default it will reuse the current context. + If the provided value does not inherit from hudson.security.AccessControlled, + the tag will look for the first ancestor satisfying the condition. + The hasPermission will be performed against that value. + + permission object to check. If this is null, the body will be also rendered. @@ -34,4 +40,4 @@ THE SOFTWARE. - \ No newline at end of file + diff --git a/war/src/main/webapp/css/style.css b/war/src/main/webapp/css/style.css index b57dfd5dc4..252c86423b 100644 --- a/war/src/main/webapp/css/style.css +++ b/war/src/main/webapp/css/style.css @@ -522,6 +522,10 @@ table.bigtable.pane > tbody > tr > td:last-child { white-space: nowrap; } +.bigtable th.minimum-width { + width: 1px; +} + .bigtable td { vertical-align: middle; padding: 3px 4px 3px 4px; From 8b60510cb48d6d190e24cc5f5225843da4b2934b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20Gond=C5=BEa?= Date: Mon, 10 Feb 2020 12:43:21 +0100 Subject: [PATCH 10/54] Clarify what exception gets propagated --- .../java/hudson/util/AtomicFileWriter.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/hudson/util/AtomicFileWriter.java b/core/src/main/java/hudson/util/AtomicFileWriter.java index ca689c2bbe..95415a8b68 100644 --- a/core/src/main/java/hudson/util/AtomicFileWriter.java +++ b/core/src/main/java/hudson/util/AtomicFileWriter.java @@ -189,13 +189,13 @@ public class AtomicFileWriter extends Writer { try { // Try to make an atomic move. Files.move(tmpPath, destPath, StandardCopyOption.ATOMIC_MOVE); - } catch (IOException e) { + } catch (IOException moveFailed) { // If it falls here that can mean many things. Either that the atomic move is not supported, // or something wrong happened. Anyway, let's try to be over-diagnosing - if (e instanceof AtomicMoveNotSupportedException) { - LOGGER.log(Level.WARNING, "Atomic move not supported. falling back to non-atomic move.", e); + if (moveFailed instanceof AtomicMoveNotSupportedException) { + LOGGER.log(Level.WARNING, "Atomic move not supported. falling back to non-atomic move.", moveFailed); } else { - LOGGER.log(Level.WARNING, "Unable to move atomically, falling back to non-atomic move.", e); + LOGGER.log(Level.WARNING, "Unable to move atomically, falling back to non-atomic move.", moveFailed); } if (destPath.toFile().exists()) { @@ -204,19 +204,19 @@ public class AtomicFileWriter extends Writer { try { Files.move(tmpPath, destPath, StandardCopyOption.REPLACE_EXISTING); - } catch (IOException e1) { - e.addSuppressed(e1); + } catch (IOException replaceFailed) { + replaceFailed.addSuppressed(moveFailed); LOGGER.log(Level.WARNING, "Unable to move {0} to {1}. Attempting to delete {0} and abandoning.", new Path[]{tmpPath, destPath}); try { Files.deleteIfExists(tmpPath); - } catch (IOException e2) { - e1.addSuppressed(e2); + } catch (IOException deleteFailed) { + replaceFailed.addSuppressed(deleteFailed); LOGGER.log(Level.WARNING, "Unable to delete {0}, good bye then!", tmpPath); - throw e; + throw replaceFailed; } - throw e; + throw replaceFailed; } } } From 495030350f960fd2f2550bc66ecf12e0d92158d1 Mon Sep 17 00:00:00 2001 From: Allan Burdajewicz Date: Mon, 10 Feb 2020 15:38:01 +1000 Subject: [PATCH 11/54] [JENKINS-60966] Order admin monitors in Global Configuration --- .../AdministrativeMonitorsConfiguration/config.groovy | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/resources/jenkins/management/AdministrativeMonitorsConfiguration/config.groovy b/core/src/main/resources/jenkins/management/AdministrativeMonitorsConfiguration/config.groovy index 18416259db..5698abc807 100644 --- a/core/src/main/resources/jenkins/management/AdministrativeMonitorsConfiguration/config.groovy +++ b/core/src/main/resources/jenkins/management/AdministrativeMonitorsConfiguration/config.groovy @@ -34,7 +34,8 @@ f.section(title: _("Administrative monitors configuration")) { f.entry(title: _("Enabled administrative monitors")) { p(_("blurb")) table(width: "100%") { - for (AdministrativeMonitor am : AdministrativeMonitor.all()) { + for (AdministrativeMonitor am : new ArrayList<>(AdministrativeMonitor.all()) + .sort({ o1, o2 -> o1.getDisplayName() <=> o2.getDisplayName() })) { f.block() { f.checkbox(name: "administrativeMonitor", title: am.displayName, From 9c4dac702e1feac4ebb983f98d5d83bf3308da79 Mon Sep 17 00:00:00 2001 From: Raihaan Shouhell Date: Thu, 13 Feb 2020 16:40:35 +0800 Subject: [PATCH 12/54] Add preauth filtering overload of getAllItems(), getItems() and allItems() (#4469) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add preauth filtering overload of getAllItems() Signed-off-by: Raihaan Shouhell * Add allItems filtering * Apply suggestions from code review Co-Authored-By: Evaristo Gutiérrez * Use streams * Add stream functions Signed-off-by: Raihaan Shouhell * Missed simplification Co-authored-by: Evaristo Gutiérrez --- .../src/main/java/hudson/model/ItemGroup.java | 48 +++++++++++ core/src/main/java/hudson/model/Items.java | 80 ++++++++++++++++--- core/src/main/java/jenkins/model/Jenkins.java | 14 +++- .../src/test/java/hudson/model/ItemsTest.java | 42 ++++++++++ 4 files changed, 173 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/hudson/model/ItemGroup.java b/core/src/main/java/hudson/model/ItemGroup.java index 2397587a0b..f79df6bae4 100644 --- a/core/src/main/java/hudson/model/ItemGroup.java +++ b/core/src/main/java/hudson/model/ItemGroup.java @@ -25,9 +25,13 @@ package hudson.model; import hudson.model.listeners.ItemListener; import java.io.IOException; +import java.util.ArrayList; import java.util.Collection; import java.io.File; import java.util.List; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.annotation.CheckForNull; import org.acegisecurity.AccessDeniedException; @@ -55,6 +59,33 @@ public interface ItemGroup extends PersistenceRoot, ModelObject */ Collection getItems(); + /** + * Gets all the items in this collection in a read-only view + * that matches supplied Predicate + * @since TODO + */ + default Collection getItems(Predicate pred) { + return getItemsStream(pred) + .collect(Collectors.toList()); + } + + /** + * Gets a read-only stream of all the items in this collection + * @since TODO + */ + default Stream getItemsStream() { + return getItems().stream(); + } + + /** + * Gets a read-only stream of all the items in this collection + * that matches supplied Predicate + * @since TODO + */ + default Stream getItemsStream(Predicate pred) { + return getItemsStream().filter(pred); + } + /** * Returns the path relative to the context root, * like "foo/bar/zot/". Note no leading slash but trailing slash. @@ -99,6 +130,14 @@ public interface ItemGroup extends PersistenceRoot, ModelObject return Items.getAllItems(this, type); } + /** + * Similar to {@link #getAllItems(Class)} with additional predicate filtering + * @since TODO + */ + default List getAllItems(Class type, Predicate pred) { + return Items.getAllItems(this, type, pred); + } + /** * Gets all the {@link Item}s unordered, lazily and recursively in the {@link ItemGroup} tree * and filter them by the given type. @@ -108,6 +147,15 @@ public interface ItemGroup extends PersistenceRoot, ModelObject return Items.allItems(this, type); } + /** + * Gets all the {@link Item}s unordered, lazily and recursively in the {@link ItemGroup} tree + * and filter them by the given type and given predicate + * @since TODO + */ + default Iterable allItems(Class type, Predicate pred) { + return Items.allItems(this, type, pred); + } + /** * Gets all the items recursively. * @since 2.93 diff --git a/core/src/main/java/hudson/model/Items.java b/core/src/main/java/hudson/model/Items.java index c6b06e322c..8c31c0bf1d 100644 --- a/core/src/main/java/hudson/model/Items.java +++ b/core/src/main/java/hudson/model/Items.java @@ -47,6 +47,7 @@ import java.util.List; import java.util.NoSuchElementException; import java.util.Stack; import java.util.StringTokenizer; +import java.util.function.Predicate; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import jenkins.model.DirectlyModifiableTopLevelItemGroup; @@ -393,26 +394,44 @@ public class Items { *

* If you do not need to iterate all items, or if the order of the items is not required, consider using * {@link #allItems(ItemGroup, Class)} instead. + * + * @param root Root node to start searching from + * @param type Given type of of items being searched for + * @return List of items matching given criteria * * @since 1.512 */ public static List getAllItems(final ItemGroup root, Class type) { + return getAllItems(root ,type, t -> true); + } + + /** + * Similar to {@link #getAllItems(ItemGroup, Class)} but with a predicate to pre-filter items to + * avoid checking ACLs unnecessarily and returning items not required by the caller + * @param root Root node to start searching from + * @param type Given type of of items being searched for + * @param pred Predicate condition to filter items + * @return List of items matching given criteria + * + * @since TODO + */ + public static List getAllItems(final ItemGroup root, Class type, Predicate pred) { List r = new ArrayList<>(); - getAllItems(root, type, r); + getAllItems(root, type, r, pred); return r; } - private static void getAllItems(final ItemGroup root, Class type, List r) { + private static void getAllItems(final ItemGroup root, Class type, List r, Predicate pred) { List items = new ArrayList<>(((ItemGroup) root).getItems()); // because we add items depth first, we can use the quicker BY_NAME comparison items.sort(BY_NAME); for (Item i : items) { - if (type.isInstance(i)) { + if (type.isInstance(i) && pred.test(type.cast(i))) { if (i.hasPermission(Item.READ)) { r.add(type.cast(i)); } } if (i instanceof ItemGroup) { - getAllItems((ItemGroup) i, type, r); + getAllItems((ItemGroup) i, type, r, pred); } } } @@ -434,6 +453,24 @@ public class Items { return allItems(Jenkins.getAuthentication(), root, type); } + /** + * Gets a read-only view of all the {@link Item}s recursively matching type and predicate + * in the {@link ItemGroup} tree visible to + * {@link Jenkins#getAuthentication()} without concern for the order in which items are returned. Each iteration + * of the view will be "live" reflecting the items available between the time the iteration was started and the + * time the iteration was completed, however if items are moved during an iteration - depending on the move - it + * may be possible for such items to escape the entire iteration. + * + * @param root the root. + * @param type the type. + * @param the type. + * @param the predicate. + * @return An {@link Iterable} for all items. + * @since TODO + */ + public static Iterable allItems(ItemGroup root, Class type, Predicate pred) { + return allItems(Jenkins.getAuthentication(), root, type, pred); + } /** * Gets a read-only view all the {@link Item}s recursively in the {@link ItemGroup} tree visible to the supplied @@ -449,7 +486,26 @@ public class Items { * @since 2.37 */ public static Iterable allItems(Authentication authentication, ItemGroup root, Class type) { - return new AllItemsIterable<>(root, authentication, type); + return allItems(authentication, root, type, t -> true); + } + + /** + * Gets a read-only view all the {@link Item}s recursively matching supplied type and predicate conditions + * in the {@link ItemGroup} tree visible to the supplied + * authentication without concern for the order in which items are returned. Each iteration + * of the view will be "live" reflecting the items available between the time the iteration was started and the + * time the iteration was completed, however if items are moved during an iteration - depending on the move - it + * may be possible for such items to escape the entire iteration. + * + * @param root the root. + * @param type the type. + * @param the type. + * @param pred the predicate. + * @return An {@link Iterable} for all items. + * @since TODO + */ + public static Iterable allItems(Authentication authentication, ItemGroup root, Class type, Predicate pred) { + return new AllItemsIterable<>(root, authentication, type, pred); } /** @@ -518,11 +574,16 @@ public class Items { * The type of item we want to return. */ private final Class type; + /** + * Predicate to filter items with + */ + private final Predicate pred; - private AllItemsIterable(ItemGroup root, Authentication authentication, Class type) { + private AllItemsIterable(ItemGroup root, Authentication authentication, Class type, Predicate pred) { this.root = root; this.authentication = authentication; this.type = type; + this.pred = pred; } @Override @@ -561,6 +622,7 @@ public class Items { if (next != null) { return true; } + Predicate search = t -> t instanceof ItemGroup || (type.isInstance(t) && pred.test(type.cast(t))); while (true) { if (delegate == null || !delegate.hasNext()) { if (stack.isEmpty()) { @@ -569,13 +631,13 @@ public class Items { ItemGroup group = stack.pop(); // group.getItems() is responsible for performing the permission check so we will not repeat it if (Jenkins.getAuthentication() == authentication) { - delegate = group.getItems().iterator(); + delegate = group.getItems(search).iterator(); } else { // slower path because the caller has switched authentication // we need to keep the original authentication so that allItems() can be used // like getAllItems() without the cost of building the entire list up front try (ACLContext ctx = ACL.as(authentication)) { - delegate = group.getItems().iterator(); + delegate = group.getItems(search).iterator(); } } } @@ -584,7 +646,7 @@ public class Items { if (item instanceof ItemGroup) { stack.push((ItemGroup) item); } - if (type.isInstance(item)) { + if (type.isInstance(item) && pred.test(type.cast(item))) { next = type.cast(item); return true; } diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index b7a51f00ef..065251d881 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -292,6 +292,7 @@ import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.LogRecord; import java.util.logging.Logger; @@ -1761,12 +1762,21 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve */ @Exported(name="jobs") public List getItems() { + return getItems(t -> true); + } + + /** + * Gets just the immediate children of {@link Jenkins} based on supplied predicate. + * + * @see #getAllItems(Class) + * @since TODO + */ + public List getItems(Predicate pred) { List viewableItems = new ArrayList<>(); for (TopLevelItem item : items.values()) { - if (item.hasPermission(Item.READ)) + if (pred.test(item) && item.hasPermission(Item.READ)) viewableItems.add(item); } - return viewableItems; } diff --git a/test/src/test/java/hudson/model/ItemsTest.java b/test/src/test/java/hudson/model/ItemsTest.java index 1cab9a96a9..ffaac3a4a6 100644 --- a/test/src/test/java/hudson/model/ItemsTest.java +++ b/test/src/test/java/hudson/model/ItemsTest.java @@ -88,6 +88,27 @@ public class ItemsTest { assertEquals(Arrays.asList(sub2a, sub2ap, sub2alpha, sub2b, sub2bp, sub2BRAVO, sub2c, sub2cp, sub2charlie), sub2.getAllItems(Item.class)); } + @Test public void getAllItemsPredicate() throws Exception { + MockFolder d = r.createFolder("d"); + MockFolder sub2 = d.createProject(MockFolder.class, "sub2"); + MockFolder sub2a = sub2.createProject(MockFolder.class, "a"); + MockFolder sub2c = sub2.createProject(MockFolder.class, "c"); + MockFolder sub2b = sub2.createProject(MockFolder.class, "b"); + MockFolder sub1 = d.createProject(MockFolder.class, "sub1"); + FreeStyleProject root = r.createFreeStyleProject("root"); + FreeStyleProject dp = d.createProject(FreeStyleProject.class, "p"); + FreeStyleProject sub1q = sub1.createProject(FreeStyleProject.class, "q"); + FreeStyleProject sub1p = sub1.createProject(FreeStyleProject.class, "p"); + FreeStyleProject sub2ap = sub2a.createProject(FreeStyleProject.class, "p"); + FreeStyleProject sub2bp = sub2b.createProject(FreeStyleProject.class, "p"); + FreeStyleProject sub2cp = sub2c.createProject(FreeStyleProject.class, "p"); + FreeStyleProject sub2alpha = sub2.createProject(FreeStyleProject.class, "alpha"); + FreeStyleProject sub2BRAVO = sub2.createProject(FreeStyleProject.class, "BRAVO"); + FreeStyleProject sub2charlie = sub2.createProject(FreeStyleProject.class, "charlie"); + assertEquals(Arrays.asList(dp, sub1p, sub2ap, sub2bp, sub2cp), d.getAllItems(FreeStyleProject.class, t -> t.getName().equals("p"))); + assertEquals(Arrays.asList(sub2a, sub2alpha), sub2.getAllItems(Item.class, t -> t.getName().startsWith("a"))); + } + @Issue("JENKINS-40252") @Test public void allItems() throws Exception { @@ -113,6 +134,27 @@ public class ItemsTest { sub2BRAVO, sub2c, sub2cp, sub2charlie)); } + @Test public void allItemsPredicate() throws Exception { + MockFolder d = r.createFolder("d"); + MockFolder sub2 = d.createProject(MockFolder.class, "sub2"); + MockFolder sub2a = sub2.createProject(MockFolder.class, "a"); + MockFolder sub2c = sub2.createProject(MockFolder.class, "c"); + MockFolder sub2b = sub2.createProject(MockFolder.class, "b"); + MockFolder sub1 = d.createProject(MockFolder.class, "sub1"); + FreeStyleProject root = r.createFreeStyleProject("root"); + FreeStyleProject dp = d.createProject(FreeStyleProject.class, "p"); + FreeStyleProject sub1q = sub1.createProject(FreeStyleProject.class, "q"); + FreeStyleProject sub1p = sub1.createProject(FreeStyleProject.class, "p"); + FreeStyleProject sub2ap = sub2a.createProject(FreeStyleProject.class, "p"); + FreeStyleProject sub2bp = sub2b.createProject(FreeStyleProject.class, "p"); + FreeStyleProject sub2cp = sub2c.createProject(FreeStyleProject.class, "p"); + FreeStyleProject sub2alpha = sub2.createProject(FreeStyleProject.class, "alpha"); + FreeStyleProject sub2BRAVO = sub2.createProject(FreeStyleProject.class, "BRAVO"); + FreeStyleProject sub2charlie = sub2.createProject(FreeStyleProject.class, "charlie"); + assertThat(d.allItems(FreeStyleProject.class, t -> t.getName().equals("p")), containsInAnyOrder(dp, sub1p, sub2ap, sub2bp, sub2cp)); + assertThat(sub2.allItems(Item.class, t -> t.getName().startsWith("a")), containsInAnyOrder(sub2a, sub2alpha)); + } + @Issue("JENKINS-24825") @Test public void moveItem() throws Exception { File tmp = tmpRule.getRoot(); From 7777d8f28bb20e50dc901ac52c3d7405c77805c7 Mon Sep 17 00:00:00 2001 From: Daniel Beck <1831569+daniel-beck@users.noreply.github.com> Date: Thu, 13 Feb 2020 09:40:58 +0100 Subject: [PATCH 13/54] Do not show disabled permissions in permission errors (#4482) --- core/src/main/java/hudson/security/ACL.java | 6 +++++- test/src/test/java/hudson/cli/GetNodeCommandTest.java | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/hudson/security/ACL.java b/core/src/main/java/hudson/security/ACL.java index 1f39620c66..e02083e6aa 100644 --- a/core/src/main/java/hudson/security/ACL.java +++ b/core/src/main/java/hudson/security/ACL.java @@ -69,8 +69,12 @@ public abstract class ACL { if (a == SYSTEM) { return; } - if(!hasPermission(a,p)) + if (!hasPermission(a,p)) { + while (!p.enabled && p.impliedBy != null) { + p = p.impliedBy; + } throw new AccessDeniedException2(a,p); + } } /** diff --git a/test/src/test/java/hudson/cli/GetNodeCommandTest.java b/test/src/test/java/hudson/cli/GetNodeCommandTest.java index 71cc405915..e90e75aa90 100644 --- a/test/src/test/java/hudson/cli/GetNodeCommandTest.java +++ b/test/src/test/java/hudson/cli/GetNodeCommandTest.java @@ -60,7 +60,7 @@ public class GetNodeCommandTest { .invokeWithArgs("MySlave") ; - assertThat(result.stderr(), containsString("ERROR: user is missing the Agent/ExtendedRead permission")); + assertThat(result.stderr(), containsString("ERROR: user is missing the Agent/Configure permission")); assertThat(result, failedWith(6)); assertThat(result, hasNoStandardOutput()); } From d67c1b024706484f4d8a84394442a932201f92d9 Mon Sep 17 00:00:00 2001 From: James Nord Date: Thu, 13 Feb 2020 17:03:45 +0000 Subject: [PATCH 14/54] allert-error does not exist, update javadoc to allert-danger --- core/src/main/java/hudson/model/AdministrativeMonitor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/model/AdministrativeMonitor.java b/core/src/main/java/hudson/model/AdministrativeMonitor.java index e0f0a63ff9..4554809c54 100644 --- a/core/src/main/java/hudson/model/AdministrativeMonitor.java +++ b/core/src/main/java/hudson/model/AdministrativeMonitor.java @@ -68,7 +68,7 @@ import org.kohsuke.stapler.interceptor.RequirePOST; * If {@link #isActivated()} returns true, Jenkins will use the {@code message.jelly} * view of this object to render the warning text. This happens in the * {@code http://SERVER/jenkins/manage} page. This view should typically render - * a DIV box with class='alert alert-error' or class='alert alert-warning' with a human-readable text + * a DIV box with class='alert alert-danger' or class='alert alert-warning' with a human-readable text * inside it. It often also contains a link to a page that provides more details * about the problem. * From 7026b4d042686411a6be0e2698bbe56a2a27f733 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Thu, 13 Feb 2020 12:08:13 -0700 Subject: [PATCH 15/54] Do not use Azure maven cache - unavailable The Azure maven cache was removed at the end of January 2020. When the DNS entry was also removed, builds began failing. Fix the builds. --- Jenkinsfile | 4 ++-- settings-azure.xml | 9 --------- 2 files changed, 2 insertions(+), 11 deletions(-) delete mode 100644 settings-azure.xml diff --git a/Jenkinsfile b/Jenkinsfile index fd76d7f129..62d7f503cc 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -43,7 +43,7 @@ for(j = 0; j < jdks.size(); j++) { "MAVEN_OPTS=-Xmx1536m -Xms512m"], buildType, jdk) { // Actually run Maven! // -Dmaven.repo.local=… tells Maven to create a subdir in the temporary directory for the local Maven repository - def mvnCmd = "mvn -Pdebug -U -Dset.changelist help:evaluate -Dexpression=changelist -Doutput=$changelistF clean install ${runTests ? '-Dmaven.test.failure.ignore' : '-DskipTests'} -V -B -ntp -Dmaven.repo.local=$m2repo -s settings-azure.xml -e" + def mvnCmd = "mvn -Pdebug -U -Dset.changelist help:evaluate -Dexpression=changelist -Doutput=$changelistF clean install ${runTests ? '-Dmaven.test.failure.ignore' : '-DskipTests'} -V -B -ntp -Dmaven.repo.local=$m2repo -e" if(isUnix()) { sh mvnCmd @@ -87,7 +87,7 @@ builds.ath = { checkout scm withMavenEnv(["JAVA_OPTS=-Xmx1536m -Xms512m", "MAVEN_OPTS=-Xmx1536m -Xms512m"], 8) { - sh "mvn --batch-mode --show-version -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn -DskipTests -am -pl war package -Dmaven.repo.local=${pwd tmp: true}/m2repo -s settings-azure.xml" + sh "mvn --batch-mode --show-version -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn -DskipTests -am -pl war package -Dmaven.repo.local=${pwd tmp: true}/m2repo" } dir("war/target") { fileUri = "file://" + pwd() + "/jenkins.war" diff --git a/settings-azure.xml b/settings-azure.xml deleted file mode 100644 index a1e96394d9..0000000000 --- a/settings-azure.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - azure - https://repo.azure.jenkins.io/public/ - repo.jenkins-ci.org - - - From e93e236d8452df68126ceecd094a317a74cb8b2f Mon Sep 17 00:00:00 2001 From: Raihaan Shouhell Date: Fri, 14 Feb 2020 10:01:28 +0800 Subject: [PATCH 16/54] Optimize getAllItems() --- core/src/main/java/hudson/model/Items.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/model/Items.java b/core/src/main/java/hudson/model/Items.java index 8c31c0bf1d..df4af2e221 100644 --- a/core/src/main/java/hudson/model/Items.java +++ b/core/src/main/java/hudson/model/Items.java @@ -421,7 +421,7 @@ public class Items { return r; } private static void getAllItems(final ItemGroup root, Class type, List r, Predicate pred) { - List items = new ArrayList<>(((ItemGroup) root).getItems()); + List items = new ArrayList<>(((ItemGroup) root).getItems(t -> t instanceof ItemGroup || (type.isInstance(t) && pred.test(type.cast(t))))); // because we add items depth first, we can use the quicker BY_NAME comparison items.sort(BY_NAME); for (Item i : items) { From 19be5e2862c912332719354290a08eb4f2e7f158 Mon Sep 17 00:00:00 2001 From: Raihaan Shouhell Date: Fri, 14 Feb 2020 10:26:55 +0800 Subject: [PATCH 17/54] Optimize getTiedJobs() Signed-off-by: Raihaan Shouhell --- core/src/main/java/hudson/model/Computer.java | 4 ++-- core/src/main/java/hudson/model/Label.java | 5 ++--- .../src/test/java/hudson/model/ComputerTest.java | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index de123b9368..0fe348beda 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -778,7 +778,7 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces @Exported public Set getAssignedLabels() { Node node = getNode(); - return (node != null) ? node.getAssignedLabels() : Collections.EMPTY_SET; + return (node != null) ? node.getAssignedLabels() : Collections.emptySet(); } /** @@ -786,7 +786,7 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces */ public List getTiedJobs() { Node node = getNode(); - return (node != null) ? node.getSelfLabel().getTiedJobs() : Collections.EMPTY_LIST; + return (node != null) ? node.getSelfLabel().getTiedJobs() : Collections.emptyList(); } public RunList getBuilds() { diff --git a/core/src/main/java/hudson/model/Label.java b/core/src/main/java/hudson/model/Label.java index ddf419604a..93749a5a24 100644 --- a/core/src/main/java/hudson/model/Label.java +++ b/core/src/main/java/hudson/model/Label.java @@ -389,9 +389,8 @@ public abstract class Label extends Actionable implements Comparable

+ ${%User ID} ${%Name} +
${user.id} ${user} - + - +
+

${%Memory Usage}

+ + + + + + + + + + +
+ ${%Timespan}: + + + ${%Short} + + + ${%Short} + + + + + + ${%Medium} + + + ${%Medium} + + + + + + ${%Long} + + + ${%Long} + + +
+ +

${%Thread Dumps}

${%threadDump_blurb('threadDump')}

From 55b2bc7f3fe037e46a06deb44b93b02787df28ef Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Fri, 14 Feb 2020 19:33:19 +0100 Subject: [PATCH 22/54] [JEP-223] Overall/Manage permission Co-Authored-By: Esther Alvarez Feijoo Co-Authored-By: aHenryJard Co-Authored-By: michael cirioli --- core/src/main/java/hudson/FilePath.java | 2 +- core/src/main/java/hudson/Functions.java | 4 ++++ .../main/java/hudson/model/Descriptor.java | 14 +++++++++++ .../hudson/model/ManageJenkinsAction.java | 2 +- .../java/hudson/model/ManagementLink.java | 2 +- .../main/java/jenkins/management/CliLink.java | 10 ++++++++ .../jenkins/management/ConfigureLink.java | 10 ++++++++ .../java/jenkins/management/NodesLink.java | 10 ++++++++ .../jenkins/management/StatisticsLink.java | 10 ++++++++ ...balProjectNamingStrategyConfiguration.java | 9 ++++++++ .../model/GlobalQuietPeriodConfiguration.java | 8 +++++++ .../GlobalSCMRetryCountConfiguration.java | 8 +++++++ core/src/main/java/jenkins/model/Jenkins.java | 23 ++++++++++++++++++- .../hudson/PluginManager/sidepanel.groovy | 2 +- .../hudson/model/ComputerSet/sidepanel.jelly | 2 +- .../hudson/model/Messages.properties | 4 ++++ .../sidepanel.jelly | 2 +- .../jenkins/model/Jenkins/configure.jelly | 2 +- .../jenkins/model/Jenkins/manage.jelly | 2 +- 19 files changed, 117 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index 01a8d3cc0e..6b5cf97304 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -2971,7 +2971,7 @@ public final class FilePath implements SerializableOnlyOverRemoting { private static void checkPermissionForValidate() { AccessControlled subject = Stapler.getCurrentRequest().findAncestorObject(AbstractProject.class); if (subject == null) - Jenkins.get().checkPermission(Jenkins.ADMINISTER); + Jenkins.get().checkPermission(Jenkins.MANAGE); else subject.checkPermission(Item.CONFIGURE); } diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java index 50339569b6..62208aaf93 100644 --- a/core/src/main/java/hudson/Functions.java +++ b/core/src/main/java/hudson/Functions.java @@ -1062,6 +1062,10 @@ public class Functions { Descriptor d = c.getInstance(); if (d.getGlobalConfigPage()==null) continue; + if (!Jenkins.get().hasPermission(d.getPermission())) { + continue; + } + if (predicate.apply(d.getCategory())) { r.add(new Tag(c.ordinal(), d)); } diff --git a/core/src/main/java/hudson/model/Descriptor.java b/core/src/main/java/hudson/model/Descriptor.java index 463cdee366..3a055d8119 100644 --- a/core/src/main/java/hudson/model/Descriptor.java +++ b/core/src/main/java/hudson/model/Descriptor.java @@ -24,6 +24,7 @@ package hudson.model; import hudson.DescriptorExtensionList; +import hudson.Extension; import hudson.PluginWrapper; import hudson.RelativePath; import hudson.XmlFile; @@ -31,6 +32,7 @@ import hudson.BulkChange; import hudson.ExtensionList; import hudson.Util; import hudson.model.listeners.SaveableListener; +import hudson.security.Permission; import hudson.util.FormApply; import hudson.util.FormValidation.CheckMethod; import hudson.util.ReflectionUtils; @@ -830,6 +832,18 @@ public abstract class Descriptor> implements Saveable, return GlobalConfigurationCategory.get(GlobalConfigurationCategory.Unclassified.class); } + /** + * Returns the permission type needed in order to configure the descriptor if and only if it is configured through the global (Unclassified) configuration. + * By default, requires {@link Jenkins.ADMINISTER} permission. + * Override to return something different if appropriate. The only currently supported alternative return value is {@link Jenkins.MANAGE}. + * + * @return Permission required to configure this descriptor. + */ + public @Nonnull + Permission getPermission() { + return Jenkins.ADMINISTER; + } + private String getViewPage(Class clazz, String pageName, String defaultValue) { return getViewPage(clazz,Collections.singleton(pageName),defaultValue); } diff --git a/core/src/main/java/hudson/model/ManageJenkinsAction.java b/core/src/main/java/hudson/model/ManageJenkinsAction.java index b1fb854a10..06659b19d8 100644 --- a/core/src/main/java/hudson/model/ManageJenkinsAction.java +++ b/core/src/main/java/hudson/model/ManageJenkinsAction.java @@ -35,7 +35,7 @@ import org.jenkinsci.Symbol; @Extension(ordinal=100) @Symbol("manageJenkins") public class ManageJenkinsAction implements RootAction { public String getIconFileName() { - if (Jenkins.get().hasPermission(Jenkins.ADMINISTER)) + if (Jenkins.get().hasPermission(Jenkins.MANAGE)) return "gear2.png"; else return null; diff --git a/core/src/main/java/hudson/model/ManagementLink.java b/core/src/main/java/hudson/model/ManagementLink.java index 00d6871097..05db6030f0 100644 --- a/core/src/main/java/hudson/model/ManagementLink.java +++ b/core/src/main/java/hudson/model/ManagementLink.java @@ -115,7 +115,7 @@ public abstract class ManagementLink implements ExtensionPoint, Action { * @return permission required for user to access this management link, in addition to {@link Jenkins#ADMINISTER} */ public @CheckForNull Permission getRequiredPermission() { - return null; + return Jenkins.ADMINISTER; } /** diff --git a/core/src/main/java/jenkins/management/CliLink.java b/core/src/main/java/jenkins/management/CliLink.java index 718504727f..6a2c67f791 100644 --- a/core/src/main/java/jenkins/management/CliLink.java +++ b/core/src/main/java/jenkins/management/CliLink.java @@ -26,8 +26,12 @@ package jenkins.management; import hudson.Extension; import hudson.model.ManagementLink; +import hudson.security.Permission; +import jenkins.model.Jenkins; import org.jenkinsci.Symbol; +import javax.annotation.CheckForNull; + /** * @author Nicolas De Loof */ @@ -48,6 +52,12 @@ public class CliLink extends ManagementLink { return Messages.CliLink_Description(); } + @CheckForNull + @Override + public Permission getRequiredPermission() { + return Jenkins.MANAGE; + } + @Override public String getUrlName() { return "cli"; diff --git a/core/src/main/java/jenkins/management/ConfigureLink.java b/core/src/main/java/jenkins/management/ConfigureLink.java index a10df71b22..61fe8f1103 100644 --- a/core/src/main/java/jenkins/management/ConfigureLink.java +++ b/core/src/main/java/jenkins/management/ConfigureLink.java @@ -26,8 +26,12 @@ package jenkins.management; import hudson.Extension; import hudson.model.ManagementLink; +import hudson.security.Permission; +import jenkins.model.Jenkins; import org.jenkinsci.Symbol; +import javax.annotation.CheckForNull; + /** * @author Nicolas De Loof */ @@ -48,6 +52,12 @@ public class ConfigureLink extends ManagementLink { return Messages.ConfigureLink_Description(); } + @CheckForNull + @Override + public Permission getRequiredPermission() { + return Jenkins.MANAGE; + } + @Override public String getUrlName() { return "configure"; diff --git a/core/src/main/java/jenkins/management/NodesLink.java b/core/src/main/java/jenkins/management/NodesLink.java index 90038450ce..3f85b701b9 100644 --- a/core/src/main/java/jenkins/management/NodesLink.java +++ b/core/src/main/java/jenkins/management/NodesLink.java @@ -26,9 +26,13 @@ package jenkins.management; import hudson.Extension; import hudson.model.ManagementLink; +import hudson.security.Permission; import jenkins.management.Messages; +import jenkins.model.Jenkins; import org.jenkinsci.Symbol; +import javax.annotation.CheckForNull; + /** * @author Nicolas De Loof */ @@ -49,6 +53,12 @@ public class NodesLink extends ManagementLink { return Messages.NodesLink_Description(); } + @CheckForNull + @Override + public Permission getRequiredPermission() { + return Jenkins.MANAGE; + } + @Override public String getUrlName() { return "computer"; diff --git a/core/src/main/java/jenkins/management/StatisticsLink.java b/core/src/main/java/jenkins/management/StatisticsLink.java index 9821781d78..83ff09edd3 100644 --- a/core/src/main/java/jenkins/management/StatisticsLink.java +++ b/core/src/main/java/jenkins/management/StatisticsLink.java @@ -26,8 +26,12 @@ package jenkins.management; import hudson.Extension; import hudson.model.ManagementLink; +import hudson.security.Permission; +import jenkins.model.Jenkins; import org.jenkinsci.Symbol; +import javax.annotation.CheckForNull; + /** * @author Nicolas De Loof */ @@ -48,6 +52,12 @@ public class StatisticsLink extends ManagementLink { return Messages.StatisticsLink_Description(); } + @CheckForNull + @Override + public Permission getRequiredPermission() { + return Jenkins.MANAGE; + } + @Override public String getUrlName() { return "load-statistics"; diff --git a/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java b/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java index 2502df1b6e..4f046291f2 100644 --- a/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java +++ b/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java @@ -24,12 +24,15 @@ package jenkins.model; import hudson.Extension; +import hudson.security.Permission; import jenkins.model.ProjectNamingStrategy.DefaultProjectNamingStrategy; import net.sf.json.JSONObject; import org.jenkinsci.Symbol; import org.kohsuke.stapler.StaplerRequest; +import javax.annotation.Nonnull; + /** * Configures the project naming strategy. * @@ -59,4 +62,10 @@ public class GlobalProjectNamingStrategyConfiguration extends GlobalConfiguratio } return true; } + + @Nonnull + @Override + public Permission getPermission() { + return Jenkins.MANAGE; + } } diff --git a/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java b/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java index b0cdd84d3d..93495cd384 100644 --- a/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java +++ b/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java @@ -24,10 +24,12 @@ package jenkins.model; import hudson.Extension; +import hudson.security.Permission; import net.sf.json.JSONObject; import org.jenkinsci.Symbol; import org.kohsuke.stapler.StaplerRequest; +import javax.annotation.Nonnull; import java.io.IOException; /** @@ -57,4 +59,10 @@ public class GlobalQuietPeriodConfiguration extends GlobalConfiguration { throw new FormException(e,"quietPeriod"); } } + + @Nonnull + @Override + public Permission getPermission() { + return Jenkins.MANAGE; + } } diff --git a/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java b/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java index 32d0c155c6..373eb62828 100644 --- a/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java +++ b/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java @@ -24,11 +24,13 @@ package jenkins.model; import hudson.Extension; +import hudson.security.Permission; import net.sf.json.JSONException; import net.sf.json.JSONObject; import org.jenkinsci.Symbol; import org.kohsuke.stapler.StaplerRequest; +import javax.annotation.Nonnull; import java.io.IOException; /** @@ -54,4 +56,10 @@ public class GlobalSCMRetryCountConfiguration extends GlobalConfiguration { throw new FormException(e.getMessage(), "quietPeriod"); } } + + @Nonnull + @Override + public Permission getPermission() { + return Jenkins.MANAGE; + } } diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 065251d881..9a44b86e6f 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -2199,6 +2199,9 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve * @since 2.64 */ public List getActiveAdministrativeMonitors() { + if (!Jenkins.get().hasPermission(ADMINISTER)) { + return Collections.emptyList(); + } return administrativeMonitors.stream().filter(m -> { try { return m.isEnabled() && m.isActivated(); @@ -3762,7 +3765,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve public synchronized void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException { BulkChange bc = new BulkChange(this); try { - checkPermission(ADMINISTER); + checkPermission(MANAGE); JSONObject json = req.getSubmittedForm(); @@ -5236,7 +5239,25 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve private static final Logger LOGGER = Logger.getLogger(Jenkins.class.getName()); public static final PermissionGroup PERMISSIONS = Permission.HUDSON_PERMISSIONS; + /** + * Grants ability to configure any and all aspects of the Jenkins instance + */ public static final Permission ADMINISTER = Permission.HUDSON_ADMINISTER; + + /** + * Allows non-privilege escalating configuration permission for a Jenkins instance. Actions which could result + * in a privilege escalation (such as RUN_SCRIPTS) require explicit ADMINISTER permission. + * + * As an experimental feature, making the manage permission able to be disabled by default (keep as ADMINISTER), can + * be enabled with "jenkins.security.ManagePermission" system property. + */ + public static final Permission MANAGE = new Permission(PERMISSIONS, "Manage", + Messages._Jenkins_Manage_Description(), + ADMINISTER, + SystemProperties.getBoolean("jenkins.security.ManagePermission"), + new PermissionScope[]{PermissionScope.JENKINS}); + + public static final Permission READ = new Permission(PERMISSIONS,"Read",Messages._Hudson_ReadPermission_Description(),Permission.READ,PermissionScope.JENKINS); public static final Permission RUN_SCRIPTS = new Permission(PERMISSIONS, "RunScripts", Messages._Hudson_RunScriptsPermission_Description(),ADMINISTER,PermissionScope.JENKINS); diff --git a/core/src/main/resources/hudson/PluginManager/sidepanel.groovy b/core/src/main/resources/hudson/PluginManager/sidepanel.groovy index 375fda352d..a3aa54878d 100644 --- a/core/src/main/resources/hudson/PluginManager/sidepanel.groovy +++ b/core/src/main/resources/hudson/PluginManager/sidepanel.groovy @@ -28,7 +28,7 @@ l.header() l.side_panel { l.tasks { l.task(icon:"icon-up icon-md", href:rootURL+'/', title:_("Back to Dashboard")) - l.task(icon:"icon-gear2 icon-md", href:"${rootURL}/manage", title:_("Manage Jenkins"), permission:app.ADMINISTER, it:app) + l.task(icon:"icon-gear2 icon-md", href:"${rootURL}/manage", title:_("Manage Jenkins"), permission:app.MANAGE, it:app) if (!app.updateCenter.jobs.isEmpty()) { l.task(icon:"icon-plugin icon-md", href:"${rootURL}/updateCenter/", title:_("Update Center")) } diff --git a/core/src/main/resources/hudson/model/ComputerSet/sidepanel.jelly b/core/src/main/resources/hudson/model/ComputerSet/sidepanel.jelly index 1a9a257225..919e67e648 100644 --- a/core/src/main/resources/hudson/model/ComputerSet/sidepanel.jelly +++ b/core/src/main/resources/hudson/model/ComputerSet/sidepanel.jelly @@ -32,7 +32,7 @@ THE SOFTWARE. - + diff --git a/core/src/main/resources/hudson/model/Messages.properties b/core/src/main/resources/hudson/model/Messages.properties index 8807077497..5e1553e12a 100644 --- a/core/src/main/resources/hudson/model/Messages.properties +++ b/core/src/main/resources/hudson/model/Messages.properties @@ -163,6 +163,10 @@ Hudson.AdministerPermission.Description=\ This permission grants the ability to make system-wide configuration changes, \ as well as perform highly sensitive operations that amounts to full local system access \ (within the scope granted by the underlying OS.) +Jenkins.Manage.Description=\ + This permission grants the ability to make non Permission escalating system \ + configuration changes. \ + System configuration which would allow the execution of arbitrary commands or code on the master requires Administer. Hudson.ReadPermission.Description=\ The read permission is necessary for viewing almost all pages of Jenkins. \ This permission is useful when you don\u2019t want unauthenticated users to see \ diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly index 8d864d01e9..6cc9f44569 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/sidepanel.jelly @@ -28,7 +28,7 @@ THE SOFTWARE. - + diff --git a/core/src/main/resources/jenkins/model/Jenkins/configure.jelly b/core/src/main/resources/jenkins/model/Jenkins/configure.jelly index a85ab58ade..6c6cfe895e 100644 --- a/core/src/main/resources/jenkins/model/Jenkins/configure.jelly +++ b/core/src/main/resources/jenkins/model/Jenkins/configure.jelly @@ -27,7 +27,7 @@ THE SOFTWARE. --> - + diff --git a/core/src/main/resources/jenkins/model/Jenkins/manage.jelly b/core/src/main/resources/jenkins/model/Jenkins/manage.jelly index d9dd6de40d..16a54119f6 100644 --- a/core/src/main/resources/jenkins/model/Jenkins/manage.jelly +++ b/core/src/main/resources/jenkins/model/Jenkins/manage.jelly @@ -27,7 +27,7 @@ THE SOFTWARE. --> - + From 3b2d0d2c78fefc1fd9428a55ee7a112abd28fe51 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Fri, 14 Feb 2020 19:59:03 +0100 Subject: [PATCH 23/54] As this is experimental, annotate with Restricted(Beta) --- core/src/main/java/jenkins/model/Jenkins.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 9a44b86e6f..cf914824d9 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -225,6 +225,7 @@ import org.jvnet.hudson.reactor.TaskBuilder; import org.jvnet.hudson.reactor.TaskGraphBuilder; import org.jvnet.hudson.reactor.TaskGraphBuilder.Handle; import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.Beta; import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.args4j.Argument; import org.kohsuke.stapler.HttpRedirect; @@ -5251,6 +5252,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve * As an experimental feature, making the manage permission able to be disabled by default (keep as ADMINISTER), can * be enabled with "jenkins.security.ManagePermission" system property. */ + @Restricted(Beta.class) public static final Permission MANAGE = new Permission(PERMISSIONS, "Manage", Messages._Jenkins_Manage_Description(), ADMINISTER, From a1d097f30c9d2f18c8d5a63bf7f95c4a9605266b Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Fri, 14 Feb 2020 21:23:29 +0100 Subject: [PATCH 24/54] Add test back Co-Authored-By: Esther Alvarez Feijoo Co-Authored-By: A. Jard Co-Authored-By: michael cirioli --- .../model/JenkinsManagePermissionTest.java | 214 ++++++++++++++++++ 1 file changed, 214 insertions(+) create mode 100644 test/src/test/java/jenkins/model/JenkinsManagePermissionTest.java diff --git a/test/src/test/java/jenkins/model/JenkinsManagePermissionTest.java b/test/src/test/java/jenkins/model/JenkinsManagePermissionTest.java new file mode 100644 index 0000000000..f2dc14c217 --- /dev/null +++ b/test/src/test/java/jenkins/model/JenkinsManagePermissionTest.java @@ -0,0 +1,214 @@ +package jenkins.model; + +import java.net.HttpURLConnection; +import com.gargoylesoftware.htmlunit.WebResponse; +import com.gargoylesoftware.htmlunit.html.HtmlForm; +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.hamcrest.Matcher; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.MockAuthorizationStrategy; +import org.jvnet.hudson.test.recipes.WithPlugin; + +import hudson.PluginWrapper; +import hudson.cli.CLICommandInvoker; +import hudson.cli.DisablePluginCommand; +import hudson.model.Descriptor; +import hudson.model.MyView; +import hudson.model.View; +import hudson.model.labels.LabelAtom; +import hudson.tasks.Shell; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +import static hudson.cli.CLICommandInvoker.Matcher.failedWith; + +/** + * As Jenkins.MANAGE can be enabled on startup with jenkins.security.ManagePermission property, we need a test class + * with this property activated. + */ +public class JenkinsManagePermissionTest { + + @Rule + public JenkinsRule j = new JenkinsRule(); + + @BeforeClass //TODO: remove once Jenkins.MANAGE is no longer an experimental feature + public static void enableManagePermission() { + System.setProperty("jenkins.security.ManagePermission", "true"); + } + + @AfterClass //TODO: remove once Jenkins.MANAGE is no longer an experimental feature + public static void disableManagePermission() { + System.clearProperty("jenkins.security.ManagePermission"); + } + + + // ----------------------------- + //Moved from DisablePluginCommandTest + @Issue("JENKINS-60266") + @Test + @WithPlugin({ "depender-0.0.2.hpi", "dependee-0.0.2.hpi"}) + public void managerCanNotDisablePlugin() { + + //GIVEN a user with Jenkins.MANAGE permission + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.MANAGE).everywhere().to("manager") + ); + + //WHEN trying to disable a plugin + assertThat(disablePluginsCLiCommandAs("manager", "dependee"), failedWith(6)); + //THEN it's refused and the plugin is not disabled. + assertPluginEnabled("dependee"); + } + + /** + * Disable a list of plugins using the CLI command. + * @param user Username + * @param args Arguments to pass to the command. + * @return Result of the command. 0 if succeed, 16 if some plugin couldn't be disabled due to dependent plugins. + */ + private CLICommandInvoker.Result disablePluginsCLiCommandAs(String user, String... args) { + return new CLICommandInvoker(j, new DisablePluginCommand()).asUser(user).invokeWithArgs(args); + } + + + private void assertPluginEnabled(String name) { + PluginWrapper plugin = j.getPluginManager().getPlugin(name); + assertThat(plugin, is(notNullValue())); + assertTrue(plugin.isEnabled()); + } + + // End of Moved from DisablePluginCommandTest + //------- + + // ----------------------------- + //Moved from ComputerTest + @Issue("JENKINS-60266") + @Test + public void dumpExportTableForbiddenWithoutAdminPermission() throws Exception { + final String READER = "reader"; + final String MANAGER = "manager"; + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.READ).everywhere().to(READER) + .grant(Jenkins.MANAGE).everywhere().to(MANAGER) + .grant(Jenkins.READ).everywhere().to(MANAGER) + ); + j.createWebClient().login(READER).assertFails("computer/(master)/dumpExportTable", HttpURLConnection.HTTP_FORBIDDEN); + j.createWebClient().login(MANAGER).assertFails("computer/(master)/dumpExportTable", HttpURLConnection.HTTP_FORBIDDEN); + } + + // End of Moved from ComputerTest + //------- + + // ----------------------------- + //Moved from HusdonTest + @Issue("JENKINS-60266") + @Test + public void someGlobalConfigurationIsNotDisplayedWithManagePermission() throws Exception { + //GIVEN a user with Jenkins.MANAGE permission + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.MANAGE, Jenkins.READ).everywhere().toEveryone()); + + //WHEN the user goes to /configure page + HtmlForm form = j.createWebClient().goTo("configure").getFormByName("config"); + String formText = form.asText(); + //THEN items restricted to ADMINISTER only should not be displayed. + assertThat("Should be able to configure system message", formText, not(containsString("systemMessage"))); + assertThat("Should be able to configure project naming strategy", formText, not(containsString("useProjectNamingStrategy"))); + assertThat("Shouldn't be able to configure primary view", formText, not(containsString("primaryView"))); + assertThat("Shouldn't be able to configure # of executors", formText, not(containsString("executors"))); + assertThat("Shouldn't be able to configure Global properties", formText, + not(containsString("Global properties"))); + assertThat("Shouldn't be able to configure Administrative monitors", formText, not(containsString( + "Administrative " + + "monitors"))); + assertThat("Shouldn't be able to configure Shell", formText, not(containsString("Shell"))); + } + + @Issue("JENKINS-60266") + @Test + public void someGlobalConfigCanNotBeModifiedWithManagePermission() throws Exception { + j.jenkins.addView(new MyView("testView", j.jenkins)); + + //GIVEN the Global Configuration Form, with some changes unsaved + int currentNumberExecutors = j.getInstance().getNumExecutors(); + String shell = getShell(); + View view = j.jenkins.getPrimaryView(); + HtmlForm form = j.createWebClient().goTo("configure").getFormByName("config"); + form.getInputByName("_.numExecutors").setValueAttribute(""+(currentNumberExecutors+1)); + form.getInputByName("_.shell").setValueAttribute("/fakeShell"); + form.getSelectByName("primaryView").setSelectedAttribute("testView", true); + + // WHEN a user with only Jenkins.MANAGE permission try to save those changes + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.MANAGE, Jenkins.READ).everywhere().toEveryone()); + j.submit(form); + // THEN the changes on fields forbidden to a Jenkins.MANAGE permission are not saved + assertEquals("shouldn't be allowed to change the number of executors", currentNumberExecutors, j.getInstance().getNumExecutors()); + assertEquals("shouldn't be allowed to change the shell executable", shell, getShell()); + assertEquals("shouldn't be allowed to change the primary view", view, j.getInstance().getPrimaryView()); + } + + @Issue("JENKINS-60266") + @Test + public void globalConfigAllowedWithManagePermission() throws Exception { + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.MANAGE, Jenkins.READ).everywhere().toEveryone()); + + HtmlForm form = j.createWebClient().goTo("configure").getFormByName("config"); + HtmlPage updated = j.submit(form); + assertThat("User with Jenkins.MANAGE permission should be able to update global configuration", + updated.getWebResponse(), hasResponseCode(HttpURLConnection.HTTP_OK)); + } + + private String getShell() { + Descriptor descriptorByName = j.getInstance().getDescriptorByName("hudson.tasks.Shell"); + return ((Shell.DescriptorImpl) descriptorByName).getShell(); + } + + private static Matcher hasResponseCode(final int httpStatus) { + return new BaseMatcher() { + @Override + public boolean matches(final Object item) { + final WebResponse response = (WebResponse) item; + return (response.getStatusCode() == httpStatus); + } + + @Override + public void describeTo(final Description description) { + description.appendText("Jenkins to return ").appendValue(httpStatus); + } + + @Override + public void describeMismatch(Object item, Description description) { + WebResponse response = (WebResponse) item; + description.appendText("Response code was: "); + description.appendValue(response.getStatusCode()); + description.appendText(" with error message: "); + description.appendText(response.getStatusMessage()); + description.appendText("\n with headers ").appendValueList("", "\n ", "", response.getResponseHeaders()); + description.appendText("\nPage content: ").appendValue(response.getContentAsString()); + } + }; + } + + // End of Moved from HusdonTest + //------- +} \ No newline at end of file From 17f1a7c15543ffae19afe7541494898ac3fdb972 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Fri, 14 Feb 2020 21:31:28 +0100 Subject: [PATCH 25/54] Fix Javadoc --- core/src/main/java/hudson/model/Descriptor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/hudson/model/Descriptor.java b/core/src/main/java/hudson/model/Descriptor.java index 3a055d8119..4aa8ae7ab9 100644 --- a/core/src/main/java/hudson/model/Descriptor.java +++ b/core/src/main/java/hudson/model/Descriptor.java @@ -834,8 +834,8 @@ public abstract class Descriptor> implements Saveable, /** * Returns the permission type needed in order to configure the descriptor if and only if it is configured through the global (Unclassified) configuration. - * By default, requires {@link Jenkins.ADMINISTER} permission. - * Override to return something different if appropriate. The only currently supported alternative return value is {@link Jenkins.MANAGE}. + * By default, requires {@link Jenkins#ADMINISTER} permission. + * Override to return something different if appropriate. The only currently supported alternative return value is {@link Jenkins#MANAGE}. * * @return Permission required to configure this descriptor. */ From 311fe0a3ef5310bef31a81b163902f7534ffc5e9 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Fri, 14 Feb 2020 21:37:15 +0100 Subject: [PATCH 26/54] Only show the link to the cloud configuration to admins --- core/src/main/resources/jenkins/model/Jenkins/configure.jelly | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/main/resources/jenkins/model/Jenkins/configure.jelly b/core/src/main/resources/jenkins/model/Jenkins/configure.jelly index 6c6cfe895e..97df594a8a 100644 --- a/core/src/main/resources/jenkins/model/Jenkins/configure.jelly +++ b/core/src/main/resources/jenkins/model/Jenkins/configure.jelly @@ -52,7 +52,9 @@ THE SOFTWARE. - + + + From 042dc75a8f650523916ca789e938cf2b01ed365b Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Fri, 14 Feb 2020 21:55:18 +0100 Subject: [PATCH 27/54] For consistency with ManagementLink, name this #getRequiredPermission() --- core/src/main/java/hudson/Functions.java | 2 +- core/src/main/java/hudson/model/Descriptor.java | 2 +- .../jenkins/model/GlobalProjectNamingStrategyConfiguration.java | 2 +- .../main/java/jenkins/model/GlobalQuietPeriodConfiguration.java | 2 +- .../java/jenkins/model/GlobalSCMRetryCountConfiguration.java | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java index 62208aaf93..9e1dbe61b2 100644 --- a/core/src/main/java/hudson/Functions.java +++ b/core/src/main/java/hudson/Functions.java @@ -1062,7 +1062,7 @@ public class Functions { Descriptor d = c.getInstance(); if (d.getGlobalConfigPage()==null) continue; - if (!Jenkins.get().hasPermission(d.getPermission())) { + if (!Jenkins.get().hasPermission(d.getRequiredPermission())) { continue; } diff --git a/core/src/main/java/hudson/model/Descriptor.java b/core/src/main/java/hudson/model/Descriptor.java index 4aa8ae7ab9..f1e17749d1 100644 --- a/core/src/main/java/hudson/model/Descriptor.java +++ b/core/src/main/java/hudson/model/Descriptor.java @@ -840,7 +840,7 @@ public abstract class Descriptor> implements Saveable, * @return Permission required to configure this descriptor. */ public @Nonnull - Permission getPermission() { + Permission getRequiredPermission() { return Jenkins.ADMINISTER; } diff --git a/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java b/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java index 4f046291f2..0385d9dfc2 100644 --- a/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java +++ b/core/src/main/java/jenkins/model/GlobalProjectNamingStrategyConfiguration.java @@ -65,7 +65,7 @@ public class GlobalProjectNamingStrategyConfiguration extends GlobalConfiguratio @Nonnull @Override - public Permission getPermission() { + public Permission getRequiredPermission() { return Jenkins.MANAGE; } } diff --git a/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java b/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java index 93495cd384..915d04162f 100644 --- a/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java +++ b/core/src/main/java/jenkins/model/GlobalQuietPeriodConfiguration.java @@ -62,7 +62,7 @@ public class GlobalQuietPeriodConfiguration extends GlobalConfiguration { @Nonnull @Override - public Permission getPermission() { + public Permission getRequiredPermission() { return Jenkins.MANAGE; } } diff --git a/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java b/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java index 373eb62828..bd2e97f677 100644 --- a/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java +++ b/core/src/main/java/jenkins/model/GlobalSCMRetryCountConfiguration.java @@ -59,7 +59,7 @@ public class GlobalSCMRetryCountConfiguration extends GlobalConfiguration { @Nonnull @Override - public Permission getPermission() { + public Permission getRequiredPermission() { return Jenkins.MANAGE; } } From 58a3f01c1ec14d58aa65017e4459f4ad36d9c0e1 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 15 Feb 2020 02:43:40 +0100 Subject: [PATCH 28/54] Address review feedback --- .../resources/hudson/model/Messages.properties | 5 ++--- .../model/JenkinsManagePermissionTest.java | 17 +++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/core/src/main/resources/hudson/model/Messages.properties b/core/src/main/resources/hudson/model/Messages.properties index 5e1553e12a..b1883a14a8 100644 --- a/core/src/main/resources/hudson/model/Messages.properties +++ b/core/src/main/resources/hudson/model/Messages.properties @@ -164,9 +164,8 @@ Hudson.AdministerPermission.Description=\ as well as perform highly sensitive operations that amounts to full local system access \ (within the scope granted by the underlying OS.) Jenkins.Manage.Description=\ - This permission grants the ability to make non Permission escalating system \ - configuration changes. \ - System configuration which would allow the execution of arbitrary commands or code on the master requires Administer. + This permission grants the ability to configure parts of the overall system configuration \ + that are not expected to impact the overall stability and security of the system. Hudson.ReadPermission.Description=\ The read permission is necessary for viewing almost all pages of Jenkins. \ This permission is useful when you don\u2019t want unauthenticated users to see \ diff --git a/test/src/test/java/jenkins/model/JenkinsManagePermissionTest.java b/test/src/test/java/jenkins/model/JenkinsManagePermissionTest.java index f2dc14c217..e9e818a430 100644 --- a/test/src/test/java/jenkins/model/JenkinsManagePermissionTest.java +++ b/test/src/test/java/jenkins/model/JenkinsManagePermissionTest.java @@ -39,24 +39,25 @@ import static hudson.cli.CLICommandInvoker.Matcher.failedWith; * As Jenkins.MANAGE can be enabled on startup with jenkins.security.ManagePermission property, we need a test class * with this property activated. */ +// TODO move tests to indicated test classes when we no longer need to set the system property public class JenkinsManagePermissionTest { @Rule public JenkinsRule j = new JenkinsRule(); - @BeforeClass //TODO: remove once Jenkins.MANAGE is no longer an experimental feature + @BeforeClass public static void enableManagePermission() { System.setProperty("jenkins.security.ManagePermission", "true"); } - @AfterClass //TODO: remove once Jenkins.MANAGE is no longer an experimental feature + @AfterClass public static void disableManagePermission() { System.clearProperty("jenkins.security.ManagePermission"); } // ----------------------------- - //Moved from DisablePluginCommandTest + // DisablePluginCommandTest @Issue("JENKINS-60266") @Test @WithPlugin({ "depender-0.0.2.hpi", "dependee-0.0.2.hpi"}) @@ -91,11 +92,11 @@ public class JenkinsManagePermissionTest { assertTrue(plugin.isEnabled()); } - // End of Moved from DisablePluginCommandTest + // End of DisablePluginCommandTest //------- // ----------------------------- - //Moved from ComputerTest + //ComputerTest @Issue("JENKINS-60266") @Test public void dumpExportTableForbiddenWithoutAdminPermission() throws Exception { @@ -111,11 +112,11 @@ public class JenkinsManagePermissionTest { j.createWebClient().login(MANAGER).assertFails("computer/(master)/dumpExportTable", HttpURLConnection.HTTP_FORBIDDEN); } - // End of Moved from ComputerTest + // End of ComputerTest //------- // ----------------------------- - //Moved from HusdonTest + // HusdonTest @Issue("JENKINS-60266") @Test public void someGlobalConfigurationIsNotDisplayedWithManagePermission() throws Exception { @@ -209,6 +210,6 @@ public class JenkinsManagePermissionTest { }; } - // End of Moved from HusdonTest + // End of HusdonTest //------- } \ No newline at end of file From 32a51374ce7beadac88ab1a6fce5a660510223ba Mon Sep 17 00:00:00 2001 From: testfixer <60407054+testfixer@users.noreply.github.com> Date: Sat, 15 Feb 2020 05:37:21 -0600 Subject: [PATCH 29/54] [JENKINS-61102] Use XMLUnit to fix test (#4502) --- .../test/java/jenkins/util/xstream/XStreamDOMTest.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java b/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java index 3acf389a42..a53dff38e2 100644 --- a/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java +++ b/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java @@ -27,8 +27,11 @@ import hudson.util.XStream2; import org.apache.commons.io.IOUtils; import org.junit.Before; import org.junit.Test; +import org.xmlunit.diff.DefaultNodeMatcher; +import org.xmlunit.diff.ElementSelectors; import static org.junit.Assert.*; +import static org.xmlunit.matchers.CompareMatcher.isSimilarTo; import java.io.IOException; import java.io.InputStream; @@ -62,7 +65,8 @@ public class XStreamDOMTest { Foo foo = createSomeFoo(); String xml = xs.toXML(foo); System.out.println(xml); - assertEquals(getTestData1().trim(), xml.trim()); + assertThat(getTestData1().trim(), isSimilarTo(xml.trim()).ignoreWhitespace() + .withNodeMatcher(new DefaultNodeMatcher(ElementSelectors.byNameAndText))); } private String getTestData1() throws IOException { @@ -100,7 +104,8 @@ public class XStreamDOMTest { String xml = xs.toXML(foo); System.out.println(xml); - assertEquals(getTestData1().trim(), xml.trim()); + assertThat(getTestData1().trim(), isSimilarTo(xml.trim()).ignoreWhitespace() + .withNodeMatcher(new DefaultNodeMatcher(ElementSelectors.byNameAndText))); } @Test From 84f43e2fa4e38cba4baa16a490a760858c1abec6 Mon Sep 17 00:00:00 2001 From: Daniel Beck <1831569+daniel-beck@users.noreply.github.com> Date: Sat, 15 Feb 2020 12:39:57 +0100 Subject: [PATCH 30/54] Update script-security to 1.70 (#4490) --- war/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/war/pom.xml b/war/pom.xml index fa9d4986a7..1d33e2299e 100644 --- a/war/pom.xml +++ b/war/pom.xml @@ -360,7 +360,7 @@ THE SOFTWARE. org.jenkins-ci.plugins script-security - 1.68 + 1.70 hpi From 7ff5f097d0b6580045531c0aaa199b7482de9b7b Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Sat, 15 Feb 2020 06:51:03 -0500 Subject: [PATCH 31/54] [JENKINS-61071] Ensure that initialization tasks run as SYSTEM (#4491) --- .../src/main/java/jenkins/InitReactorRunner.java | 4 +++- test/src/test/java/hudson/PluginManagerTest.java | 12 ++++++++++++ .../plugins/require-system-in-initializer.jpi | Bin 0 -> 6094 bytes 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 test/src/test/resources/plugins/require-system-in-initializer.jpi diff --git a/core/src/main/java/jenkins/InitReactorRunner.java b/core/src/main/java/jenkins/InitReactorRunner.java index fd76f91214..be183b47e1 100644 --- a/core/src/main/java/jenkins/InitReactorRunner.java +++ b/core/src/main/java/jenkins/InitReactorRunner.java @@ -4,10 +4,12 @@ import com.google.common.collect.Lists; import jenkins.util.SystemProperties; import hudson.init.InitMilestone; import hudson.init.InitReactorListener; +import hudson.security.ACL; import hudson.util.DaemonThreadFactory; import hudson.util.NamingThreadFactory; import jenkins.model.Configuration; import jenkins.model.Jenkins; +import jenkins.security.ImpersonatingExecutorService; import org.jvnet.hudson.reactor.Milestone; import org.jvnet.hudson.reactor.Reactor; import org.jvnet.hudson.reactor.ReactorException; @@ -45,7 +47,7 @@ public class InitReactorRunner { else es = Executors.newSingleThreadExecutor(new NamingThreadFactory(new DaemonThreadFactory(), "InitReactorRunner")); try { - reactor.execute(es,buildReactorListener()); + reactor.execute(new ImpersonatingExecutorService(es, ACL.SYSTEM), buildReactorListener()); } finally { es.shutdownNow(); // upon a successful return the executor queue should be empty. Upon an exception, we want to cancel all pending tasks } diff --git a/test/src/test/java/hudson/PluginManagerTest.java b/test/src/test/java/hudson/PluginManagerTest.java index 57cd87d9f9..bb9ce486b8 100644 --- a/test/src/test/java/hudson/PluginManagerTest.java +++ b/test/src/test/java/hudson/PluginManagerTest.java @@ -503,6 +503,18 @@ public class PluginManagerTest { } } + @Issue("JENKINS-61071") + @Test + public void requireSystemInInitializer() throws Exception { + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()); + String pluginShortName = "require-system-in-initializer"; + dynamicLoad(pluginShortName + ".jpi"); + try (ACLContext context = ACL.as(User.getById("underprivileged", true).impersonate())) { + r.jenkins.pluginManager.start(Collections.singletonList(r.jenkins.pluginManager.getPlugin(pluginShortName))); + } + } + private void dynamicLoad(String plugin) throws IOException, InterruptedException, RestartRequiredException { PluginManagerUtil.dynamicLoad(plugin, r.jenkins); } diff --git a/test/src/test/resources/plugins/require-system-in-initializer.jpi b/test/src/test/resources/plugins/require-system-in-initializer.jpi new file mode 100644 index 0000000000000000000000000000000000000000..8c0aaf186cdf684b5279721818a996758e19ea76 GIT binary patch literal 6094 zcmb_gbySqw79Uc&J49jtX_OK{8l)Sf%b{nep+N~ji2-Q|$pH}*lx`_OO1hC88V97C z2lqa%_df5t>;3W0eDj@|wf1lCb@tw8@3ns#%4p~~fUET*RYl{sAOC(~U+$`MI#S$< zYVy1q{}#gnY+Z>FN=PT5002B_007ya#Z;x#6y@c#b$C?eVOpx{lafUB#bp;3(Yqfi z7F;tMF!GhMshEI^;T7vm>8;b>kk93g*K6B&o(1TzC@0Qzr+Pc3D`#yHr!60t)7$HQ zFo@N>Q(4kg#A~YArWV;vGRK zZ$VI&nnzAzYouZJn!M4)yhyV(nj!93xf@+bN9hhro}kH5aA(;zjgM`4*kX{}Ow+5TqdloG0-%|l zW>x|25;%bJD-P%s!`#(`r@~Qs+WoW)+(fep;~WrxXsX@dWbCg{oBY_G>(Yqp!dxqy zM&nQV1gPb?N*3D+jSNf0N>588@$hk1I!C<@Tl@JRG0>!78MZ+=KhszCg;7ezp239K)zGC)HEr z-3HaxSE1T3MBoU727F|It>pMecB^uYZ$qIc;iSCoC;GI;a#KB%+{0swIT#Z#8Nt{@k8J6905o3|Sj z1m?DbTt0SCI}1lUACN1Ly@l(0eQ%K=2io)TN`c4{Yhmh>BCEGAfdczLEfs3&Wa=(F ztM$@nFLJCX6+E;$k@1Vq;2nwF$|H_h8_L0l(BY%jEVl+XpW>O5c3+cTe~HnP2fZh? zGn0}@k!ci%VoRjG)PNbhl<(Mse46BgBZKjLhcxo|-TnkRgDoMw%qd@tOaALGqL=VP_xVshfzI&V-7&Yd>*a5Nl@T;Mio9cRO!-m zt~yt_x&uqp%8Vv-FE-f|;tej^rtM)?s>(3J-A$`bcdXkABWg@9_sY^OH-s*Hs5kR& zoaeNO^B#UtRx?;GILE?B+$RZH2>}k}FFfRBS%&lBve?4dS#iML-wCRnvtk{!zM!U( zvzrF5POmi;-cGlq+i*ZCsc=ySr#*2NdB`*>q1($XzHjgfG!efP!W{yLse4Odym9I; zsRd0>4W>{Cs?^_})ZWePnOtx*;kW(K z%QZi7Uk4v%f-kCy-veVRmNUg_$~P25Vpy}GX?jq7yl3f&v$T>i4BN=9 z*Bsv5LNgx0SlEi+IkgI=^t@ACjo(VequXmVetuDEjl3JZ*u+ffx-Ht`M%|Qn1`EPb z_n{^pkN+}fddRl(@uAJsi|4r*FT@n$Y|5jG78gt2q^e@7X-B1Xv`OY^b`O1~7D#Bq z8!_1H_(XhK@KEMUD>l(a@7a-$^=Me0`Ea^+gLR~Xv+#-KWUljZsjBz3kd;j3S3dMu zy}EVF0}IeI_eTlj!m?4~$j)g|y8)GVFupCqwuAEU8O6J(1B)>( zX@tF+SnCBz!s#R~@h2a1E@-F!BK#S~Ru>VvO*_K`d+a^8aoaHiE##VR`dgS(2@U3F zgKy*ntbA^)?#?G&+-mIN{qGT9hrF8JR)RN|NTOXlCq|hTm=lX&5OPJ{se{{n%0-=^ zvXQC;)7)AtUVf4?gtu_hsqYrpX_}il)!QNUC}!m@!*adD2P2Uq{pVYrxS#8a^J=UN^whuz8k1h3B z565*%6K8yqlCaozDBo0bQsm_LW&NhFac!*QJyweWr&!!N3nY5o`=y)VFN@~qKjP#V zW~4Ujs;RDWOVEhFsHTC&KdvvrryCL9cX_LYx0iIC{D< zB*@Pd$3bdJPzjv$4QU=w)iVo7CdrswyIqGzTl$jRRB9aiLp7NYdP+Y2yikvpUP_Mj z?TrDsp0d4m`Bi>BCR_J*Klzr0z4~@0T#8I-DdVCCdYFWx`l4h{O&GaUg^V&o;Pllq zlXlLkm67|UY`z%+q}^xtQiZ9%o^+Q7)y!J6wiJ-DJRM94SxQUtM%7`RNLj;q6U*?U z`J3&NZ~awvY8)X1Ae}I+Lc!104osfoal&V33CBusD5!-ZjSD9l7WM&~F(Nd*S*rr; zov>+yd>IrsX~dNCb*-j9ed7F^SGh8fw*9buM)lJ8QU)n&^J(4jx+AT*?tW^U9nLqh z_nyXP93i9vN~S+iB*l?w--j!nn7@EKd>uION<>0~Vx~lgkwIN@Zt?6+#cCIfDz--G zrc)?$D=V5hec6MuRW`1AD(dRHj%-|&XnFX9Ro2?n+R2mCMt!-dy$(P{3JN7-0X z6opLo=l#)k%E^s|gk*JC1+M}#5MX--@Ls&Q5Qrc=tw{3ISH;Pg#7M_e%jN`?P=kAutyy>mb5E)UPe#W^*Zcon7;}}VK^|o zvLQd__2Ra3#dZ`Qs7kt;}m2Y1e)tv8`U;wtdVOaj}Tu zoO`_0_`|TS(-{0VdnQ(&ZNL8GW2ZuA1FGjuGIC;cA*x8qcp3@E_l6zoU#(~}6dzwi zmo28s+CyU#c0M!(^Es!^u zq}*}4;}Q>Fw^dVEwqmf_rBD%4G;y!HyXGv=p^IkO@kWiacz&5O-DOB3xlMt%G2~4Z zjbQPyk4{q2H^Srx!t+o(J>~?Yn>+$DV!K+$Y@DNKrd{>j-c9`Vusa_)fjCT00Ef%;AYmn&q?B1Nn% zI$o2P-*t7arDPM}>rd6Gn_NGS0yF!u>*>o|8QrBq<@7 z&txLL5SMDoZn8J2d5u4!m)5afDJ=nqn1o>l#`&F3fS7AFR9(xeP!4Bfa z zc>-3#;nTBszgvfs!-xWeG7TSFAAUd5b{4OL-Qc==O#X~{<^4vF^MbKZF0>0*gR!@2 zOftVGh3DoHltdmxv3c*Ck@xQOcaJ$_&H=4LKlvGdhPcIS;tef>_5?s=iK~6R$Y9WY z!o5*TMC^0QAj@r%dfB;Y))p_xt(FSWz+K;A=!Q1mL_bGmB&|k~sFwv8n*NQAe~G|M zs-DsF*5g;XM>J3G7*3{QLDGy}<}}{bQGpdXHJRgq=~SEfbNppEpHYgbWCaK`$bVYWF;gohP|gK_ z?t=)ma=dhZaJTN9*xV`Xj`Yc0k53igsqCFUwqOLgLuAYS+!@2~QE}6!hL>q?d%dH^ ze>|b|M)klaYTGwjI+3{>>dO`rpqYG%a-f3VvklDdBb|D(m^d)ySZ))&Jnq|eG2L`@ z%;q!Lb0^7;gmj+~>xB+e59zI8D0g~CQmsPCKX109$ocu$vf7zvd%QVeX*uu3w2OVmb5TW6}c5*m1=; zOTMsOrtk>wl+^TG@N@9Y^$q-e-Z!qM6brgc*NDrn+W#qSon4)rL9S3ckQ;tj2Tn5| z8DR4$T(TG2K#js@Yu3X#kV5RuzCrp3yn!X;cn=dZ)S>{*>sFHa>{(|o1f{pY_pSJ@ zF>R4>`EYr;->AnFd(Idt8ULZYGH{}6f8=H{J?(A8k$?ad0{4{GKYk~^-ExV2ZctpS z8hs`bovDbwNgd#Nu-}8!B!dD9( z$aeYu1NJAD@u!AgtJf8!_4~RqjJzbJt_i7sLS0c@KT%|twdNZ2JK=SG(BA|5pOn`% z79RNEFUX$+*iYMkPX+zdV0+1~{@M0F*2ybk=%?ymopDw7e_vP57`goWhdZyk!1c*~ z1v6b^e+wZR(J$b??F#>0pX6Wk$^1|H{_*seUE-}i}YgYdwt|AV!Eca1+Az2ZOr p^OpZK`iS&@aoIl~@yh7WJFlUPaoGX^0Nl%u>t&+IUlFc=e*y8QSSbJi literal 0 HcmV?d00001 From 60b3ef12607ed9e1b5b8830a095d34e53d482ec0 Mon Sep 17 00:00:00 2001 From: Raihaan Shouhell Date: Mon, 17 Feb 2020 11:27:39 +0800 Subject: [PATCH 32/54] Use Streams --- core/src/main/java/hudson/model/Label.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/hudson/model/Label.java b/core/src/main/java/hudson/model/Label.java index 93749a5a24..f9ce39340d 100644 --- a/core/src/main/java/hudson/model/Label.java +++ b/core/src/main/java/hudson/model/Label.java @@ -66,6 +66,8 @@ import java.util.List; import java.util.Set; import java.util.Stack; import java.util.TreeSet; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import com.thoughtworks.xstream.converters.Converter; import com.thoughtworks.xstream.converters.MarshallingContext; @@ -388,12 +390,10 @@ public abstract class Label extends Actionable implements Comparable
-
- -
- + + + + - - - - -
diff --git a/core/src/main/resources/lib/layout/breadcrumbs.css b/core/src/main/resources/lib/layout/breadcrumbs.css index 26c4683a8a..30b65c2c42 100644 --- a/core/src/main/resources/lib/layout/breadcrumbs.css +++ b/core/src/main/resources/lib/layout/breadcrumbs.css @@ -1,70 +1,115 @@ #left-top-nav { - text-align: left; - color: #222; -} - -#right-top-nav { - float: right; - margin-right: 1em; - height: 2em; - line-height: 2em; + text-align: left; + color: #222; } #left-top-nav a { - color: black; + color: black; } #top-nav .a { - color: white; + color: white; +} + +.breadcrumbs__wrapper { + display: flex; + justify-content: space-between; + align-items: center; + min-height: 2.5rem; + font-size: 1rem; + line-height: 1.5; + font-family: Roboto, Helvetica, Arial, sans-serif; + padding: 0 1.25rem; + background-color: #F8F8F8; +} + +/* + Revert the font change to the options menu. + It's used all over the application and having Roboto may make it clash with other UI elements +*/ +#breadcrumb-menu { + font-family: Helvetica, Arial, sans-serif; } #breadcrumbs { list-style-type: none; - padding: 0 0 0 8px; margin: 0; - height: 2em; - border-bottom: 1px solid #d3d7cf; - display:block; + padding: 0; + min-height: 2.5rem; + display: block; + display: flex; + align-items: center; + flex: 1; + border-bottom: none; + flex-wrap: wrap; } #breadcrumbs LI { - float:left; - line-height:2em; - height: 2em; - color:#555753; display:inline-block; + min-height: 2.5rem; } -#breadcrumbs LI A { - display: block; - padding-left: 0.75em; - line-height: 2em; +#breadcrumbs LI A, +.breadcrumbs__auto-refresh { + display: inline-block; + display: inline-flex; + align-items: center; + height: 2.5rem; + padding: 0 0.5rem; } #breadcrumbs LI:hover, A.breadcrumbBarAnchor.mouseIsOverMenuSelector { background-color: #e0e4dc; } -#breadcrumbs LI A:link, #breadcrumbs LI A:visited { - text-decoration:none; - color:#555753; - display: block; + +#breadcrumbs LI A, #breadcrumbs LI A:link, #breadcrumbs LI A:visited, +.breadcrumbs__auto-refresh:link, .breadcrumbs__auto-refresh:visited { + /* is this still needed? */ max-width: 1330px; - float: left; + text-overflow: ellipsis; + text-decoration: none; overflow: hidden; white-space: nowrap; + font-weight: bold; + + display: inline-block; + display: inline-flex; + color: #4d545d; } -#breadcrumbs LI A:hover, #crumbs LI A:focus { - color:#3465a4; +#breadcrumbs LI A:hover, #breadcrumbs LI A:focus, +.breadcrumbs__auto-refresh:hover, .breadcrumbs__auto-refresh:focus { + /* TODO: change hover state + add it in the LI element so that it applies to the menuSeparator */ + text-decoration: underline; + outline-color: #3FB3F7; +} + +#breadcrumbs LI A:active, .breadcrumbs__auto-refresh:active { + background-color: #C8CEC2; +} + +#breadcrumbs LI A { + cursor: pointer; +} + +.breadcrumbs__auto-refresh { + text-transform: lowercase; + margin-left: 1.5rem; } #breadcrumbs LI.children, #breadcrumbs LI.separator {/* '>' separator between two items */ - width: 16px; + width: 1.25rem; + height: 100%; + display: inline-block; + display: inline-flex; + align-items: center; background-image: url(menu_right_arrow2.png); background-position: center center; background-repeat: no-repeat; + } #breadcrumbs LI.children { @@ -80,21 +125,21 @@ } #menuSelector {/* used for showing 'v' on the right of the anchor */ - background-color:transparent; - background-image: url(menu_down_arrow.png); - background-position: center center; - background-repeat: no-repeat; - width: 15px; - height:16px; - position: absolute; - visibility: hidden; - cursor: pointer; - z-index: 2000; + background-color:transparent; + background-image: url(menu_down_arrow.png); + background-position: center center; + background-repeat: no-repeat; + width: 15px; + height:16px; + position: absolute; + visibility: hidden; + cursor: pointer; + z-index: 2000; } #menuSelector.inverse { - background-image: url(menu_down_arrow2.png); + background-image: url(menu_down_arrow2.png); } A.model-link.inside, #breadcrumbs A.inside {/* additional 'inside' class allows pre-allocation of the context menu space */ - padding-right: 16px; + padding-right: 16px; } diff --git a/core/src/main/resources/lib/layout/html.jelly b/core/src/main/resources/lib/layout/html.jelly index 0a56c939ad..5fa717b347 100644 --- a/core/src/main/resources/lib/layout/html.jelly +++ b/core/src/main/resources/lib/layout/html.jelly @@ -80,7 +80,7 @@ THE SOFTWARE. - + ${h.checkPermission(it,permission)} diff --git a/core/src/main/resources/lib/layout/layout.jelly b/core/src/main/resources/lib/layout/layout.jelly index cac4b0d893..3cb5787cc4 100644 --- a/core/src/main/resources/lib/layout/layout.jelly +++ b/core/src/main/resources/lib/layout/layout.jelly @@ -92,7 +92,7 @@ THE SOFTWARE. - + ${h.checkPermission(it,permission)} @@ -101,8 +101,8 @@ THE SOFTWARE. ${h.appendIfNotNull(title, ' [Jenkins]', 'Jenkins')} - - + + @@ -156,6 +156,9 @@ THE SOFTWARE. + + + @@ -185,60 +188,12 @@ THE SOFTWARE.
-