Repository: karaf Updated Branches: refs/heads/karaf-2.x 8cb7d02f0 -> fcc3e44c5
[KARAF-4348] Correct ordering of wildcard PID for RBAC (cherry picked from commit 3e83c1d45a6dc9c4a06ba4efed47e2de2ec36cd2) Project: http://git-wip-us.apache.org/repos/asf/karaf/repo Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/fcc3e44c Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/fcc3e44c Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/fcc3e44c Branch: refs/heads/karaf-2.x Commit: fcc3e44c565d62eb8e9e6dbbfd6589f132a0863b Parents: 8cb7d02 Author: Grzegorz Grzybek <[email protected]> Authored: Thu Feb 18 18:28:29 2016 +0100 Committer: Grzegorz Grzybek <[email protected]> Committed: Thu Feb 18 18:33:53 2016 +0100 ---------------------------------------------------------------------- .../karaf/management/KarafMBeanServerGuard.java | 63 +++++++++++++++- .../management/KarafMBeanServerGuardTest.java | 77 ++++++++++++++++++++ 2 files changed, 136 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/karaf/blob/fcc3e44c/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java ---------------------------------------------------------------------- diff --git a/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java b/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java index d17d3d5..884f8f6 100644 --- a/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java +++ b/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java @@ -23,9 +23,14 @@ import java.security.AccessControlContext; import java.security.AccessController; import java.security.Principal; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.Enumeration; +import java.util.Iterator; import java.util.List; +import java.util.Set; +import java.util.TreeSet; import java.util.regex.Pattern; import javax.management.Attribute; @@ -60,6 +65,8 @@ public class KarafMBeanServerGuard implements InvocationHandler { private static final String JMX_OBJECTNAME_PROPERTY_WILDCARD = "_"; + private static final Comparator<String[]> WILDCARD_PID_COMPARATOR = new WildcardPidComparator(); + private ConfigurationAdmin configAdmin; public ConfigurationAdmin getConfigAdmin() { @@ -326,6 +333,7 @@ public class KarafMBeanServerGuard implements InvocationHandler { private String getGeneralPid(List<String> allPids, String pid) { String ret = ""; String[] pidStrArray = pid.split(Pattern.quote(".")); + Set<String[]> rets = new TreeSet<String[]>(WILDCARD_PID_COMPARATOR); for (String id : allPids) { String[] idStrArray = id.split(Pattern.quote(".")); if (idStrArray.length == pidStrArray.length) { @@ -340,13 +348,24 @@ public class KarafMBeanServerGuard implements InvocationHandler { } } if (match) { - ret = id; - return ret; + rets.add(idStrArray); } } } - - return ret; + + Iterator<String[]> it = rets.iterator(); + if (!it.hasNext()) { + return ""; + } else { + StringBuilder buffer = new StringBuilder(); + for (String segment : it.next()) { + if (buffer.length() > 0) { + buffer.append("."); + } + buffer.append(segment); + } + return buffer.toString(); + } } private List<String> getNameSegments(ObjectName objectName) { @@ -433,4 +452,40 @@ public class KarafMBeanServerGuard implements InvocationHandler { return false; } + /** + * <code>nulls</code>-last comparator of PIDs split to segments. {@link #JMX_OBJECTNAME_PROPERTY_WILDCARD} + * in a segment makes the PID more generic, thus - with lower prioroty. + */ + private static class WildcardPidComparator implements Comparator<String[]> { + @Override + public int compare(String[] o1, String[] o2) { + if (o1 == null && o2 == null) { + return 0; + } + if (o1 == null) { + return 1; + } + if (o2 == null) { + return -1; + } + if (o1.length != o2.length) { + // not necessary - not called with PIDs of different segment count + return o1.length - o2.length; + } + for (int n = 0; n < o1.length; n++) { + if (o1[n].equals(o2[n])) { + continue; + } + if (o1[n].equals(JMX_OBJECTNAME_PROPERTY_WILDCARD)) { + return 1; + } + if (o2[n].equals(JMX_OBJECTNAME_PROPERTY_WILDCARD)) { + return -1; + } + return o1[n].compareTo(o2[n]); + } + return 0; + } + } + } http://git-wip-us.apache.org/repos/asf/karaf/blob/fcc3e44c/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java ---------------------------------------------------------------------- diff --git a/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java b/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java index d153ffc..6a24144 100644 --- a/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java +++ b/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java @@ -291,6 +291,83 @@ public class KarafMBeanServerGuardTest extends TestCase { guard.getRequiredRoles(on, "zar", new Object[]{}, new String[]{})); } + @SuppressWarnings("unchecked") + public void testRequiredRolesHierarchyWildcard1() throws Exception { + Dictionary<String, Object> conf1 = new Hashtable<String, Object>(); + conf1.put("foo", "viewer"); + conf1.put(Constants.SERVICE_PID, "jmx.acl._.bar.Test"); + Dictionary<String, Object> conf2 = new Hashtable<String, Object>(); + conf2.put("foo", "editor"); + conf2.put(Constants.SERVICE_PID, "jmx.acl.foo.bar.Test"); + + ConfigurationAdmin ca = getMockConfigAdmin2(conf1, conf2); + assertEquals("Precondition", 2, ca.listConfigurations("(service.pid=jmx.acl*)").length); + + KarafMBeanServerGuard guard = new KarafMBeanServerGuard(); + guard.setConfigAdmin(ca); + + ObjectName on1 = ObjectName.getInstance("foo.bar:type=Test"); + assertEquals("Should only return the most specific definition", + Collections.singletonList("editor"), + guard.getRequiredRoles(on1, "foo", new Object[]{}, new String[]{})); + ObjectName on2 = ObjectName.getInstance("tar.bar:type=Test"); + assertEquals("Should return definition from wildcard PID", + Collections.singletonList("viewer"), + guard.getRequiredRoles(on2, "foo", new Object[]{}, new String[]{})); + } + + @SuppressWarnings("unchecked") + public void testRequiredRolesHierarchyWildcard2() throws Exception { + Dictionary<String, Object> conf1 = new Hashtable<String, Object>(); + conf1.put("foo", "viewer"); + conf1.put(Constants.SERVICE_PID, "jmx.acl.foo.bar.Test"); + Dictionary<String, Object> conf2 = new Hashtable<String, Object>(); + conf2.put("foo", "editor"); + conf2.put(Constants.SERVICE_PID, "jmx.acl._.bar.Test"); + + ConfigurationAdmin ca = getMockConfigAdmin2(conf1, conf2); + assertEquals("Precondition", 2, ca.listConfigurations("(service.pid=jmx.acl*)").length); + + KarafMBeanServerGuard guard = new KarafMBeanServerGuard(); + guard.setConfigAdmin(ca); + + ObjectName on1 = ObjectName.getInstance("foo.bar:type=Test"); + assertEquals("Should only return the most specific definition", + Collections.singletonList("viewer"), + guard.getRequiredRoles(on1, "foo", new Object[]{}, new String[]{})); + ObjectName on2 = ObjectName.getInstance("tar.bar:type=Test"); + assertEquals("Should return definition from wildcard PID", + Collections.singletonList("editor"), + guard.getRequiredRoles(on2, "foo", new Object[]{}, new String[]{})); + } + + @SuppressWarnings("unchecked") + public void testRequiredRolesHierarchyWildcard3() throws Exception { + Dictionary<String, Object> conf1 = new Hashtable<String, Object>(); + conf1.put("foo", "viewer"); + conf1.put(Constants.SERVICE_PID, "jmx.acl._.bar.Test"); + Dictionary<String, Object> conf2 = new Hashtable<String, Object>(); + conf2.put("foo", "editor"); + conf2.put(Constants.SERVICE_PID, "jmx.acl.foo._.Test"); + + ConfigurationAdmin ca = getMockConfigAdmin2(conf1, conf2); + assertEquals("Precondition", 2, ca.listConfigurations("(service.pid=jmx.acl*)").length); + + KarafMBeanServerGuard guard = new KarafMBeanServerGuard(); + guard.setConfigAdmin(ca); + + ObjectName on1 = ObjectName.getInstance("foo.bar:type=Test"); + assertEquals("Should only return the most specific definition", + Collections.singletonList("editor"), + guard.getRequiredRoles(on1, "foo", new Object[]{}, new String[]{})); + ObjectName on2 = ObjectName.getInstance("foo.tar:type=Test"); + assertEquals(Collections.singletonList("editor"), + guard.getRequiredRoles(on2, "foo", new Object[]{}, new String[]{})); + ObjectName on3 = ObjectName.getInstance("boo.bar:type=Test"); + assertEquals(Collections.singletonList("viewer"), + guard.getRequiredRoles(on3, "foo", new Object[]{}, new String[]{})); + } + public void testRequiredRolesMethodNameWildcard() throws Exception { Dictionary<String, Object> configuration = new Hashtable<String, Object>(); configuration.put("getFoo", "viewer");
