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.

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

19 files changed, 537 insertions, 313 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicConsumeMethodHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicConsumeMethodHandler.java
index 5342a7f518..08610f24cd 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicConsumeMethodHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicConsumeMethodHandler.java
@@ -97,8 +97,13 @@ public class BasicConsumeMethodHandler implements StateAwareMethodListener<Basic
 
                 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)
                 {
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicGetMethodHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicGetMethodHandler.java
index be1135dd91..001b7858ec 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicGetMethodHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicGetMethodHandler.java
@@ -94,7 +94,10 @@ public class BasicGetMethodHandler implements StateAwareMethodListener<BasicGetB
             {

 

                 //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()))

                 {

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicPublishMethodHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicPublishMethodHandler.java
index e8e42454de..a7d3ad6217 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicPublishMethodHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/BasicPublishMethodHandler.java
@@ -87,7 +87,12 @@ public class BasicPublishMethodHandler implements StateAwareMethodListener<Basic
             }
 
             //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);
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java
index f99e650979..824f084f57 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java
@@ -78,7 +78,10 @@ public class ConnectionOpenMethodHandler implements StateAwareMethodListener<Con
             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)
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java
index 39b048aecb..ba60808492 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java
@@ -61,8 +61,14 @@ public class ExchangeDeclareHandler implements StateAwareMethodListener<Exchange
 
         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())
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeleteHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeleteHandler.java
index 888ffcb2e5..bd4b610933 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeleteHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeleteHandler.java
@@ -23,6 +23,7 @@ package org.apache.qpid.server.handler;
 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 @@ public class ExchangeDeleteHandler implements StateAwareMethodListener<ExchangeD
         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
         {
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueBindHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueBindHandler.java
index 46182e8c00..84491c1d2e 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueBindHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueBindHandler.java
@@ -108,7 +108,11 @@ public class QueueBindHandler implements StateAwareMethodListener<QueueBindBody>
         {
 
             //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))
             {
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java
index 3047643021..71f38cb28a 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java
@@ -78,11 +78,14 @@ public class QueueDeclareHandler implements StateAwareMethodListener<QueueDeclar
 
         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
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeleteHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeleteHandler.java
index dfc36f5b93..b397db9246 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeleteHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeleteHandler.java
@@ -105,7 +105,10 @@ public class QueueDeleteHandler implements StateAwareMethodListener<QueueDeleteB
             {
                 
                 //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();
 
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueuePurgeHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueuePurgeHandler.java
index 7377862875..2768518f53 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueuePurgeHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueuePurgeHandler.java
@@ -101,7 +101,10 @@ public class QueuePurgeHandler implements StateAwareMethodListener<QueuePurgeBod
         {

 

                 //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());

 

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueUnbindHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueUnbindHandler.java
index 6331a0365d..d4272239d1 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueUnbindHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/QueueUnbindHandler.java
@@ -101,7 +101,10 @@ public class QueueUnbindHandler implements StateAwareMethodListener<QueueUnbindB
         }
 
         //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
         {
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ACLPlugin.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ACLPlugin.java
index 7855f147b4..164fbad911 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ACLPlugin.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ACLPlugin.java
@@ -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);
+
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/Permission.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/Permission.java
index 00757a4f8c..b65b0cdc6c 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/Permission.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/Permission.java
@@ -28,7 +28,8 @@ public enum Permission
 {
     CONSUME,
     PUBLISH,
-    CREATE,
+    CREATEQUEUE,
+    CREATEEXCHANGE,
     ACCESS,
     BIND,
     UNBIND,
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/PrincipalPermissions.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/PrincipalPermissions.java
index dd207dc001..00c63ede7c 100755
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/PrincipalPermissions.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/PrincipalPermissions.java
@@ -68,9 +68,8 @@ public class PrincipalPermissions
      *  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 @@ public class PrincipalPermissions
 
 
                 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 @@ public class PrincipalPermissions
                     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 @@ public class PrincipalPermissions
      *  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 @@ public class PrincipalPermissions
                 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 @@ public class PrincipalPermissions
                     return true;
                 }
 
-            case CREATE:// Paramters : QueueDeclareBody || ExchangeDeclareBody
+            case CREATEQUEUE:// Parameters : boolean autodelete, AMQShortString name
 
                 Map createRights = (Map) _permissions.get(permission);
 
@@ -445,46 +445,33 @@ public class PrincipalPermissions
                     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 @@ public class PrincipalPermissions
                 // 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 @@ public class PrincipalPermissions
                 {
 
                     // 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)
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java
index 9b784069dd..f78c9a2e16 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java
@@ -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 class AllowAll implements ACLPlugin
 
     public void setConfiguaration(Configuration config)
     {
-        //no-op
+        // no-op
     }
 
+    @Override
+    protected boolean getResult()
+    {
+        // Always allow
+        return true;
+    }
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicACLPlugin.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicACLPlugin.java
new file mode 100644
index 0000000000..26d3162f3a
--- /dev/null
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicACLPlugin.java
@@ -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
+    }
+
+}
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java
index 80c125e737..1645236382 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java
@@ -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 class DenyAll implements ACLPlugin
 
     public void setConfiguaration(Configuration config)
     {
-        //no-op
+        // no-op
     }
+
+    @Override 
+    protected boolean getResult()
+    {
+        // Always deny
+        return false;
+    }
+
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java
index 251f4e6330..4fe1f8e777 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java
@@ -32,11 +32,13 @@ import org.apache.qpid.framing.BasicPublishBody;
 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 @@ public class SimpleXML implements ACLPlugin
     }
 
     /**
-     * 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 @@ public class SimpleXML implements ACLPlugin
 
         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 @@ public class SimpleXML implements ACLPlugin
                 grant(Permission.PUBLISH, user, exchangeName);
             }
 
-            //Apply permissions to Groups
+            // Apply permissions to Groups
             exchangeCount++;
             exchangeConfig = publishConfig.subset("exchanges.exchange(" + exchangeCount + ")");
         }
@@ -155,20 +157,20 @@ public class SimpleXML implements ACLPlugin
 
         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 @@ public class SimpleXML implements ACLPlugin
 
         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 @@ public class SimpleXML implements ACLPlugin
                 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 @@ public class SimpleXML implements ACLPlugin
 
             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 @@ public class SimpleXML implements ACLPlugin
             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 @@ public class SimpleXML implements ACLPlugin
 
         for (String user : users)
         {
-            grant(Permission.CREATE, user);
+            grant(Permission.CREATEEXCHANGE, user);
+            grant(Permission.CREATEQUEUE, user);
         }
 
-
     }
 
     public String getPluginName()
@@ -272,71 +273,153 @@ public class SimpleXML implements ACLPlugin
         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);
+        }
     }
 }
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/access/PrincipalPermissionsTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/access/PrincipalPermissionsTest.java
index 7927339491..df41ac9dc2 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/access/PrincipalPermissionsTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/access/PrincipalPermissionsTest.java
@@ -54,7 +54,7 @@ public class PrincipalPermissionsTest extends TestCase
     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 @@ public class PrincipalPermissionsTest extends TestCase
         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 class PrincipalPermissionsTest extends TestCase
     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 @@ public class PrincipalPermissionsTest extends TestCase
         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 class PrincipalPermissionsTest extends TestCase
     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));
     }