QPID-3703: ACL test improvements

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

4 files changed, 213 insertions, 177 deletions

diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java
index 94f05ed265..402b991419 100644
--- a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java
@@ -21,6 +21,7 @@ package org.apache.qpid.server.security.access.config;
 import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.EnumMap;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -149,7 +150,6 @@ public class RuleSet
         return rules;
     }
 
-
     public boolean isValidNumber(Integer number)
     {
         return !_rules.containsKey(number);
@@ -425,6 +425,15 @@ public class RuleSet
         _config.put(key, value);
     }
 
+     /**
+      * Returns all rules in the {@link RuleSet}.   Primarily intended to support unit-testing.
+      * @return map of rules
+      */
+     public Map<Integer, Rule> getAllRules()
+     {
+         return Collections.unmodifiableMap(_rules);
+     }
+
     private boolean isRelevant(final Set<Principal> principals, final Rule rule)
     {
         if (rule.getIdentity().equalsIgnoreCase(Rule.ALL))
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
index e16f9943ba..aa3982df71 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/plugins/PlainConfigurationTest.java
@@ -22,12 +22,19 @@ import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.FileWriter;
 import java.io.PrintWriter;
+import java.util.Map;
 
 import junit.framework.TestCase;
 
 import org.apache.commons.configuration.ConfigurationException;
+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;
+import org.apache.qpid.server.security.access.Operation;
 import org.apache.qpid.server.security.access.config.ConfigurationFile;
 import org.apache.qpid.server.security.access.config.PlainConfiguration;
+import org.apache.qpid.server.security.access.config.Rule;
+import org.apache.qpid.server.security.access.config.RuleSet;
 
 /**
  * These tests check that the ACL file parsing works correctly.
@@ -37,7 +44,7 @@ import org.apache.qpid.server.security.access.config.PlainConfiguration;
  */
 public class PlainConfigurationTest extends TestCase
 {
-    public void writeACLConfig(String...aclData) throws Exception
+    private PlainConfiguration writeACLConfig(String...aclData) throws Exception
     {
         File acl = File.createTempFile(getClass().getName() + getName(), "acl");
         acl.deleteOnExit();
@@ -51,8 +58,9 @@ public class PlainConfigurationTest extends TestCase
         aclWriter.close();
 
         // Load ruleset
-        ConfigurationFile configFile = new PlainConfiguration(acl);
+        PlainConfiguration configFile = new PlainConfiguration(acl);
         configFile.load();
+        return configFile;
     }
 
     public void testMissingACLConfig() throws Exception
@@ -191,4 +199,197 @@ public class PlainConfigurationTest extends TestCase
             assertEquals(String.format(PlainConfiguration.PROPERTY_NO_VALUE_MSG, 1), ce.getMessage());
         }
     }
+
+    /**
+     * Tests interpretation of an acl rule with no object properties.
+     *
+     */
+    public void testValidRule() throws Exception
+    {
+        final PlainConfiguration config = writeACLConfig("ACL DENY-LOG user1 ACCESS VIRTUALHOST");
+        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", "user1", rule.getIdentity());
+        assertEquals("Rule has unexpected operation", Operation.ACCESS, rule.getAction().getOperation());
+        assertEquals("Rule has unexpected operation", ObjectType.VIRTUALHOST, rule.getAction().getObjectType());
+        assertEquals("Rule has unexpected object properties", ObjectProperties.EMPTY, rule.getAction().getProperties());
+    }
+
+    /**
+     * Tests interpretation of an acl rule with object properties quoted in single quotes.
+     */
+    public void testValidRuleWithSingleQuotedProperty() throws Exception
+    {
+        final PlainConfiguration config = writeACLConfig("ACL ALLOW all CREATE EXCHANGE name = \'value\'");
+        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", "all", rule.getIdentity());
+        assertEquals("Rule has unexpected operation", Operation.CREATE, rule.getAction().getOperation());
+        assertEquals("Rule has unexpected operation", ObjectType.EXCHANGE, rule.getAction().getObjectType());
+        final ObjectProperties expectedProperties = new ObjectProperties();
+        expectedProperties.setName("value");
+        assertEquals("Rule has unexpected object properties", expectedProperties, rule.getAction().getProperties());
+    }
+
+    /**
+     * Tests interpretation of an acl rule with object properties quoted in double quotes.
+     */
+    public void testValidRuleWithDoubleQuotedProperty() throws Exception
+    {
+        final PlainConfiguration config = writeACLConfig("ACL ALLOW all CREATE EXCHANGE name = \"value\"");
+        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", "all", rule.getIdentity());
+        assertEquals("Rule has unexpected operation", Operation.CREATE, rule.getAction().getOperation());
+        assertEquals("Rule has unexpected operation", ObjectType.EXCHANGE, rule.getAction().getObjectType());
+        final ObjectProperties expectedProperties = new ObjectProperties();
+        expectedProperties.setName("value");
+        assertEquals("Rule has unexpected object properties", expectedProperties, rule.getAction().getProperties());
+    }
+
+    /**
+     * Tests interpretation of an acl rule with many object properties.
+     */
+    public void testValidRuleWithManyProperties() throws Exception
+    {
+        final PlainConfiguration config = writeACLConfig("ACL ALLOW admin DELETE QUEUE name=name1 owner = owner1");
+        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", "admin", rule.getIdentity());
+        assertEquals("Rule has unexpected operation", Operation.DELETE, rule.getAction().getOperation());
+        assertEquals("Rule has unexpected operation", ObjectType.QUEUE, rule.getAction().getObjectType());
+        final ObjectProperties expectedProperties = new ObjectProperties();
+        expectedProperties.setName("name1");
+        expectedProperties.put(Property.OWNER, "owner1");
+        assertEquals("Rule has unexpected operation", expectedProperties, rule.getAction().getProperties());
+    }
+
+    /**
+     * Tests interpretation of an acl rule with object properties containing wildcards.  Values containing
+     * hashes must be quoted otherwise they are interpreted as comments.
+     */
+    public void testValidRuleWithWildcardProperties() throws Exception
+    {
+        final PlainConfiguration config = writeACLConfig("ACL ALLOW all CREATE EXCHANGE routingKey = \'news.#\'",
+                                                         "ACL ALLOW all CREATE EXCHANGE routingKey = \'news.co.#\'",
+                                                         "ACL ALLOW all CREATE EXCHANGE routingKey = *.co.medellin");
+        final RuleSet rs = config.getConfiguration();
+        assertEquals(3, rs.getRuleCount());
+
+        final Map<Integer, Rule> rules = rs.getAllRules();
+        assertEquals(3, rules.size());
+        final Rule rule1 = rules.get(0);
+        assertEquals("Rule has unexpected identity", "all", rule1.getIdentity());
+        assertEquals("Rule has unexpected operation", Operation.CREATE, rule1.getAction().getOperation());
+        assertEquals("Rule has unexpected operation", ObjectType.EXCHANGE, rule1.getAction().getObjectType());
+        final ObjectProperties expectedProperties1 = new ObjectProperties();
+        expectedProperties1.put(Property.ROUTING_KEY,"news.#");
+        assertEquals("Rule has unexpected object properties", expectedProperties1, rule1.getAction().getProperties());
+
+        final Rule rule2 = rules.get(10);
+        final ObjectProperties expectedProperties2 = new ObjectProperties();
+        expectedProperties2.put(Property.ROUTING_KEY,"news.co.#");
+        assertEquals("Rule has unexpected object properties", expectedProperties2, rule2.getAction().getProperties());
+
+        final Rule rule3 = rules.get(20);
+        final ObjectProperties expectedProperties3 = new ObjectProperties();
+        expectedProperties3.put(Property.ROUTING_KEY,"*.co.medellin");
+        assertEquals("Rule has unexpected object properties", expectedProperties3, rule3.getAction().getProperties());
+    }
+
+    /**
+     * Tests that rules are case insignificant.
+     */
+    public void testMixedCaseRuleInterpretation() throws Exception
+    {
+        final PlainConfiguration config = writeACLConfig("AcL deny-LOG user1 BiND Exchange name=AmQ.dIrect");
+        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", "user1", rule.getIdentity());
+        assertEquals("Rule has unexpected operation", Operation.BIND, rule.getAction().getOperation());
+        assertEquals("Rule has unexpected operation", ObjectType.EXCHANGE, rule.getAction().getObjectType());
+        final ObjectProperties expectedProperties = new ObjectProperties("amq.direct");
+        assertEquals("Rule has unexpected object properties", expectedProperties, rule.getAction().getProperties());
+    }
+
+    /**
+     * Tests whitespace is supported. Note that currently the Java implementation permits comments to
+     * be introduced anywhere in the ACL, whereas the C++ supports only whitespace at the beginning of
+     * of line.
+     */
+    public void testCommentsSuppported() throws Exception
+    {
+        final PlainConfiguration config = writeACLConfig("#Comment",
+                                                         "ACL DENY-LOG user1 ACCESS VIRTUALHOST # another comment",
+                                                         "  # final comment with leading whitespace");
+        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", "user1", rule.getIdentity());
+        assertEquals("Rule has unexpected operation", Operation.ACCESS, rule.getAction().getOperation());
+        assertEquals("Rule has unexpected operation", ObjectType.VIRTUALHOST, rule.getAction().getObjectType());
+        assertEquals("Rule has unexpected object properties", ObjectProperties.EMPTY, rule.getAction().getProperties());
+    }
+
+    /**
+     * Tests interpretation of an acl rule using mixtures of tabs/spaces as token separators.
+     *
+     */
+    public void testWhitespace() throws Exception
+    {
+        final PlainConfiguration config = writeACLConfig("ACL\tDENY-LOG\t\t user1\t \tACCESS VIRTUALHOST");
+        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", "user1", rule.getIdentity());
+        assertEquals("Rule has unexpected operation", Operation.ACCESS, rule.getAction().getOperation());
+        assertEquals("Rule has unexpected operation", ObjectType.VIRTUALHOST, rule.getAction().getObjectType());
+        assertEquals("Rule has unexpected object properties", ObjectProperties.EMPTY, rule.getAction().getProperties());
+    }
+
+    /**
+     * Tests interpretation of an acl utilising line continuation.
+     */
+    public void testLineContination() throws Exception
+    {
+        final PlainConfiguration config = writeACLConfig("ACL DENY-LOG user1 \\",
+                                                         "ACCESS VIRTUALHOST");
+        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", "user1", rule.getIdentity());
+        assertEquals("Rule has unexpected operation", Operation.ACCESS, rule.getAction().getOperation());
+        assertEquals("Rule has unexpected operation", ObjectType.VIRTUALHOST, rule.getAction().getObjectType());
+        assertEquals("Rule has unexpected object properties", ObjectProperties.EMPTY, rule.getAction().getProperties());
+    }
+
 }
diff --git a/java/systests/src/main/java/org/apache/qpid/server/security/acl/AbstractACLTestCase.java b/java/systests/src/main/java/org/apache/qpid/server/security/acl/AbstractACLTestCase.java
index ec8d837ee8..262051ff89 100644
--- a/java/systests/src/main/java/org/apache/qpid/server/security/acl/AbstractACLTestCase.java
+++ b/java/systests/src/main/java/org/apache/qpid/server/security/acl/AbstractACLTestCase.java
@@ -51,7 +51,6 @@ import org.apache.qpid.url.URLSyntaxException;
  * TODO move the pre broker-startup setup method invocation code to {@link QpidBrokerTestCase}
  * 
  * @see ExternalACLTest
- * @see ExternalACLFileTest
  * @see ExternalACLJMXTest
  * @see ExhaustiveACLTest
  */
diff --git a/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalACLFileTest.java b/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalACLFileTest.java
deleted file mode 100644
index 5ab2fede83..0000000000
--- a/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalACLFileTest.java
+++ /dev/null
@@ -1,173 +0,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.
- */
-package org.apache.qpid.server.security.acl;
-
-import javax.jms.Connection;
-import javax.jms.Session;
-
-import org.apache.qpid.client.AMQSession;
-import org.apache.qpid.framing.AMQShortString;
-
-/**
- * Tests that ACL version 2/3 files following the specification work correctly.
- * 
- * ACL lines that are identical in meaning apart from differences allowed by the specification, such as whitespace or case
- * of tokens are set up for numbered queues and the queues are then created to show that the meaning is correctly parsed by
- * the plugin.
- * 
- * TODO move this to the access-control plugin unit tests instead
- */
-public class ExternalACLFileTest extends AbstractACLTestCase
-{
-    private void createQueuePrefixList(String prefix, int count)
-    {
-        try
-        {
-            Connection conn = getConnection("test", "client", "guest");
-            Session sess = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
-            conn.start();
-
-            //Create n queues
-            for (int n = 0; n < count; n++)
-            {
-                AMQShortString queueName = new AMQShortString(String.format("%s.%03d", prefix, n));
-                ((AMQSession<?, ?>) sess).createQueue(queueName, false, false, false);
-            }
-
-            conn.close();
-        }
-        catch (Exception e)
-        {
-            fail("Test failed due to:" + e.getMessage());
-        }
-    }
-
-    private void createQueueNameList(String...queueNames)
-    {
-        try
-        {
-            Connection conn = getConnection("test", "client", "guest");
-            Session sess = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
-            conn.start();
-
-            //Create all queues
-            for (String queueName : queueNames)
-            {
-                ((AMQSession<?, ?>) sess).createQueue(new AMQShortString(queueName), false, false, false);
-            }
-
-            conn.close();
-        }
-        catch (Exception e)
-        {
-            fail("Test failed due to:" + e.getMessage());
-        }
-    }
-
-    public void setUpCreateQueueMixedCase() throws Exception
-    {
-        writeACLFile(
-            "test",
-            "ACL ALLOW-LOG client ACCESS VIRTUALHOST",
-            "acl allow client create queue name=mixed.000",
-            "ACL ALLOW client CREATE QUEUE NAME=mixed.001",
-            "Acl Allow client Create Queue Name=mixed.002",
-            "aCL aLLOW client cREATE qUEUE nAME=mixed.003",
-            "aCl AlLoW client cReAtE qUeUe NaMe=mixed.004"
-        );
-    }
-
-    public void testCreateQueueMixedCase()
-    {
-        createQueuePrefixList("mixed", 5);
-    }
-
-    public void setUpCreateQueueContinuation() throws Exception
-    {
-        writeACLFile(
-            "test",
-            "ACL ALLOW-LOG client ACCESS VIRTUALHOST",
-            "acl allow client create queue name=continuation.000",
-            "acl allow client create queue \\",
-            "   name=continuation.001",
-            "acl allow client \\",
-            "   create queue \\",
-            "   name=continuation.002",
-            "acl allow \\",
-            "   client \\",
-            "   create queue \\",
-            "   name=continuation.003",
-            "acl \\",
-            "   allow \\",
-            "   client \\",
-            "   create queue \\",
-            "   name=continuation.004"
-        );
-    }
-
-    public void testCreateQueueContinuation()
-    {
-        createQueuePrefixList("continuation", 5);
-    }
-
-    public void setUpCreateQueueWhitespace() throws Exception
-    {
-        writeACLFile(
-            "test",
-            "ACL ALLOW-LOG client ACCESS VIRTUALHOST",
-            "acl allow client create queue name=whitespace.000",
-            "acl\tallow\tclient\tcreate\tqueue\tname=whitespace.001",
-            "acl allow client create queue name = whitespace.002",
-            "acl\tallow\tclient\tcreate\tqueue\tname\t=\twhitespace.003",
-            "acl   allow\t\tclient\t   \tcreate\t\t queue\t \t name  \t =\t \twhitespace.004"
-        );
-    }
-
-    public void testCreateQueueWhitespace()
-    {
-        createQueuePrefixList("whitespace", 5);
-    }
-
-    public void setUpCreateQueueQuoting() throws Exception
-    {
-        writeACLFile(
-            "test",
-            "ACL ALLOW-LOG client ACCESS VIRTUALHOST",
-            "acl allow client create queue name='quoting.ABC.000'",
-            "acl allow client create queue name='quoting.*.000'",
-            "acl allow client create queue name='quoting.#.000'",
-            "acl allow client create queue name='quoting. .000'",
-            "acl allow client create queue name='quoting.!@$%.000'"
-        );
-    }
-
-    public void testCreateQueueQuoting()
-    {
-        createQueueNameList( 
-            "quoting.ABC.000",
-            "quoting.*.000",
-            "quoting.#.000",
-            "quoting. .000",
-            "quoting.!@$%.000"
-        );
-    }
-}
-
-
-