From 201b035cfccf3c0292faff3492acd008b5f2dace Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Wed, 24 Sep 2014 09:29:34 -0700 Subject: [PATCH] Updated to reflect the signature change around role checking in remoting --- core/src/main/java/hudson/FilePath.java | 20 +++++++++---------- .../security/CallableDirectionChecker.java | 14 ++++++++----- .../security/MasterToSlaveCallable.java | 5 +++-- .../security/SlaveToMasterCallable.java | 5 +++-- .../security/DefaultFilePathFilterTest.java | 9 +++++---- 5 files changed, 29 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index ed911a4a0a..88c7a1e016 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -67,7 +67,7 @@ import org.apache.tools.ant.types.FileSet; import org.apache.tools.tar.TarEntry; import org.apache.tools.zip.ZipEntry; import org.apache.tools.zip.ZipFile; -import org.jenkinsci.remoting.Role; +import org.jenkinsci.remoting.RoleChecker; import org.jenkinsci.remoting.RoleSensitive; import org.kohsuke.stapler.Stapler; @@ -95,8 +95,6 @@ import java.net.URL; import java.net.URLConnection; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; import java.util.Comparator; import java.util.Enumeration; import java.util.List; @@ -912,8 +910,8 @@ public final class FilePath implements Serializable { */ public static abstract class MasterToSlaveFileCallable implements FileCallable { @Override - public Collection getRecipients() { - return Roles.FOR_SLAVE; + public void checkRoles(RoleChecker checker) throws SecurityException { + checker.check(this,Roles.SLAVE); } private static final long serialVersionUID = 1L; } @@ -923,8 +921,8 @@ public final class FilePath implements Serializable { */ public static abstract class SlaveToMasterFileCallable implements FileCallable { @Override - public Collection getRecipients() { - return Roles.FOR_MASTER; + public void checkRoles(RoleChecker checker) throws SecurityException { + checker.check(this,Roles.MASTER); } private static final long serialVersionUID = 1L; } @@ -1085,8 +1083,8 @@ public final class FilePath implements Serializable { } @Override - public Collection getRecipients() { - return task.getRecipients(); + public void checkRoles(RoleChecker checker) throws SecurityException { + task.checkRoles(checker); } private static final long serialVersionUID = 1L; @@ -2537,8 +2535,8 @@ public final class FilePath implements Serializable { * Role check comes from {@link FileCallable}s. */ @Override - public Collection getRecipients() { - return callable.getRecipients(); + public void checkRoles(RoleChecker checker) throws SecurityException { + callable.checkRoles(checker); } public ClassLoader getClassLoader() { diff --git a/core/src/main/java/jenkins/security/CallableDirectionChecker.java b/core/src/main/java/jenkins/security/CallableDirectionChecker.java index 836cacecad..dff410cd44 100644 --- a/core/src/main/java/jenkins/security/CallableDirectionChecker.java +++ b/core/src/main/java/jenkins/security/CallableDirectionChecker.java @@ -48,7 +48,11 @@ public class CallableDirectionChecker extends RoleChecker { */ public static ConcurrentMap BYPASS_CALLABLES = new ConcurrentHashMap(); + /** + * Test feature that is meant to replace all logging, and log what would have been violations. + */ private static final PrintWriter BYPASS_LOG; + static { String log = System.getProperty(CallableDirectionChecker.class.getName()+".log"); if (log == null) { @@ -65,16 +69,16 @@ public class CallableDirectionChecker extends RoleChecker { private CallableDirectionChecker() {} @Override - public void check(RoleSensitive subject, @Nonnull Collection roles) throws SecurityException { - if (Roles.FOR_MASTER.containsAll(roles)) { + public void check(RoleSensitive subject, @Nonnull Collection expected) throws SecurityException { + final String name = subject.getClass().getName(); + + if (expected.contains(Roles.MASTER)) { if (BYPASS_LOG == null) { - LOGGER.log(Level.FINE, "Executing {0} is allowed since it is targeted for the master role"); + LOGGER.log(Level.FINE, "Executing {0} is allowed since it is targeted for the master role", name); } return; // known to be safe } - final String name = subject.getClass().getName(); - if (BYPASS_LOG != null) { BYPASS_LOG.println(name); return; diff --git a/core/src/main/java/jenkins/security/MasterToSlaveCallable.java b/core/src/main/java/jenkins/security/MasterToSlaveCallable.java index 1d498d290e..8be5b98305 100644 --- a/core/src/main/java/jenkins/security/MasterToSlaveCallable.java +++ b/core/src/main/java/jenkins/security/MasterToSlaveCallable.java @@ -2,6 +2,7 @@ package jenkins.security; import hudson.remoting.Callable; import org.jenkinsci.remoting.Role; +import org.jenkinsci.remoting.RoleChecker; import java.util.Collection; @@ -13,8 +14,8 @@ import java.util.Collection; */ public abstract class MasterToSlaveCallable implements Callable { @Override - public Collection getRecipients() { - return Roles.FOR_SLAVE; + public void checkRoles(RoleChecker checker) throws SecurityException { + checker.check(this,Roles.SLAVE); } private static final long serialVersionUID = 1L; diff --git a/core/src/main/java/jenkins/security/SlaveToMasterCallable.java b/core/src/main/java/jenkins/security/SlaveToMasterCallable.java index cb5fd1a082..61265cbdea 100644 --- a/core/src/main/java/jenkins/security/SlaveToMasterCallable.java +++ b/core/src/main/java/jenkins/security/SlaveToMasterCallable.java @@ -2,6 +2,7 @@ package jenkins.security; import hudson.remoting.Callable; import org.jenkinsci.remoting.Role; +import org.jenkinsci.remoting.RoleChecker; import java.util.Collection; @@ -13,8 +14,8 @@ import java.util.Collection; */ public abstract class SlaveToMasterCallable implements Callable { @Override - public Collection getRecipients() { - return Roles.FOR_MASTER; + public void checkRoles(RoleChecker checker) throws SecurityException { + checker.check(this,Roles.MASTER); } private static final long serialVersionUID = 1L; diff --git a/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java b/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java index 3d10d97a26..509bac52fd 100644 --- a/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java +++ b/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java @@ -31,6 +31,7 @@ import java.io.File; import java.util.Collection; import org.jenkinsci.remoting.Role; +import org.jenkinsci.remoting.RoleChecker; import org.junit.Test; import static org.junit.Assert.*; import org.junit.Rule; @@ -70,8 +71,8 @@ public class DefaultFilePathFilterTest { return p.readToString(); } @Override - public Collection getRecipients() { - return null; // simulate legacy Callable impls + public void checkRoles(RoleChecker checker) throws SecurityException { + throw new NoSuchMethodError(); // simulate legacy Callable impls } } @@ -86,8 +87,8 @@ public class DefaultFilePathFilterTest { return null; } @Override - public Collection getRecipients() { - return null; // simulate legacy Callable impls + public void checkRoles(RoleChecker checker) throws SecurityException { + throw new NoSuchMethodError(); // simulate legacy Callable impls } } }