You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ai...@apache.org on 2009/02/04 16:33:27 UTC

svn commit: r740769 - in /qpid/trunk/qpid/java/broker/src: main/java/org/apache/qpid/server/handler/ main/java/org/apache/qpid/server/security/access/ main/java/org/apache/qpid/server/security/access/plugins/ test/java/org/apache/qpid/server/security/a...

Author: aidan
Date: Wed Feb  4 15:33:26 2009
New Revision: 740769

URL: http://svn.apache.org/viewvc?rev=740769&view=rev
Log:
QPID-1626: Make ACLPlugin a more sensible interface, get rid of the giant switch in SimpleXML. Handlers shouldn't rely on the plugin throwing an exception for flow control, they now check the return value and do the right thing themselves.

AllowAll, DenyAll now extend BasicACLPlugin.

PrinciplePermissions(Test): futz with the interface a little so that it's easier to call from an ACLPlugin implementation. Leave the giant switch alone as it's quite fragile, and throws rocks at cats.

Added:
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicACLPlugin.java
Modified:
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicConsumeMethodHandler.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicGetMethodHandler.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicPublishMethodHandler.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeleteHandler.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueBindHandler.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeleteHandler.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueuePurgeHandler.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueUnbindHandler.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ACLPlugin.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/Permission.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/PrincipalPermissions.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java
    qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/security/access/PrincipalPermissionsTest.java

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicConsumeMethodHandler.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicConsumeMethodHandler.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicConsumeMethodHandler.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicConsumeMethodHandler.java Wed Feb  4 15:33:26 2009
@@ -97,8 +97,13 @@
 
                 final AMQShortString consumerTagName;
 
-                //Perform ACLs
-                vHost.getAccessManager().authorise(session, Permission.CONSUME, body, queue);
+                // Check authz
+                if (!vHost.getAccessManager().authoriseConsume(session,
+                        body.getExclusive(), body.getNoAck(),
+                        body.getNoLocal(), body.getNowait(), queue))
+                {
+                    throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied");
+                }
 
                 if (body.getConsumerTag() != null)
                 {

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicGetMethodHandler.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicGetMethodHandler.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicGetMethodHandler.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicGetMethodHandler.java Wed Feb  4 15:33:26 2009
@@ -94,7 +94,10 @@
             {
 
                 //Perform ACLs
-                vHost.getAccessManager().authorise(session, Permission.CONSUME, body, queue);
+                if (!vHost.getAccessManager().authoriseConsume(session, body.getNoAck(), queue))
+                {
+                    throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied");
+                }
 
                 if (!performGet(queue,session, channel, !body.getNoAck()))
                 {

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicPublishMethodHandler.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicPublishMethodHandler.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicPublishMethodHandler.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicPublishMethodHandler.java Wed Feb  4 15:33:26 2009
@@ -87,7 +87,12 @@
             }
 
             //Access Control
-            vHost.getAccessManager().authorise(session, Permission.PUBLISH, body, e);
+            if (!vHost.getAccessManager().authorisePublish(session, 
+                    body.getImmediate(), body.getMandatory(), 
+                    body.getRoutingKey(), e))
+            {
+                throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied");
+            }
 
             MessagePublishInfo info = session.getMethodRegistry().getProtocolVersionMethodConverter().convertToInfo(body);
             info.setExchange(exchange);

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java Wed Feb  4 15:33:26 2009
@@ -78,7 +78,10 @@
             session.setVirtualHost(virtualHost);
 
             //Perform ACL
-            virtualHost.getAccessManager().authorise(session, Permission.ACCESS ,body, virtualHost);
+            if (!virtualHost.getAccessManager().authoriseConnect(session, virtualHost))
+            {
+                throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied");
+            }
 
             // See Spec (0.8.2). Section  3.1.2 Virtual Hosts
             if (session.getContextKey() == null)

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java Wed Feb  4 15:33:26 2009
@@ -61,8 +61,14 @@
 
         if (!body.getPassive())
         {
-            //Perform ACL if request is not passive
-            virtualHost.getAccessManager().authorise(session, Permission.CREATE, body);
+            // Perform ACL if request is not passive
+            if (!virtualHost.getAccessManager().authoriseCreateExchange(session, body.getAutoDelete(),
+                    body.getDurable(), body.getExchange(), body.getInternal(), body.getNowait(), body.getPassive(),
+                    body.getType()))
+            {
+                throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied");
+            }
+
         }
 
         if (_logger.isDebugEnabled())

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeleteHandler.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeleteHandler.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeleteHandler.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeleteHandler.java Wed Feb  4 15:33:26 2009
@@ -23,6 +23,7 @@
 import org.apache.qpid.AMQException;
 import org.apache.qpid.framing.ExchangeDeleteBody;
 import org.apache.qpid.framing.ExchangeDeleteOkBody;
+import org.apache.qpid.protocol.AMQConstant;
 import org.apache.qpid.server.exchange.ExchangeInUseException;
 import org.apache.qpid.server.exchange.ExchangeRegistry;
 import org.apache.qpid.server.protocol.AMQProtocolSession;
@@ -51,8 +52,11 @@
         ExchangeRegistry exchangeRegistry = virtualHost.getExchangeRegistry();
 
         //Perform ACLs
-        virtualHost.getAccessManager().authorise(session, Permission.DELETE,body,
-                                                 exchangeRegistry.getExchange(body.getExchange()));
+        if (!virtualHost.getAccessManager().authoriseDelete(session,
+                exchangeRegistry.getExchange(body.getExchange())))
+        {
+            throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied");
+        }
 
         try
         {

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueBindHandler.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueBindHandler.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueBindHandler.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueBindHandler.java Wed Feb  4 15:33:26 2009
@@ -108,7 +108,11 @@
         {
 
             //Perform ACLs
-            virtualHost.getAccessManager().authorise(session, Permission.BIND, body, exch, queue, routingKey);
+            if (!virtualHost.getAccessManager().authoriseBind(session, exch,
+                    queue, routingKey))
+            {
+                throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied");
+            }
 
             if (!exch.isBound(routingKey, body.getArguments(), queue))
             {

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java Wed Feb  4 15:33:26 2009
@@ -78,11 +78,14 @@
 
         if (!body.getPassive())
         {
-            //Perform ACL if request is not passive
-            virtualHost.getAccessManager().authorise(session, Permission.CREATE, body);
+            // Perform ACL if request is not passive
+            if (!virtualHost.getAccessManager().authoriseCreateQueue(session, body.getAutoDelete(), body.getDurable(),
+                    body.getExclusive(), body.getNowait(), body.getPassive(), body.getQueue()))
+            {
+                throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied");
+            }
         }
 
-
         final AMQShortString queueName;
 
         // if we aren't given a queue name, we create one which we return to the client

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeleteHandler.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeleteHandler.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeleteHandler.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeleteHandler.java Wed Feb  4 15:33:26 2009
@@ -105,7 +105,10 @@
             {
                 
                 //Perform ACLs
-                virtualHost.getAccessManager().authorise(session, Permission.DELETE, body, queue);
+                if (!virtualHost.getAccessManager().authoriseDelete(session, queue))
+                {
+                    throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied");
+                }
 
                 int purged = queue.delete();
 

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueuePurgeHandler.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueuePurgeHandler.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueuePurgeHandler.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueuePurgeHandler.java Wed Feb  4 15:33:26 2009
@@ -101,7 +101,10 @@
         {
 
                 //Perform ACLs
-                virtualHost.getAccessManager().authorise(session, Permission.PURGE, body, queue);
+                if (!virtualHost.getAccessManager().authorisePurge(session, queue))
+                {
+                    throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied");
+                }
 
                 long purged = queue.clearQueue(channel.getStoreContext());
 

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueUnbindHandler.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueUnbindHandler.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueUnbindHandler.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueUnbindHandler.java Wed Feb  4 15:33:26 2009
@@ -101,7 +101,10 @@
         }
 
         //Perform ACLs
-        virtualHost.getAccessManager().authorise(session, Permission.UNBIND, body, queue);
+        if (!virtualHost.getAccessManager().authoriseUnbind(session, exch, routingKey, queue))
+        {
+            throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied");
+        }
 
         try
         {

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ACLPlugin.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ACLPlugin.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ACLPlugin.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ACLPlugin.java Wed Feb  4 15:33:26 2009
@@ -20,39 +20,45 @@
  */
 package org.apache.qpid.server.security.access;
 
-import org.apache.qpid.framing.AMQMethodBody;
-
-import org.apache.qpid.server.protocol.AMQProtocolSession;
-import org.apache.qpid.AMQConnectionException;
 import org.apache.commons.configuration.Configuration;
-
+import org.apache.qpid.framing.AMQShortString;
+import org.apache.qpid.server.exchange.Exchange;
+import org.apache.qpid.server.protocol.AMQProtocolSession;
+import org.apache.qpid.server.queue.AMQQueue;
+import org.apache.qpid.server.virtualhost.VirtualHost;
 
 public interface ACLPlugin
 {
-    /**
-     * Pseudo-Code:
-     * Identify requested RighConnectiont
-     * Lookup users ability for that right.
-     * if rightsExists
-     * Validate right on object
-     * Return result
-     * e.g
-     * User, CONSUME , Queue
-     * User, CONSUME , Exchange + RoutingKey
-     * User, PUBLISH , Exchange + RoutingKey
-     * User, CREATE  , Exchange || Queue
-     * User, BIND    , Exchange + RoutingKey + Queue
-     *
-     * @param session      - The session requesting access
-     * @param permission   - The permission requested
-     * @param parameters   - The above objects that are used to authorise the request.
-     * @return The AccessResult decision
-     */
-    //todo potential refactor this ConnectionException Out of here
-    AccessResult authorise(AMQProtocolSession session, Permission permission, AMQMethodBody body, Object... parameters) throws AMQConnectionException;
-
     String getPluginName();
 
     void setConfiguaration(Configuration config);
 
+    // These return true if the plugin thinks the action should be allowed, and false if not. 
+    
+    boolean authoriseBind(AMQProtocolSession session, Exchange exch, AMQQueue queue, AMQShortString routingKey);
+
+    boolean authoriseCreateExchange(AMQProtocolSession session, boolean autoDelete, boolean durable,
+            AMQShortString exchangeName, boolean internal, boolean nowait, boolean passive, AMQShortString exchangeType);
+
+    boolean authoriseCreateQueue(AMQProtocolSession session, boolean autoDelete, boolean durable, boolean exclusive,
+            boolean nowait, boolean passive, AMQShortString queue);
+
+    boolean authoriseConnect(AMQProtocolSession session, VirtualHost virtualHost);
+
+    boolean authoriseConsume(AMQProtocolSession session, boolean noAck, AMQQueue queue);
+
+    boolean authoriseConsume(AMQProtocolSession session, boolean exclusive, boolean noAck, boolean noLocal,
+            boolean nowait, AMQQueue queue);
+
+    boolean authoriseDelete(AMQProtocolSession session, AMQQueue queue);
+
+    boolean authoriseDelete(AMQProtocolSession session, Exchange exchange);
+
+    boolean authorisePublish(AMQProtocolSession session, boolean immediate, boolean mandatory,
+            AMQShortString routingKey, Exchange e);
+
+    boolean authorisePurge(AMQProtocolSession session, AMQQueue queue);
+
+    boolean authoriseUnbind(AMQProtocolSession session, Exchange exch, AMQShortString routingKey, AMQQueue queue);
+
 }

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/Permission.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/Permission.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/Permission.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/Permission.java Wed Feb  4 15:33:26 2009
@@ -28,7 +28,8 @@
 {
     CONSUME,
     PUBLISH,
-    CREATE,
+    CREATEQUEUE,
+    CREATEEXCHANGE,
     ACCESS,
     BIND,
     UNBIND,

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/PrincipalPermissions.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/PrincipalPermissions.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/PrincipalPermissions.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/PrincipalPermissions.java Wed Feb  4 15:33:26 2009
@@ -68,9 +68,8 @@
      *  ACCESS: none
      *  BIND: none
      *  CONSUME: AMQShortString queueName, Boolean temporary, Boolean ownQueueOnly
-     *  CREATE:  Boolean temporary, AMQShortString queueName, AMQShortString exchangeName, AMQShortString routingKey
-     *                      or
-     *           AMQShortString exchangeName, AMQShortString Class
+     *  CREATEQUEUE:  Boolean temporary, AMQShortString queueName, AMQShortString exchangeName, AMQShortString routingKey
+     *  CREATEEXCHANGE: AMQShortString exchangeName, AMQShortString Class
      *  DELETE: none
      *  PUBLISH: Exchange exchange, AMQShortString routingKey
      *  PURGE: none
@@ -134,9 +133,8 @@
 
 
                 break;
-            case CREATE:  // Parameters : Boolean temporary, AMQShortString queueName
+            case CREATEQUEUE:  // Parameters : Boolean temporary, AMQShortString queueName
                 // , AMQShortString exchangeName , AMQShortString routingKey
-                // || AMQShortString exchangeName , AMQShortString Class
 
                 Map createRights = (Map) _permissions.get(permission);
 
@@ -153,121 +151,122 @@
                     return;
                 }
 
+                Boolean temporary = (Boolean) parameters[0];
 
-                if (parameters[0] instanceof Boolean) //Create Queue :
-                // Boolean temporary, [AMQShortString queueName, AMQShortString exchangeName , AMQShortString routingKey]
-                {
-                    Boolean temporary = (Boolean) parameters[0];
+                AMQShortString queueName = parameters.length > 1 ? (AMQShortString) parameters[1] : null;
+                AMQShortString exchangeName = parameters.length > 2 ? (AMQShortString) parameters[2] : null;
+                //Set the routingkey to the specified value or the queueName if present
+                AMQShortString routingKey = parameters.length > 3 ? (AMQShortString) parameters[3] : queueName;
 
-                    AMQShortString queueName = parameters.length > 1 ? (AMQShortString) parameters[1] : null;
-                    AMQShortString exchangeName = parameters.length > 2 ? (AMQShortString) parameters[2] : null;
-                    //Set the routingkey to the specified value or the queueName if present
-                    AMQShortString routingKey = parameters.length > 3 ? (AMQShortString) parameters[3] : queueName;
+                // Get the queues map
+                Map create_queues = (Map) createRights.get(CREATE_QUEUES_KEY);
 
-                    // Get the queues map
-                    Map create_queues = (Map) createRights.get(CREATE_QUEUES_KEY);
+                if (create_queues == null)
+                {
+                    create_queues = new ConcurrentHashMap();
+                    createRights.put(CREATE_QUEUES_KEY, create_queues);
+                }
 
-                    if (create_queues == null)
-                    {
-                        create_queues = new ConcurrentHashMap();
-                        createRights.put(CREATE_QUEUES_KEY, create_queues);
-                    }
+                //Allow all temp queues to be created
+                create_queues.put(CREATE_QUEUE_TEMPORARY_KEY, temporary);
 
-                    //Allow all temp queues to be created
-                    create_queues.put(CREATE_QUEUE_TEMPORARY_KEY, temporary);
+                //Create empty list of queues
+                Map create_queues_queues = (Map) create_queues.get(CREATE_QUEUE_QUEUES_KEY);
 
-                    //Create empty list of queues
-                    Map create_queues_queues = (Map) create_queues.get(CREATE_QUEUE_QUEUES_KEY);
+                if (create_queues_queues == null)
+                {
+                    create_queues_queues = new ConcurrentHashMap();
+                    create_queues.put(CREATE_QUEUE_QUEUES_KEY, create_queues_queues);
+                }
 
-                    if (create_queues_queues == null)
+                // We are granting CREATE rights to all temporary queues only
+                if (parameters.length == 1)
+                {
+                    return;
+                }
+
+                // if we have a queueName then we need to store any associated exchange / rk bindings
+                if (queueName != null)
+                {
+                    Map queue = (Map) create_queues_queues.get(queueName);
+                    if (queue == null)
                     {
-                        create_queues_queues = new ConcurrentHashMap();
-                        create_queues.put(CREATE_QUEUE_QUEUES_KEY, create_queues_queues);
+                        queue = new ConcurrentHashMap();
+                        create_queues_queues.put(queueName, queue);
                     }
 
-                    // We are granting CREATE rights to all temporary queues only
-                    if (parameters.length == 1)
+                    if (exchangeName != null)
                     {
-                        return;
+                        queue.put(exchangeName, routingKey);
                     }
 
-                    // if we have a queueName then we need to store any associated exchange / rk bindings
-                    if (queueName != null)
-                    {
-                        Map queue = (Map) create_queues_queues.get(queueName);
-                        if (queue == null)
-                        {
-                            queue = new ConcurrentHashMap();
-                            create_queues_queues.put(queueName, queue);
-                        }
+                    //If no exchange is specified then the presence of the queueName in the map says any exchange is ok
+                }
 
-                        if (exchangeName != null)
-                        {
-                            queue.put(exchangeName, routingKey);
-                        }
+                // Store the exchange that we are being granted rights to. This will be used as part of binding
 
-                        //If no exchange is specified then the presence of the queueName in the map says any exchange is ok
-                    }
+                //Lookup the list of exchanges
+                Map create_queues_exchanges = (Map) create_queues.get(CREATE_QUEUE_EXCHANGES_KEY);
 
-                    // Store the exchange that we are being granted rights to. This will be used as part of binding
+                if (create_queues_exchanges == null)
+                {
+                    create_queues_exchanges = new ConcurrentHashMap();
+                    create_queues.put(CREATE_QUEUE_EXCHANGES_KEY, create_queues_exchanges);
+                }
 
-                    //Lookup the list of exchanges
-                    Map create_queues_exchanges = (Map) create_queues.get(CREATE_QUEUE_EXCHANGES_KEY);
+                //if we have an exchange
+                if (exchangeName != null)
+                {
+                    //Retrieve the list of permitted exchanges.
+                    Map exchanges = (Map) create_queues_exchanges.get(exchangeName);
 
-                    if (create_queues_exchanges == null)
+                    if (exchanges == null)
                     {
-                        create_queues_exchanges = new ConcurrentHashMap();
-                        create_queues.put(CREATE_QUEUE_EXCHANGES_KEY, create_queues_exchanges);
+                        exchanges = new ConcurrentHashMap();
+                        create_queues_exchanges.put(exchangeName, exchanges);
                     }
 
-                    //if we have an exchange
-                    if (exchangeName != null)
+                    //Store the temporary setting CREATE_QUEUE_EXCHANGES_ROUTINGKEYS_KEY
+                    exchanges.put(CREATE_QUEUE_EXCHANGES_TEMPORARY_KEY, temporary);
+
+                    //Store the binding details of queue/rk for this exchange.
+                    if (queueName != null)
                     {
-                        //Retrieve the list of permitted exchanges.
-                        Map exchanges = (Map) create_queues_exchanges.get(exchangeName);
+                        //Retrieve the list of permitted routingKeys.
+                        Map rKeys = (Map) exchanges.get(exchangeName);
 
-                        if (exchanges == null)
+                        if (rKeys == null)
                         {
-                            exchanges = new ConcurrentHashMap();
-                            create_queues_exchanges.put(exchangeName, exchanges);
+                            rKeys = new ConcurrentHashMap();
+                            exchanges.put(CREATE_QUEUE_EXCHANGES_ROUTINGKEYS_KEY, rKeys);
                         }
 
-                        //Store the temporary setting CREATE_QUEUE_EXCHANGES_ROUTINGKEYS_KEY
-                        exchanges.put(CREATE_QUEUE_EXCHANGES_TEMPORARY_KEY, temporary);
-
-                        //Store the binding details of queue/rk for this exchange.
-                        if (queueName != null)
-                        {
-                            //Retrieve the list of permitted routingKeys.
-                            Map rKeys = (Map) exchanges.get(exchangeName);
-
-                            if (rKeys == null)
-                            {
-                                rKeys = new ConcurrentHashMap();
-                                exchanges.put(CREATE_QUEUE_EXCHANGES_ROUTINGKEYS_KEY, rKeys);
-                            }
-
-                            rKeys.put(queueName, routingKey);
-                        }
+                        rKeys.put(queueName, routingKey);
                     }
                 }
-                else // Create Exchange : AMQShortString exchangeName , AMQShortString Class
+                break;
+            case CREATEEXCHANGE:
+                // Parameters AMQShortString exchangeName , AMQShortString Class
+                Map rights = (Map) _permissions.get(permission);
+                if (rights == null)
                 {
-                    Map create_exchanges = (Map) createRights.get(CREATE_EXCHANGES_KEY);
+                    rights = new ConcurrentHashMap();
+                    _permissions.put(permission, rights);
+                }
 
-                    if (create_exchanges == null)
-                    {
-                        create_exchanges = new ConcurrentHashMap();
-                        createRights.put(CREATE_EXCHANGES_KEY, create_exchanges);
-                    }
+                Map create_exchanges = (Map) rights.get(CREATE_EXCHANGES_KEY);
+                if (create_exchanges == null)
+                {
+                    create_exchanges = new ConcurrentHashMap();
+                    rights.put(CREATE_EXCHANGES_KEY, create_exchanges);
+                }
 
-                    //Should perhaps error if parameters[0] is null;
-                    AMQShortString exchangeName = parameters.length > 0 ? (AMQShortString) parameters[0] : null;
-                    AMQShortString className = parameters.length > 1 ? (AMQShortString) parameters[1] : null;
+                //Should perhaps error if parameters[0] is null;
+                AMQShortString name = parameters.length > 0 ? (AMQShortString) parameters[0] : null;
+                AMQShortString className = parameters.length > 1 ? (AMQShortString) parameters[1] : new AMQShortString("direct");
 
-                    //Store the exchangeName / class mapping if the mapping is null
-                    createRights.put(exchangeName, className);
-                }
+                //Store the exchangeName / class mapping if the mapping is null
+                rights.put(name, className);
                 break;
             case DELETE:
                 break;
@@ -330,7 +329,8 @@
      *  ACCESS: none
      *  BIND: QueueBindBody bindmethod, Exchange exchange, AMQQueue queue, AMQShortString routingKey
      *  CONSUME: AMQQueue queue
-     *  CREATE:  QueueDeclareBody obj || ExchangeDeclareBody obj
+     *  CREATEQUEUE:  Boolean autodelete, AMQShortString name
+     *  CREATEEXCHANGE: AMQShortString exchangeName
      *  DELETE: none
      *  PUBLISH: Exchange exchange, AMQShortString routingKey
      *  PURGE: none
@@ -352,7 +352,7 @@
                 AMQShortString routingKey = (AMQShortString) parameters[3];
 
                 //Get all Create Rights for this user
-                Map bindCreateRights = (Map) _permissions.get(Permission.CREATE);
+                Map bindCreateRights = (Map) _permissions.get(Permission.CREATEQUEUE);
 
                 //Look up the Queue Creation Rights
                 Map bind_create_queues = (Map) bindCreateRights.get(CREATE_QUEUES_KEY);
@@ -435,7 +435,7 @@
                     return true;
                 }
 
-            case CREATE:// Paramters : QueueDeclareBody || ExchangeDeclareBody
+            case CREATEQUEUE:// Parameters : boolean autodelete, AMQShortString name
 
                 Map createRights = (Map) _permissions.get(permission);
 
@@ -445,46 +445,33 @@
                     return false;
                 }
 
-                if (parameters.length == 1)
-                {
-                    if (parameters[0] instanceof QueueDeclareBody)
-                    {
-                        QueueDeclareBody body = (QueueDeclareBody) parameters[0];
-
-                        //Look up the Queue Creation Rights
-                        Map create_queues = (Map) createRights.get(CREATE_QUEUES_KEY);
-
-                        //Lookup the list of queues allowed to be created
-                        Map create_queues_queues = (Map) create_queues.get(CREATE_QUEUE_QUEUES_KEY);
-
-
-                        AMQShortString queueName = body.getQueue();
+                //Look up the Queue Creation Rights
+                Map create_queues = (Map) createRights.get(CREATE_QUEUES_KEY);
 
+                //Lookup the list of queues allowed to be created
+                Map create_queues_queues = (Map) create_queues.get(CREATE_QUEUE_QUEUES_KEY);
 
-                        if (body.getAutoDelete())// we have a temporary queue
-                        {
-                            return (Boolean) create_queues.get(CREATE_QUEUE_TEMPORARY_KEY);
-                        }
-                        else
-                        {
-                            // If there is a white list then check
-                            return create_queues_queues == null || create_queues_queues.containsKey(queueName);
-                        }
 
-                    }
-                    else if (parameters[0] instanceof ExchangeDeclareBody)
-                    {
-                        ExchangeDeclareBody body = (ExchangeDeclareBody) parameters[0];
+                AMQShortString queueName = (AMQShortString) parameters[1];
+                Boolean autoDelete = (Boolean) parameters[0];
 
-                        AMQShortString exchangeName = body.getExchange();
+                if (autoDelete)// we have a temporary queue
+                {
+                    return (Boolean) create_queues.get(CREATE_QUEUE_TEMPORARY_KEY);
+                }
+                else
+                {
+                    // If there is a white list then check
+                    return create_queues_queues == null || create_queues_queues.containsKey(queueName);
+                }
+            case CREATEEXCHANGE:
+                Map rights = (Map) _permissions.get(permission);
 
-                        Map create_exchanges = (Map) createRights.get(CREATE_EXCHANGES_KEY);
+                AMQShortString exchangeName = (AMQShortString) parameters[0];
 
-                        // If the exchange list is doesn't exist then all is allowed else check the valid exchanges
-                        return create_exchanges == null || create_exchanges.containsKey(exchangeName);
-                    }
-                }
-                break;
+                // If the exchange list is doesn't exist then all is allowed else
+                // check the valid exchanges
+                return rights == null || rights.containsKey(exchangeName);
             case CONSUME: // Parameters :  AMQQueue
 
                 if (parameters.length == 1 && parameters[0] instanceof AMQQueue)
@@ -557,7 +544,7 @@
                 // Otherwise exchange must be listed in the white list
 
                 // If the map doesn't have the exchange then it isn't allowed
-                if (!exchanges.containsKey(parameters[0]))
+                if (!exchanges.containsKey(((Exchange) parameters[0]).getName()))
                 {
                     return false;
                 }
@@ -565,7 +552,7 @@
                 {
 
                     // Get valid routing keys
-                    HashSet routingKeys = (HashSet) exchanges.get(parameters[0]);
+                    HashSet routingKeys = (HashSet) exchanges.get(((Exchange)parameters[0]).getName());
 
                     // Having no routingKeys in the map then all are allowed.
                     if (routingKeys == null)

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java Wed Feb  4 15:33:26 2009
@@ -20,40 +20,10 @@
  */
 package org.apache.qpid.server.security.access.plugins;
 
-import org.apache.qpid.framing.AMQMethodBody;
-import org.apache.qpid.server.protocol.AMQProtocolSession;
-import org.apache.qpid.server.security.access.ACLPlugin;
-import org.apache.qpid.server.security.access.ACLManager;
-import org.apache.qpid.server.security.access.AccessResult;
-import org.apache.qpid.server.security.access.Accessable;
-import org.apache.qpid.server.security.access.Permission;
 import org.apache.commons.configuration.Configuration;
 
-public class AllowAll implements ACLPlugin
+public class AllowAll extends BasicACLPlugin
 {
-    public AccessResult authorise(AMQProtocolSession session, Permission permission, AMQMethodBody body, Object... parameters)
-    {
-        if (ACLManager.getLogger().isDebugEnabled())
-        {
-            ACLManager.getLogger().debug("Allowing user:" + session.getAuthorizedID() + " for :" + permission.toString()
-                                        + " on " + body.getClass().getSimpleName()
-                                        + (parameters == null || parameters.length == 0 ? "" : "-" + accessablesToString(parameters)));
-        }
-
-        return new AccessResult(this, AccessResult.AccessStatus.GRANTED);
-    }
-
-    public static String accessablesToString(Object[] accessObject)
-    {
-        StringBuilder sb = new StringBuilder();
-
-        for (Object access : accessObject)
-        {
-            sb.append(access.getClass().getSimpleName() + ":" + access.toString() + ", ");
-        }
-
-        return sb.delete(sb.length() - 2, sb.length()).toString();
-    }
 
     public String getPluginName()
     {
@@ -62,7 +32,13 @@
 
     public void setConfiguaration(Configuration config)
     {
-        //no-op
+        // no-op
     }
 
+    @Override
+    protected boolean getResult()
+    {
+        // Always allow
+        return true;
+    }
 }

Added: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicACLPlugin.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicACLPlugin.java?rev=740769&view=auto
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicACLPlugin.java (added)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicACLPlugin.java Wed Feb  4 15:33:26 2009
@@ -0,0 +1,123 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ *
+ *
+ */
+package org.apache.qpid.server.security.access.plugins;
+
+import org.apache.commons.configuration.Configuration;
+import org.apache.qpid.AMQConnectionException;
+import org.apache.qpid.framing.AMQShortString;
+import org.apache.qpid.server.exchange.Exchange;
+import org.apache.qpid.server.protocol.AMQProtocolSession;
+import org.apache.qpid.server.queue.AMQQueue;
+import org.apache.qpid.server.security.access.ACLPlugin;
+import org.apache.qpid.server.virtualhost.VirtualHost;
+
+public abstract class BasicACLPlugin implements ACLPlugin
+{
+
+    // Returns true or false if the plugin should authorise or deny the request    
+    protected abstract boolean getResult();
+    
+    @Override
+    public boolean authoriseBind(AMQProtocolSession session, Exchange exch,
+            AMQQueue queue, AMQShortString routingKey)
+    {
+        return getResult();
+    }
+
+    @Override
+    public boolean authoriseConnect(AMQProtocolSession session,
+            VirtualHost virtualHost)
+    {
+        return getResult();
+    }
+
+    @Override
+    public boolean authoriseConsume(AMQProtocolSession session, boolean noAck,
+            AMQQueue queue)
+    {
+        return getResult();    
+    }
+
+    @Override
+    public boolean authoriseConsume(AMQProtocolSession session,
+            boolean exclusive, boolean noAck, boolean noLocal, boolean nowait,
+            AMQQueue queue)
+    {
+        return getResult();
+    }
+
+    @Override
+    public boolean authoriseCreateExchange(AMQProtocolSession session,
+            boolean autoDelete, boolean durable, AMQShortString exchangeName,
+            boolean internal, boolean nowait, boolean passive,
+            AMQShortString exchangeType)
+    {
+        return getResult();
+    }
+
+    @Override
+    public boolean authoriseCreateQueue(AMQProtocolSession session,
+            boolean autoDelete, boolean durable, boolean exclusive,
+            boolean nowait, boolean passive, AMQShortString queue)
+    {
+        return getResult();
+    }
+
+    @Override
+    public boolean authoriseDelete(AMQProtocolSession session, AMQQueue queue)
+    {
+        return getResult();
+    }
+
+    @Override
+    public boolean authoriseDelete(AMQProtocolSession session, Exchange exchange)
+    {
+        return getResult();
+    }
+
+    @Override
+    public boolean authorisePublish(AMQProtocolSession session,
+            boolean immediate, boolean mandatory, AMQShortString routingKey,
+            Exchange e)
+    {
+        return getResult();
+    }
+
+    @Override
+    public boolean authorisePurge(AMQProtocolSession session, AMQQueue queue)
+    {
+        return getResult();
+    }
+
+    @Override
+    public boolean authoriseUnbind(AMQProtocolSession session, Exchange exch,
+            AMQShortString routingKey, AMQQueue queue)
+    {
+        return getResult();
+    }
+
+    @Override
+    public void setConfiguaration(Configuration config)
+    {
+        // no-op
+    }
+
+}

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java Wed Feb  4 15:33:26 2009
@@ -20,29 +20,29 @@
  */
 package org.apache.qpid.server.security.access.plugins;
 
+import org.apache.commons.configuration.Configuration;
+import org.apache.qpid.AMQConnectionException;
 import org.apache.qpid.framing.AMQMethodBody;
 import org.apache.qpid.protocol.AMQConstant;
 import org.apache.qpid.server.protocol.AMQProtocolSession;
 import org.apache.qpid.server.security.access.ACLManager;
-import org.apache.qpid.server.security.access.ACLPlugin;
 import org.apache.qpid.server.security.access.AccessResult;
 import org.apache.qpid.server.security.access.Permission;
-import org.apache.qpid.AMQConnectionException;
-import org.apache.commons.configuration.Configuration;
 
-public class DenyAll implements ACLPlugin
+public class DenyAll extends BasicACLPlugin
 {
-    public AccessResult authorise(AMQProtocolSession session, Permission permission, AMQMethodBody body, Object... parameters) throws AMQConnectionException
+    public AccessResult authorise(AMQProtocolSession session,
+            Permission permission, AMQMethodBody body, Object... parameters)
+            throws AMQConnectionException
     {
 
         if (ACLManager.getLogger().isInfoEnabled())
         {
+            ACLManager.getLogger().info(
+                    "Denying user:" + session.getAuthorizedID());
         }
-        ACLManager.getLogger().info("Denying user:" + session.getAuthorizedID() + " for :" + permission.toString()
-                                    + " on " + body.getClass().getSimpleName()
-                                    + (parameters == null || parameters.length == 0 ? "" : "-" + AllowAll.accessablesToString(parameters)));
-
-        throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "DenyAll Plugin");
+        throw body.getConnectionException(AMQConstant.ACCESS_REFUSED,
+                "DenyAll Plugin");
     }
 
     public String getPluginName()
@@ -52,6 +52,14 @@
 
     public void setConfiguaration(Configuration config)
     {
-        //no-op
+        // no-op
     }
+
+    @Override 
+    protected boolean getResult()
+    {
+        // Always deny
+        return false;
+    }
+
 }

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java Wed Feb  4 15:33:26 2009
@@ -32,11 +32,13 @@
 import org.apache.qpid.protocol.AMQConstant;
 import org.apache.qpid.server.exchange.Exchange;
 import org.apache.qpid.server.protocol.AMQProtocolSession;
+import org.apache.qpid.server.queue.AMQQueue;
 import org.apache.qpid.server.security.access.ACLManager;
 import org.apache.qpid.server.security.access.ACLPlugin;
 import org.apache.qpid.server.security.access.AccessResult;
 import org.apache.qpid.server.security.access.Permission;
 import org.apache.qpid.server.security.access.PrincipalPermissions;
+import org.apache.qpid.server.virtualhost.VirtualHost;
 
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -69,16 +71,16 @@
     }
 
     /**
-     * Publish format takes
-     * Exchange + Routing Key Pairs
-     *
-     * @param config XML Configuration
+     * Publish format takes Exchange + Routing Key Pairs
+     * 
+     * @param config
+     *            XML Configuration
      */
     private void processPublish(Configuration config)
     {
         Configuration publishConfig = config.subset("security.access_control_list.publish");
 
-        //Process users that have full publish permission
+        // Process users that have full publish permission
         String[] users = publishConfig.getStringArray("users.user");
 
         for (String user : users)
@@ -92,33 +94,33 @@
 
         while (!exchangeConfig.isEmpty())
         {
-            //Get Exchange Name
+            // Get Exchange Name
             AMQShortString exchangeName = new AMQShortString(exchangeConfig.getString("name"));
 
-            //Get Routing Keys
+            // Get Routing Keys
             int keyCount = 0;
             Configuration routingkeyConfig = exchangeConfig.subset("routing_keys.routing_key(" + keyCount + ")");
 
             while (!routingkeyConfig.isEmpty())
             {
-                //Get RoutingKey Value
+                // Get RoutingKey Value
                 AMQShortString routingKeyValue = new AMQShortString(routingkeyConfig.getString("value"));
 
-                //Apply Exchange + RoutingKey permissions to Users
+                // Apply Exchange + RoutingKey permissions to Users
                 users = routingkeyConfig.getStringArray("users.user");
                 for (String user : users)
                 {
                     grant(Permission.PUBLISH, user, exchangeName, routingKeyValue);
                 }
 
-                //Apply permissions to Groups
+                // Apply permissions to Groups
 
                 // Check for more configs
                 keyCount++;
                 routingkeyConfig = exchangeConfig.subset("routing_keys.routing_key(" + keyCount + ")");
             }
 
-            //Apply Exchange wide permissions to Users
+            // Apply Exchange wide permissions to Users
             users = exchangeConfig.getStringArray("exchange(" + exchangeCount + ").users.user");
 
             for (String user : users)
@@ -126,7 +128,7 @@
                 grant(Permission.PUBLISH, user, exchangeName);
             }
 
-            //Apply permissions to Groups
+            // Apply permissions to Groups
             exchangeCount++;
             exchangeConfig = publishConfig.subset("exchanges.exchange(" + exchangeCount + ")");
         }
@@ -155,20 +157,20 @@
 
         while (!queueConfig.isEmpty())
         {
-            //Get queue Name
+            // Get queue Name
             AMQShortString queueName = new AMQShortString(queueConfig.getString("name"));
             // if there is no name then there may be a temporary element
             boolean temporary = queueConfig.containsKey("temporary");
             boolean ownQueues = queueConfig.containsKey("own_queues");
 
-            //Process permissions for this queue
+            // Process permissions for this queue
             String[] users = queueConfig.getStringArray("users.user");
             for (String user : users)
             {
                 grant(Permission.CONSUME, user, queueName, temporary, ownQueues);
             }
 
-            //See if we have another config
+            // See if we have another config
             queueCount++;
             queueConfig = consumeConfig.subset("queues.queue(" + queueCount + ")");
         }
@@ -192,7 +194,7 @@
 
         while (!queueConfig.isEmpty())
         {
-            //Get queue Name
+            // Get queue Name
             AMQShortString queueName = new AMQShortString(queueConfig.getString("name"));
 
             // if there is no name then there may be a temporary element
@@ -207,17 +209,16 @@
                 AMQShortString exchange = new AMQShortString(exchangeConfig.getString("name"));
                 AMQShortString routingKey = new AMQShortString(exchangeConfig.getString("routing_key"));
 
-                //Process permissions for this queue
+                // Process permissions for this queue
                 String[] users = exchangeConfig.getStringArray("users.user");
                 for (String user : users)
                 {
-                    grant(Permission.CREATE, user, temporary,
-                          (queueName.equals("") ? null : queueName),
-                          (exchange.equals("") ? null : exchange),
-                          (routingKey.equals("") ? null : routingKey));
+                    grant(Permission.CREATEEXCHANGE, user, exchange);
+                    grant(Permission.CREATEQUEUE, user, temporary, (queueName.equals("") ? null : queueName), (exchange
+                            .equals("") ? null : exchange), (routingKey.equals("") ? null : routingKey));
                 }
 
-                //See if we have another config
+                // See if we have another config
                 exchangeCount++;
                 exchangeConfig = queueConfig.subset("exchanges.exchange(" + exchangeCount + ")");
             }
@@ -227,10 +228,10 @@
 
             for (String user : users)
             {
-                grant(Permission.CREATE, user, temporary, queueName);
+                grant(Permission.CREATEQUEUE, user, temporary, queueName);
             }
 
-            //See if we have another config
+            // See if we have another config
             queueCount++;
             queueConfig = createConfig.subset("queues.queue(" + queueCount + ")");
         }
@@ -244,14 +245,14 @@
             AMQShortString exchange = new AMQShortString(exchangeConfig.getString("name"));
             AMQShortString clazz = new AMQShortString(exchangeConfig.getString("class"));
 
-            //Process permissions for this queue
+            // Process permissions for this queue
             String[] users = exchangeConfig.getStringArray("users.user");
             for (String user : users)
             {
-                grant(Permission.CREATE, user, exchange, clazz);
+                grant(Permission.CREATEEXCHANGE, user, exchange, clazz);
             }
 
-            //See if we have another config
+            // See if we have another config
             exchangeCount++;
             exchangeConfig = queueConfig.subset("exchanges.exchange(" + exchangeCount + ")");
         }
@@ -261,10 +262,10 @@
 
         for (String user : users)
         {
-            grant(Permission.CREATE, user);
+            grant(Permission.CREATEEXCHANGE, user);
+            grant(Permission.CREATEQUEUE, user);
         }
 
-
     }
 
     public String getPluginName()
@@ -272,71 +273,153 @@
         return "Simple";
     }
 
-    public AccessResult authorise(AMQProtocolSession session, Permission permission, AMQMethodBody body, Object... parameters) throws AMQConnectionException
+    @Override
+    public boolean authoriseBind(AMQProtocolSession session, Exchange exch, AMQQueue queue, AMQShortString routingKey)
     {
-        String error = "";
-        
-        if (ACLManager.getLogger().isInfoEnabled())
+        PrincipalPermissions principalPermissions = _users.get(session.getAuthorizedID().getName());
+        if (principalPermissions == null)
+        {
+            return false;
+        }
+        else
         {
-            ACLManager.getLogger().info("Simple Authorisation processing user:" + session.getAuthorizedID() + " for :" + permission.toString()
-                    + " on " + body.getClass().getSimpleName()
-                    + (parameters == null || parameters.length == 0 ? "" : "-" + AllowAll.accessablesToString(parameters)));
+            return principalPermissions.authorise(Permission.BIND, null, exch, queue, routingKey);
         }
+    }
 
-        String username = session.getAuthorizedID().getName();
+    @Override
+    public boolean authoriseConnect(AMQProtocolSession session, VirtualHost virtualHost)
+    {
+        PrincipalPermissions principalPermissions = _users.get(session.getAuthorizedID().getName());
+        if (principalPermissions == null)
+        {
+            return false;
+        }
+        else
+        {
+            return principalPermissions.authorise(Permission.ACCESS);
+        }
+    }
 
-        //Get the Users Permissions
-        PrincipalPermissions permissions = _users.get(username);
+    @Override
+    public boolean authoriseConsume(AMQProtocolSession session, boolean noAck, AMQQueue queue)
+    {
+        PrincipalPermissions principalPermissions = _users.get(session.getAuthorizedID().getName());
+        if (principalPermissions == null)
+        {
+            return false;
+        }
+        else
+        {
+            return principalPermissions.authorise(Permission.CONSUME, queue);
+        }
+    }
 
-        if (permissions != null)
+    @Override
+    public boolean authoriseConsume(AMQProtocolSession session, boolean exclusive, boolean noAck, boolean noLocal,
+            boolean nowait, AMQQueue queue)
+    {
+        return authoriseConsume(session, noAck, queue);
+    }
+
+    @Override
+    public boolean authoriseCreateExchange(AMQProtocolSession session, boolean autoDelete, boolean durable,
+            AMQShortString exchangeName, boolean internal, boolean nowait, boolean passive, AMQShortString exchangeType)
+    {
+        PrincipalPermissions principalPermissions = _users.get(session.getAuthorizedID().getName());
+        if (principalPermissions == null)
         {
-            switch (permission)
-            {
-                case ACCESS:
-                    return GRANTED;
-                case BIND:  // Body QueueDeclareBody - Parameters : Exchange, Queue, QueueName
-                    // Body QueueBindBody - Paramters : Exchange, Queue, QueueName
-                    if (parameters.length == 3)
-                    {
-                        // Parameters : Exchange, Queue, RoutingKey
-                        if (permissions.authorise(Permission.BIND, body, parameters[0], parameters[1], parameters[2]))
-                        {
-                            return GRANTED;
-                        }
-                    }
-                    break;
-                case CONSUME: // Parameters : none
-                    if (parameters.length == 1 && permissions.authorise(Permission.CONSUME, parameters[0]))
-                    {
-                        return GRANTED;
-                    }
-                    break;
-                case CREATE: // Body : QueueDeclareBody | ExchangeDeclareBody - Parameters : none
-                    if (permissions.authorise(Permission.CREATE, body))
-                    {
-                        return GRANTED;
-                    }
-                    break;
-                case PUBLISH: // Body : BasicPublishBody  Parameters : exchange
-                    if (parameters.length == 1 && parameters[0] instanceof Exchange)
-                    {
-                        if (permissions.authorise(Permission.PUBLISH, ((Exchange) parameters[0]).getName(),
-                                                  ((BasicPublishBody) body).getRoutingKey()))
-                        {
-                            return GRANTED;
-                        }
-                    }
-                    break;
-                case PURGE:
-                    break;
-                case DELETE:
-                    break;
-                case UNBIND:
-                    break;
-            }
+            return false;
+        }
+        else
+        {
+            return principalPermissions.authorise(Permission.CREATEEXCHANGE, exchangeName);
+        }
+    }
+
+    @Override
+    public boolean authoriseCreateQueue(AMQProtocolSession session, boolean autoDelete, boolean durable, boolean exclusive,
+            boolean nowait, boolean passive, AMQShortString queue)
+    {
+        PrincipalPermissions principalPermissions = _users.get(session.getAuthorizedID().getName());
+        if (principalPermissions == null)
+        {
+            return false;
         }
+        else
+        {
+            return principalPermissions.authorise(Permission.CREATEQUEUE, autoDelete, queue);
+        }
+    }
 
-        //todo potential refactor this ConnectionException Out of here
-        throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, error);
+    @Override
+    public boolean authoriseDelete(AMQProtocolSession session, AMQQueue queue)
+    {
+        PrincipalPermissions principalPermissions = _users.get(session.getAuthorizedID().getName());
+        if (principalPermissions == null)
+        {
+            return false;
+        }
+        else
+        {
+            return principalPermissions.authorise(Permission.DELETE);
+        }
+    }
+
+    @Override
+    public boolean authoriseDelete(AMQProtocolSession session, Exchange exchange)
+    {
+        PrincipalPermissions principalPermissions = _users.get(session.getAuthorizedID().getName());
+        if (principalPermissions == null)
+        {
+            return false;
+        }
+        else
+        {
+            return principalPermissions.authorise(Permission.DELETE);
+        }
+    }
+
+    @Override
+    public boolean authorisePublish(AMQProtocolSession session, boolean immediate, boolean mandatory,
+            AMQShortString routingKey, Exchange e)
+    {
+        PrincipalPermissions principalPermissions = _users.get(session.getAuthorizedID().getName());
+        if (principalPermissions == null)
+        {
+            return false;
+        }
+        else
+        {
+            return principalPermissions.authorise(Permission.PUBLISH, e, routingKey);
+        }
+    }
+
+    @Override
+    public boolean authorisePurge(AMQProtocolSession session, AMQQueue queue)
+    {
+        PrincipalPermissions principalPermissions = _users.get(session.getAuthorizedID().getName());
+        if (principalPermissions == null)
+        {
+            return false;
+        }
+        else
+        {
+            return principalPermissions.authorise(Permission.PURGE);
+        }
+    }
+
+    @Override
+    public boolean authoriseUnbind(AMQProtocolSession session, Exchange exch, AMQShortString routingKey, AMQQueue queue)
+    {
+        PrincipalPermissions principalPermissions = _users.get(session.getAuthorizedID().getName());
+        if (principalPermissions == null)
+        {
+            return false;
+        }
+        else
+        {
+            return principalPermissions.authorise(Permission.UNBIND);
+        }
     }
 }

Modified: qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/security/access/PrincipalPermissionsTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/security/access/PrincipalPermissionsTest.java?rev=740769&r1=740768&r2=740769&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/security/access/PrincipalPermissionsTest.java (original)
+++ qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/security/access/PrincipalPermissionsTest.java Wed Feb  4 15:33:26 2009
@@ -54,7 +54,7 @@
     private AMQShortString _exchangeType = new AMQShortString("direct");
     private boolean _internal = false;
 
-    private DirectExchange _exchange = new DirectExchange();
+    private DirectExchange _exchange;
     private VirtualHost _virtualHost;
     private AMQShortString _owner = new AMQShortString(this.getClass().getName()+"owner");
     private AMQQueue _queue;
@@ -67,6 +67,7 @@
         try 
         {
             _virtualHost = new VirtualHost("localhost", new SkeletonMessageStore());
+            _exchange = DirectExchange.TYPE.newInstance(_virtualHost, _exchangeName, _durable, _ticket, _autoDelete);
             _queue = AMQQueueFactory.createAMQQueueImpl(_queueName, false, _owner , false, _virtualHost, _arguments);
         } 
         catch (Exception e)
@@ -96,15 +97,11 @@
     public void testQueueCreate()
     {
         Object[] grantArgs = new Object[]{_temporary , _queueName, _exchangeName, _routingKey};
+        Object[] authArgs = new Object[]{_autoDelete, _queueName};
         
-        
-        QueueDeclareBodyImpl queueDeclare = new QueueDeclareBodyImpl(
-                _ticket, _queueName, _passive, _durable, _exclusive, _autoDelete, _nowait, _arguments);
-        Object[] authArgs = new Object[]{queueDeclare};
-        
-        assertFalse(_perms.authorise(Permission.CREATE, authArgs));
-        _perms.grant(Permission.CREATE, grantArgs);
-        assertTrue(_perms.authorise(Permission.CREATE, authArgs));
+        assertFalse(_perms.authorise(Permission.CREATEQUEUE, authArgs));
+        _perms.grant(Permission.CREATEQUEUE, grantArgs);
+        assertTrue(_perms.authorise(Permission.CREATEQUEUE, authArgs));
     }
     
     
@@ -117,9 +114,9 @@
         Object[] authArgs = new Object[]{exchangeDeclare};
         Object[] grantArgs = new Object[]{_exchangeName, _exchangeType};
         
-        assertFalse(_perms.authorise(Permission.CREATE, authArgs));
-        _perms.grant(Permission.CREATE, grantArgs);
-        assertTrue(_perms.authorise(Permission.CREATE, authArgs));
+        assertFalse(_perms.authorise(Permission.CREATEEXCHANGE, authArgs));
+        _perms.grant(Permission.CREATEEXCHANGE, grantArgs);
+        assertTrue(_perms.authorise(Permission.CREATEEXCHANGE, authArgs));
     }
     
     public void testConsume()
@@ -137,9 +134,10 @@
     public void testPublish()
     {
         Object[] authArgs = new Object[]{_exchange, _routingKey};
+        Object[] grantArgs = new Object[]{_exchange.getName(), _routingKey};
         
         assertFalse(_perms.authorise(Permission.PUBLISH, authArgs));
-        _perms.grant(Permission.PUBLISH, authArgs);
+        _perms.grant(Permission.PUBLISH, grantArgs);
         assertTrue(_perms.authorise(Permission.PUBLISH, authArgs));
     }
     



---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:commits-subscribe@qpid.apache.org