From f635dc00fe03b7930154cc8e8a06dc0aa17039ab Mon Sep 17 00:00:00 2001 From: Aidan Skinner Date: Wed, 4 Feb 2009 15:33:26 +0000 Subject: 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 --- .../server/handler/BasicConsumeMethodHandler.java | 9 +- .../qpid/server/handler/BasicGetMethodHandler.java | 5 +- .../server/handler/BasicPublishMethodHandler.java | 7 +- .../handler/ConnectionOpenMethodHandler.java | 5 +- .../server/handler/ExchangeDeclareHandler.java | 10 +- .../qpid/server/handler/ExchangeDeleteHandler.java | 8 +- .../qpid/server/handler/QueueBindHandler.java | 6 +- .../qpid/server/handler/QueueDeclareHandler.java | 9 +- .../qpid/server/handler/QueueDeleteHandler.java | 5 +- .../qpid/server/handler/QueuePurgeHandler.java | 5 +- .../qpid/server/handler/QueueUnbindHandler.java | 5 +- .../qpid/server/security/access/ACLPlugin.java | 60 ++--- .../qpid/server/security/access/Permission.java | 3 +- .../security/access/PrincipalPermissions.java | 241 +++++++++---------- .../server/security/access/plugins/AllowAll.java | 40 +--- .../security/access/plugins/BasicACLPlugin.java | 123 ++++++++++ .../server/security/access/plugins/DenyAll.java | 30 ++- .../server/security/access/plugins/SimpleXML.java | 255 ++++++++++++++------- .../security/access/PrincipalPermissionsTest.java | 24 +- 19 files changed, 537 insertions(+), 313 deletions(-) create mode 100644 qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicACLPlugin.java (limited to 'qpid/java/broker') 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 { //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 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)); } -- cgit v1.2.1