QPID-2654: Add Actor logging to the ACL plugin

Applied patch from Andrew Kennedy <andrew.international@gmail.com>


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

10 files changed, 275 insertions, 15 deletions

diff --git a/java/broker-plugins/access-control/MANIFEST.MF b/java/broker-plugins/access-control/MANIFEST.MF
index ea8b709a70..1cd285ba20 100644
--- a/java/broker-plugins/access-control/MANIFEST.MF
+++ b/java/broker-plugins/access-control/MANIFEST.MF
@@ -4,13 +4,13 @@ Bundle-Name: Qpid Broker-Plugins Access Control
 Bundle-SymbolicName: broker-plugins-access-control
 Bundle-Description: Access control plugin for Qpid.
 Bundle-License: http://www.apache.org/licenses/LICENSE-2.0.txt
-Bundle-DocURL: http://www.apache.org/
+Bundle-DocURL: http://qpid.apache.org/acl.html
 Bundle-Version: 1.0.0
 Bundle-Activator: org.apache.qpid.server.security.access.plugins.AccessControlActivator
 Bundle-RequiredExecutionEnvironment: JavaSE-1.5
 Bundle-ClassPath: .
 Bundle-ActivationPolicy: lazy
-Import-Package:  org.apache.qpid,
+Import-Package: org.apache.qpid,
  org.apache.qpid.exchange,
  org.apache.qpid.framing,
  org.apache.qpid.junit.extensions.util,
@@ -19,8 +19,12 @@ Import-Package:  org.apache.qpid,
  org.apache.qpid.server.configuration.plugins,
  org.apache.qpid.server.exchange,
  org.apache.qpid.server.management,
+ org.apache.qpid.server.logging,
+ org.apache.qpid.server.logging.actors,
+ org.apache.qpid.server.logging.subjects,
  org.apache.qpid.server.plugins,
  org.apache.qpid.server.queue,
+ org.apache.qpid.server.registry,
  org.apache.qpid.server.security,
  org.apache.qpid.server.security.access,
  org.apache.qpid.server.virtualhost,
@@ -33,5 +37,6 @@ Import-Package:  org.apache.qpid,
  javax.management.openmbean;version=1.0.0,
  org.osgi.util.tracker;version=1.0.0,
  org.osgi.framework;version=1.3
-Private-Package: org.apache.qpid.server.security.access.config
+Private-Package: org.apache.qpid.server.security.access.config,
+ org.apache.qpid.server.security.access.logging
 Export-Package: org.apache.qpid.server.security.access.plugins
diff --git a/java/broker-plugins/access-control/build.xml b/java/broker-plugins/access-control/build.xml
index f3f7e4de74..89f8240fd5 100644
--- a/java/broker-plugins/access-control/build.xml
+++ b/java/broker-plugins/access-control/build.xml
@@ -25,5 +25,7 @@
 
     <import file="../../module.xml" />
 
-    <target name="bundle" depends="bundle-tasks" />
+    <target name="bundle" depends="bundle-tasks"/>
+
+    <target name="precompile" depends="gen_logging"/>
 </project>
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 3c471f2f55..ebc73440ed 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
@@ -33,11 +33,13 @@ import org.apache.commons.lang.BooleanUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 import org.apache.qpid.exchange.ExchangeDefaults;
+import org.apache.qpid.server.logging.actors.CurrentActor;
 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.logging.AccessControlMessages;
 
 /**
  * Models the rule configuration for the access control plugin.
@@ -183,6 +185,20 @@ public class RuleSet
 		}
 		return false;
     }
+    
+    private Permission noLog(Permission permission)
+    {
+        switch (permission)
+        {
+            case ALLOW:
+            case ALLOW_LOG:
+                return Permission.ALLOW;
+            case DENY:
+            case DENY_LOG:
+            default:
+                return Permission.DENY;
+        }
+    }
 
     // TODO make this work when group membership is not known at file parse time
     public void addRule(Integer number, String identity, Permission permission, Action action)
@@ -201,7 +217,7 @@ public class RuleSet
         {
             if (action.getOperation() == Operation.CREATE && action.getObjectType() == ObjectType.TOPIC)
             {
-                addRule(null, identity, permission, new Action(Operation.BIND, ObjectType.EXCHANGE,
+                addRule(null, identity, noLog(permission), new Action(Operation.BIND, ObjectType.EXCHANGE,
                         new ObjectProperties("amq.topic", action.getProperties().get(ObjectProperties.Property.NAME))));
                 ObjectProperties topicProperties = new ObjectProperties();
                 topicProperties.put(ObjectProperties.Property.DURABLE, true);
@@ -210,7 +226,7 @@ public class RuleSet
             }
             if (action.getOperation() == Operation.DELETE && action.getObjectType() == ObjectType.TOPIC)
             {
-                addRule(null, identity, permission, new Action(Operation.UNBIND, ObjectType.EXCHANGE,
+                addRule(null, identity, noLog(permission), new Action(Operation.UNBIND, ObjectType.EXCHANGE,
                         new ObjectProperties("amq.topic", action.getProperties().get(ObjectProperties.Property.NAME))));
                 ObjectProperties topicProperties = new ObjectProperties();
                 topicProperties.put(ObjectProperties.Property.DURABLE, true);
@@ -227,10 +243,10 @@ public class RuleSet
                 ObjectProperties exchProperties = new ObjectProperties(action.getProperties());
                 exchProperties.setName(ExchangeDefaults.DEFAULT_EXCHANGE_NAME);
                 exchProperties.put(ObjectProperties.Property.ROUTING_KEY, action.getProperties().get(ObjectProperties.Property.NAME));
-                addRule(null, identity, permission, new Action(Operation.BIND, ObjectType.EXCHANGE, exchProperties));
+                addRule(null, identity, noLog(permission), new Action(Operation.BIND, ObjectType.EXCHANGE, exchProperties));
 				if (action.getProperties().isSet(ObjectProperties.Property.AUTO_DELETE))
 				{
-					addRule(null, identity, permission, new Action(Operation.DELETE, ObjectType.QUEUE, action.getProperties()));
+					addRule(null, identity, noLog(permission), new Action(Operation.DELETE, ObjectType.QUEUE, action.getProperties()));
 				}
             }
             else if (action.getOperation() == Operation.DELETE && action.getObjectType() == ObjectType.QUEUE)
@@ -238,11 +254,11 @@ public class RuleSet
                 ObjectProperties exchProperties = new ObjectProperties(action.getProperties());
                 exchProperties.setName(ExchangeDefaults.DEFAULT_EXCHANGE_NAME);
                 exchProperties.put(ObjectProperties.Property.ROUTING_KEY, action.getProperties().get(ObjectProperties.Property.NAME));
-                addRule(null, identity, permission, new Action(Operation.UNBIND, ObjectType.EXCHANGE, exchProperties));
+                addRule(null, identity, noLog(permission), new Action(Operation.UNBIND, ObjectType.EXCHANGE, exchProperties));
             }
             else if (action.getOperation() != Operation.ACCESS && action.getObjectType() != ObjectType.VIRTUALHOST)
             {
-                addRule(null, identity, permission, new Action(Operation.ACCESS, ObjectType.VIRTUALHOST));
+                addRule(null, identity, noLog(permission), new Action(Operation.ACCESS, ObjectType.VIRTUALHOST));
             }
         }
         
@@ -409,11 +425,13 @@ public class RuleSet
                 switch (permission)
                 {
                     case ALLOW_LOG:
-                        _logger.info("ALLOWED " + action);
+                        CurrentActor.get().message(AccessControlMessages.ALLOWED(
+                                action.getOperation().toString(), action.getObjectType().toString(), action.getProperties().toString()));
                     case ALLOW:
                         return Result.ALLOWED;
                     case DENY_LOG:
-                        _logger.info("DENIED " + action);
+                        CurrentActor.get().message(AccessControlMessages.DENIED(
+                                action.getOperation().toString(), action.getObjectType().toString(), action.getProperties().toString()));
                     case DENY:
                         return Result.DENIED;
                 }
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/logging/AccessControl_logmessages.properties b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/logging/AccessControl_logmessages.properties
new file mode 100644
index 0000000000..bf80df3722
--- /dev/null
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/logging/AccessControl_logmessages.properties
@@ -0,0 +1,28 @@
+#
+# 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.
+#
+
+# org.apache.qpid.server.security.access.logging.AccessControl
+
+# Access Control logging message i18n strings.
+
+# 'accept-log' rule message
+ALLOWED = ACL-1001 : Allowed : {0} {1} {2}
+
+# 'deny-log' rule message
+DENIED = ACL-1002 : Denied : {0} {1} {2}
\ No newline at end of file
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java
index e6e0059902..69cfa173bd 100644
--- a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java
@@ -20,7 +20,6 @@
  */
 package org.apache.qpid.server.security.access.plugins;
 
-import java.io.File;
 import java.security.Principal;
 
 import org.apache.commons.configuration.ConfigurationException;
@@ -33,8 +32,6 @@ import org.apache.qpid.server.security.SecurityPluginFactory;
 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.config.ConfigurationFile;
-import org.apache.qpid.server.security.access.config.PlainConfiguration;
 import org.apache.qpid.server.security.access.config.RuleSet;
 
 /**
diff --git a/java/broker/src/main/java/org/apache/qpid/server/plugins/PluginManager.java b/java/broker/src/main/java/org/apache/qpid/server/plugins/PluginManager.java
index 8364635632..62d8f87641 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/plugins/PluginManager.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/plugins/PluginManager.java
@@ -124,6 +124,7 @@ public class PluginManager implements Closeable
                 "org.apache.qpid.server.exchange; version=0.7," +
                 "org.apache.qpid.server.logging; version=0.7," +
                 "org.apache.qpid.server.logging.actors; version=0.7," +                
+                "org.apache.qpid.server.logging.subjects; version=0.7," +                
                 "org.apache.qpid.server.management; version=0.7," +
                 "org.apache.qpid.server.persistent; version=0.7," +
                 "org.apache.qpid.server.plugins; version=0.7," +
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java b/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java
index 66ef388976..1fedc8134a 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java
@@ -87,4 +87,10 @@ public enum ObjectType
         }
         throw new IllegalArgumentException("Not a valid object type: " + text);
     }
+    
+    public String toString()
+    {
+        String name = name();
+        return name.charAt(0) + name.substring(1).toLowerCase();
+    }
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/access/Operation.java b/java/broker/src/main/java/org/apache/qpid/server/security/access/Operation.java
index 7077257d01..06d7f8fd0c 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/access/Operation.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/access/Operation.java
@@ -46,4 +46,10 @@ public enum Operation
         }
         throw new IllegalArgumentException("Not a valid operation: " + text);
     }
+    
+    public String toString()
+    {
+        String name = name();
+        return name.charAt(0) + name.substring(1).toLowerCase();
+    }
 }
\ No newline at end of file
diff --git a/java/systests/etc/test-logging.txt b/java/systests/etc/test-logging.txt
new file mode 100644
index 0000000000..76c6e442e0
--- /dev/null
+++ b/java/systests/etc/test-logging.txt
@@ -0,0 +1,23 @@
+#
+# 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.
+#
+
+ACL ALLOW client CREATE QUEUE name="allow"
+ACL ALLOW-LOG client CREATE QUEUE name="allow-log"
+ACL DENY client CREATE QUEUE name="deny"
+ACL DENY-LOG client CREATE QUEUE name="deny-log"
diff --git a/java/systests/src/main/java/org/apache/qpid/server/logging/AccessControlLoggingTest.java b/java/systests/src/main/java/org/apache/qpid/server/logging/AccessControlLoggingTest.java
new file mode 100644
index 0000000000..da11117962
--- /dev/null
+++ b/java/systests/src/main/java/org/apache/qpid/server/logging/AccessControlLoggingTest.java
@@ -0,0 +1,174 @@
+/*
+ *  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.logging;
+
+import java.io.File;
+import java.util.List;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Session;
+
+import org.apache.qpid.AMQException;
+import org.apache.qpid.client.AMQSession;
+import org.apache.qpid.framing.AMQShortString;
+import org.apache.qpid.protocol.AMQConstant;
+
+/**
+ * ACL version 2/3 file testing to verify that ACL actor logging works correctly.
+ * 
+ * This suite of tests validate that the AccessControl messages occur correctly
+ * and according to the following format:
+ * 
+ * <pre>
+ * ACL-1001 : Allowed Operation Object {PROPERTIES}
+ * ACL-1002 : Denied Operation Object {PROPERTIES}
+ * </pre>
+ */
+public class AccessControlLoggingTest extends AbstractTestLogging
+{
+    private static final String ACL_LOG_PREFIX = "ACL-";
+    private static final String USER = "client";
+    private static final String PASS = "guest";
+
+    public void setUp() throws Exception
+    {
+        setConfigurationProperty("virtualhosts.virtualhost.test.security.aclv2",
+                QpidHome + File.separator + "etc" + File.separator + "test-logging.txt");
+        
+        super.setUp();
+    }
+
+    /** FIXME This comes from {@link SimpleACLTest} and makes me suspicious. */
+    @Override
+    public void tearDown() throws Exception
+    {
+        try
+        {
+            super.tearDown();
+        }
+        catch (JMSException e)
+        {
+            //we're throwing this away as it can happen in this test as the state manager remembers exceptions
+            //that we provoked with authentication failures, where the test passes - we can ignore on con close
+        }
+    }
+    
+    /**
+     * Test that {@code allow} ACL entries do not log anything.
+     */
+    public void testAllow() throws Exception
+	{
+        Connection conn = getConnection(USER, PASS);
+        Session sess = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
+        conn.start();
+        ((AMQSession<?, ?>) sess).createQueue(new AMQShortString("allow"), false, false, false);
+        
+        List<String> matches = _monitor.findMatches(ACL_LOG_PREFIX);
+        
+        assertTrue("Should be no ACL log messages", matches.isEmpty());
+    }
+    
+    /**
+     * Test that {@code allow-log} ACL entries log correctly.
+     */
+    public void testAllowLog() throws Exception
+    {
+        Connection conn = getConnection(USER, PASS);
+        Session sess = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
+        conn.start();
+        ((AMQSession<?, ?>) sess).createQueue(new AMQShortString("allow-log"), false, false, false);
+        
+        List<String> matches = _monitor.findMatches(ACL_LOG_PREFIX);
+        
+        assertEquals("Should only be one ACL log message", 1, matches.size());
+        
+        String log = getLog(matches.get(0));
+        String actor = fromActor(log);
+        String subject = fromSubject(log);
+        String message = getMessageString(fromMessage(log));
+        
+        validateMessageID(ACL_LOG_PREFIX + 1001, log);
+        
+        assertTrue("Actor should contain the user identity", actor.contains(USER));
+        assertTrue("Subject should be empty", subject.length() == 0);
+        assertTrue("Message should start with 'Allowed'", message.startsWith("Allowed"));
+        assertTrue("Message should contain 'Create Queue'", message.contains("Create Queue"));
+        assertTrue("Message should have contained the queue name", message.contains("allow-log"));
+    }
+    
+    /**
+     * Test that {@code deny-log} ACL entries log correctly.
+     */
+    public void testDenyLog() throws Exception
+    {
+        Connection conn = getConnection(USER, PASS);
+        Session sess = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
+        conn.start();
+        try {
+            ((AMQSession<?, ?>) sess).createQueue(new AMQShortString("deny-log"), false, false, false);
+	        fail("Should have denied queue creation");
+        }
+        catch (AMQException amqe)
+        {
+            // Denied, so exception thrown
+            assertEquals("Expected ACCESS_REFUSED error code", AMQConstant.ACCESS_REFUSED, amqe.getErrorCode());
+        }
+        
+        List<String> matches = _monitor.findMatches(ACL_LOG_PREFIX);
+        
+        assertEquals("Should only be one ACL log message", 1, matches.size());
+        
+        String log = getLog(matches.get(0));
+        String actor = fromActor(log);
+        String subject = fromSubject(log);
+        String message = getMessageString(fromMessage(log));
+        
+        validateMessageID(ACL_LOG_PREFIX + 1002, log);
+        
+        assertTrue("Actor should contain the user identity", actor.contains(USER));
+        assertTrue("Subject should be empty", subject.length() == 0);
+        assertTrue("Message should start with 'Denied'", message.startsWith("Denied"));
+        assertTrue("Message should contain 'Create Queue'", message.contains("Create Queue"));
+        assertTrue("Message should have contained the queue name", message.contains("deny-log"));
+    }
+    
+    /**
+     * Test that {@code deny} ACL entries do not log anything.
+     */
+    public void testDeny() throws Exception
+    {
+        Connection conn = getConnection(USER, PASS);
+        Session sess = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
+        conn.start();
+        try {
+            ((AMQSession<?, ?>) sess).createQueue(new AMQShortString("deny"), false, false, false);
+            fail("Should have denied queue creation");
+        }
+        catch (AMQException amqe)
+        {
+            // Denied, so exception thrown
+            assertEquals("Expected ACCESS_REFUSED error code", AMQConstant.ACCESS_REFUSED, amqe.getErrorCode());
+        }
+        
+        List<String> matches = _monitor.findMatches(ACL_LOG_PREFIX);
+        
+        assertTrue("Should be no ACL log messages", matches.isEmpty());
+    }
+}