Update from trunk r1375509 through r1450773asyncstore

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

11 files changed, 866 insertions, 133 deletions

diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclActionTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclActionTest.java
new file mode 100644
index 0000000000..14620cff70
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclActionTest.java
@@ -0,0 +1,66 @@
+/*
+ * 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.config;
+
+import static org.mockito.Mockito.*;
+
+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.firewall.FirewallRule;
+
+import junit.framework.TestCase;
+
+public class AclActionTest extends TestCase
+{
+    public void testEqualsAndHashCode()
+    {
+        AclRulePredicates predicates = createAclRulePredicates();
+        ObjectType objectType = ObjectType.EXCHANGE;
+        Operation operation = Operation.ACCESS;
+
+        AclAction aclAction = new AclAction(operation, objectType, predicates);
+        AclAction equalAclAction = new AclAction(operation, objectType, predicates);
+
+        assertTrue(aclAction.equals(aclAction));
+        assertTrue(aclAction.equals(equalAclAction));
+        assertTrue(equalAclAction.equals(aclAction));
+
+        assertTrue(aclAction.hashCode() == equalAclAction.hashCode());
+
+        assertFalse("Different operation should cause aclActions to be unequal",
+                aclAction.equals(new AclAction(Operation.BIND, objectType, predicates)));
+
+        assertFalse("Different operation type should cause aclActions to be unequal",
+                aclAction.equals(new AclAction(operation, ObjectType.GROUP, predicates)));
+
+        assertFalse("Different predicates should cause aclActions to be unequal",
+                aclAction.equals(new AclAction(operation, objectType, createAclRulePredicates())));
+
+    }
+
+    private AclRulePredicates createAclRulePredicates()
+    {
+        AclRulePredicates predicates = mock(AclRulePredicates.class);
+        when(predicates.getFirewallRule()).thenReturn(mock(FirewallRule.class));
+        when(predicates.getObjectProperties()).thenReturn(mock(ObjectProperties.class));
+        return predicates;
+    }
+
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclRulePredicatesTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclRulePredicatesTest.java
new file mode 100644
index 0000000000..93b765d0fb
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclRulePredicatesTest.java
@@ -0,0 +1,87 @@
+/*
+ * 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.config;
+
+import static org.apache.qpid.server.security.access.ObjectProperties.Property.*;
+
+import org.apache.qpid.server.security.access.firewall.FirewallRule;
+import org.apache.qpid.server.security.access.firewall.FirewallRuleFactory;
+
+import static org.mockito.Mockito.*;
+
+import junit.framework.TestCase;
+
+public class AclRulePredicatesTest extends TestCase
+{
+    private AclRulePredicates _aclRulePredicates = new AclRulePredicates();
+    private FirewallRuleFactory _firewallRuleFactory = mock(FirewallRuleFactory.class);
+
+    @Override
+    protected void setUp() throws Exception
+    {
+        _aclRulePredicates.setFirewallRuleFactory(_firewallRuleFactory);
+
+        when(_firewallRuleFactory.createForHostname((String[]) any())).thenReturn(mock(FirewallRule.class));
+        when(_firewallRuleFactory.createForNetwork((String[]) any())).thenReturn(mock(FirewallRule.class));
+    }
+
+    public void testParse()
+    {
+        String name = "name";
+        String className = "class";
+
+        _aclRulePredicates.parse(NAME.name(), name);
+        _aclRulePredicates.parse(CLASS.name(), className);
+
+        assertEquals(name, _aclRulePredicates.getObjectProperties().get(NAME));
+        assertEquals(className, _aclRulePredicates.getObjectProperties().get(CLASS));
+    }
+
+    public void testParseHostnameFirewallRule()
+    {
+        String hostname = "hostname1,hostname2";
+        _aclRulePredicates.parse(FROM_HOSTNAME.name(), hostname);
+
+        verify(_firewallRuleFactory).createForHostname(new String[] {"hostname1", "hostname2"});
+    }
+
+    public void testParseNetworkFirewallRule()
+    {
+        _aclRulePredicates.setFirewallRuleFactory(_firewallRuleFactory);
+
+        String networks = "network1,network2";
+        _aclRulePredicates.parse(FROM_NETWORK.name(), networks);
+
+        verify(_firewallRuleFactory).createForNetwork(new String[] {"network1", "network2"});
+    }
+
+    public void testParseThrowsExceptionIfBothHostnameAndNetworkSpecified()
+    {
+        _aclRulePredicates.parse(FROM_NETWORK.name(), "network1,network2");
+        try
+        {
+            _aclRulePredicates.parse(FROM_HOSTNAME.name(), "hostname1,hostname2");
+            fail("Exception not thrown");
+        }
+        catch(IllegalStateException e)
+        {
+            // pass
+        }
+    }
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ActionTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ActionTest.java
new file mode 100644
index 0000000000..00e06106bf
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ActionTest.java
@@ -0,0 +1,95 @@
+/*
+ * 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.config;
+
+import static org.mockito.Mockito.*;
+
+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 junit.framework.TestCase;
+
+public class ActionTest extends TestCase
+{
+    private ObjectProperties _properties1 = mock(ObjectProperties.class);
+    private ObjectProperties _properties2 = mock(ObjectProperties.class);
+
+    public void testMatchesReturnsTrueForMatchingActions()
+    {
+        when(_properties1.matches(_properties2)).thenReturn(true);
+
+        assertMatches(
+                new Action(Operation.CONSUME, ObjectType.QUEUE, _properties1),
+                new Action(Operation.CONSUME, ObjectType.QUEUE, _properties2));
+    }
+
+    public void testMatchesReturnsFalseWhenOperationsDiffer()
+    {
+        assertDoesntMatch(
+                new Action(Operation.CONSUME, ObjectType.QUEUE, _properties1),
+                new Action(Operation.CREATE, ObjectType.QUEUE, _properties1));
+    }
+
+    public void testMatchesReturnsFalseWhenOperationTypesDiffer()
+    {
+        assertDoesntMatch(
+                new Action(Operation.CREATE, ObjectType.QUEUE, _properties1),
+                new Action(Operation.CREATE, ObjectType.EXCHANGE, _properties1));
+    }
+
+    public void testMatchesReturnsFalseWhenOperationPropertiesDiffer()
+    {
+        assertDoesntMatch(
+                new Action(Operation.CREATE, ObjectType.QUEUE, _properties1),
+                new Action(Operation.CREATE, ObjectType.QUEUE, _properties2));
+    }
+
+    public void testMatchesReturnsFalseWhenMyOperationPropertiesIsNull()
+    {
+        assertDoesntMatch(
+                new Action(Operation.CREATE, ObjectType.QUEUE, (ObjectProperties)null),
+                new Action(Operation.CREATE, ObjectType.QUEUE, _properties1));
+    }
+
+    public void testMatchesReturnsFalseWhenOtherOperationPropertiesIsNull()
+    {
+        assertDoesntMatch(
+                new Action(Operation.CREATE, ObjectType.QUEUE, _properties1),
+                new Action(Operation.CREATE, ObjectType.QUEUE, (ObjectProperties)null));
+    }
+
+    public void testMatchesReturnsTrueWhenBothOperationPropertiesAreNull()
+    {
+        assertMatches(
+                new Action(Operation.CREATE, ObjectType.QUEUE, (ObjectProperties)null),
+                new Action(Operation.CREATE, ObjectType.QUEUE, (ObjectProperties)null));
+    }
+
+    private void assertMatches(Action action1, Action action2)
+    {
+        assertTrue(action1 + " should match " + action2, action1.matches(action2));
+    }
+
+    private void assertDoesntMatch(Action action1, Action action2)
+    {
+        assertFalse(action1 + " should not match " + action2, action1.matches(action2));
+    }
+
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ClientActionTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ClientActionTest.java
new file mode 100644
index 0000000000..ae5d3fda74
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ClientActionTest.java
@@ -0,0 +1,79 @@
+/*
+ * 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.config;
+
+import static org.mockito.Mockito.*;
+
+import java.net.InetAddress;
+
+import org.apache.qpid.server.security.access.firewall.FirewallRule;
+
+import junit.framework.TestCase;
+
+public class ClientActionTest extends TestCase
+{
+    private Action _action = mock(Action.class);
+    private AclAction _ruleAction = mock(AclAction.class);
+    private InetAddress _addressOfClient = mock(InetAddress.class);
+
+    private ClientAction _clientAction = new ClientAction(_action);
+
+    public void testMatches_returnsTrueWhenActionsMatchAndNoFirewallRule()
+    {
+        when(_action.matches(any(Action.class))).thenReturn(true);
+        when(_ruleAction.getFirewallRule()).thenReturn(null);
+
+        assertTrue(_clientAction.matches(_ruleAction, _addressOfClient));
+    }
+
+    public void testMatches_returnsFalseWhenActionsDontMatch()
+    {
+        FirewallRule firewallRule = mock(FirewallRule.class);
+        when(firewallRule.matches(_addressOfClient)).thenReturn(true);
+
+        when(_action.matches(any(Action.class))).thenReturn(false);
+        when(_ruleAction.getFirewallRule()).thenReturn(firewallRule);
+
+        assertFalse(_clientAction.matches(_ruleAction, _addressOfClient));
+    }
+
+    public void testMatches_returnsTrueWhenActionsAndFirewallRuleMatch()
+    {
+        FirewallRule firewallRule = mock(FirewallRule.class);
+        when(firewallRule.matches(_addressOfClient)).thenReturn(true);
+
+        when(_action.matches(any(Action.class))).thenReturn(true);
+        when(_ruleAction.getFirewallRule()).thenReturn(firewallRule);
+
+        assertTrue(_clientAction.matches(_ruleAction, _addressOfClient));
+    }
+
+    public void testMatches_ignoresFirewallRuleIfClientAddressIsNull()
+    {
+        FirewallRule firewallRule = mock(FirewallRule.class);
+
+        when(_action.matches(any(Action.class))).thenReturn(true);
+        when(_ruleAction.getFirewallRule()).thenReturn(firewallRule);
+
+        assertTrue(_clientAction.matches(_ruleAction, null));
+
+        verifyZeroInteractions(firewallRule);
+    }
+
+}
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/config/PlainConfigurationTest.java
index c2282694fb..cbfc9003c8 100644
--- 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/config/PlainConfigurationTest.java
@@ -16,7 +16,7 @@
  *  specific language governing permissions and limitations
  *  under the License.
  */
-package org.apache.qpid.server.security.access.plugins;
+package org.apache.qpid.server.security.access.config;
 
 import java.io.File;
 import java.io.FileNotFoundException;
@@ -26,7 +26,7 @@ import java.util.Map;
 
 import junit.framework.TestCase;
 
-import org.apache.commons.configuration.ConfigurationException;
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.security.access.ObjectProperties;
 import org.apache.qpid.server.security.access.ObjectProperties.Property;
 import org.apache.qpid.server.security.access.ObjectType;
@@ -73,7 +73,7 @@ public class PlainConfigurationTest extends TestCase
 
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.CONFIG_NOT_FOUND_MSG, "doesnotexist"), ce.getMessage());
             assertTrue(ce.getCause() instanceof FileNotFoundException);
@@ -87,7 +87,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL ALLOW ALL \\ ALL");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.PREMATURE_CONTINUATION_MSG, 1), ce.getMessage());
         }
@@ -100,7 +100,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL unparsed ALL ALL");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.PARSE_TOKEN_FAILED_MSG, 1), ce.getMessage());
             assertTrue(ce.getCause() instanceof IllegalArgumentException);
@@ -108,19 +108,6 @@ public class PlainConfigurationTest extends TestCase
         }
     }
 
-    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
@@ -128,7 +115,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL ALLOW");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_ACL_MSG, 1), ce.getMessage());
         }
@@ -141,7 +128,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("CONFIG");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_TOKENS_MSG, 1), ce.getMessage());
         }
@@ -154,7 +141,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("INVALID");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.NOT_ENOUGH_TOKENS_MSG, 1), ce.getMessage());
         }
@@ -167,7 +154,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL ALLOW adk CREATE QUEUE name");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.PROPERTY_KEY_ONLY_MSG, 1), ce.getMessage());
         }
@@ -180,7 +167,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL ALLOW adk CREATE QUEUE name test");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.PROPERTY_NO_EQUALS_MSG, 1), ce.getMessage());
         }
@@ -193,7 +180,7 @@ public class PlainConfigurationTest extends TestCase
             writeACLConfig("ACL ALLOW adk CREATE QUEUE name =");
             fail("fail");
         }
-        catch (ConfigurationException ce)
+        catch (IllegalConfigurationException ce)
         {
             assertEquals(String.format(PlainConfiguration.PROPERTY_NO_VALUE_MSG, 1), ce.getMessage());
         }
@@ -391,4 +378,86 @@ public class PlainConfigurationTest extends TestCase
         assertEquals("Rule has unexpected object properties", ObjectProperties.EMPTY, rule.getAction().getProperties());
     }
 
+    public void testUserRuleParsing() throws Exception
+    {
+        validateRule(writeACLConfig("ACL ALLOW user1 CREATE USER"),
+                "user1", Operation.CREATE, ObjectType.USER, ObjectProperties.EMPTY);
+        validateRule(writeACLConfig("ACL ALLOW user1 CREATE USER name=\"otherUser\""),
+                "user1", Operation.CREATE, ObjectType.USER, new ObjectProperties("otherUser"));
+
+        validateRule(writeACLConfig("ACL ALLOW user1 DELETE USER"),
+                "user1", Operation.DELETE, ObjectType.USER, ObjectProperties.EMPTY);
+        validateRule(writeACLConfig("ACL ALLOW user1 DELETE USER name=\"otherUser\""),
+                "user1", Operation.DELETE, ObjectType.USER, new ObjectProperties("otherUser"));
+
+        validateRule(writeACLConfig("ACL ALLOW user1 UPDATE USER"),
+                "user1", Operation.UPDATE, ObjectType.USER, ObjectProperties.EMPTY);
+        validateRule(writeACLConfig("ACL ALLOW user1 UPDATE USER name=\"otherUser\""),
+                "user1", Operation.UPDATE, ObjectType.USER, new ObjectProperties("otherUser"));
+
+        validateRule(writeACLConfig("ACL ALLOW user1 ALL USER"),
+                "user1", Operation.ALL, ObjectType.USER, ObjectProperties.EMPTY);
+        validateRule(writeACLConfig("ACL ALLOW user1 ALL USER name=\"otherUser\""),
+                "user1", Operation.ALL, ObjectType.USER, new ObjectProperties("otherUser"));
+    }
+
+    public void testGroupRuleParsing() throws Exception
+    {
+        validateRule(writeACLConfig("ACL ALLOW user1 CREATE GROUP"),
+                "user1", Operation.CREATE, ObjectType.GROUP, ObjectProperties.EMPTY);
+        validateRule(writeACLConfig("ACL ALLOW user1 CREATE GROUP name=\"groupName\""),
+                "user1", Operation.CREATE, ObjectType.GROUP, new ObjectProperties("groupName"));
+
+        validateRule(writeACLConfig("ACL ALLOW user1 DELETE GROUP"),
+                "user1", Operation.DELETE, ObjectType.GROUP, ObjectProperties.EMPTY);
+        validateRule(writeACLConfig("ACL ALLOW user1 DELETE GROUP name=\"groupName\""),
+                "user1", Operation.DELETE, ObjectType.GROUP, new ObjectProperties("groupName"));
+
+        validateRule(writeACLConfig("ACL ALLOW user1 UPDATE GROUP"),
+                "user1", Operation.UPDATE, ObjectType.GROUP, ObjectProperties.EMPTY);
+        validateRule(writeACLConfig("ACL ALLOW user1 UPDATE GROUP name=\"groupName\""),
+                "user1", Operation.UPDATE, ObjectType.GROUP, new ObjectProperties("groupName"));
+
+        validateRule(writeACLConfig("ACL ALLOW user1 ALL GROUP"),
+                "user1", Operation.ALL, ObjectType.GROUP, ObjectProperties.EMPTY);
+        validateRule(writeACLConfig("ACL ALLOW user1 ALL GROUP name=\"groupName\""),
+                "user1", Operation.ALL, ObjectType.GROUP, new ObjectProperties("groupName"));
+    }
+
+    /** explicitly test for exception indicating that this functionality has been moved to Group Providers */
+    public void testGroupDefinitionThrowsException() throws Exception
+    {
+        try
+        {
+            writeACLConfig("GROUP group1 bob alice");
+            fail("Expected exception not thrown");
+        }
+        catch(IllegalConfigurationException e)
+        {
+            assertTrue(e.getMessage().contains("GROUP keyword not supported"));
+        }
+    }
+
+    public void testManagementRuleParsing() throws Exception
+    {
+        validateRule(writeACLConfig("ACL ALLOW user1 ALL MANAGEMENT"),
+                "user1", Operation.ALL, ObjectType.MANAGEMENT, ObjectProperties.EMPTY);
+
+        validateRule(writeACLConfig("ACL ALLOW user1 ACCESS MANAGEMENT"),
+                "user1", Operation.ACCESS, ObjectType.MANAGEMENT, ObjectProperties.EMPTY);
+    }
+
+    private void validateRule(final PlainConfiguration config, String username, Operation operation, ObjectType objectType, ObjectProperties objectProperties)
+    {
+        final RuleSet rs = config.getConfiguration();
+        assertEquals(1, rs.getRuleCount());
+
+        final Map<Integer, Rule> rules = rs.getAllRules();
+        assertEquals(1, rules.size());
+        final Rule rule = rules.get(0);
+        assertEquals("Rule has unexpected identity", username, rule.getIdentity());
+        assertEquals("Rule has unexpected operation", operation, rule.getAction().getOperation());
+        assertEquals("Rule has unexpected operation", objectType, rule.getAction().getObjectType());
+        assertEquals("Rule has unexpected object properties", objectProperties, rule.getAction().getProperties());
+    }
 }
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleTest.java
new file mode 100644
index 0000000000..2ae7759679
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleTest.java
@@ -0,0 +1,53 @@
+/*
+ * 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.config;
+
+import static org.mockito.Mockito.*;
+
+import org.apache.qpid.server.security.access.Permission;
+
+import junit.framework.TestCase;
+
+public class RuleTest extends TestCase
+{
+    public void testEqualsAndHashCode()
+    {
+        AclAction aclAction = mock(AclAction.class);
+        String identity = "identity";
+        Permission allow = Permission.ALLOW;
+
+        Rule rule = new Rule(identity, aclAction, allow);
+        Rule equalRule = new Rule(identity, aclAction, allow);
+
+        assertTrue(rule.equals(rule));
+        assertTrue(rule.equals(equalRule));
+        assertTrue(equalRule.equals(rule));
+
+        assertTrue(rule.hashCode() == equalRule.hashCode());
+
+        assertFalse("Different identity should cause rules to be unequal",
+                rule.equals(new Rule("identity2", aclAction, allow)));
+
+        assertFalse("Different action should cause rules to be unequal",
+                rule.equals(new Rule(identity, mock(AclAction.class), allow)));
+
+        assertFalse("Different permission should cause rules to be unequal",
+                rule.equals(new Rule(identity, aclAction, Permission.DENY)));
+    }
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRuleTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRuleTest.java
new file mode 100644
index 0000000000..be82cb294a
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRuleTest.java
@@ -0,0 +1,99 @@
+/*
+ * 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.firewall;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.net.InetAddress;
+
+import org.apache.qpid.server.security.access.firewall.HostnameFirewallRule;
+
+import junit.framework.TestCase;
+
+public class HostnameFirewallRuleTest extends TestCase
+{
+    private InetAddress _addressNotInRule;
+
+    private HostnameFirewallRule _HostnameFirewallRule;
+
+    @Override
+    protected void setUp() throws Exception
+    {
+        _addressNotInRule = InetAddress.getByName("127.0.0.1");
+    }
+
+    public void testSingleHostname() throws Exception
+    {
+        String hostnameInRule = "hostnameInRule";
+        InetAddress addressWithMatchingHostname = mock(InetAddress.class);
+        when(addressWithMatchingHostname.getCanonicalHostName()).thenReturn(hostnameInRule);
+
+        _HostnameFirewallRule = new HostnameFirewallRule(hostnameInRule);
+
+        assertFalse(_HostnameFirewallRule.matches(_addressNotInRule));
+        assertTrue(_HostnameFirewallRule.matches(addressWithMatchingHostname));
+    }
+
+    public void testSingleHostnameWilcard() throws Exception
+    {
+        String hostnameInRule = ".*FOO.*";
+        InetAddress addressWithMatchingHostname = mock(InetAddress.class);
+        when(addressWithMatchingHostname.getCanonicalHostName()).thenReturn("xxFOOxx");
+
+        _HostnameFirewallRule = new HostnameFirewallRule(hostnameInRule);
+
+        assertFalse(_HostnameFirewallRule.matches(_addressNotInRule));
+        assertTrue(_HostnameFirewallRule.matches(addressWithMatchingHostname));
+    }
+
+    public void testMultipleHostnames() throws Exception
+    {
+        String[] hostnamesInRule = new String[] {"hostnameInRule1", "hostnameInRule2"};
+
+        _HostnameFirewallRule = new HostnameFirewallRule(hostnamesInRule);
+
+        assertFalse(_HostnameFirewallRule.matches(_addressNotInRule));
+        for (String hostnameInRule : hostnamesInRule)
+        {
+            InetAddress addressWithMatchingHostname = mock(InetAddress.class);
+            when(addressWithMatchingHostname.getCanonicalHostName()).thenReturn(hostnameInRule);
+
+            assertTrue(_HostnameFirewallRule.matches(addressWithMatchingHostname));
+        }
+    }
+
+    public void testEqualsAndHashCode()
+    {
+        String hostname1 = "hostname1";
+        String hostname2 = "hostname2";
+
+        HostnameFirewallRule rule = new HostnameFirewallRule(hostname1, hostname2);
+        HostnameFirewallRule equalRule = new HostnameFirewallRule(hostname1, hostname2);
+
+        assertTrue(rule.equals(rule));
+        assertTrue(rule.equals(equalRule));
+        assertTrue(equalRule.equals(rule));
+
+        assertTrue(rule.hashCode() == equalRule.hashCode());
+
+        assertFalse("Different hostnames should cause rules to be unequal",
+                rule.equals(new HostnameFirewallRule(hostname1, "different-hostname")));
+    }
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRuleTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRuleTest.java
new file mode 100644
index 0000000000..e521039db2
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRuleTest.java
@@ -0,0 +1,115 @@
+/*
+ * 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.firewall;
+
+import java.net.InetAddress;
+
+import org.apache.qpid.server.security.access.firewall.NetworkFirewallRule;
+
+import junit.framework.TestCase;
+
+public class NetworkFirewallRuleTest extends TestCase
+{
+    private static final String LOCALHOST_IP = "127.0.0.1";
+    private static final String OTHER_IP_1 = "192.168.23.1";
+    private static final String OTHER_IP_2 = "192.168.23.2";
+
+    private InetAddress _addressNotInRule;
+
+    private NetworkFirewallRule _networkFirewallRule;
+
+    @Override
+    protected void setUp() throws Exception
+    {
+        _addressNotInRule = InetAddress.getByName(LOCALHOST_IP);
+    }
+
+    public void testIpRule() throws Exception
+    {
+        String ipAddressInRule = OTHER_IP_1;
+
+        _networkFirewallRule = new NetworkFirewallRule(ipAddressInRule);
+
+        assertFalse(_networkFirewallRule.matches(_addressNotInRule));
+        assertTrue(_networkFirewallRule.matches(InetAddress.getByName(ipAddressInRule)));
+    }
+
+    public void testNetMask() throws Exception
+    {
+        String ipAddressInRule = "192.168.23.0/24";
+        _networkFirewallRule = new NetworkFirewallRule(ipAddressInRule);
+
+        assertFalse(_networkFirewallRule.matches(InetAddress.getByName("192.168.24.1")));
+        assertTrue(_networkFirewallRule.matches(InetAddress.getByName("192.168.23.0")));
+        assertTrue(_networkFirewallRule.matches(InetAddress.getByName("192.168.23.255")));
+    }
+
+    public void testWildcard() throws Exception
+    {
+        // Test xxx.xxx.*
+
+        assertFalse(new NetworkFirewallRule("192.168.*")
+             .matches(InetAddress.getByName("192.169.1.0")));
+
+        assertTrue(new NetworkFirewallRule("192.168.*")
+            .matches(InetAddress.getByName("192.168.1.0")));
+
+        assertTrue(new NetworkFirewallRule("192.168.*")
+            .matches(InetAddress.getByName("192.168.255.255")));
+
+        // Test xxx.xxx.xxx.*
+
+        assertFalse(new NetworkFirewallRule("192.168.1.*")
+             .matches(InetAddress.getByName("192.169.2.0")));
+
+        assertTrue(new NetworkFirewallRule("192.168.1.*")
+            .matches(InetAddress.getByName("192.168.1.0")));
+
+        assertTrue(new NetworkFirewallRule("192.168.1.*")
+            .matches(InetAddress.getByName("192.168.1.255")));
+    }
+
+    public void testMultipleNetworks() throws Exception
+    {
+        String[] ipAddressesInRule = new String[] {OTHER_IP_1, OTHER_IP_2};
+
+        _networkFirewallRule = new NetworkFirewallRule(ipAddressesInRule);
+
+        assertFalse(_networkFirewallRule.matches(_addressNotInRule));
+        for (String ipAddressInRule : ipAddressesInRule)
+        {
+            assertTrue(_networkFirewallRule.matches(InetAddress.getByName(ipAddressInRule)));
+        }
+    }
+
+    public void testEqualsAndHashCode()
+    {
+        NetworkFirewallRule rule = new NetworkFirewallRule(LOCALHOST_IP, OTHER_IP_1);
+        NetworkFirewallRule equalRule = new NetworkFirewallRule(LOCALHOST_IP, OTHER_IP_1);
+
+        assertTrue(rule.equals(rule));
+        assertTrue(rule.equals(equalRule));
+        assertTrue(equalRule.equals(rule));
+
+        assertTrue(rule.hashCode() == equalRule.hashCode());
+
+        assertFalse("Different networks should cause rules to be unequal",
+                rule.equals(new NetworkFirewallRule(LOCALHOST_IP, OTHER_IP_2)));
+    }
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactoryTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactoryTest.java
new file mode 100644
index 0000000000..ca1f19098f
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactoryTest.java
@@ -0,0 +1,69 @@
+package org.apache.qpid.server.security.access.plugins;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
+import org.apache.qpid.server.security.AccessControl;
+import org.apache.qpid.test.utils.QpidTestCase;
+import org.apache.qpid.test.utils.TestFileUtils;
+
+public class DefaultAccessControlFactoryTest extends QpidTestCase
+{
+    public void testCreateInstanceWhenAclFileIsNotPresent()
+    {
+        DefaultAccessControlFactory factory = new DefaultAccessControlFactory();
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        AccessControl acl = factory.createInstance(attributes);
+        assertNull("ACL was created without a configuration file", acl);
+    }
+
+    public void testCreateInstanceWhenAclFileIsSpecified()
+    {
+        File aclFile = TestFileUtils.createTempFile(this, ".acl", "ACL ALLOW all all");
+        DefaultAccessControlFactory factory = new DefaultAccessControlFactory();
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        attributes.put(DefaultAccessControlFactory.ATTRIBUTE_ACL_FILE, aclFile.getAbsolutePath());
+        AccessControl acl = factory.createInstance(attributes);
+
+        assertNotNull("ACL was not created from acl file: " + aclFile.getAbsolutePath(), acl);
+    }
+
+    public void testCreateInstanceWhenAclFileIsSpecifiedButDoesNotExist()
+    {
+        File aclFile = new File(TMP_FOLDER, "my-non-existing-acl-" + System.currentTimeMillis());
+        assertFalse("ACL file " + aclFile.getAbsolutePath() + " actually exists but should not", aclFile.exists());
+        DefaultAccessControlFactory factory = new DefaultAccessControlFactory();
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        attributes.put(DefaultAccessControlFactory.ATTRIBUTE_ACL_FILE, aclFile.getAbsolutePath());
+        try
+        {
+            factory.createInstance(attributes);
+            fail("It should not be possible to create ACL from non existing file");
+        }
+        catch (IllegalConfigurationException e)
+        {
+            assertTrue("Unexpected exception message", Pattern.matches("ACL file '.*' is not found", e.getMessage()));
+        }
+    }
+
+    public void testCreateInstanceWhenAclFileIsSpecifiedAsNonString()
+    {
+        DefaultAccessControlFactory factory = new DefaultAccessControlFactory();
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        Integer aclFile = new Integer(0);
+        attributes.put(DefaultAccessControlFactory.ATTRIBUTE_ACL_FILE, aclFile);
+        try
+        {
+            factory.createInstance(attributes);
+            fail("It should not be possible to create ACL from Integer");
+        }
+        catch (IllegalConfigurationException e)
+        {
+            assertEquals("Unexpected exception message", "Expected '" + DefaultAccessControlFactory.ATTRIBUTE_ACL_FILE
+                    + "' attribute value of type String but was " + Integer.class + ": " + aclFile, e.getMessage());
+        }
+    }
+}
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/DefaultAccessControlTest.java
index 5db02d10ce..a8406308c0 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/DefaultAccessControlTest.java
@@ -20,12 +20,16 @@
  */
 package org.apache.qpid.server.security.access.plugins;
 
-import java.util.Arrays;
+import static org.mockito.Mockito.*;
+
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+
+import javax.security.auth.Subject;
 
 import junit.framework.TestCase;
 
 import org.apache.commons.configuration.ConfigurationException;
-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;
@@ -37,20 +41,19 @@ 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.TestPrincipalUtils;
 
 /**
- * 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.
- * 
+ * In these tests, the ruleset is configured programmatically rather than from an external file.
+ *
  * @see RuleSetTest
  */
-public class AccessControlTest extends TestCase
+public class DefaultAccessControlTest extends TestCase
 {
-    private AccessControl _plugin = null;  // Class under test
+    private static final String ALLOWED_GROUP = "allowed_group";
+    private static final String DENIED_GROUP = "denied_group";
+
+    private DefaultAccessControl _plugin = null;  // Class under test
     private final UnitTestMessageLogger messageLogger = new UnitTestMessageLogger();
 
     private void setUpGroupAccessControl() throws ConfigurationException
@@ -60,7 +63,7 @@ public class AccessControlTest extends TestCase
 
     private void configureAccessControl(final RuleSet rs) throws ConfigurationException
     {
-        _plugin = (AccessControl) AccessControl.FACTORY.newInstance(createConfiguration(rs));
+        _plugin = new DefaultAccessControl(rs);
         SecurityManager.setThreadSubject(null);
         CurrentActor.set(new TestLogActor(messageLogger));
     }
@@ -68,14 +71,12 @@ public class AccessControlTest extends TestCase
     private RuleSet createGroupRuleSet()
     {
         final RuleSet rs = new RuleSet();
-        rs.addGroup("aclGroup1", Arrays.asList(new String[] {"member1", "Member2"}));
 
         // 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);
+        // Rules expressed with groups
+        rs.grant(1, ALLOWED_GROUP, Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        rs.grant(2, DENIED_GROUP, 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);
 
@@ -117,31 +118,23 @@ public class AccessControlTest extends TestCase
      * 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() throws ConfigurationException
+    public void testGroupMembershipAllowsOperation() throws ConfigurationException
     {
         setUpGroupAccessControl();
-        SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("member1"));
 
-        Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
-        assertEquals(Result.ALLOWED, result);
-
-        SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("Member2"));
-
-        result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
-        assertEquals(Result.ALLOWED, result);
+        authoriseAndAssertResult(Result.ALLOWED, "member of allowed group", ALLOWED_GROUP);
+        authoriseAndAssertResult(Result.DENIED, "member of denied group", DENIED_GROUP);
+        authoriseAndAssertResult(Result.ALLOWED, "another member of allowed group", ALLOWED_GROUP);
     }
 
     /**
-     * Tests that a deny rule expressed with an <b>External groupname</b> denies an operation performed by a thread running
+     * Tests that a deny rule expressed with a <b>groupname</b> denies an operation performed by a thread running
      * by a user who belongs to the same group.
      */
-    public void testExternalGroupMembershipDeniesOperation() throws ConfigurationException
+    public void testGroupMembershipDeniesOperation() throws ConfigurationException
     {
         setUpGroupAccessControl();
-        SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("user3", "extGroup1"));
-
-        final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
-        assertEquals(Result.DENIED, result);
+        authoriseAndAssertResult(Result.DENIED, "user3", DENIED_GROUP);
     }
 
     /**
@@ -203,6 +196,46 @@ public class AccessControlTest extends TestCase
         assertEquals(Result.DEFER, result);
     }
 
+    public void testAccess() throws Exception
+    {
+        Subject subject = TestPrincipalUtils.createTestSubject("user1");
+        SecurityManager.setThreadSubject(subject);
+
+        RuleSet mockRuleSet = mock(RuleSet.class);
+
+        InetAddress inetAddress = InetAddress.getLocalHost();
+        InetSocketAddress inetSocketAddress = new InetSocketAddress(inetAddress, 1);
+
+        DefaultAccessControl accessControl = new DefaultAccessControl(mockRuleSet);
+
+        accessControl.access(ObjectType.VIRTUALHOST, inetSocketAddress);
+
+        verify(mockRuleSet).check(subject, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY, inetAddress);
+    }
+
+    public void testAccessIsDeniedIfRuleThrowsException() throws Exception
+    {
+        Subject subject = TestPrincipalUtils.createTestSubject("user1");
+        SecurityManager.setThreadSubject(subject);
+
+        InetAddress inetAddress = InetAddress.getLocalHost();
+        InetSocketAddress inetSocketAddress = new InetSocketAddress(inetAddress, 1);
+
+        RuleSet mockRuleSet = mock(RuleSet.class);
+        when(mockRuleSet.check(
+                subject,
+                Operation.ACCESS,
+                ObjectType.VIRTUALHOST,
+                ObjectProperties.EMPTY,
+                inetAddress)).thenThrow(new RuntimeException());
+
+        DefaultAccessControl accessControl = new DefaultAccessControl(mockRuleSet);
+        Result result = accessControl.access(ObjectType.VIRTUALHOST, inetSocketAddress);
+
+        assertEquals(Result.DENIED, result);
+    }
+
+
     /**
      * Tests that a grant access method rule allows any access operation to be performed on a specified component
      */
@@ -325,31 +358,11 @@ public class AccessControlTest extends TestCase
         assertEquals(Result.DEFER, result);
     }
 
-    /**
-     * Creates a configuration plugin for the {@link AccessControl} plugin.
-     */
-    private ConfigurationPlugin createConfiguration(final RuleSet rs)
+    private void authoriseAndAssertResult(Result expectedResult, String userName, String... groups)
     {
-        final ConfigurationPlugin cp = new ConfigurationPlugin()
-        {
-            @SuppressWarnings("unchecked")
-            public AccessControlConfiguration  getConfiguration(final String plugin)
-            {
-                return new AccessControlConfiguration()
-                {
-                    public RuleSet getRuleSet()
-                    {
-                        return rs;
-                    }
-                };
-            }
-
-            public String[] getElementsProcessed()
-            {
-                throw new UnsupportedOperationException();
-            }
-        };
-
-        return cp;
+        SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject(userName, groups));
+
+        Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(expectedResult, result);
     }
 }
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 f7cc60543d..181d693614 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
@@ -22,7 +22,6 @@
 package org.apache.qpid.server.security.access.plugins;
 
 import java.security.Principal;
-import java.util.Arrays;
 
 import javax.security.auth.Subject;
 
@@ -34,8 +33,7 @@ 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.server.security.auth.TestPrincipalUtils;
 import org.apache.qpid.test.utils.QpidTestCase;
 
 /**
@@ -46,10 +44,7 @@ import org.apache.qpid.test.utils.QpidTestCase;
  * access control mechanism is validated by checking whether operations would be authorised by calling the
  * {@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).
-
+ * It ensure that permissions can be granted correctly on users directly and on groups.
  */
 public class RuleSetTest extends QpidTestCase
 {
@@ -316,63 +311,36 @@ public class RuleSetTest extends QpidTestCase
         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()
+    public void testGroupsSupported()
     {
-        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());
+        String allowGroup = "allowGroup";
+        String deniedGroup = "deniedGroup";
 
-        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));
-    }
+        _ruleSet.grant(1, allowGroup, Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        _ruleSet.grant(2, deniedGroup, Permission.DENY, 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));
+        assertEquals(Result.ALLOWED, _ruleSet.check(TestPrincipalUtils.createTestSubject("usera", allowGroup),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+        assertEquals(Result.DENIED, _ruleSet.check(TestPrincipalUtils.createTestSubject("userb", deniedGroup),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+        assertEquals(Result.DEFER, _ruleSet.check(TestPrincipalUtils.createTestSubject("user", "group not mentioned in acl"),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
+     * granted explicit permission on an object, is granted that access even though a group
      * to which the user belongs is later denied the permission.
      */
     public void testAllowDeterminedByRuleOrder()
     {
-        assertTrue(_ruleSet.addGroup("aclgroup", Arrays.asList(new String[] {"usera"})));
+        String group = "group";
+        String user = "user";
 
-        _ruleSet.grant(1, "usera", Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
-        _ruleSet.grant(2, "aclgroup", Permission.DENY, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        _ruleSet.grant(1, user, Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        _ruleSet.grant(2, group, 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));
+        assertEquals(Result.ALLOWED, _ruleSet.check(TestPrincipalUtils.createTestSubject(user, group),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
     }
 
     /**
@@ -381,13 +349,33 @@ public class RuleSetTest extends QpidTestCase
      */
     public void testDenyDeterminedByRuleOrder()
     {
-        assertTrue(_ruleSet.addGroup("aclgroup", Arrays.asList(new String[] {"usera"})));
+        String group = "aclgroup";
+        String user = "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);
+        _ruleSet.grant(1, group, Permission.DENY, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        _ruleSet.grant(2, user, 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));
+        assertEquals(Result.DENIED, _ruleSet.check(TestPrincipalUtils.createTestSubject(user, group),Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+    }
+
+    public void testUserInMultipleGroups()
+    {
+        String allowedGroup = "group1";
+        String deniedGroup = "group2";
+
+        _ruleSet.grant(1, allowedGroup, Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        _ruleSet.grant(2, deniedGroup, Permission.DENY, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+
+        Subject subjectInBothGroups = TestPrincipalUtils.createTestSubject("user", allowedGroup, deniedGroup);
+        Subject subjectInDeniedGroupAndOneOther = TestPrincipalUtils.createTestSubject("user", deniedGroup, "some other group");
+        Subject subjectInAllowedGroupAndOneOther = TestPrincipalUtils.createTestSubject("user", allowedGroup, "some other group");
+
+        assertEquals(Result.ALLOWED, _ruleSet.check(subjectInBothGroups,Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+
+        assertEquals(Result.DENIED, _ruleSet.check(subjectInDeniedGroupAndOneOther,Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
+
+        assertEquals(Result.ALLOWED, _ruleSet.check(subjectInAllowedGroupAndOneOther,Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
     }
 }