Merged out from trunkQPID-2519

git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/QPID-2519@1187375 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 490 insertions, 214 deletions

diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java
index 309a3aeb2c..09d26e5451 100644
--- a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java
@@ -1,195 +1,172 @@
 /*
- *  Licensed to the Apache Software Foundation (ASF) under one
- *  or more contributor license agreements.  See the NOTICE file
- *  distributed with this work for additional information
- *  regarding copyright ownership.  The ASF licenses this file
- *  to you under the Apache License, Version 2.0 (the
- *  "License"); you may not use this file except in compliance
- *  with the License.  You may obtain a copy of the License at
  *
- *    http://www.apache.org/licenses/LICENSE-2.0
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
  *
- *  Unless required by applicable law or agreed to in writing,
- *  software distributed under the License is distributed on an
- *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- *  KIND, either express or implied.  See the License for the
- *  specific language governing permissions and limitations
- *  under the License.
  */
 package org.apache.qpid.server.security.access.plugins;
 
-import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.FileWriter;
-import java.io.PrintWriter;
+import java.util.Arrays;
 
 import junit.framework.TestCase;
 
-import org.apache.commons.configuration.ConfigurationException;
-import org.apache.qpid.server.security.access.config.ConfigurationFile;
-import org.apache.qpid.server.security.access.config.PlainConfiguration;
+import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
+import org.apache.qpid.server.logging.UnitTestMessageLogger;
+import org.apache.qpid.server.logging.actors.CurrentActor;
+import org.apache.qpid.server.logging.actors.TestLogActor;
+import org.apache.qpid.server.security.Result;
+import org.apache.qpid.server.security.SecurityManager;
+import org.apache.qpid.server.security.access.ObjectProperties;
+import org.apache.qpid.server.security.access.ObjectType;
+import org.apache.qpid.server.security.access.Operation;
+import org.apache.qpid.server.security.access.Permission;
+import org.apache.qpid.server.security.access.config.Rule;
 import org.apache.qpid.server.security.access.config.RuleSet;
+import org.apache.qpid.server.security.auth.sasl.TestPrincipalUtils;
 
 /**
- * These tests check that the ACL file parsing works correctly.
+ * Unit test for ACL V2 plugin.  
+ * 
+ * This unit test tests the AccessControl class and it collaboration with {@link RuleSet},
+ * {@link SecurityManager} and {@link CurrentActor}.   The ruleset is configured programmatically,
+ * rather than from an external file.
  * 
- * For each message that can be returned in a {@link ConfigurationException}, an ACL file is created that should trigger this
- * particular message.
+ * @see RuleSetTest
  */
 public class AccessControlTest extends TestCase
 {
-    public void writeACLConfig(String...aclData) throws Exception
+    private AccessControl _plugin = null;  // Class under test
+    private final UnitTestMessageLogger messageLogger = new UnitTestMessageLogger();
+
+    protected void setUp() throws Exception
     {
-        File acl = File.createTempFile(getClass().getName() + getName(), "acl");
-        acl.deleteOnExit();
-        
-        // Write ACL file
-        PrintWriter aclWriter = new PrintWriter(new FileWriter(acl));
-        for (String line : aclData)
-        {
-            aclWriter.println(line);
-        }
-        aclWriter.close();
+        super.setUp();
 
-        // Load ruleset
-        ConfigurationFile configFile = new PlainConfiguration(acl);
-        RuleSet ruleSet = configFile.load();
-    }
+        final RuleSet rs = new RuleSet();
+        rs.addGroup("aclGroup1", Arrays.asList(new String[] {"member1", "member2"}));
 
-    public void testMissingACLConfig() throws Exception
-    {
-        try
-        {
-            // Load ruleset
-	        ConfigurationFile configFile = new PlainConfiguration(new File("doesnotexist"));
-	        RuleSet ruleSet = configFile.load();
-            
-            fail("fail");
-        }
-        catch (ConfigurationException ce)
-        {
-            assertEquals(String.format(PlainConfiguration.CONFIG_NOT_FOUND_MSG, "doesnotexist"), ce.getMessage());
-            assertTrue(ce.getCause() instanceof FileNotFoundException);
-            assertEquals("doesnotexist (No such file or directory)", ce.getCause().getMessage());
-        }
-    }
+        // Rule expressed with username
+        rs.grant(0, "user1", Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        // Rule expressed with a acl group
+        rs.grant(1, "aclGroup1", Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        // Rule expressed with an external group
+        rs.grant(2, "extGroup1", Permission.DENY, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        // Catch all rule
+        rs.grant(3, Rule.ALL, Permission.DENY_LOG, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
 
-    public void testACLFileSyntaxContinuation() throws Exception
-    {
-        try
-        {
-            writeACLConfig("ACL ALLOW ALL \\ ALL");
-            fail("fail");
-        }
-        catch (ConfigurationException ce)
-        {
-            assertEquals(String.format(PlainConfiguration.PREMATURE_CONTINUATION_MSG, 1), ce.getMessage());
-        }
-    }
+        _plugin = (AccessControl) AccessControl.FACTORY.newInstance(createConfiguration(rs));
 
-    public void testACLFileSyntaxTokens() throws Exception
-    {
-        try
-        {
-            writeACLConfig("ACL unparsed ALL ALL");
-            fail("fail");
-        }
-        catch (ConfigurationException ce)
-        {
-            assertEquals(String.format(PlainConfiguration.PARSE_TOKEN_FAILED_MSG, 1), ce.getMessage());
-            assertTrue(ce.getCause() instanceof IllegalArgumentException);
-            assertEquals("Not a valid permission: unparsed", ce.getCause().getMessage());
-        }
+        SecurityManager.setThreadSubject(null);
+        
+        CurrentActor.set(new TestLogActor(messageLogger));
     }
 
-    public void testACLFileSyntaxNotEnoughGroup() throws Exception
+    protected void tearDown() throws Exception
     {
-        try
-        {
-            writeACLConfig("GROUP blah");
-            fail("fail");
-        }
-        catch (ConfigurationException ce)
-        {
-            assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_GROUP_MSG, 1), ce.getMessage());
-        }
+        super.tearDown();
+        SecurityManager.setThreadSubject(null);
     }
 
-    public void testACLFileSyntaxNotEnoughACL() throws Exception
+    /** 
+     * ACL plugin must always abstain if there is no  subject attached to the thread.
+     */
+    public void testNoSubjectAlwaysAbstains()
     {
-        try
-        {
-            writeACLConfig("ACL ALLOW");
-            fail("fail");
-        }
-        catch (ConfigurationException ce)
-        {
-            assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_ACL_MSG, 1), ce.getMessage());
-        }
+        SecurityManager.setThreadSubject(null);
+
+        final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(Result.ABSTAIN, result);
     }
 
-    public void testACLFileSyntaxNotEnoughConfig() throws Exception
+    /** 
+     * Tests that an allow rule expressed with a username allows an operation performed by a thread running
+     * with the same username.
+     */
+    public void testUsernameAllowsOperation()
     {
-        try
-        {
-            writeACLConfig("CONFIG");
-            fail("fail");
-        }
-        catch (ConfigurationException ce)
-        {
-            assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_TOKENS_MSG, 1), ce.getMessage());
-        }
+        SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("user1"));
+
+        final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(Result.ALLOWED, result);
     }
 
-    public void testACLFileSyntaxNotEnough() throws Exception
+    /** 
+     * Tests that an allow rule expressed with an <b>ACL groupname</b> allows an operation performed by a thread running
+     * by a user who belongs to the same group..
+     */
+    public void testAclGroupMembershipAllowsOperation()
     {
-        try
-        {
-            writeACLConfig("INVALID");
-            fail("fail");
-        }
-        catch (ConfigurationException ce)
-        {
-            assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_TOKENS_MSG, 1), ce.getMessage());
-        }
+        SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("member1"));
+
+        final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(Result.ALLOWED, result);
     }
 
-    public void testACLFileSyntaxPropertyKeyOnly() throws Exception
+    /** 
+     * Tests that a deny rule expressed with an <b>External groupname</b> denies an operation performed by a thread running
+     * by a user who belongs to the same group.
+     */
+    public void testExternalGroupMembershipDeniesOperation()
     {
-        try
-        {
-            writeACLConfig("ACL ALLOW adk CREATE QUEUE name");
-            fail("fail");
-        }
-        catch (ConfigurationException ce)
-        {
-            assertEquals(String.format(PlainConfiguration.PROPERTY_KEY_ONLY_MSG, 1), ce.getMessage());
-        }
+        SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("user3", "extGroup1"));
+        
+        final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(Result.DENIED, result);
     }
 
-    public void testACLFileSyntaxPropertyNoEquals() throws Exception
+    /** 
+     * Tests that the catch all deny denies the operation and logs with the logging actor.
+     */
+    public void testCatchAllRuleDeniesUnrecognisedUsername()
     {
-        try
-        {
-            writeACLConfig("ACL ALLOW adk CREATE QUEUE name test");
-            fail("fail");
-        }
-        catch (ConfigurationException ce)
-        {
-            assertEquals(String.format(PlainConfiguration.PROPERTY_NO_EQUALS_MSG, 1), ce.getMessage());
-        }
+        SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("unknown", "unkgroup1", "unkgroup2"));
+        
+        assertEquals("Expecting zero messages before test", 0, messageLogger.getLogMessages().size());
+        final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(Result.DENIED, result);
+        
+        assertEquals("Expecting one message before test", 1, messageLogger.getLogMessages().size());
+        assertTrue("Logged message does not contain expected string", messageLogger.messageContains(0, "ACL-1002"));
     }
-
-    public void testACLFileSyntaxPropertyNoValue() throws Exception
+    
+    /**
+     * Creates a configuration plugin for the {@link AccessControl} plugin.
+     */
+    private ConfigurationPlugin createConfiguration(final RuleSet rs)
     {
-        try
-        {
-            writeACLConfig("ACL ALLOW adk CREATE QUEUE name =");
-            fail("fail");
-        }
-        catch (ConfigurationException ce)
-        {
-            assertEquals(String.format(PlainConfiguration.PROPERTY_NO_VALUE_MSG, 1), ce.getMessage());
-        }
+        final ConfigurationPlugin cp = new ConfigurationPlugin()
+        {
+            public AccessControlConfiguration  getConfiguration(final String plugin)
+            {
+                return new AccessControlConfiguration()
+                {
+                    public RuleSet getRuleSet()
+                    {
+                        return rs;
+                    }
+                };
+            }
+
+            public String[] getElementsProcessed()
+            {
+                throw new UnsupportedOperationException();
+            }
+        };
+
+        return cp;
     }
 }
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/PlainConfigurationTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/PlainConfigurationTest.java
new file mode 100644
index 0000000000..e16f9943ba
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/PlainConfigurationTest.java
@@ -0,0 +1,194 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.qpid.server.security.access.plugins;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileWriter;
+import java.io.PrintWriter;
+
+import junit.framework.TestCase;
+
+import org.apache.commons.configuration.ConfigurationException;
+import org.apache.qpid.server.security.access.config.ConfigurationFile;
+import org.apache.qpid.server.security.access.config.PlainConfiguration;
+
+/**
+ * These tests check that the ACL file parsing works correctly.
+ * 
+ * For each message that can be returned in a {@link ConfigurationException}, an ACL file is created that should trigger this
+ * particular message.
+ */
+public class PlainConfigurationTest extends TestCase
+{
+    public void writeACLConfig(String...aclData) throws Exception
+    {
+        File acl = File.createTempFile(getClass().getName() + getName(), "acl");
+        acl.deleteOnExit();
+        
+        // Write ACL file
+        PrintWriter aclWriter = new PrintWriter(new FileWriter(acl));
+        for (String line : aclData)
+        {
+            aclWriter.println(line);
+        }
+        aclWriter.close();
+
+        // Load ruleset
+        ConfigurationFile configFile = new PlainConfiguration(acl);
+        configFile.load();
+    }
+
+    public void testMissingACLConfig() throws Exception
+    {
+        try
+        {
+            // Load ruleset
+            ConfigurationFile configFile = new PlainConfiguration(new File("doesnotexist"));
+            configFile.load();
+            
+            fail("fail");
+        }
+        catch (ConfigurationException ce)
+        {
+            assertEquals(String.format(PlainConfiguration.CONFIG_NOT_FOUND_MSG, "doesnotexist"), ce.getMessage());
+            assertTrue(ce.getCause() instanceof FileNotFoundException);
+            assertEquals("doesnotexist (No such file or directory)", ce.getCause().getMessage());
+        }
+    }
+
+    public void testACLFileSyntaxContinuation() throws Exception
+    {
+        try
+        {
+            writeACLConfig("ACL ALLOW ALL \\ ALL");
+            fail("fail");
+        }
+        catch (ConfigurationException ce)
+        {
+            assertEquals(String.format(PlainConfiguration.PREMATURE_CONTINUATION_MSG, 1), ce.getMessage());
+        }
+    }
+
+    public void testACLFileSyntaxTokens() throws Exception
+    {
+        try
+        {
+            writeACLConfig("ACL unparsed ALL ALL");
+            fail("fail");
+        }
+        catch (ConfigurationException ce)
+        {
+            assertEquals(String.format(PlainConfiguration.PARSE_TOKEN_FAILED_MSG, 1), ce.getMessage());
+            assertTrue(ce.getCause() instanceof IllegalArgumentException);
+            assertEquals("Not a valid permission: unparsed", ce.getCause().getMessage());
+        }
+    }
+
+    public void testACLFileSyntaxNotEnoughGroup() throws Exception
+    {
+        try
+        {
+            writeACLConfig("GROUP blah");
+            fail("fail");
+        }
+        catch (ConfigurationException ce)
+        {
+            assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_GROUP_MSG, 1), ce.getMessage());
+        }
+    }
+
+    public void testACLFileSyntaxNotEnoughACL() throws Exception
+    {
+        try
+        {
+            writeACLConfig("ACL ALLOW");
+            fail("fail");
+        }
+        catch (ConfigurationException ce)
+        {
+            assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_ACL_MSG, 1), ce.getMessage());
+        }
+    }
+
+    public void testACLFileSyntaxNotEnoughConfig() throws Exception
+    {
+        try
+        {
+            writeACLConfig("CONFIG");
+            fail("fail");
+        }
+        catch (ConfigurationException ce)
+        {
+            assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_TOKENS_MSG, 1), ce.getMessage());
+        }
+    }
+
+    public void testACLFileSyntaxNotEnough() throws Exception
+    {
+        try
+        {
+            writeACLConfig("INVALID");
+            fail("fail");
+        }
+        catch (ConfigurationException ce)
+        {
+            assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_TOKENS_MSG, 1), ce.getMessage());
+        }
+    }
+
+    public void testACLFileSyntaxPropertyKeyOnly() throws Exception
+    {
+        try
+        {
+            writeACLConfig("ACL ALLOW adk CREATE QUEUE name");
+            fail("fail");
+        }
+        catch (ConfigurationException ce)
+        {
+            assertEquals(String.format(PlainConfiguration.PROPERTY_KEY_ONLY_MSG, 1), ce.getMessage());
+        }
+    }
+
+    public void testACLFileSyntaxPropertyNoEquals() throws Exception
+    {
+        try
+        {
+            writeACLConfig("ACL ALLOW adk CREATE QUEUE name test");
+            fail("fail");
+        }
+        catch (ConfigurationException ce)
+        {
+            assertEquals(String.format(PlainConfiguration.PROPERTY_NO_EQUALS_MSG, 1), ce.getMessage());
+        }
+    }
+
+    public void testACLFileSyntaxPropertyNoValue() throws Exception
+    {
+        try
+        {
+            writeACLConfig("ACL ALLOW adk CREATE QUEUE name =");
+            fail("fail");
+        }
+        catch (ConfigurationException ce)
+        {
+            assertEquals(String.format(PlainConfiguration.PROPERTY_NO_VALUE_MSG, 1), ce.getMessage());
+        }
+    }
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleSetTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleSetTest.java
index aad7290557..bd9deac153 100644
--- a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleSetTest.java
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleSetTest.java
@@ -21,13 +21,21 @@
 
 package org.apache.qpid.server.security.access.plugins;
 
+import java.security.Principal;
+import java.util.Arrays;
+
+import javax.security.auth.Subject;
+
 import org.apache.qpid.framing.AMQShortString;
 import org.apache.qpid.server.security.Result;
 import org.apache.qpid.server.security.access.ObjectProperties;
 import org.apache.qpid.server.security.access.ObjectType;
 import org.apache.qpid.server.security.access.Operation;
 import org.apache.qpid.server.security.access.Permission;
+import org.apache.qpid.server.security.access.config.Rule;
 import org.apache.qpid.server.security.access.config.RuleSet;
+import org.apache.qpid.server.security.auth.sasl.TestPrincipalUtils;
+import org.apache.qpid.server.security.auth.sasl.UsernamePrincipal;
 import org.apache.qpid.test.utils.QpidTestCase;
 
 /**
@@ -36,16 +44,24 @@ import org.apache.qpid.test.utils.QpidTestCase;
  * The ruleset is configured directly rather than using an external file by adding rules individually, calling the
  * {@link RuleSet#grant(Integer, String, Permission, Operation, ObjectType, ObjectProperties)} method. Then, the
  * access control mechanism is validated by checking whether operations would be authorised by calling the
- * {@link RuleSet#check(String, Operation, ObjectType, ObjectProperties)} method.
+ * {@link RuleSet#check(Principal, Operation, ObjectType, ObjectProperties)} method.
+ * 
+ * It ensure that permissions can be granted correctly on users directly, ACL groups (that is those
+ * groups declared directly in the ACL itself), and External groups (that is a group from an External
+ * Authentication Provider, such as an LDAP).
+
  */
 public class RuleSetTest extends QpidTestCase
 {
-    private RuleSet _ruleSet;
+    private RuleSet _ruleSet; // Object under test
+
+    private static final String TEST_USER = "user";
 
     // Common things that are passed to frame constructors
     private AMQShortString _queueName = new AMQShortString(this.getClass().getName() + "queue");
     private AMQShortString _exchangeName = new AMQShortString("amq.direct");
     private AMQShortString _exchangeType = new AMQShortString("direct");
+    private Subject _testSubject = TestPrincipalUtils.createTestSubject(TEST_USER);
 
     @Override
     public void setUp() throws Exception
@@ -63,34 +79,36 @@ public class RuleSetTest extends QpidTestCase
         super.tearDown();
     }
 
-    public void assertDenyGrantAllow(String identity, Operation operation, ObjectType objectType)
+    public void assertDenyGrantAllow(Subject subject, Operation operation, ObjectType objectType)
     {
-        assertDenyGrantAllow(identity, operation, objectType, ObjectProperties.EMPTY);
+        assertDenyGrantAllow(subject, operation, objectType, ObjectProperties.EMPTY);
     }
     
-    public void assertDenyGrantAllow(String identity, Operation operation, ObjectType objectType, ObjectProperties properties)
+    public void assertDenyGrantAllow(Subject subject, Operation operation, ObjectType objectType, ObjectProperties properties)
     {
-        assertEquals(Result.DENIED, _ruleSet.check(identity, operation, objectType, properties));
-        _ruleSet.grant(0, identity, Permission.ALLOW, operation, objectType, properties);
+        final Principal identity = UsernamePrincipal.getUsernamePrincipalFromSubject(subject);
+        
+        assertEquals(Result.DENIED, _ruleSet.check(subject, operation, objectType, properties));
+        _ruleSet.grant(0, identity.getName(), Permission.ALLOW, operation, objectType, properties);
         assertEquals(1, _ruleSet.getRuleCount());
-        assertEquals(Result.ALLOWED, _ruleSet.check(identity, operation, objectType, properties));
+        assertEquals(Result.ALLOWED, _ruleSet.check(subject, operation, objectType, properties));
     }
 
     public void testEmptyRuleSet()
     {
         assertNotNull(_ruleSet);
         assertEquals(_ruleSet.getRuleCount(), 0);
-        assertEquals(_ruleSet.getDefault(), _ruleSet.check("user", Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+        assertEquals(_ruleSet.getDefault(), _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
     }
     
     public void testVirtualHostAccess() throws Exception
     {
-        assertDenyGrantAllow("user", Operation.ACCESS, ObjectType.VIRTUALHOST);
+        assertDenyGrantAllow(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST);
     }
 
     public void testQueueCreateNamed() throws Exception
     {
-        assertDenyGrantAllow("user", Operation.CREATE, ObjectType.QUEUE, new ObjectProperties(_queueName));
+        assertDenyGrantAllow(_testSubject, Operation.CREATE, ObjectType.QUEUE, new ObjectProperties(_queueName));
     }
 
     public void testQueueCreatenamedNullRoutingKey()
@@ -98,7 +116,7 @@ public class RuleSetTest extends QpidTestCase
         ObjectProperties properties = new ObjectProperties(_queueName);
         properties.put(ObjectProperties.Property.ROUTING_KEY, (String) null);
         
-        assertDenyGrantAllow("user", Operation.CREATE, ObjectType.QUEUE, properties);
+        assertDenyGrantAllow(_testSubject, Operation.CREATE, ObjectType.QUEUE, properties);
     }
 
     public void testExchangeCreate()
@@ -106,17 +124,17 @@ public class RuleSetTest extends QpidTestCase
         ObjectProperties properties = new ObjectProperties(_exchangeName);
         properties.put(ObjectProperties.Property.TYPE, _exchangeType.asString());
         
-        assertDenyGrantAllow("user", Operation.CREATE, ObjectType.EXCHANGE, properties);
+        assertDenyGrantAllow(_testSubject, Operation.CREATE, ObjectType.EXCHANGE, properties);
     }
 
     public void testConsume()
     {
-        assertDenyGrantAllow("user", Operation.CONSUME, ObjectType.QUEUE);
+        assertDenyGrantAllow(_testSubject, Operation.CONSUME, ObjectType.QUEUE);
     }
 
     public void testPublish()
     {
-        assertDenyGrantAllow("user", Operation.PUBLISH, ObjectType.EXCHANGE);
+        assertDenyGrantAllow(_testSubject, Operation.PUBLISH, ObjectType.EXCHANGE);
     }
 
     /**
@@ -131,13 +149,13 @@ public class RuleSetTest extends QpidTestCase
         ObjectProperties normal = new ObjectProperties();
         normal.put(ObjectProperties.Property.AUTO_DELETE, Boolean.FALSE);
         
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CONSUME, ObjectType.QUEUE, temporary));
-        _ruleSet.grant(0, "user", Permission.ALLOW, Operation.CONSUME, ObjectType.QUEUE, temporary);
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CONSUME, ObjectType.QUEUE, temporary));
+        _ruleSet.grant(0, TEST_USER, Permission.ALLOW, Operation.CONSUME, ObjectType.QUEUE, temporary);
         assertEquals(1, _ruleSet.getRuleCount());
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CONSUME, ObjectType.QUEUE, temporary));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CONSUME, ObjectType.QUEUE, temporary));
         
         // defer to global if exists, otherwise default answer - this is handled by the security manager
-        assertEquals(Result.DEFER, _ruleSet.check("user", Operation.CONSUME, ObjectType.QUEUE, normal));
+        assertEquals(Result.DEFER, _ruleSet.check(_testSubject, Operation.CONSUME, ObjectType.QUEUE, normal));
     }
 
     /**
@@ -151,15 +169,15 @@ public class RuleSetTest extends QpidTestCase
         ObjectProperties normal = new ObjectProperties(_queueName);
         normal.put(ObjectProperties.Property.AUTO_DELETE, Boolean.FALSE);
         
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CONSUME, ObjectType.QUEUE, temporary));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CONSUME, ObjectType.QUEUE, temporary));
 
         // should not matter if the temporary permission is processed first or last
-        _ruleSet.grant(1, "user", Permission.ALLOW, Operation.CONSUME, ObjectType.QUEUE, normal);
-        _ruleSet.grant(2, "user", Permission.ALLOW, Operation.CONSUME, ObjectType.QUEUE, temporary);
+        _ruleSet.grant(1, TEST_USER, Permission.ALLOW, Operation.CONSUME, ObjectType.QUEUE, normal);
+        _ruleSet.grant(2, TEST_USER, Permission.ALLOW, Operation.CONSUME, ObjectType.QUEUE, temporary);
         assertEquals(2, _ruleSet.getRuleCount());
         
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CONSUME, ObjectType.QUEUE, normal));
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CONSUME, ObjectType.QUEUE, temporary));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CONSUME, ObjectType.QUEUE, normal));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CONSUME, ObjectType.QUEUE, temporary));
     }
 
     /**
@@ -173,15 +191,15 @@ public class RuleSetTest extends QpidTestCase
         ObjectProperties normal = new ObjectProperties(_queueName);
         normal.put(ObjectProperties.Property.AUTO_DELETE, Boolean.FALSE);
         
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CONSUME, ObjectType.QUEUE, temporary));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CONSUME, ObjectType.QUEUE, temporary));
 
         // should not matter if the temporary permission is processed first or last
-        _ruleSet.grant(1, "user", Permission.ALLOW, Operation.CONSUME, ObjectType.QUEUE, temporary);
-        _ruleSet.grant(2, "user", Permission.ALLOW, Operation.CONSUME, ObjectType.QUEUE, normal);
+        _ruleSet.grant(1, TEST_USER, Permission.ALLOW, Operation.CONSUME, ObjectType.QUEUE, temporary);
+        _ruleSet.grant(2, TEST_USER, Permission.ALLOW, Operation.CONSUME, ObjectType.QUEUE, normal);
         assertEquals(2, _ruleSet.getRuleCount());
         
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CONSUME, ObjectType.QUEUE, normal));
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CONSUME, ObjectType.QUEUE, temporary));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CONSUME, ObjectType.QUEUE, normal));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CONSUME, ObjectType.QUEUE, temporary));
     }
 
     /*
@@ -197,15 +215,15 @@ public class RuleSetTest extends QpidTestCase
         ObjectProperties namedTemporary = new ObjectProperties(_queueName);
         namedTemporary.put(ObjectProperties.Property.AUTO_DELETE, Boolean.TRUE);
         
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, named));
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedTemporary));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, named));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedTemporary));
 
-        _ruleSet.grant(1, "user", Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, named);
-        _ruleSet.grant(2, "user", Permission.DENY, Operation.CREATE, ObjectType.QUEUE, namedTemporary);
+        _ruleSet.grant(1, TEST_USER, Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, named);
+        _ruleSet.grant(2, TEST_USER, Permission.DENY, Operation.CREATE, ObjectType.QUEUE, namedTemporary);
         assertEquals(2, _ruleSet.getRuleCount());
         
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, named));
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedTemporary));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, named));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedTemporary));
     }
     
     /**
@@ -217,15 +235,15 @@ public class RuleSetTest extends QpidTestCase
         ObjectProperties namedTemporary = new ObjectProperties(_queueName);
         namedTemporary.put(ObjectProperties.Property.AUTO_DELETE, Boolean.TRUE);
         
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, named));
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedTemporary));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, named));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedTemporary));
 
-        _ruleSet.grant(1, "user", Permission.DENY, Operation.CREATE, ObjectType.QUEUE, namedTemporary);
-        _ruleSet.grant(2, "user", Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, named);
+        _ruleSet.grant(1, TEST_USER, Permission.DENY, Operation.CREATE, ObjectType.QUEUE, namedTemporary);
+        _ruleSet.grant(2, TEST_USER, Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, named);
         assertEquals(2, _ruleSet.getRuleCount());
         
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, named));
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedTemporary));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, named));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedTemporary));
     }
     
     /**
@@ -239,18 +257,18 @@ public class RuleSetTest extends QpidTestCase
         ObjectProperties namedDurable = new ObjectProperties(_queueName);
         namedDurable.put(ObjectProperties.Property.DURABLE, Boolean.TRUE);
         
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, named));
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedTemporary));
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedDurable));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, named));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedTemporary));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedDurable));
 
-        _ruleSet.grant(1, "user", Permission.DENY, Operation.CREATE, ObjectType.QUEUE, namedTemporary);
-        _ruleSet.grant(2, "user", Permission.DENY, Operation.CREATE, ObjectType.QUEUE, namedDurable);
-        _ruleSet.grant(3, "user", Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, named);
+        _ruleSet.grant(1, TEST_USER, Permission.DENY, Operation.CREATE, ObjectType.QUEUE, namedTemporary);
+        _ruleSet.grant(2, TEST_USER, Permission.DENY, Operation.CREATE, ObjectType.QUEUE, namedDurable);
+        _ruleSet.grant(3, TEST_USER, Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, named);
         assertEquals(3, _ruleSet.getRuleCount());
         
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, named));
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedTemporary));
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedDurable));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, named));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedTemporary));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedDurable));
     }
     
     public void testNamedTemporaryQueueAllowed()
@@ -259,15 +277,15 @@ public class RuleSetTest extends QpidTestCase
         ObjectProperties namedTemporary = new ObjectProperties(_queueName);
         namedTemporary.put(ObjectProperties.Property.AUTO_DELETE, Boolean.TRUE);
         
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, named));
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedTemporary));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, named));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedTemporary));
 
-        _ruleSet.grant(1, "user", Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, namedTemporary);
-        _ruleSet.grant(2, "user", Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, named);
+        _ruleSet.grant(1, TEST_USER, Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, namedTemporary);
+        _ruleSet.grant(2, TEST_USER, Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, named);
         assertEquals(2, _ruleSet.getRuleCount());
         
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, named));
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedTemporary));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, named));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedTemporary));
     }
     
     public void testNamedTemporaryQueueDeniedAllowed()
@@ -276,14 +294,101 @@ public class RuleSetTest extends QpidTestCase
         ObjectProperties namedTemporary = new ObjectProperties(_queueName);
         namedTemporary.put(ObjectProperties.Property.AUTO_DELETE, Boolean.TRUE);
         
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, named));
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedTemporary));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, named));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedTemporary));
 
-        _ruleSet.grant(1, "user", Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, namedTemporary);
-        _ruleSet.grant(2, "user", Permission.DENY, Operation.CREATE, ObjectType.QUEUE, named);
+        _ruleSet.grant(1, TEST_USER, Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, namedTemporary);
+        _ruleSet.grant(2, TEST_USER, Permission.DENY, Operation.CREATE, ObjectType.QUEUE, named);
         assertEquals(2, _ruleSet.getRuleCount());
         
-        assertEquals(Result.DENIED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, named));
-        assertEquals(Result.ALLOWED, _ruleSet.check("user", Operation.CREATE, ObjectType.QUEUE, namedTemporary));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, named));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, namedTemporary));
+    }
+
+    /** 
+     * Tests support for the {@link Rule#ALL} keyword. 
+     */
+    public void testAllowToAll()
+    {
+        _ruleSet.grant(1, Rule.ALL, Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(1, _ruleSet.getRuleCount());
+
+        assertEquals(Result.ALLOWED, _ruleSet.check(TestPrincipalUtils.createTestSubject("usera"),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+        assertEquals(Result.ALLOWED, _ruleSet.check(TestPrincipalUtils.createTestSubject("userb"),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+    }
+
+    /** 
+     * Tests support for ACL groups (i.e. inline groups declared in the ACL file itself).
+     */
+    public void testAclGroupsSupported()
+    {
+        assertTrue(_ruleSet.addGroup("aclgroup", Arrays.asList(new String[] {"usera", "userb"})));  
+        
+        _ruleSet.grant(1, "aclgroup", Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(1, _ruleSet.getRuleCount());
+
+        assertEquals(Result.ALLOWED, _ruleSet.check(TestPrincipalUtils.createTestSubject("usera"),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+        assertEquals(Result.ALLOWED, _ruleSet.check(TestPrincipalUtils.createTestSubject("userb"),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+        assertEquals(Result.DEFER, _ruleSet.check(TestPrincipalUtils.createTestSubject("userc"),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+    }
+
+    /** 
+     * Tests support for nested ACL groups.
+     */
+    public void testNestedAclGroupsSupported()
+    {
+        assertTrue(_ruleSet.addGroup("aclgroup1", Arrays.asList(new String[] {"userb"})));
+        assertTrue(_ruleSet.addGroup("aclgroup2", Arrays.asList(new String[] {"usera", "aclgroup1"})));  
+        
+        _ruleSet.grant(1, "aclgroup2", Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(1, _ruleSet.getRuleCount());
+
+        assertEquals(Result.ALLOWED, _ruleSet.check(TestPrincipalUtils.createTestSubject("usera"),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+        assertEquals(Result.ALLOWED, _ruleSet.check(TestPrincipalUtils.createTestSubject("userb"),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+    }
+
+    /** 
+     * Tests support for nested External groups (i.e. those groups coming from an external source such as an LDAP).
+     */
+    public void testExternalGroupsSupported()
+    {
+        _ruleSet.grant(1, "extgroup1", Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        _ruleSet.grant(2, "extgroup2", Permission.DENY, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(2, _ruleSet.getRuleCount());
+
+        assertEquals(Result.ALLOWED, _ruleSet.check(TestPrincipalUtils.createTestSubject("usera", "extgroup1"),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+        assertEquals(Result.DENIED, _ruleSet.check(TestPrincipalUtils.createTestSubject("userb", "extgroup2"),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+    }
+    
+    /**
+     * Rule order in the ACL determines the outcome of the check.  This test ensures that a user who is
+     * granted explicit permission on an object, is granted that access even although late a group
+     * to which the user belongs is later denied the permission.
+     */
+    public void testAllowDeterminedByRuleOrder()
+    {
+        assertTrue(_ruleSet.addGroup("aclgroup", Arrays.asList(new String[] {"usera"})));
+        
+        _ruleSet.grant(1, "usera", Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        _ruleSet.grant(2, "aclgroup", Permission.DENY, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(2, _ruleSet.getRuleCount());
+
+        assertEquals(Result.ALLOWED, _ruleSet.check(TestPrincipalUtils.createTestSubject("usera"),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+    }
+
+    /**
+     * Rule order in the ACL determines the outcome of the check.  This tests ensures that a user who is denied
+     * access by group, is denied access, despite there being a later rule granting permission to that user.
+     */
+    public void testDenyDeterminedByRuleOrder()
+    {
+        assertTrue(_ruleSet.addGroup("aclgroup", Arrays.asList(new String[] {"usera"})));
+        
+        _ruleSet.grant(1, "aclgroup", Permission.DENY, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        _ruleSet.grant(2, "usera", Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        
+        assertEquals(2, _ruleSet.getRuleCount());
+
+        assertEquals(Result.DENIED, _ruleSet.check(TestPrincipalUtils.createTestSubject("usera"),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
     }
 }