https://issues.apache.org/jira/browse/AMQ-3749 - Composite destinations break simple authorisation through role aggregation. additional tests and fix, the union of the roles for composite destinations provides the auth list

git-svn-id: https://svn.apache.org/repos/asf/activemq/trunk@1295545 13f79535-47bb-0310-9956-ffa450edef68

activemq-core/src/main/java/org/apache/activemq/security/DefaultAuthorizationMap.java

@@ -124,6 +124,53 @@ public class DefaultAuthorizationMap extends DestinationMap implements Authoriza
         return answer;
     }
 
+
+    /**
+     * Looks up the value(s) matching the given Destination key. For simple
+     * destinations this is typically a List of one single value, for wildcards
+     * or composite destinations this will typically be a Union of matching
+     * values.
+     *
+     * @param key the destination to lookup
+     * @return a Union of matching values or an empty list if there are no
+     *         matching values.
+     */
+    @Override
+    @SuppressWarnings({ "rawtypes", "unchecked" })
+    public synchronized Set get(ActiveMQDestination key) {
+        if (key.isComposite()) {
+            ActiveMQDestination[] destinations = key.getCompositeDestinations();
+            Set answer = null;
+            for (int i = 0; i < destinations.length; i++) {
+                ActiveMQDestination childDestination = destinations[i];
+                answer = union(answer, get(childDestination));
+                if (answer == null  || answer.isEmpty()) {
+                    break;
+                }
+            }
+            return answer;
+        }
+        return findWildcardMatches(key);
+    }
+
+    private Set union(Set existing, Set candidates) {
+        if ( candidates != null ) {
+            if (existing != null) {
+                for (Iterator<Object> iterator = existing.iterator(); iterator.hasNext();) {
+                    Object toMatch = iterator.next();
+                    if (!candidates.contains(toMatch)) {
+                        iterator.remove();
+                    }
+                }
+            } else {
+                existing = candidates;
+            }
+        } else if ( existing != null ) {
+            existing.clear();
+        }
+        return existing;
+    }
+
     /**
      * Sets the individual entries on the authorization map
      *

activemq-core/src/test/java/org/apache/activemq/security/AuthorizationMapTest.java

@@ -45,6 +45,14 @@ public class AuthorizationMapTest extends TestCase {
 
     }
 
+    public void testCompositeDoesNotBypassAuthorizationMap() {
+        AuthorizationMap map = createAuthorizationMap();
+
+        Set<?> readACLs = map.getReadACLs(new ActiveMQQueue("USERS.FOO.BAR,DENIED"));
+        assertEquals("set size: " + readACLs, 1, readACLs.size());
+        assertTrue("Contains users group", readACLs.contains(ADMINS));
+    }
+
     public void testAuthorizationMapWithTempDest() {
         AuthorizationMap map = createAuthorizationMapWithTempDest();
 

activemq-core/src/test/java/org/apache/activemq/security/LDAPSecurityTest.java

@@ -34,6 +34,7 @@ import org.junit.runner.RunWith;
 import javax.jms.*;
 
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
 
 
 @RunWith( FrameworkRunner.class )
@@ -76,6 +77,38 @@ public class LDAPSecurityTest extends AbstractLdapTestUnit {
         assertNotNull(msg);
     }
 
+    @Test
+    public void testSendDenied() throws Exception {
+        ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory("tcp://localhost:61616");
+        Connection conn = factory.createQueueConnection("jdoe", "sunflower");
+        Session sess = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
+        conn.start();
+        Queue queue = sess.createQueue("ADMIN.FOO");
+
+        MessageProducer producer = sess.createProducer(queue);
+        try {
+            producer.send(sess.createTextMessage("test"));
+            fail("expect auth exception");
+        } catch (JMSException expected) {
+        }
+    }
+
+    @Test
+    public void testCompositeSendDenied() throws Exception {
+        ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory("tcp://localhost:61616");
+        Connection conn = factory.createQueueConnection("jdoe", "sunflower");
+        Session sess = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
+        conn.start();
+        Queue queue = sess.createQueue("TEST.FOO,ADMIN.FOO");
+
+        MessageProducer producer = sess.createProducer(queue);
+        try {
+            producer.send(sess.createTextMessage("test"));
+            fail("expect auth exception");
+        } catch (JMSException expected) {
+        }
+    }
+
     @Test
     public void testTempDestinations() throws Exception {
         ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory("tcp://localhost:61616");

activemq-core/src/test/java/org/apache/activemq/security/SecurityTestSupport.java

@@ -186,7 +186,7 @@ public class SecurityTestSupport extends JmsTestSupport {
     public void initCombosForTestUserReceiveFails() {
         addCombinationValues("userName", new Object[] {"user"});
         addCombinationValues("password", new Object[] {"password"});
-        addCombinationValues("destination", new Object[] {new ActiveMQQueue("TEST"), new ActiveMQTopic("TEST"), new ActiveMQQueue("GUEST.BAR"), new ActiveMQTopic("GUEST.BAR")});
+        addCombinationValues("destination", new Object[] {new ActiveMQQueue("USERS.BY_PASS, TEST"), new ActiveMQQueue("TEST"), new ActiveMQTopic("TEST"), new ActiveMQQueue("GUEST.BAR"), new ActiveMQTopic("GUEST.BAR")});
     }
 
     /**
@@ -221,7 +221,7 @@ public class SecurityTestSupport extends JmsTestSupport {
     public void initCombosForTestGuestReceiveFails() {
         addCombinationValues("userName", new Object[] {"guest"});
         addCombinationValues("password", new Object[] {"password"});
-        addCombinationValues("destination", new Object[] {new ActiveMQQueue("TEST"), new ActiveMQTopic("TEST"), new ActiveMQQueue("USERS.FOO"), new ActiveMQTopic("USERS.FOO") });
+        addCombinationValues("destination", new Object[] {new ActiveMQQueue("GUESTS.BY_PASS,TEST"), new ActiveMQQueue("TEST"), new ActiveMQTopic("TEST"), new ActiveMQQueue("USERS.FOO"), new ActiveMQTopic("USERS.FOO") });
     }
 
     /**
@@ -240,7 +240,7 @@ public class SecurityTestSupport extends JmsTestSupport {
     public void initCombosForTestUserSendFails() {
         addCombinationValues("userName", new Object[] {"user"});
         addCombinationValues("password", new Object[] {"password"});
-        addCombinationValues("destination", new Object[] {new ActiveMQQueue("TEST"), new ActiveMQTopic("TEST")});
+        addCombinationValues("destination", new Object[] {new ActiveMQQueue("USERS.BY_PASS,TEST"), new ActiveMQQueue("TEST"), new ActiveMQTopic("TEST")});
     }
 
     /**
@@ -249,7 +249,7 @@ public class SecurityTestSupport extends JmsTestSupport {
     public void initCombosForTestGuestSendFails() {
         addCombinationValues("userName", new Object[] {"guest"});
         addCombinationValues("password", new Object[] {"password"});
-        addCombinationValues("destination", new Object[] {new ActiveMQQueue("TEST"), new ActiveMQTopic("TEST"), new ActiveMQQueue("USERS.FOO"), new ActiveMQTopic("USERS.FOO")});
+        addCombinationValues("destination", new Object[] {new ActiveMQQueue("GUESTS.BY_PASS,TEST"), new ActiveMQQueue("TEST"), new ActiveMQTopic("TEST"), new ActiveMQQueue("USERS.FOO"), new ActiveMQTopic("USERS.FOO")});
     }
 
     /**

activemq-core/src/test/java/org/apache/activemq/security/SimpleSecurityBrokerSystemTest.java

@@ -73,7 +73,7 @@ public class SimpleSecurityBrokerSystemTest extends SecurityTestSupport {
     }
 
     public static AuthorizationMap createAuthorizationMap() {
-        DestinationMap readAccess = new DestinationMap();
+        DestinationMap readAccess = new DefaultAuthorizationMap();
         readAccess.put(new ActiveMQQueue(">"), ADMINS);
         readAccess.put(new ActiveMQQueue("USERS.>"), USERS);
         readAccess.put(new ActiveMQQueue("GUEST.>"), GUESTS);
@@ -81,7 +81,7 @@ public class SimpleSecurityBrokerSystemTest extends SecurityTestSupport {
         readAccess.put(new ActiveMQTopic("USERS.>"), USERS);
         readAccess.put(new ActiveMQTopic("GUEST.>"), GUESTS);
 
-        DestinationMap writeAccess = new DestinationMap();
+        DestinationMap writeAccess = new DefaultAuthorizationMap();
         writeAccess.put(new ActiveMQQueue(">"), ADMINS);
         writeAccess.put(new ActiveMQQueue("USERS.>"), USERS);
         writeAccess.put(new ActiveMQQueue("GUEST.>"), USERS);
@@ -96,7 +96,7 @@ public class SimpleSecurityBrokerSystemTest extends SecurityTestSupport {
         writeAccess.put(new ActiveMQTopic("ActiveMQ.Advisory.>"), GUESTS);
         writeAccess.put(new ActiveMQTopic("ActiveMQ.Advisory.>"), USERS);
 
-        DestinationMap adminAccess = new DestinationMap();
+        DestinationMap adminAccess = new DefaultAuthorizationMap();
         adminAccess.put(new ActiveMQTopic(">"), ADMINS);
         adminAccess.put(new ActiveMQTopic(">"), USERS);
         adminAccess.put(new ActiveMQTopic(">"), GUESTS);