diff options
| author | Robert Godfrey <rgodfrey@apache.org> | 2014-02-18 23:07:41 +0000 |
|---|---|---|
| committer | Robert Godfrey <rgodfrey@apache.org> | 2014-02-18 23:07:41 +0000 |
| commit | d6f465d6a10b4d1d9ced48a10ae980c98697ff5b (patch) | |
| tree | 69c47633c086c1b0c2f725c37a0acf80cd9fb34a /qpid/java | |
| parent | 0ef258cebe7b0fbb4b1f1c6cbb5c74d24ea6115d (diff) | |
| download | qpid-python-d6f465d6a10b4d1d9ced48a10ae980c98697ff5b.tar.gz | |
QPID-5562 : [Java Broker] make all failed ACL checks throw AccessControlException
git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1569552 13f79535-47bb-0310-9956-ffa450edef68
Diffstat (limited to 'qpid/java')
58 files changed, 386 insertions, 631 deletions
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java index 6f1db59b1a..cb5902d234 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java @@ -22,7 +22,6 @@ package org.apache.qpid.server.exchange; import java.util.ArrayList; import org.apache.log4j.Logger; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.binding.Binding; import org.apache.qpid.server.consumer.Consumer; import org.apache.qpid.server.logging.LogSubject; @@ -134,7 +133,7 @@ public abstract class AbstractExchange implements Exchange return _autoDelete; } - public void close() throws QpidSecurityException + public void close() { if(_closed.compareAndSet(false,true)) @@ -526,7 +525,6 @@ public abstract class AbstractExchange implements Exchange @Override public boolean addBinding(String bindingKey, AMQQueue queue, Map<String, Object> arguments) - throws QpidSecurityException { return makeBinding(null, bindingKey, queue, arguments, false, false); } @@ -535,7 +533,6 @@ public abstract class AbstractExchange implements Exchange public boolean replaceBinding(final UUID id, final String bindingKey, final AMQQueue queue, final Map<String, Object> arguments) - throws QpidSecurityException { return makeBinding(id, bindingKey, queue, arguments, false, true); } @@ -543,20 +540,18 @@ public abstract class AbstractExchange implements Exchange @Override public void restoreBinding(final UUID id, final String bindingKey, final AMQQueue queue, final Map<String, Object> argumentMap) - throws QpidSecurityException { makeBinding(id, bindingKey,queue, argumentMap,true, false); } @Override - public void removeBinding(final Binding b) throws QpidSecurityException + public void removeBinding(final Binding b) { removeBinding(b.getBindingKey(), b.getQueue(), b.getArguments()); } @Override public Binding removeBinding(String bindingKey, AMQQueue queue, Map<String, Object> arguments) - throws QpidSecurityException { assert queue != null; @@ -569,14 +564,8 @@ public abstract class AbstractExchange implements Exchange arguments = Collections.emptyMap(); } - // The default exchange bindings must reflect the existence of queues, allow - // all operations on it to succeed. It is up to the broker to prevent illegal - // attempts at binding to this exchange, not the ACLs. // Check access - if (!_virtualHost.getSecurityManager().authoriseUnbind(this, bindingKey, queue)) - { - throw new QpidSecurityException("Permission denied: unbinding " + bindingKey); - } + _virtualHost.getSecurityManager().authoriseUnbind(this, bindingKey, queue); BindingImpl b = _bindingsMap.remove(new BindingImpl(null, bindingKey,queue,arguments)); @@ -622,7 +611,7 @@ public abstract class AbstractExchange implements Exchange AMQQueue queue, Map<String, Object> arguments, boolean restore, - boolean force) throws QpidSecurityException + boolean force) { assert queue != null; @@ -636,10 +625,7 @@ public abstract class AbstractExchange implements Exchange } //Perform ACLs - if (!_virtualHost.getSecurityManager().authoriseBind(AbstractExchange.this, queue, bindingKey)) - { - throw new QpidSecurityException("Permission denied: binding " + bindingKey); - } + _virtualHost.getSecurityManager().authoriseBind(AbstractExchange.this, queue, bindingKey); if (id == null) { @@ -690,7 +676,7 @@ public abstract class AbstractExchange implements Exchange } - public void onClose(final Exchange exchange) throws QpidSecurityException + public void onClose(final Exchange exchange) { removeBinding(this); } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchange.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchange.java index d7dfcfbfc5..cc6131f6b5 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchange.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchange.java @@ -18,6 +18,7 @@ */ package org.apache.qpid.server.exchange; +import java.security.AccessControlException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -27,7 +28,6 @@ import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.log4j.Logger; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.exchange.ExchangeDefaults; import org.apache.qpid.server.binding.Binding; import org.apache.qpid.server.consumer.Consumer; @@ -97,59 +97,55 @@ public class DefaultExchange implements Exchange @Override public long getByteDrops() { - return 0; //To change body of implemented methods use File | Settings | File Templates. + return 0; } @Override public long getByteReceives() { - return 0; //To change body of implemented methods use File | Settings | File Templates. + return 0; } @Override public long getMsgDrops() { - return 0; //To change body of implemented methods use File | Settings | File Templates. + return 0; } @Override public long getMsgReceives() { - return 0; //To change body of implemented methods use File | Settings | File Templates. + return 0; } @Override public boolean addBinding(String bindingKey, AMQQueue queue, Map<String, Object> arguments) - throws QpidSecurityException { - throw new QpidSecurityException("Cannot add bindings to the default exchange"); + throw new AccessControlException("Cannot add bindings to the default exchange"); } @Override public boolean replaceBinding(UUID id, String bindingKey, AMQQueue queue, Map<String, Object> arguments) - throws QpidSecurityException { - throw new QpidSecurityException("Cannot replace bindings on the default exchange"); + throw new AccessControlException("Cannot replace bindings on the default exchange"); } @Override public void restoreBinding(UUID id, String bindingKey, AMQQueue queue, Map<String, Object> argumentMap) - throws QpidSecurityException { _logger.warn("Bindings to the default exchange should not be stored in the configuration store"); } @Override - public void removeBinding(Binding b) throws QpidSecurityException + public void removeBinding(Binding b) { - throw new QpidSecurityException("Cannot remove bindings to the default exchange"); + throw new AccessControlException("Cannot remove bindings to the default exchange"); } @Override public Binding removeBinding(String bindingKey, AMQQueue queue, Map<String, Object> arguments) - throws QpidSecurityException { - throw new QpidSecurityException("Cannot remove bindings to the default exchange"); + throw new AccessControlException("Cannot remove bindings to the default exchange"); } @Override diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeFactory.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeFactory.java index a63e1afa3a..21586c6a4a 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeFactory.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeFactory.java @@ -22,7 +22,6 @@ package org.apache.qpid.server.exchange; import org.apache.log4j.Logger; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.exchange.ExchangeDefaults; import org.apache.qpid.server.model.UUIDGenerator; import org.apache.qpid.server.plugin.ExchangeType; @@ -107,7 +106,7 @@ public class DefaultExchangeFactory implements ExchangeFactory } public Exchange createExchange(String exchange, String type, boolean durable, boolean autoDelete) - throws AMQUnknownExchangeType, QpidSecurityException + throws AMQUnknownExchangeType { UUID id = UUIDGenerator.generateExchangeUUID(exchange, _host.getName()); @@ -115,14 +114,10 @@ public class DefaultExchangeFactory implements ExchangeFactory } public Exchange createExchange(UUID id, String exchange, String type, boolean durable, boolean autoDelete) - throws QpidSecurityException, AMQUnknownExchangeType + throws AMQUnknownExchangeType { // Check access - if (!_host.getSecurityManager().authoriseCreateExchange(autoDelete, durable, exchange, null, null, null, type)) - { - String description = "Permission denied: exchange-name '" + exchange + "'"; - throw new QpidSecurityException(description); - } + _host.getSecurityManager().authoriseCreateExchange(autoDelete, durable, exchange, null, null, null, type); ExchangeType<? extends Exchange> exchType = _exchangeClassMap.get(type); if (exchType == null) @@ -136,7 +131,7 @@ public class DefaultExchangeFactory implements ExchangeFactory @Override public Exchange restoreExchange(UUID id, String exchange, String type, boolean autoDelete) - throws AMQUnknownExchangeType, QpidSecurityException + throws AMQUnknownExchangeType { return createExchange(id, exchange, type, true, autoDelete); } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeRegistry.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeRegistry.java index 8d2d04a464..ffd515e385 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeRegistry.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeRegistry.java @@ -21,7 +21,6 @@ package org.apache.qpid.server.exchange; import org.apache.log4j.Logger; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.exchange.ExchangeDefaults; import org.apache.qpid.server.model.UUIDGenerator; import org.apache.qpid.server.plugin.ExchangeType; @@ -100,16 +99,13 @@ public class DefaultExchangeRegistry implements ExchangeRegistry return _defaultExchange; } - public boolean unregisterExchange(String name, boolean inUse) throws QpidSecurityException + public boolean unregisterExchange(String name, boolean inUse) { final Exchange exchange = _exchangeMap.get(name); if (exchange != null) { - if (!_host.getSecurityManager().authoriseDelete(exchange)) - { - throw new QpidSecurityException(); - } + _host.getSecurityManager().authoriseDelete(exchange); // TODO: check inUse argument diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/Exchange.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/Exchange.java index 38249e6b1f..5625a0aca4 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/Exchange.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/Exchange.java @@ -20,7 +20,6 @@ */ package org.apache.qpid.server.exchange; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.binding.Binding; import org.apache.qpid.server.message.MessageDestination; import org.apache.qpid.server.plugin.ExchangeType; @@ -66,26 +65,22 @@ public interface Exchange extends ExchangeReferrer, MessageDestination long getMsgReceives(); - boolean addBinding(String bindingKey, AMQQueue queue, Map<String, Object> arguments) - throws QpidSecurityException; + boolean addBinding(String bindingKey, AMQQueue queue, Map<String, Object> arguments); boolean replaceBinding(UUID id, String bindingKey, AMQQueue queue, - Map<String, Object> arguments) - throws QpidSecurityException; + Map<String, Object> arguments); void restoreBinding(UUID id, String bindingKey, AMQQueue queue, - Map<String, Object> argumentMap) - throws QpidSecurityException; + Map<String, Object> argumentMap); - void removeBinding(Binding b) throws QpidSecurityException; + void removeBinding(Binding b); - Binding removeBinding(String bindingKey, AMQQueue queue, Map<String, Object> arguments) - throws QpidSecurityException; + Binding removeBinding(String bindingKey, AMQQueue queue, Map<String, Object> arguments); Binding getBinding(String bindingKey, AMQQueue queue, Map<String, Object> arguments); - void close() throws QpidSecurityException; + void close(); /** * Determines whether a message would be isBound to a particular queue using a specific routing key and arguments diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeFactory.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeFactory.java index 06aa3aee2d..a8839d2dfd 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeFactory.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeFactory.java @@ -21,7 +21,6 @@ package org.apache.qpid.server.exchange; import org.apache.qpid.server.plugin.ExchangeType; -import org.apache.qpid.server.security.QpidSecurityException; import java.util.Collection; import java.util.UUID; @@ -35,11 +34,10 @@ public interface ExchangeFactory Collection<ExchangeType<? extends Exchange>> getPublicCreatableTypes(); Exchange createExchange(String exchange, String type, boolean durable, boolean autoDelete) - throws AMQUnknownExchangeType, QpidSecurityException; + throws AMQUnknownExchangeType; - Exchange createExchange(UUID id, String exchange, String type, boolean durable, boolean autoDelete) throws AMQUnknownExchangeType, - QpidSecurityException; + Exchange createExchange(UUID id, String exchange, String type, boolean durable, boolean autoDelete) throws AMQUnknownExchangeType; Exchange restoreExchange(UUID id, String exchange, String type, boolean autoDelete) - throws AMQUnknownExchangeType, QpidSecurityException; + throws AMQUnknownExchangeType; } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeInitialiser.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeInitialiser.java index aa3f0de24f..1443074e18 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeInitialiser.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeInitialiser.java @@ -21,7 +21,6 @@ package org.apache.qpid.server.exchange; import org.apache.qpid.server.plugin.ExchangeType; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.store.DurableConfigurationStoreHelper; import org.apache.qpid.server.store.DurableConfigurationStore; import org.apache.qpid.server.util.ServerScopedRuntimeException; @@ -52,11 +51,6 @@ public class ExchangeInitialiser } } } - catch (QpidSecurityException e) - { - throw new ServerScopedRuntimeException("Security Exception when attempting to initialise exchanges - " + - "this is likely a programming error", e); - } catch (AMQUnknownExchangeType e) { throw new ServerScopedRuntimeException("Unknown exchange type while attempting to initialise exchanges - " + diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeRegistry.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeRegistry.java index aa66b98a5c..de05bfb4d9 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeRegistry.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/ExchangeRegistry.java @@ -20,8 +20,6 @@ */ package org.apache.qpid.server.exchange; -import org.apache.qpid.server.security.QpidSecurityException; - import java.util.Collection; import java.util.UUID; @@ -41,7 +39,7 @@ public interface ExchangeRegistry * @param exchange name of the exchange to delete * @param ifUnused if true, do NOT delete the exchange if it is in use (has queues bound to it) */ - boolean unregisterExchange(String exchange, boolean ifUnused) throws QpidSecurityException; + boolean unregisterExchange(String exchange, boolean ifUnused); void clearAndUnregisterMbeans(); diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/message/MessageSource.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/message/MessageSource.java index 07f7660f62..cb1c8a5384 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/message/MessageSource.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/message/MessageSource.java @@ -24,8 +24,6 @@ import org.apache.qpid.server.consumer.Consumer; import org.apache.qpid.server.consumer.ConsumerTarget; import org.apache.qpid.server.filter.FilterManager; import org.apache.qpid.server.protocol.AMQSessionModel; -import org.apache.qpid.server.security.AuthorizationHolder; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.store.TransactionLogResource; import java.util.Collection; @@ -36,7 +34,7 @@ public interface MessageSource<C extends Consumer, S extends MessageSource<C,S>> <T extends ConsumerTarget> C addConsumer(T target, FilterManager filters, Class<? extends ServerMessage> messageClass, String consumerName, EnumSet<Consumer.Option> options) - throws ExistingExclusiveConsumer, ExistingConsumerPreventsExclusive, QpidSecurityException, + throws ExistingExclusiveConsumer, ExistingConsumerPreventsExclusive, ConsumerAccessRefused; Collection<C> getConsumers(); diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java index 0fcdb779c1..162f579114 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java @@ -472,27 +472,17 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana @Override public boolean createUser(String username, String password, Map<String, String> attributes) { - if(getSecurityManager().authoriseUserOperation(Operation.CREATE, username)) - { - return getPrincipalDatabase().createPrincipal(new UsernamePrincipal(username), password.toCharArray()); - } - else - { - throw new AccessControlException("Do not have permission to create new user"); - } + getSecurityManager().authoriseUserOperation(Operation.CREATE, username); + return getPrincipalDatabase().createPrincipal(new UsernamePrincipal(username), password.toCharArray()); + } @Override public void deleteUser(String username) throws AccountNotFoundException { - if(getSecurityManager().authoriseUserOperation(Operation.DELETE, username)) - { - getPrincipalDatabase().deletePrincipal(new UsernamePrincipal(username)); - } - else - { - throw new AccessControlException("Cannot delete user " + username); - } + getSecurityManager().authoriseUserOperation(Operation.DELETE, username); + getPrincipalDatabase().deletePrincipal(new UsernamePrincipal(username)); + } private SecurityManager getSecurityManager() @@ -508,14 +498,10 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana @Override public void setPassword(String username, String password) throws AccountNotFoundException { - if(getSecurityManager().authoriseUserOperation(Operation.UPDATE, username)) - { - getPrincipalDatabase().updatePassword(new UsernamePrincipal(username), password.toCharArray()); - } - else - { - throw new AccessControlException("Do not have permission to set password"); - } + getSecurityManager().authoriseUserOperation(Operation.UPDATE, username); + + getPrincipalDatabase().updatePassword(new UsernamePrincipal(username), password.toCharArray()); + } @Override diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BindingAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BindingAdapter.java index 265d4318f1..5111810556 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BindingAdapter.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BindingAdapter.java @@ -26,7 +26,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.model.Binding; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.model.Exchange; @@ -139,14 +138,7 @@ final class BindingAdapter extends AbstractAdapter implements Binding public void delete() { - try - { - _exchange.getExchange().removeBinding(_binding); - } - catch(QpidSecurityException e) - { - throw new AccessControlException(e.getMessage()); - } + _exchange.getExchange().removeBinding(_binding); } @Override diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ExchangeAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ExchangeAdapter.java index d7b6b8bb75..52226d503a 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ExchangeAdapter.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ExchangeAdapter.java @@ -27,7 +27,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.binding.Binding; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.model.ConfiguredObjectFinder; @@ -127,28 +126,21 @@ final class ExchangeAdapter extends AbstractAdapter implements Exchange, org.apa { AMQQueue amqQueue = ((QueueAdapter)queue).getAMQQueue(); - try + if(!_exchange.addBinding(bindingKey, amqQueue, bindingArguments)) { - if(!_exchange.addBinding(bindingKey, amqQueue, bindingArguments)) - { - Binding oldBinding = _exchange.getBinding(bindingKey, amqQueue, bindingArguments); + Binding oldBinding = _exchange.getBinding(bindingKey, amqQueue, bindingArguments); - Map<String, Object> oldArgs = oldBinding.getArguments(); - if((oldArgs == null && !bindingArguments.isEmpty()) || (oldArgs != null && !oldArgs.equals(bindingArguments))) - { - _exchange.replaceBinding(oldBinding.getId(), bindingKey, amqQueue, bindingArguments); - } - } - Binding binding = _exchange.getBinding(bindingKey, amqQueue, bindingArguments); - - synchronized (_bindingAdapters) + Map<String, Object> oldArgs = oldBinding.getArguments(); + if((oldArgs == null && !bindingArguments.isEmpty()) || (oldArgs != null && !oldArgs.equals(bindingArguments))) { - return binding == null ? null : _bindingAdapters.get(binding); + _exchange.replaceBinding(oldBinding.getId(), bindingKey, amqQueue, bindingArguments); } } - catch(QpidSecurityException e) + Binding binding = _exchange.getBinding(bindingKey, amqQueue, bindingArguments); + + synchronized (_bindingAdapters) { - throw new AccessControlException(e.toString()); + return binding == null ? null : _bindingAdapters.get(binding); } } @@ -166,10 +158,6 @@ final class ExchangeAdapter extends AbstractAdapter implements Exchange, org.apa { throw new IllegalStateException(e); } - catch (QpidSecurityException e) - { - throw new AccessControlException(e.toString()); - } } public String getName() @@ -384,19 +372,13 @@ final class ExchangeAdapter extends AbstractAdapter implements Exchange, org.apa @Override protected void authoriseSetAttribute(String name, Object expected, Object desired) throws AccessControlException { - if (!_vhost.getSecurityManager().authoriseUpdate(_exchange)) - { - throw new AccessControlException("Setting of exchange attribute is denied"); - } + _vhost.getSecurityManager().authoriseUpdate(_exchange); } @Override protected void authoriseSetAttributes(Map<String, Object> attributes) throws AccessControlException { - if (!_vhost.getSecurityManager().authoriseUpdate(_exchange)) - { - throw new AccessControlException("Setting of exchange attributes is denied"); - } + _vhost.getSecurityManager().authoriseUpdate(_exchange); } private class ExchangeStatistics implements Statistics diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java index 9323606c83..973fb6e416 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java @@ -209,16 +209,10 @@ public class GroupProviderAdapter extends AbstractAdapter implements { String groupName = (String) attributes.get(Group.NAME); - if (getSecurityManager().authoriseGroupOperation(Operation.CREATE, groupName)) - { + getSecurityManager().authoriseGroupOperation(Operation.CREATE, groupName); _groupManager.createGroup(groupName); return (C) new GroupAdapter(groupName, getTaskExecutor()); - } - else - { - throw new AccessControlException("Do not have permission" + - " to create new group"); - } + } throw new IllegalArgumentException( @@ -487,16 +481,11 @@ public class GroupProviderAdapter extends AbstractAdapter implements { String memberName = (String) attributes.get(GroupMember.NAME); - if (getSecurityManager().authoriseGroupOperation(Operation.UPDATE, _group)) - { - _groupManager.addUserToGroup(memberName, _group); - return (C) new GroupMemberAdapter(memberName, getTaskExecutor()); - } - else - { - throw new AccessControlException("Do not have permission" + - " to add new group member"); - } + getSecurityManager().authoriseGroupOperation(Operation.UPDATE, _group); + + _groupManager.addUserToGroup(memberName, _group); + return (C) new GroupMemberAdapter(memberName, getTaskExecutor()); + } throw new IllegalArgumentException( @@ -530,15 +519,9 @@ public class GroupProviderAdapter extends AbstractAdapter implements { if (desiredState == State.DELETED) { - if (getSecurityManager().authoriseGroupOperation(Operation.DELETE, _group)) - { - _groupManager.removeGroup(_group); - return true; - } - else - { - throw new AccessControlException("Do not have permission to delete group"); - } + getSecurityManager().authoriseGroupOperation(Operation.DELETE, _group); + _groupManager.removeGroup(_group); + return true; } return false; @@ -677,15 +660,11 @@ public class GroupProviderAdapter extends AbstractAdapter implements { if (desiredState == State.DELETED) { - if (getSecurityManager().authoriseGroupOperation(Operation.UPDATE, _group)) - { - _groupManager.removeUserFromGroup(_memberName, _group); - return true; - } - else - { - throw new AccessControlException("Do not have permission to remove group member"); - } + getSecurityManager().authoriseGroupOperation(Operation.UPDATE, _group); + + _groupManager.removeUserFromGroup(_memberName, _group); + return true; + } return false; } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/QueueAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/QueueAdapter.java index 5d09cfa8e2..5223977136 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/QueueAdapter.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/QueueAdapter.java @@ -35,7 +35,6 @@ import org.apache.qpid.server.model.*; import org.apache.qpid.server.protocol.AMQConnectionModel; import org.apache.qpid.server.protocol.AMQSessionModel; import org.apache.qpid.server.queue.*; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.store.DurableConfigurationStoreHelper; import org.apache.qpid.server.consumer.Consumer; import org.apache.qpid.server.util.MapValueConverter; @@ -160,14 +159,7 @@ final class QueueAdapter<Q extends AMQQueue<?,Q,?>> extends AbstractAdapter impl public void delete() { - try - { - _queue.getVirtualHost().removeQueue(_queue); - } - catch (QpidSecurityException e) - { - throw new AccessControlException(e.toString()); - } + _queue.getVirtualHost().removeQueue(_queue); } public String getName() @@ -771,19 +763,13 @@ final class QueueAdapter<Q extends AMQQueue<?,Q,?>> extends AbstractAdapter impl @Override protected void authoriseSetAttribute(String name, Object expected, Object desired) throws AccessControlException { - if (!_vhost.getSecurityManager().authoriseUpdate(_queue)) - { - throw new AccessControlException("Setting of queue attribute is denied"); - } + _vhost.getSecurityManager().authoriseUpdate(_queue); } @Override protected void authoriseSetAttributes(Map<String, Object> attributes) throws AccessControlException { - if (!_vhost.getSecurityManager().authoriseUpdate(_queue)) - { - throw new AccessControlException("Setting of queue attributes is denied"); - } + _vhost.getSecurityManager().authoriseUpdate(_queue); } @Override diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java index 4cd7432f75..e76b9f15fc 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java @@ -64,7 +64,6 @@ import org.apache.qpid.server.plugin.ExchangeType; import org.apache.qpid.server.protocol.AMQConnectionModel; import org.apache.qpid.server.queue.AMQQueue; import org.apache.qpid.server.queue.ConflationQueue; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.security.SecurityManager; import org.apache.qpid.server.security.access.Operation; import org.apache.qpid.server.stats.StatisticsGatherer; @@ -377,10 +376,6 @@ public final class VirtualHostAdapter extends AbstractAdapter implements Virtual { throw new IllegalArgumentException(e); } - catch (QpidSecurityException e) - { - throw new AccessControlException(e.toString()); - } } public Queue createQueue(Map<String, Object> attributes) @@ -431,10 +426,6 @@ public final class VirtualHostAdapter extends AbstractAdapter implements Virtual { throw new IllegalArgumentException("Queue with name "+MapValueConverter.getStringAttribute(Queue.NAME,attributes)+" already exists"); } - catch (QpidSecurityException e) - { - throw new AccessControlException(e.toString()); - } } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AMQQueue.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AMQQueue.java index a5ab77283f..7dcafedc18 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AMQQueue.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AMQQueue.java @@ -21,7 +21,6 @@ package org.apache.qpid.server.queue; import org.apache.qpid.server.binding.Binding; -import org.apache.qpid.server.configuration.QueueConfiguration; import org.apache.qpid.server.exchange.Exchange; import org.apache.qpid.server.exchange.ExchangeReferrer; import org.apache.qpid.server.logging.LogSubject; @@ -31,8 +30,6 @@ import org.apache.qpid.server.model.ExclusivityPolicy; import org.apache.qpid.server.model.LifetimePolicy; import org.apache.qpid.server.protocol.CapacityChecker; import org.apache.qpid.server.consumer.Consumer; -import org.apache.qpid.server.security.QpidSecurityException; -import org.apache.qpid.server.util.Action; import org.apache.qpid.server.util.Deletable; import org.apache.qpid.server.virtualhost.VirtualHost; @@ -96,7 +93,7 @@ public interface AMQQueue<E extends QueueEntry<E,Q,C>, Q extends AMQQueue<E,Q,C> boolean isDeleted(); - int delete() throws QpidSecurityException; + int delete(); void requeue(E entry); @@ -164,7 +161,7 @@ public interface AMQQueue<E extends QueueEntry<E,Q,C>, Q extends AMQQueue<E,Q,C> boolean isOverfull(); - long clearQueue() throws QpidSecurityException; + long clearQueue(); /** * Checks the status of messages on the queue, purging expired ones, firing age related alerts etc. diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AMQQueueFactory.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AMQQueueFactory.java index 5003db1385..399586fcff 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AMQQueueFactory.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AMQQueueFactory.java @@ -28,7 +28,6 @@ import org.apache.qpid.server.exchange.AMQUnknownExchangeType; import org.apache.qpid.server.model.ExclusivityPolicy; import org.apache.qpid.server.model.LifetimePolicy; import org.apache.qpid.server.protocol.AMQSessionModel; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.exchange.ExchangeDefaults; import org.apache.qpid.server.configuration.BrokerProperties; import org.apache.qpid.server.configuration.QueueConfiguration; @@ -64,7 +63,7 @@ public class AMQQueueFactory implements QueueFactory } @Override - public AMQQueue restoreQueue(Map<String, Object> attributes) throws QpidSecurityException + public AMQQueue restoreQueue(Map<String, Object> attributes) { return createOrRestoreQueue(null, attributes, false); @@ -72,13 +71,13 @@ public class AMQQueueFactory implements QueueFactory @Override public AMQQueue createQueue(final AMQSessionModel creatingSession, - Map<String, Object> attributes) throws QpidSecurityException + Map<String, Object> attributes) { return createOrRestoreQueue(creatingSession, attributes, true); } private AMQQueue createOrRestoreQueue(final AMQSessionModel creatingSession, Map<String, Object> attributes, - boolean createInStore) throws QpidSecurityException + boolean createInStore) { @@ -179,7 +178,7 @@ public class AMQQueueFactory implements QueueFactory return queue; } - private void createDLQ(final AMQQueue queue) throws QpidSecurityException + private void createDLQ(final AMQQueue queue) { final String queueName = queue.getName(); final String dlExchangeName = getDeadLetterExchangeName(queueName); @@ -256,7 +255,7 @@ public class AMQQueueFactory implements QueueFactory queue.setAlternateExchange(dlExchange); } - public AMQQueue createAMQQueueImpl(QueueConfiguration config) throws QpidSecurityException + public AMQQueue createAMQQueueImpl(QueueConfiguration config) { Map<String, Object> arguments = createQueueAttributesFromConfig(_virtualHost, config); diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueFactory.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueFactory.java index 62a2d93b0f..c80018799b 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueFactory.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueFactory.java @@ -21,16 +21,14 @@ package org.apache.qpid.server.queue; import java.util.Map; -import java.util.UUID; import org.apache.qpid.server.protocol.AMQSessionModel; -import org.apache.qpid.server.security.QpidSecurityException; public interface QueueFactory { AMQQueue createQueue(final AMQSessionModel creatingSession, - Map<String, Object> arguments) throws QpidSecurityException; + Map<String, Object> arguments); - AMQQueue restoreQueue(Map<String, Object> arguments) throws QpidSecurityException; + AMQQueue restoreQueue(Map<String, Object> arguments); } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/SimpleAMQQueue.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/SimpleAMQQueue.java index 0135b11fb9..ef3eea19b3 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/SimpleAMQQueue.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/SimpleAMQQueue.java @@ -29,11 +29,9 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import org.apache.log4j.Logger; -import org.apache.qpid.server.message.MessageSource; import org.apache.qpid.server.model.ExclusivityPolicy; import org.apache.qpid.server.model.LifetimePolicy; import org.apache.qpid.server.protocol.AMQConnectionModel; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.pool.ReferenceCountingExecutorService; import org.apache.qpid.server.binding.Binding; import org.apache.qpid.server.configuration.BrokerProperties; @@ -51,7 +49,6 @@ import org.apache.qpid.server.message.MessageReference; import org.apache.qpid.server.message.ServerMessage; import org.apache.qpid.server.model.Queue; import org.apache.qpid.server.protocol.AMQSessionModel; -import org.apache.qpid.server.security.AuthorizationHolder; import org.apache.qpid.server.consumer.Consumer; import org.apache.qpid.server.consumer.ConsumerTarget; import org.apache.qpid.server.security.auth.AuthenticatedPrincipal; @@ -60,7 +57,6 @@ import org.apache.qpid.server.txn.AutoCommitTransaction; import org.apache.qpid.server.txn.LocalTransaction; import org.apache.qpid.server.txn.ServerTransaction; import org.apache.qpid.server.util.Action; -import org.apache.qpid.server.util.ConnectionScopedRuntimeException; import org.apache.qpid.server.util.Deletable; import org.apache.qpid.server.util.MapValueConverter; import org.apache.qpid.server.util.ServerScopedRuntimeException; @@ -437,15 +433,7 @@ abstract class SimpleAMQQueue<E extends QueueEntryImpl<E,Q,L>, Q extends SimpleA @Override public void performAction(final Deletable object) { - try - { - getVirtualHost().removeQueue(SimpleAMQQueue.this); - } - catch (QpidSecurityException e) - { - throw new ConnectionScopedRuntimeException("Unable to delete a queue even though the queue's " + - "lifetime was tied to an object being deleted"); - } + getVirtualHost().removeQueue(SimpleAMQQueue.this); } }; @@ -583,15 +571,12 @@ abstract class SimpleAMQQueue<E extends QueueEntryImpl<E,Q,L>, Q extends SimpleA final Class<? extends ServerMessage> messageClass, final String consumerName, EnumSet<Consumer.Option> optionSet) - throws ExistingExclusiveConsumer, ExistingConsumerPreventsExclusive, QpidSecurityException, + throws ExistingExclusiveConsumer, ExistingConsumerPreventsExclusive, ConsumerAccessRefused { // Access control - if (!getVirtualHost().getSecurityManager().authoriseConsume(this)) - { - throw new QpidSecurityException("Permission denied"); - } + getVirtualHost().getSecurityManager().authoriseConsume(this); if (hasExclusiveConsumer()) @@ -777,14 +762,7 @@ abstract class SimpleAMQQueue<E extends QueueEntryImpl<E,Q,L>, Q extends SimpleA _logger.info("Auto-deleting queue:" + this); } - try - { - getVirtualHost().removeQueue(this); - } - catch (QpidSecurityException e) - { - throw new ConnectionScopedRuntimeException("Auto delete queue unable to delete itself", e); - } + getVirtualHost().removeQueue(this); // we need to manually fire the event to the removed consumer (which was the last one left for this // queue. This is because the delete method uses the consumer set which has just been cleared @@ -1440,11 +1418,6 @@ abstract class SimpleAMQQueue<E extends QueueEntryImpl<E,Q,L>, Q extends SimpleA } - public void purge(final long request) throws QpidSecurityException - { - clear(request); - } - public long getCreateTime() { return _createTime; @@ -1452,18 +1425,15 @@ abstract class SimpleAMQQueue<E extends QueueEntryImpl<E,Q,L>, Q extends SimpleA // ------ Management functions - public long clearQueue() throws QpidSecurityException + public long clearQueue() { return clear(0l); } - private long clear(final long request) throws QpidSecurityException + private long clear(final long request) { //Perform ACLs - if (!getVirtualHost().getSecurityManager().authorisePurge(this)) - { - throw new QpidSecurityException("Permission denied: queue " + getName()); - } + getVirtualHost().getSecurityManager().authorisePurge(this); QueueEntryIterator<E,Q,L,QueueConsumer<?,E,Q,L>> queueListIterator = _entries.iterator(); long count = 0; @@ -1526,13 +1496,10 @@ abstract class SimpleAMQQueue<E extends QueueEntryImpl<E,Q,L>, Q extends SimpleA } // TODO list all thrown exceptions - public int delete() throws QpidSecurityException + public int delete() { // Check access - if (!_virtualHost.getSecurityManager().authoriseDelete(this)) - { - throw new QpidSecurityException("Permission denied: " + getName()); - } + _virtualHost.getSecurityManager().authoriseDelete(this); if (!_deleted.getAndSet(true)) { diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/QpidSecurityException.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/QpidSecurityException.java deleted file mode 100644 index b9d9513f9f..0000000000 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/QpidSecurityException.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - * - */ -package org.apache.qpid.server.security; - -public class QpidSecurityException extends Exception -{ - public QpidSecurityException() - { - } - - public QpidSecurityException(final String message) - { - super(message); - } - - public QpidSecurityException(final String message, final Throwable cause) - { - super(message, cause); - } - - public QpidSecurityException(final Throwable cause) - { - super(cause); - } -} diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java index ff45add206..5af035c6b3 100755 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java @@ -56,6 +56,7 @@ import static org.apache.qpid.server.security.access.Operation.UPDATE; import javax.security.auth.Subject; import java.net.SocketAddress; +import java.security.AccessControlException; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -289,20 +290,26 @@ public class SecurityManager implements ConfigurationChangeListener return true; } - public boolean authoriseBind(final Exchange exch, final AMQQueue queue, final String routingKey) + public void authoriseBind(final Exchange exch, final AMQQueue queue, final String routingKey) { - return checkAllPlugins(new AccessCheck() + boolean allowed = + checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(BIND, EXCHANGE, new ObjectProperties(exch, queue, routingKey)); } }); + + if(!allowed) + { + throw new AccessControlException("Permission denied: binding " + routingKey); + } } - public boolean authoriseMethod(final Operation operation, final String componentName, final String methodName) + public void authoriseMethod(final Operation operation, final String componentName, final String methodName) { - return checkAllPlugins(new AccessCheck() + boolean allowed = checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { @@ -316,132 +323,176 @@ public class SecurityManager implements ConfigurationChangeListener return plugin.authorise(operation, METHOD, properties); } }); + if(!allowed) + { + throw new AccessControlException("Permission denied: " + operation.name() + " " + methodName); + } } - public boolean accessManagement() + public void accessManagement() { - return checkAllPlugins(new AccessCheck() + if(!checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.access(ObjectType.MANAGEMENT, null); } - }); + })) + { + throw new AccessControlException("User not authorised for management"); + } } - public boolean accessVirtualhost(final String vhostname, final SocketAddress remoteAddress) + public void accessVirtualhost(final String vhostname, final SocketAddress remoteAddress) { - return checkAllPlugins(new AccessCheck() + if(!checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.access(VIRTUALHOST, remoteAddress); } - }); + })) + { + throw new AccessControlException("Permission denied: " + vhostname); + } } - public boolean authoriseConsume(final AMQQueue queue) + public void authoriseConsume(final AMQQueue queue) { - return checkAllPlugins(new AccessCheck() + if(!checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(CONSUME, QUEUE, new ObjectProperties(queue)); } - }); + })) + { + throw new AccessControlException("Permission denied: consume from queue '" + queue.getName() + "'."); + } } - public boolean authoriseCreateExchange(final Boolean autoDelete, final Boolean durable, final String exchangeName, - final Boolean internal, final Boolean nowait, final Boolean passive, final String exchangeType) + public void authoriseCreateExchange(final Boolean autoDelete, + final Boolean durable, + final String exchangeName, + final Boolean internal, + final Boolean nowait, + final Boolean passive, + final String exchangeType) { - return checkAllPlugins(new AccessCheck() + if(!checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(CREATE, EXCHANGE, new ObjectProperties(autoDelete, durable, exchangeName, internal, nowait, passive, exchangeType)); } - }); + })) + { + throw new AccessControlException("Permission denied: exchange-name '" + exchangeName + "'"); + } } - public boolean authoriseCreateQueue(final Boolean autoDelete, final Boolean durable, final Boolean exclusive, + public void authoriseCreateQueue(final Boolean autoDelete, final Boolean durable, final Boolean exclusive, final Boolean nowait, final Boolean passive, final String queueName, final String owner) { - return checkAllPlugins(new AccessCheck() + if(! checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(CREATE, QUEUE, new ObjectProperties(autoDelete, durable, exclusive, nowait, passive, queueName, owner)); } - }); + })) + { + throw new AccessControlException("Permission denied: queue-name '" + queueName + "'"); + } } - public boolean authoriseDelete(final AMQQueue queue) + public void authoriseDelete(final AMQQueue queue) { - return checkAllPlugins(new AccessCheck() + if(!checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(DELETE, QUEUE, new ObjectProperties(queue)); } - }); + })) + { + throw new AccessControlException("Permission denied, delete queue: " + queue.getName()); + } } - public boolean authoriseUpdate(final AMQQueue queue) + public void authoriseUpdate(final AMQQueue queue) { - return checkAllPlugins(new AccessCheck() + if(!checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(UPDATE, QUEUE, new ObjectProperties(queue)); } - }); + })) + { + throw new AccessControlException("Permission denied: update queue: " + queue.getName()); + } } - public boolean authoriseUpdate(final Exchange exchange) + public void authoriseUpdate(final Exchange exchange) { - return checkAllPlugins(new AccessCheck() + if(!checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(UPDATE, EXCHANGE, new ObjectProperties(exchange.getName())); } - }); + })) + { + throw new AccessControlException("Permission denied: update exchange: " + exchange.getName()); + } } - public boolean authoriseDelete(final Exchange exchange) + public void authoriseDelete(final Exchange exchange) { - return checkAllPlugins(new AccessCheck() + if(! checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(DELETE, EXCHANGE, new ObjectProperties(exchange.getName())); } - }); + })) + { + throw new AccessControlException("Permission denied, delete exchange: '" + exchange.getName() + "'"); + } } - public boolean authoriseGroupOperation(final Operation operation, final String groupName) + public void authoriseGroupOperation(final Operation operation, final String groupName) { - return checkAllPlugins(new AccessCheck() + if(!checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(operation, GROUP, new ObjectProperties(groupName)); } - }); + })) + { + throw new AccessControlException("Do not have permission" + + " to perform the " + operation + " on the group " + groupName); + } } - public boolean authoriseUserOperation(final Operation operation, final String userName) + public void authoriseUserOperation(final Operation operation, final String userName) { - return checkAllPlugins(new AccessCheck() + if(! checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(operation, USER, new ObjectProperties(userName)); } - }); + })) + { + throw new AccessControlException("Do not have permission" + + " to perform the " + operation + " on the user " + userName); + } } private ConcurrentHashMap<String, ConcurrentHashMap<String, PublishAccessCheck>> _immediatePublishPropsCache @@ -449,7 +500,7 @@ public class SecurityManager implements ConfigurationChangeListener private ConcurrentHashMap<String, ConcurrentHashMap<String, PublishAccessCheck>> _publishPropsCache = new ConcurrentHashMap<String, ConcurrentHashMap<String, PublishAccessCheck>>(); - public boolean authorisePublish(final boolean immediate, String routingKey, String exchangeName) + public void authorisePublish(final boolean immediate, String routingKey, String exchangeName) { if(routingKey == null) { @@ -477,29 +528,38 @@ public class SecurityManager implements ConfigurationChangeListener exchangeMap.put(routingKey, check); } - return checkAllPlugins(check); + if(!checkAllPlugins(check)) + { + throw new AccessControlException("Permission denied, publish to: exchange-name '" + exchangeName + "'"); + } } - public boolean authorisePurge(final AMQQueue queue) + public void authorisePurge(final AMQQueue queue) { - return checkAllPlugins(new AccessCheck() + if(!checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(PURGE, QUEUE, new ObjectProperties(queue)); } - }); + })) + { + throw new AccessControlException("Permission denied: queue " + queue.getName()); + } } - public boolean authoriseUnbind(final Exchange exch, final String routingKey, final AMQQueue queue) + public void authoriseUnbind(final Exchange exch, final String routingKey, final AMQQueue queue) { - return checkAllPlugins(new AccessCheck() + if(! checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) { return plugin.authorise(UNBIND, EXCHANGE, new ObjectProperties(exch, queue, routingKey)); } - }); + })) + { + throw new AccessControlException("Permission denied: unbinding " + routingKey); + } } public static boolean setAccessChecksDisabled(final boolean status) diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/jmx/JMXPasswordAuthenticator.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/jmx/JMXPasswordAuthenticator.java index bf8d489e61..4e61e4b80b 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/jmx/JMXPasswordAuthenticator.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/jmx/JMXPasswordAuthenticator.java @@ -38,7 +38,6 @@ public class JMXPasswordAuthenticator implements JMXAuthenticator static final String SHOULD_HAVE_2_ELEMENTS = "User details should have 2 elements, username, password"; static final String SHOULD_BE_NON_NULL = "Supplied username and password should be non-null"; static final String INVALID_CREDENTIALS = "Invalid user details supplied"; - static final String USER_NOT_AUTHORISED_FOR_MANAGEMENT = "User not authorised for management"; static final String CREDENTIALS_REQUIRED = "User details are required. " + "Please ensure you are using an up to date management console to connect."; @@ -121,10 +120,7 @@ public class JMXPasswordAuthenticator implements JMXAuthenticator SecurityManager.setThreadSubject(authenticatedSubject); try { - if (!_broker.getSecurityManager().accessManagement()) - { - throw new SecurityException(USER_NOT_AUTHORISED_FOR_MANAGEMENT); - } + _broker.getSecurityManager().accessManagement(); } finally { diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java index 0e9b879316..31481721d6 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java @@ -38,7 +38,6 @@ import org.apache.qpid.server.exchange.AMQUnknownExchangeType; import org.apache.qpid.server.model.ExclusivityPolicy; import org.apache.qpid.server.model.LifetimePolicy; import org.apache.qpid.server.model.Queue; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.configuration.ExchangeConfiguration; import org.apache.qpid.server.configuration.QueueConfiguration; import org.apache.qpid.server.configuration.VirtualHostConfiguration; @@ -73,7 +72,6 @@ import org.apache.qpid.server.store.DurableConfigurationStoreHelper; import org.apache.qpid.server.store.DurableConfiguredObjectRecoverer; import org.apache.qpid.server.store.Event; import org.apache.qpid.server.store.EventListener; -import org.apache.qpid.server.store.MessageStore; import org.apache.qpid.server.txn.DtxRegistry; import org.apache.qpid.server.util.MapValueConverter; import org.apache.qpid.server.util.ServerScopedRuntimeException; @@ -313,10 +311,6 @@ public abstract class AbstractVirtualHost implements VirtualHost, IConnectionReg { configureExchange(config.getExchangeConfiguration(exchangeName)); } - catch (QpidSecurityException e) - { - throw new ServerScopedRuntimeException("Could not configure exchange " + exchangeName, e); - } catch (UnknownExchangeException e) { throw new ServerScopedRuntimeException("Could not configure exchange " + exchangeName, e); @@ -344,15 +338,11 @@ public abstract class AbstractVirtualHost implements VirtualHost, IConnectionReg { throw new ServerScopedRuntimeException("Could not configure queue " + queueName, e); } - catch (QpidSecurityException e) - { - throw new ServerScopedRuntimeException("Could not configure queue " + queueName, e); - } } } private void configureExchange(ExchangeConfiguration exchangeConfiguration) - throws QpidSecurityException, UnknownExchangeException, ReservedExchangeNameException, + throws UnknownExchangeException, ReservedExchangeNameException, AMQUnknownExchangeType { boolean durable = exchangeConfiguration.getDurable(); @@ -370,7 +360,7 @@ public abstract class AbstractVirtualHost implements VirtualHost, IConnectionReg } private void configureQueue(QueueConfiguration queueConfiguration) - throws ConfigurationException, QpidSecurityException + throws ConfigurationException { AMQQueue queue = _queueFactory.createAMQQueueImpl(queueConfiguration); String queueName = queue.getName(); @@ -421,7 +411,6 @@ public abstract class AbstractVirtualHost implements VirtualHost, IConnectionReg } private void configureBinding(AMQQueue queue, Exchange exchange, String routingKey, Map<String,Object> arguments) - throws QpidSecurityException { if (_logger.isInfoEnabled()) { @@ -528,7 +517,7 @@ public abstract class AbstractVirtualHost implements VirtualHost, IConnectionReg } @Override - public int removeQueue(AMQQueue queue) throws QpidSecurityException + public int removeQueue(AMQQueue queue) { synchronized (getQueueRegistry()) { @@ -547,7 +536,7 @@ public abstract class AbstractVirtualHost implements VirtualHost, IConnectionReg } } - public AMQQueue createQueue(final AMQSessionModel creatingSession, Map<String, Object> attributes) throws QpidSecurityException, QueueExistsException + public AMQQueue createQueue(final AMQSessionModel creatingSession, Map<String, Object> attributes) throws QueueExistsException { // make a copy as we may augment (with an ID for example) attributes = new LinkedHashMap<String, Object>(attributes); @@ -562,17 +551,13 @@ public abstract class AbstractVirtualHost implements VirtualHost, IConnectionReg String owner = MapValueConverter.getStringAttribute(Queue.OWNER, attributes, null); // Access check - if (!getSecurityManager().authoriseCreateQueue(autoDelete, - durable, - exclusive != null && exclusive != ExclusivityPolicy.NONE, - null, - null, - queueName, - owner)) - { - String description = "Permission denied: queue-name '" + queueName + "'"; - throw new QpidSecurityException(description); - } + getSecurityManager().authoriseCreateQueue(autoDelete, + durable, + exclusive != null && exclusive != ExclusivityPolicy.NONE, + null, + null, + queueName, + owner); synchronized (_queueRegistry) { @@ -650,7 +635,7 @@ public abstract class AbstractVirtualHost implements VirtualHost, IConnectionReg boolean durable, boolean autoDelete, String alternateExchangeName) - throws QpidSecurityException, ExchangeExistsException, ReservedExchangeNameException, + throws ExchangeExistsException, ReservedExchangeNameException, UnknownExchangeException, AMQUnknownExchangeType { synchronized (_exchangeRegistry) @@ -698,7 +683,7 @@ public abstract class AbstractVirtualHost implements VirtualHost, IConnectionReg @Override public void removeExchange(Exchange exchange, boolean force) - throws QpidSecurityException, ExchangeIsAlternateException, RequiredExchangeException + throws ExchangeIsAlternateException, RequiredExchangeException { if(exchange.hasReferrers()) { diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/BindingRecoverer.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/BindingRecoverer.java index 6aa572d81a..948fa77048 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/BindingRecoverer.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/BindingRecoverer.java @@ -30,11 +30,9 @@ import org.apache.qpid.server.exchange.Exchange; import org.apache.qpid.server.exchange.ExchangeRegistry; import org.apache.qpid.server.model.Queue; import org.apache.qpid.server.queue.AMQQueue; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.store.AbstractDurableConfiguredObjectRecoverer; import org.apache.qpid.server.store.UnresolvedDependency; import org.apache.qpid.server.store.UnresolvedObject; -import org.apache.qpid.server.util.ServerScopedRuntimeException; public class BindingRecoverer extends AbstractDurableConfiguredObjectRecoverer<Binding> { @@ -108,23 +106,14 @@ public class BindingRecoverer extends AbstractDurableConfiguredObjectRecoverer<B @Override public Binding resolve() { - try + if(_exchange.getBinding(_bindingName, _queue, _bindingArgumentsMap) == null) { - if(_exchange.getBinding(_bindingName, _queue, _bindingArgumentsMap) == null) - { - _logger.info("Restoring binding: (Exchange: " + _exchange.getName() + ", Queue: " + _queue.getName() - + ", Routing Key: " + _bindingName + ", Arguments: " + _bindingArgumentsMap + ")"); - - _exchange.restoreBinding(_bindingId, _bindingName, _queue, _bindingArgumentsMap); - } - return _exchange.getBinding(_bindingName, _queue, _bindingArgumentsMap); - } - catch (QpidSecurityException e) - { - throw new ServerScopedRuntimeException("Security Exception thrown when recovering. The recovery " + - "thread should not be bound by permissions, this is likely " + - "a programming error.",e); + _logger.info("Restoring binding: (Exchange: " + _exchange.getName() + ", Queue: " + _queue.getName() + + ", Routing Key: " + _bindingName + ", Arguments: " + _bindingArgumentsMap + ")"); + + _exchange.restoreBinding(_bindingId, _bindingName, _queue, _bindingArgumentsMap); } + return _exchange.getBinding(_bindingName, _queue, _bindingArgumentsMap); } private class QueueDependency implements UnresolvedDependency<AMQQueue> diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/ExchangeRecoverer.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/ExchangeRecoverer.java index c687cbda92..ce91efacc3 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/ExchangeRecoverer.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/ExchangeRecoverer.java @@ -27,7 +27,6 @@ import org.apache.qpid.server.exchange.Exchange; import org.apache.qpid.server.exchange.ExchangeFactory; import org.apache.qpid.server.exchange.ExchangeRegistry; import org.apache.qpid.server.model.LifetimePolicy; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.store.AbstractDurableConfiguredObjectRecoverer; import org.apache.qpid.server.store.UnresolvedDependency; import org.apache.qpid.server.store.UnresolvedObject; @@ -82,16 +81,6 @@ public class ExchangeRecoverer extends AbstractDurableConfiguredObjectRecoverer< _exchange = _exchangeFactory.restoreExchange(id, exchangeName, exchangeType, autoDelete); _exchangeRegistry.registerExchange(_exchange); } - }/* - catch (AMQException e) - { - throw new RuntimeException("Error recovering exchange uuid " + id + " name " + exchangeName, e); - }*/ - catch (QpidSecurityException e) - { - throw new ServerScopedRuntimeException("Security Exception thrown when recovering. The recovery " + - "thread should not be bound by permissions, this is likely " + - "a programming error.",e); } catch (AMQUnknownExchangeType e) { diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/QueueRecoverer.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/QueueRecoverer.java index 621ea02059..60c7b2c7d9 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/QueueRecoverer.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/QueueRecoverer.java @@ -31,11 +31,9 @@ import org.apache.qpid.server.exchange.ExchangeRegistry; import org.apache.qpid.server.model.Queue; import org.apache.qpid.server.queue.AMQQueue; import org.apache.qpid.server.queue.QueueFactory; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.store.AbstractDurableConfiguredObjectRecoverer; import org.apache.qpid.server.store.UnresolvedDependency; import org.apache.qpid.server.store.UnresolvedObject; -import org.apache.qpid.server.util.ServerScopedRuntimeException; public class QueueRecoverer extends AbstractDurableConfiguredObjectRecoverer<AMQQueue> { @@ -105,26 +103,17 @@ public class QueueRecoverer extends AbstractDurableConfiguredObjectRecoverer<AMQ { String queueName = (String) _attributes.get(Queue.NAME); - try + _queue = _virtualHost.getQueue(_id); + if(_queue == null) { - _queue = _virtualHost.getQueue(_id); - if(_queue == null) - { - _queue = _virtualHost.getQueue(queueName); - } - - if (_queue == null) - { - Map<String, Object> attributes = new LinkedHashMap<String, Object>(_attributes); - attributes.put(Queue.ID, _id); - _queue = _queueFactory.restoreQueue(attributes); - } + _queue = _virtualHost.getQueue(queueName); } - catch (QpidSecurityException e) + + if (_queue == null) { - throw new ServerScopedRuntimeException("Security Exception thrown when recovering. The recovery " + - "thread should not be bound by permissions, this is likely " + - "a programming error.",e); + Map<String, Object> attributes = new LinkedHashMap<String, Object>(_attributes); + attributes.put(Queue.ID, _id); + _queue = _queueFactory.restoreQueue(attributes); } return _queue; } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHost.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHost.java index 9996684bad..61b2265d89 100755 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHost.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHost.java @@ -36,7 +36,6 @@ import org.apache.qpid.server.plugin.ExchangeType; import org.apache.qpid.server.protocol.AMQSessionModel; import org.apache.qpid.server.protocol.LinkRegistry; import org.apache.qpid.server.queue.AMQQueue; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.security.SecurityManager; import org.apache.qpid.server.stats.StatisticsGatherer; import org.apache.qpid.server.store.DurableConfigurationStore; @@ -58,9 +57,9 @@ public interface VirtualHost extends DurableConfigurationStore.Source, Closeable Collection<AMQQueue> getQueues(); - int removeQueue(AMQQueue queue) throws QpidSecurityException; + int removeQueue(AMQQueue queue); - AMQQueue createQueue(final AMQSessionModel creatingSession, Map<String, Object> arguments) throws QueueExistsException, QpidSecurityException; + AMQQueue createQueue(final AMQSessionModel creatingSession, Map<String, Object> arguments) throws QueueExistsException; Exchange createExchange(UUID id, @@ -69,10 +68,10 @@ public interface VirtualHost extends DurableConfigurationStore.Source, Closeable boolean durable, boolean autoDelete, String alternateExchange) - throws QpidSecurityException, ExchangeExistsException, ReservedExchangeNameException, + throws ExchangeExistsException, ReservedExchangeNameException, UnknownExchangeException, AMQUnknownExchangeType; - void removeExchange(Exchange exchange, boolean force) throws QpidSecurityException, ExchangeIsAlternateException, + void removeExchange(Exchange exchange, boolean force) throws ExchangeIsAlternateException, RequiredExchangeException; MessageDestination getMessageDestination(String name); diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/FanoutExchangeTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/FanoutExchangeTest.java index 8cbc6b49ca..f42c22c753 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/FanoutExchangeTest.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/FanoutExchangeTest.java @@ -20,9 +20,7 @@ */ package org.apache.qpid.server.exchange; -import static org.mockito.Matchers.any; import static org.mockito.Matchers.anySet; -import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -33,7 +31,6 @@ import java.util.Set; import java.util.UUID; import junit.framework.TestCase; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.common.AMQPFilterTypes; import org.apache.qpid.server.logging.LogActor; import org.apache.qpid.server.logging.actors.CurrentActor; @@ -60,8 +57,6 @@ public class FanoutExchangeTest extends TestCase _virtualHost = mock(VirtualHost.class); SecurityManager securityManager = mock(SecurityManager.class); when(_virtualHost.getSecurityManager()).thenReturn(securityManager); - when(securityManager.authoriseBind(any(Exchange.class), any(AMQQueue.class), anyString())).thenReturn(true); - when(securityManager.authoriseUnbind(any(Exchange.class), anyString(), any(AMQQueue.class))).thenReturn(true); _exchange.initialise(UUID.randomUUID(), _virtualHost, "test", false, false); } @@ -83,28 +78,28 @@ public class FanoutExchangeTest extends TestCase assertFalse("calling isBound(AMQQueue) with null queue should return false", _exchange.isBound((AMQQueue) null)); } - public void testIsBoundStringMapAMQQueue() throws QpidSecurityException + public void testIsBoundStringMapAMQQueue() { AMQQueue queue = bindQueue(); assertTrue("Should return true for a bound queue", _exchange.isBound("matters", null, queue)); } - public void testIsBoundStringAMQQueue() throws QpidSecurityException + public void testIsBoundStringAMQQueue() { AMQQueue queue = bindQueue(); assertTrue("Should return true for a bound queue", _exchange.isBound("matters", queue)); } - public void testIsBoundAMQQueue() throws QpidSecurityException + public void testIsBoundAMQQueue() { AMQQueue queue = bindQueue(); assertTrue("Should return true for a bound queue", _exchange.isBound(queue)); } - private AMQQueue bindQueue() throws QpidSecurityException + private AMQQueue bindQueue() { AMQQueue queue = mockQueue(); _exchange.addBinding("matters", queue, null); diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/HeadersExchangeTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/HeadersExchangeTest.java index fdaa147ae6..11342ee0ae 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/HeadersExchangeTest.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/HeadersExchangeTest.java @@ -29,7 +29,6 @@ import java.util.Map; import java.util.Set; import java.util.UUID; import junit.framework.TestCase; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.common.AMQPFilterTypes; import org.apache.qpid.server.logging.LogActor; import org.apache.qpid.server.logging.actors.CurrentActor; @@ -43,7 +42,6 @@ import org.apache.qpid.server.virtualhost.VirtualHost; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import static org.mockito.Matchers.any; import static org.mockito.Matchers.anySet; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; @@ -64,8 +62,6 @@ public class HeadersExchangeTest extends TestCase _virtualHost = mock(VirtualHost.class); SecurityManager securityManager = mock(SecurityManager.class); when(_virtualHost.getSecurityManager()).thenReturn(securityManager); - when(securityManager.authoriseBind(any(Exchange.class), any(AMQQueue.class), anyString())).thenReturn(true); - when(securityManager.authoriseUnbind(any(Exchange.class), anyString(), any(AMQQueue.class))).thenReturn(true); _exchange.initialise(UUID.randomUUID(), _virtualHost, "test", false, false); @@ -118,7 +114,6 @@ public class HeadersExchangeTest extends TestCase } private void bind(String bindingKey, Map<String, Object> arguments, AMQQueue q) - throws QpidSecurityException { _exchange.addBinding(bindingKey,q,arguments); } diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/TopicExchangeTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/TopicExchangeTest.java index 0af6ba125b..adb024257d 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/TopicExchangeTest.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/exchange/TopicExchangeTest.java @@ -34,7 +34,6 @@ import org.apache.qpid.server.model.Queue; import org.apache.qpid.server.model.UUIDGenerator; import org.apache.qpid.server.queue.AMQQueue; import org.apache.qpid.server.queue.BaseQueue; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.util.BrokerTestHelper; import org.apache.qpid.server.virtualhost.QueueExistsException; import org.apache.qpid.server.virtualhost.VirtualHost; @@ -76,7 +75,7 @@ public class TopicExchangeTest extends QpidTestCase } } - private AMQQueue<?,?,?> createQueue(String name) throws QpidSecurityException, QueueExistsException + private AMQQueue<?,?,?> createQueue(String name) throws QueueExistsException { Map<String,Object> attributes = new HashMap<String, Object>(); attributes.put(Queue.ID, UUIDGenerator.generateRandomUUID()); diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTestBase.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTestBase.java index 1c88df611a..c74c7bfa8b 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTestBase.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTestBase.java @@ -35,7 +35,6 @@ import org.apache.log4j.Logger; import org.apache.qpid.server.message.MessageSource; import org.apache.qpid.server.model.LifetimePolicy; import org.apache.qpid.server.model.Queue; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.exchange.ExchangeDefaults; import org.apache.qpid.server.exchange.DirectExchange; import org.apache.qpid.server.message.AMQMessageHeader; @@ -129,7 +128,7 @@ abstract class SimpleAMQQueueTestBase<E extends QueueEntryImpl<E,Q,L>, Q extends assertEquals("Virtual host was wrong", _virtualHost, _queue.getVirtualHost()); } - public void testBinding() throws QpidSecurityException + public void testBinding() { _exchange.addBinding(_routingKey, _queue, Collections.EMPTY_MAP); diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/jmx/JMXPasswordAuthenticatorTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/jmx/JMXPasswordAuthenticatorTest.java index 1aaa580ea3..99fa07e5b9 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/jmx/JMXPasswordAuthenticatorTest.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/jmx/JMXPasswordAuthenticatorTest.java @@ -22,11 +22,13 @@ package org.apache.qpid.server.security.auth.jmx; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.net.InetSocketAddress; import java.net.SocketAddress; +import java.security.AccessControlException; import java.security.Principal; import java.util.regex.Pattern; @@ -47,6 +49,7 @@ import org.apache.qpid.server.security.SecurityManager; */ public class JMXPasswordAuthenticatorTest extends TestCase { + static final String USER_NOT_AUTHORISED_FOR_MANAGEMENT = "User not authorised for management"; private static final String USERNAME = "guest"; private static final String PASSWORD = "password"; @@ -72,7 +75,6 @@ public class JMXPasswordAuthenticatorTest extends TestCase public void testAuthenticationSuccess() { when(_broker.getSubjectCreator(any(SocketAddress.class))).thenReturn(_usernamePasswordOkaySubjectCreator); - when(_securityManager.accessManagement()).thenReturn(true); Subject newSubject = _rmipa.authenticate(_credentials); assertSame("Subject must be unchanged", _loginSubject, newSubject); @@ -100,7 +102,7 @@ public class JMXPasswordAuthenticatorTest extends TestCase public void testAuthorisationFailure() { when(_broker.getSubjectCreator(any(SocketAddress.class))).thenReturn(_usernamePasswordOkaySubjectCreator); - when(_securityManager.accessManagement()).thenReturn(false); + doThrow(new AccessControlException(USER_NOT_AUTHORISED_FOR_MANAGEMENT)).when(_securityManager).accessManagement(); try { @@ -110,7 +112,7 @@ public class JMXPasswordAuthenticatorTest extends TestCase catch (SecurityException se) { assertEquals("Unexpected exception message", - JMXPasswordAuthenticator.USER_NOT_AUTHORISED_FOR_MANAGEMENT, se.getMessage()); + USER_NOT_AUTHORISED_FOR_MANAGEMENT, se.getMessage()); } } diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/util/BrokerTestHelper.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/util/BrokerTestHelper.java index f082d58d39..641fc7cb35 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/util/BrokerTestHelper.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/util/BrokerTestHelper.java @@ -46,7 +46,6 @@ import org.apache.qpid.server.logging.actors.TestLogActor; import org.apache.qpid.server.model.Broker; import org.apache.qpid.server.model.UUIDGenerator; import org.apache.qpid.server.queue.AMQQueue; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.security.SecurityManager; import org.apache.qpid.server.security.SubjectCreator; import org.apache.qpid.server.stats.StatisticsGatherer; @@ -182,7 +181,7 @@ public class BrokerTestHelper } public static AMQQueue createQueue(String queueName, VirtualHost virtualHost) - throws QpidSecurityException, QueueExistsException + throws QueueExistsException { Map<String,Object> attributes = new HashMap<String, Object>(); attributes.put(Queue.ID, UUIDGenerator.generateRandomUUID()); diff --git a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnectionDelegate.java b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnectionDelegate.java index dc26249c61..13c7e7bcd3 100644 --- a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnectionDelegate.java +++ b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnectionDelegate.java @@ -20,6 +20,7 @@ */ package org.apache.qpid.server.protocol.v0_10; +import java.security.AccessControlException; import java.security.Principal; import java.util.ArrayList; import java.util.Collection; @@ -195,12 +196,18 @@ public class ServerConnectionDelegate extends ServerDelegate { sconn.setVirtualHost(vhost); - if (!vhost.getSecurityManager().accessVirtualhost(vhostName, sconn.getRemoteAddress())) + try + { + vhost.getSecurityManager().accessVirtualhost(vhostName, sconn.getRemoteAddress()); + } + catch (AccessControlException e) { sconn.setState(Connection.State.CLOSING); - sconn.invoke(new ConnectionClose(ConnectionCloseCode.CONNECTION_FORCED, "Permission denied '"+vhostName+"'")); + sconn.invoke(new ConnectionClose(ConnectionCloseCode.CONNECTION_FORCED, e.getMessage())); + return; } - else if (vhost.getState() != State.ACTIVE) + + if (vhost.getState() != State.ACTIVE) { sconn.setState(Connection.State.CLOSING); sconn.invoke(new ConnectionClose(ConnectionCloseCode.CONNECTION_FORCED, "Virtual host '"+vhostName+"' is not active")); diff --git a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java index b0a60beaf5..d39ca73136 100644 --- a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java +++ b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java @@ -20,6 +20,7 @@ */ package org.apache.qpid.server.protocol.v0_10; +import java.security.AccessControlException; import java.util.EnumSet; import java.util.LinkedHashMap; import java.util.UUID; @@ -44,7 +45,6 @@ import org.apache.qpid.server.model.UUIDGenerator; import org.apache.qpid.server.plugin.ExchangeType; import org.apache.qpid.server.queue.AMQQueue; import org.apache.qpid.server.queue.QueueArgumentsConverter; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.security.SecurityManager; import org.apache.qpid.server.store.DurableConfigurationStore; import org.apache.qpid.server.store.MessageStore; @@ -61,7 +61,6 @@ import org.apache.qpid.server.txn.ServerTransaction; import org.apache.qpid.server.txn.SuspendAndFailDtxException; import org.apache.qpid.server.txn.TimeoutDtxException; import org.apache.qpid.server.txn.UnknownDtxBranchException; -import org.apache.qpid.server.util.Action; import org.apache.qpid.server.virtualhost.ExchangeExistsException; import org.apache.qpid.server.virtualhost.ExchangeIsAlternateException; import org.apache.qpid.server.virtualhost.RequiredExchangeException; @@ -265,7 +264,7 @@ public class ServerSessionDelegate extends SessionDelegate { exception(session, method, ExecutionErrorCode.RESOURCE_LOCKED, "Queue has an existing consumer - can't subscribe exclusively"); } - catch (QpidSecurityException e) + catch (AccessControlException e) { exception(session, method, ExecutionErrorCode.UNAUTHORIZED_ACCESS, e.getMessage()); } @@ -291,11 +290,14 @@ public class ServerSessionDelegate extends SessionDelegate final MessageMetaData_0_10 messageMetaData = new MessageMetaData_0_10(xfr); - if (!getVirtualHost(ssn).getSecurityManager().authorisePublish(messageMetaData.isImmediate(), messageMetaData.getRoutingKey(), exchange.getName())) + try + { + getVirtualHost(ssn).getSecurityManager().authorisePublish(messageMetaData.isImmediate(), messageMetaData.getRoutingKey(), exchange.getName()); + } + catch (AccessControlException e) { ExecutionErrorCode errorCode = ExecutionErrorCode.UNAUTHORIZED_ACCESS; - String description = "Permission denied: exchange-name '" + exchange.getName() + "'"; - exception(ssn, xfr, errorCode, description); + exception(ssn, xfr, errorCode, e.getMessage()); return; } @@ -749,7 +751,7 @@ public class ServerSessionDelegate extends SessionDelegate + " to " + method.getAlternateExchange() +"."); } } - catch (QpidSecurityException e) + catch (AccessControlException e) { exception(session, method, ExecutionErrorCode.UNAUTHORIZED_ACCESS, e.getMessage()); } @@ -841,7 +843,7 @@ public class ServerSessionDelegate extends SessionDelegate { exception(session, method, ExecutionErrorCode.NOT_ALLOWED, "Exchange '"+method.getExchange()+"' cannot be deleted"); } - catch (QpidSecurityException e) + catch (AccessControlException e) { exception(session, method, ExecutionErrorCode.UNAUTHORIZED_ACCESS, e.getMessage()); } @@ -935,7 +937,7 @@ public class ServerSessionDelegate extends SessionDelegate { exchange.addBinding(method.getBindingKey(), queue, method.getArguments()); } - catch (QpidSecurityException e) + catch (AccessControlException e) { exception(session, method, ExecutionErrorCode.UNAUTHORIZED_ACCESS, e.getMessage()); } @@ -988,7 +990,7 @@ public class ServerSessionDelegate extends SessionDelegate { exchange.removeBinding(method.getBindingKey(), queue, null); } - catch (QpidSecurityException e) + catch (AccessControlException e) { exception(session, method, ExecutionErrorCode.UNAUTHORIZED_ACCESS, e.getMessage()); } @@ -1236,7 +1238,7 @@ public class ServerSessionDelegate extends SessionDelegate exception(session, method, errorCode, description); } } - catch (QpidSecurityException e) + catch (AccessControlException e) { exception(session, method, ExecutionErrorCode.UNAUTHORIZED_ACCESS, e.getMessage()); } @@ -1309,7 +1311,7 @@ public class ServerSessionDelegate extends SessionDelegate { virtualHost.removeQueue(queue); } - catch (QpidSecurityException e) + catch (AccessControlException e) { exception(session, method, ExecutionErrorCode.UNAUTHORIZED_ACCESS, e.getMessage()); } @@ -1340,7 +1342,7 @@ public class ServerSessionDelegate extends SessionDelegate { queue.clearQueue(); } - catch (QpidSecurityException e) + catch (AccessControlException e) { exception(session, method, ExecutionErrorCode.UNAUTHORIZED_ACCESS, e.getMessage()); } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java index fe1cb624e5..0146d066f1 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java @@ -21,6 +21,7 @@ package org.apache.qpid.server.protocol.v0_8; import java.nio.ByteBuffer; +import java.security.AccessControlException; import java.util.*; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicBoolean; @@ -33,7 +34,6 @@ import org.apache.qpid.AMQException; import org.apache.qpid.server.filter.AMQInvalidArgumentException; import org.apache.qpid.server.filter.Filterable; import org.apache.qpid.server.filter.MessageFilter; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.common.AMQPFilterTypes; import org.apache.qpid.framing.AMQMethodBody; import org.apache.qpid.framing.AMQShortString; @@ -50,7 +50,6 @@ import org.apache.qpid.server.configuration.BrokerProperties; import org.apache.qpid.server.exchange.Exchange; import org.apache.qpid.server.filter.FilterManager; import org.apache.qpid.server.filter.FilterManagerFactory; -import org.apache.qpid.server.filter.FilterSupport; import org.apache.qpid.server.filter.SimpleFilterManager; import org.apache.qpid.server.flow.FlowCreditManager; import org.apache.qpid.server.flow.Pre0_10CreditManager; @@ -70,7 +69,6 @@ import org.apache.qpid.server.message.MessageSource; import org.apache.qpid.server.message.ServerMessage; import org.apache.qpid.server.protocol.CapacityChecker; import org.apache.qpid.server.protocol.v0_8.output.ProtocolOutputConverter; -import org.apache.qpid.server.protocol.AMQConnectionModel; import org.apache.qpid.server.protocol.AMQSessionModel; import org.apache.qpid.server.queue.AMQQueue; import org.apache.qpid.server.queue.QueueEntry; @@ -279,14 +277,13 @@ public class AMQChannel<T extends AMQProtocolSession<T>> return _channelId; } - public void setPublishFrame(MessagePublishInfo info, final MessageDestination e) throws QpidSecurityException + public void setPublishFrame(MessagePublishInfo info, final MessageDestination e) { String routingKey = info.getRoutingKey() == null ? null : info.getRoutingKey().asString(); SecurityManager securityManager = getVirtualHost().getSecurityManager(); - if (!securityManager.authorisePublish(info.isImmediate(), routingKey, e.getName())) - { - throw new QpidSecurityException("Permission denied: " + e.getName()); - } + + securityManager.authorisePublish(info.isImmediate(), routingKey, e.getName()); + _currentMessage = new IncomingMessage(info); _currentMessage.setMessageDestination(e); } @@ -533,7 +530,7 @@ public class AMQChannel<T extends AMQProtocolSession<T>> */ public AMQShortString consumeFromSource(AMQShortString tag, MessageSource source, boolean acks, FieldTable filters, boolean exclusive, boolean noLocal) - throws AMQException, QpidSecurityException, MessageSource.ExistingConsumerPreventsExclusive, + throws AMQException, MessageSource.ExistingConsumerPreventsExclusive, MessageSource.ExistingExclusiveConsumer, AMQInvalidArgumentException, MessageSource.ConsumerAccessRefused { @@ -606,7 +603,7 @@ public class AMQChannel<T extends AMQProtocolSession<T>> AMQShortString.toString(tag), options); } - catch (QpidSecurityException e) + catch (AccessControlException e) { _tag2SubscriptionTargetMap.remove(tag); throw e; diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java index c93c164978..aeb5f2d2b1 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java @@ -32,13 +32,13 @@ import org.apache.qpid.server.filter.AMQInvalidArgumentException; import org.apache.qpid.server.message.MessageSource; import org.apache.qpid.server.protocol.v0_8.AMQChannel; import org.apache.qpid.server.protocol.v0_8.AMQProtocolSession; -import org.apache.qpid.server.protocol.AMQSessionModel; import org.apache.qpid.server.queue.AMQQueue; import org.apache.qpid.server.protocol.v0_8.state.AMQStateManager; import org.apache.qpid.server.protocol.v0_8.state.StateAwareMethodListener; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.virtualhost.VirtualHost; +import java.security.AccessControlException; + public class BasicConsumeMethodHandler implements StateAwareMethodListener<BasicConsumeBody> { private static final Logger _logger = Logger.getLogger(BasicConsumeMethodHandler.class); @@ -167,7 +167,7 @@ public class BasicConsumeMethodHandler implements StateAwareMethodListener<Basic + queue.getName() + " exclusively as it already has a consumer"); } - catch (QpidSecurityException e) + catch (AccessControlException e) { throw body.getChannelException(AMQConstant.ACCESS_REFUSED, "Cannot subscribe to queue " diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicGetMethodHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicGetMethodHandler.java index 43700049e1..611999d8c6 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicGetMethodHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicGetMethodHandler.java @@ -38,7 +38,6 @@ import org.apache.qpid.server.flow.FlowCreditManager; import org.apache.qpid.server.flow.MessageOnlyCreditManager; import org.apache.qpid.server.protocol.v0_8.AMQMessage; import org.apache.qpid.server.protocol.v0_8.AMQProtocolSession; -import org.apache.qpid.server.protocol.AMQSessionModel; import org.apache.qpid.server.protocol.v0_8.ConsumerTarget_0_8; import org.apache.qpid.server.queue.AMQQueue; import org.apache.qpid.server.protocol.v0_8.state.AMQStateManager; @@ -46,9 +45,9 @@ import org.apache.qpid.server.protocol.v0_8.state.StateAwareMethodListener; import org.apache.qpid.server.protocol.v0_8.ClientDeliveryMethod; import org.apache.qpid.server.protocol.v0_8.RecordDeliveryMethod; import org.apache.qpid.server.consumer.Consumer; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.virtualhost.VirtualHost; +import java.security.AccessControlException; import java.util.EnumSet; public class BasicGetMethodHandler implements StateAwareMethodListener<BasicGetBody> @@ -111,7 +110,7 @@ public class BasicGetMethodHandler implements StateAwareMethodListener<BasicGetB protocolConnection.writeFrame(responseBody.generateFrame(channelId)); } } - catch (QpidSecurityException e) + catch (AccessControlException e) { throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, e.getMessage()); @@ -140,7 +139,7 @@ public class BasicGetMethodHandler implements StateAwareMethodListener<BasicGetB final AMQProtocolSession session, final AMQChannel channel, final boolean acks) - throws AMQException, QpidSecurityException, MessageSource.ExistingConsumerPreventsExclusive, + throws AMQException, MessageSource.ExistingConsumerPreventsExclusive, MessageSource.ExistingExclusiveConsumer, MessageSource.ConsumerAccessRefused { diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicPublishMethodHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicPublishMethodHandler.java index 318efdd125..101a92242f 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicPublishMethodHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicPublishMethodHandler.java @@ -30,13 +30,13 @@ import org.apache.qpid.framing.abstraction.MessagePublishInfo; import org.apache.qpid.protocol.AMQConstant; import org.apache.qpid.server.message.MessageDestination; import org.apache.qpid.server.protocol.v0_8.AMQChannel; -import org.apache.qpid.server.exchange.Exchange; import org.apache.qpid.server.protocol.v0_8.AMQProtocolSession; import org.apache.qpid.server.protocol.v0_8.state.AMQStateManager; import org.apache.qpid.server.protocol.v0_8.state.StateAwareMethodListener; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.virtualhost.VirtualHost; +import java.security.AccessControlException; + public class BasicPublishMethodHandler implements StateAwareMethodListener<BasicPublishBody> { private static final Logger _logger = Logger.getLogger(BasicPublishMethodHandler.class); @@ -93,7 +93,7 @@ public class BasicPublishMethodHandler implements StateAwareMethodListener<Basic { channel.setPublishFrame(info, exch); } - catch (QpidSecurityException e) + catch (AccessControlException e) { throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, e.getMessage()); } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java index 62b13baac2..1c992012da 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java @@ -35,6 +35,8 @@ import org.apache.qpid.server.protocol.v0_8.state.StateAwareMethodListener; import org.apache.qpid.server.virtualhost.State; import org.apache.qpid.server.virtualhost.VirtualHost; +import java.security.AccessControlException; + public class ConnectionOpenMethodHandler implements StateAwareMethodListener<ConnectionOpenBody> { private static final Logger _logger = Logger.getLogger(ConnectionOpenMethodHandler.class); @@ -79,11 +81,16 @@ public class ConnectionOpenMethodHandler implements StateAwareMethodListener<Con else { // Check virtualhost access - if (!virtualHost.getSecurityManager().accessVirtualhost(virtualHostName, session.getRemoteAddress())) + try + { + virtualHost.getSecurityManager().accessVirtualhost(virtualHostName, session.getRemoteAddress()); + } + catch (AccessControlException e) { - throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied: '" + virtualHost.getName() + "'"); + throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, e.getMessage()); } - else if (virtualHost.getState() != State.ACTIVE) + + if (virtualHost.getState() != State.ACTIVE) { throw body.getConnectionException(AMQConstant.CONNECTION_FORCED, "Virtual host '" + virtualHost.getName() + "' is not active"); } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java index 5c5b1f141b..87622b88e7 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java @@ -35,12 +35,13 @@ import org.apache.qpid.server.exchange.Exchange; import org.apache.qpid.server.protocol.v0_8.AMQProtocolSession; import org.apache.qpid.server.protocol.v0_8.state.AMQStateManager; import org.apache.qpid.server.protocol.v0_8.state.StateAwareMethodListener; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.virtualhost.ExchangeExistsException; import org.apache.qpid.server.virtualhost.ReservedExchangeNameException; import org.apache.qpid.server.virtualhost.UnknownExchangeException; import org.apache.qpid.server.virtualhost.VirtualHost; +import java.security.AccessControlException; + public class ExchangeDeclareHandler implements StateAwareMethodListener<ExchangeDeclareBody> { private static final Logger _logger = Logger.getLogger(ExchangeDeclareHandler.class); @@ -126,7 +127,7 @@ public class ExchangeDeclareHandler implements StateAwareMethodListener<Exchange { throw body.getConnectionException(AMQConstant.COMMAND_INVALID, "Unknown exchange: " + exchangeName,e); } - catch (QpidSecurityException e) + catch (AccessControlException e) { throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, e.getMessage()); } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java index ea3d7ded4d..bbe6028a63 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java @@ -29,11 +29,12 @@ import org.apache.qpid.server.exchange.Exchange; import org.apache.qpid.server.protocol.v0_8.AMQProtocolSession; import org.apache.qpid.server.protocol.v0_8.state.AMQStateManager; import org.apache.qpid.server.protocol.v0_8.state.StateAwareMethodListener; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.virtualhost.ExchangeIsAlternateException; import org.apache.qpid.server.virtualhost.RequiredExchangeException; import org.apache.qpid.server.virtualhost.VirtualHost; +import java.security.AccessControlException; + public class ExchangeDeleteHandler implements StateAwareMethodListener<ExchangeDeleteBody> { private static final ExchangeDeleteHandler _instance = new ExchangeDeleteHandler(); @@ -83,7 +84,7 @@ public class ExchangeDeleteHandler implements StateAwareMethodListener<ExchangeD { throw body.getChannelException(AMQConstant.NOT_ALLOWED, "Exchange '"+body.getExchange()+"' cannot be deleted"); } - catch (QpidSecurityException e) + catch (AccessControlException e) { throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, e.getMessage()); } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java index 9b875ccf39..8eb6c63542 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java @@ -23,7 +23,6 @@ package org.apache.qpid.server.protocol.v0_8.handler; import org.apache.log4j.Logger; import org.apache.qpid.AMQException; -import org.apache.qpid.exchange.ExchangeDefaults; import org.apache.qpid.framing.AMQMethodBody; import org.apache.qpid.framing.AMQShortString; import org.apache.qpid.framing.FieldTable; @@ -31,19 +30,16 @@ import org.apache.qpid.framing.MethodRegistry; import org.apache.qpid.framing.QueueBindBody; import org.apache.qpid.protocol.AMQConstant; import org.apache.qpid.server.exchange.TopicExchange; -import org.apache.qpid.server.plugin.ExchangeType; import org.apache.qpid.server.protocol.v0_8.AMQChannel; import org.apache.qpid.server.binding.Binding; import org.apache.qpid.server.exchange.Exchange; import org.apache.qpid.server.protocol.v0_8.AMQProtocolSession; -import org.apache.qpid.server.protocol.AMQSessionModel; import org.apache.qpid.server.queue.AMQQueue; -import org.apache.qpid.server.queue.QueueRegistry; import org.apache.qpid.server.protocol.v0_8.state.AMQStateManager; import org.apache.qpid.server.protocol.v0_8.state.StateAwareMethodListener; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.virtualhost.VirtualHost; +import java.security.AccessControlException; import java.util.Map; public class QueueBindHandler implements StateAwareMethodListener<QueueBindBody> @@ -135,7 +131,7 @@ public class QueueBindHandler implements StateAwareMethodListener<QueueBindBody> } } } - catch (QpidSecurityException e) + catch (AccessControlException e) { throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, e.getMessage()); } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueDeclareHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueDeclareHandler.java index 215e3f2f23..26302dbd72 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueDeclareHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueDeclareHandler.java @@ -40,10 +40,9 @@ import org.apache.qpid.server.queue.AMQQueue; import org.apache.qpid.server.queue.QueueArgumentsConverter; import org.apache.qpid.server.protocol.v0_8.state.AMQStateManager; import org.apache.qpid.server.protocol.v0_8.state.StateAwareMethodListener; -import org.apache.qpid.server.security.QpidSecurityException; -import org.apache.qpid.server.util.Action; import org.apache.qpid.server.virtualhost.VirtualHost; +import java.security.AccessControlException; import java.util.Map; import java.util.UUID; import org.apache.qpid.server.virtualhost.QueueExistsException; @@ -149,7 +148,7 @@ public class QueueDeclareHandler implements StateAwareMethodListener<QueueDeclar } } - catch (QpidSecurityException e) + catch (AccessControlException e) { throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, e.getMessage()); } @@ -181,7 +180,7 @@ public class QueueDeclareHandler implements StateAwareMethodListener<QueueDeclar QueueDeclareBody body, final VirtualHost virtualHost, final AMQProtocolSession session) - throws AMQException, QpidSecurityException, QueueExistsException + throws AMQException, QueueExistsException { final boolean durable = body.getDurable(); diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueDeleteHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueDeleteHandler.java index c939e49aab..d7545c844a 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueDeleteHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueDeleteHandler.java @@ -27,16 +27,14 @@ import org.apache.qpid.framing.QueueDeleteOkBody; import org.apache.qpid.protocol.AMQConstant; import org.apache.qpid.server.protocol.v0_8.AMQChannel; import org.apache.qpid.server.protocol.v0_8.AMQProtocolSession; -import org.apache.qpid.server.protocol.AMQSessionModel; import org.apache.qpid.server.queue.AMQQueue; -import org.apache.qpid.server.queue.QueueRegistry; import org.apache.qpid.server.protocol.v0_8.state.AMQStateManager; import org.apache.qpid.server.protocol.v0_8.state.StateAwareMethodListener; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.store.DurableConfigurationStore; -import org.apache.qpid.server.store.DurableConfigurationStoreHelper; import org.apache.qpid.server.virtualhost.VirtualHost; +import java.security.AccessControlException; + public class QueueDeleteHandler implements StateAwareMethodListener<QueueDeleteBody> { private static final QueueDeleteHandler _instance = new QueueDeleteHandler(); @@ -116,7 +114,7 @@ public class QueueDeleteHandler implements StateAwareMethodListener<QueueDeleteB { purged = virtualHost.removeQueue(queue); } - catch (QpidSecurityException e) + catch (AccessControlException e) { throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, e.getMessage()); } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueuePurgeHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueuePurgeHandler.java index 9d035a3f57..569654c64d 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueuePurgeHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueuePurgeHandler.java @@ -28,14 +28,13 @@ import org.apache.qpid.framing.QueuePurgeBody; import org.apache.qpid.protocol.AMQConstant; import org.apache.qpid.server.protocol.v0_8.AMQChannel; import org.apache.qpid.server.protocol.v0_8.AMQProtocolSession; -import org.apache.qpid.server.protocol.AMQSessionModel; import org.apache.qpid.server.queue.AMQQueue; -import org.apache.qpid.server.queue.QueueRegistry; import org.apache.qpid.server.protocol.v0_8.state.AMQStateManager; import org.apache.qpid.server.protocol.v0_8.state.StateAwareMethodListener; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.virtualhost.VirtualHost; +import java.security.AccessControlException; + public class QueuePurgeHandler implements StateAwareMethodListener<QueuePurgeBody> { private static final QueuePurgeHandler _instance = new QueuePurgeHandler(); @@ -107,7 +106,7 @@ public class QueuePurgeHandler implements StateAwareMethodListener<QueuePurgeBod { purged = queue.clearQueue(); } - catch (QpidSecurityException e) + catch (AccessControlException e) { throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, e.getMessage()); } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java index 91025dacf2..f44f831f68 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java @@ -35,12 +35,12 @@ import org.apache.qpid.server.protocol.v0_8.AMQChannel; import org.apache.qpid.server.exchange.Exchange; import org.apache.qpid.server.protocol.v0_8.AMQProtocolSession; import org.apache.qpid.server.queue.AMQQueue; -import org.apache.qpid.server.queue.QueueRegistry; import org.apache.qpid.server.protocol.v0_8.state.AMQStateManager; import org.apache.qpid.server.protocol.v0_8.state.StateAwareMethodListener; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.virtualhost.VirtualHost; +import java.security.AccessControlException; + public class QueueUnbindHandler implements StateAwareMethodListener<QueueUnbindBody> { private static final Logger _log = Logger.getLogger(QueueUnbindHandler.class); @@ -110,7 +110,7 @@ public class QueueUnbindHandler implements StateAwareMethodListener<QueueUnbindB { exch.removeBinding(String.valueOf(routingKey), queue, FieldTable.convertToMap(body.getArguments())); } - catch (QpidSecurityException e) + catch (AccessControlException e) { throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, e.getMessage()); } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/BrokerTestHelper_0_8.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/BrokerTestHelper_0_8.java index 845c1d55b4..e5a3475feb 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/BrokerTestHelper_0_8.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/BrokerTestHelper_0_8.java @@ -26,7 +26,6 @@ import org.apache.qpid.framing.BasicContentHeaderProperties; import org.apache.qpid.framing.ContentHeaderBody; import org.apache.qpid.framing.abstraction.MessagePublishInfo; import org.apache.qpid.server.exchange.Exchange; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.util.BrokerTestHelper; import org.apache.qpid.server.virtualhost.VirtualHost; @@ -66,7 +65,7 @@ public class BrokerTestHelper_0_8 extends BrokerTestHelper } public static void publishMessages(AMQChannel channel, int numberOfMessages, String queueName, String exchangeName) - throws AMQException, QpidSecurityException + throws AMQException { AMQShortString routingKey = new AMQShortString(queueName); AMQShortString exchangeNameAsShortString = new AMQShortString(exchangeName); diff --git a/qpid/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/SendingLink_1_0.java b/qpid/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/SendingLink_1_0.java index f7e2d2df50..d0e77b4878 100644 --- a/qpid/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/SendingLink_1_0.java +++ b/qpid/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/SendingLink_1_0.java @@ -20,6 +20,7 @@ */ package org.apache.qpid.server.protocol.v1_0; +import java.security.AccessControlException; import java.util.ArrayList; import java.util.Collections; import java.util.EnumSet; @@ -33,7 +34,6 @@ import org.apache.log4j.Logger; import org.apache.qpid.server.model.ExclusivityPolicy; import org.apache.qpid.server.model.LifetimePolicy; import org.apache.qpid.server.model.Queue; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.amqp_1_0.transport.DeliveryStateHandler; import org.apache.qpid.amqp_1_0.transport.LinkEndpoint; import org.apache.qpid.amqp_1_0.transport.SendingLinkEndpoint; @@ -64,7 +64,6 @@ import org.apache.qpid.server.queue.AMQQueue; import org.apache.qpid.server.consumer.Consumer; import org.apache.qpid.server.txn.AutoCommitTransaction; import org.apache.qpid.server.txn.ServerTransaction; -import org.apache.qpid.server.util.Action; import org.apache.qpid.server.util.ConnectionScopedRuntimeException; import org.apache.qpid.server.virtualhost.VirtualHost; import org.apache.qpid.server.virtualhost.QueueExistsException; @@ -312,11 +311,6 @@ public class SendingLink_1_0 implements SendingLinkListener, Link_1_0, DeliveryS qd = new QueueDestination(queue); } - catch (QpidSecurityException e) - { - _logger.error("Security error", e); - throw new ConnectionScopedRuntimeException(e); - } catch (QueueExistsException e) { _logger.error("A randomly generated temporary queue name collided with an existing queue",e); @@ -357,12 +351,6 @@ public class SendingLink_1_0 implements SendingLinkListener, Link_1_0, DeliveryS messageFilter == null ? null : new SimpleFilterManager(messageFilter), Message_1_0.class, name, options); } - catch (QpidSecurityException e) - { - //TODO - _logger.info("Error registering subscription", e); - throw new ConnectionScopedRuntimeException(e); - } catch (MessageSource.ExistingExclusiveConsumer e) { _logger.info("Cannot add a consumer to the destination as there is already an exclusive consumer"); @@ -416,7 +404,7 @@ public class SendingLink_1_0 implements SendingLinkListener, Link_1_0, DeliveryS { _vhost.removeQueue((AMQQueue)_queue); } - catch (QpidSecurityException e) + catch (AccessControlException e) { //TODO _logger.error("Error registering subscription", e); diff --git a/qpid/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/Session_1_0.java b/qpid/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/Session_1_0.java index 6840c7344a..b96738e0f6 100644 --- a/qpid/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/Session_1_0.java +++ b/qpid/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/Session_1_0.java @@ -20,6 +20,7 @@ */ package org.apache.qpid.server.protocol.v1_0; +import java.security.AccessControlException; import java.text.MessageFormat; import org.apache.log4j.Logger; @@ -39,13 +40,11 @@ import org.apache.qpid.amqp_1_0.type.transport.*; import org.apache.qpid.amqp_1_0.type.transport.Error; import org.apache.qpid.server.model.*; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.protocol.AMQConstant; import org.apache.qpid.server.exchange.Exchange; import org.apache.qpid.server.logging.LogSubject; import org.apache.qpid.server.message.MessageDestination; import org.apache.qpid.server.message.MessageSource; -import org.apache.qpid.server.protocol.AMQConnectionModel; import org.apache.qpid.server.protocol.AMQSessionModel; import org.apache.qpid.server.protocol.LinkRegistry; import org.apache.qpid.server.queue.AMQQueue; @@ -380,7 +379,7 @@ public class Session_1_0 implements SessionEventListener, AMQSessionModel<Sessio final AMQQueue tempQueue = queue = _vhost.createQueue(this, attributes ); } - catch (QpidSecurityException e) + catch (AccessControlException e) { //TODO _logger.info("Security error", e); diff --git a/qpid/java/broker-plugins/management-amqp/src/main/java/org/apache/qpid/server/management/amqp/ManagementNode.java b/qpid/java/broker-plugins/management-amqp/src/main/java/org/apache/qpid/server/management/amqp/ManagementNode.java index 6f083012e7..7fe280649f 100644 --- a/qpid/java/broker-plugins/management-amqp/src/main/java/org/apache/qpid/server/management/amqp/ManagementNode.java +++ b/qpid/java/broker-plugins/management-amqp/src/main/java/org/apache/qpid/server/management/amqp/ManagementNode.java @@ -20,7 +20,6 @@ */ package org.apache.qpid.server.management.amqp; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.consumer.Consumer; import org.apache.qpid.server.consumer.ConsumerTarget; import org.apache.qpid.server.filter.FilterManager; @@ -369,16 +368,9 @@ class ManagementNode implements MessageSource<ManagementNodeConsumer,ManagementN } response = performReadOperation(message, child); } - catch(RuntimeException e) + catch(AccessControlException e) { - if (e instanceof AccessControlException || e.getCause() instanceof QpidSecurityException) - { - response = createFailureResponse(message, STATUS_CODE_FORBIDDEN, e.getMessage()); - } - else - { - throw e; - } + response = createFailureResponse(message, STATUS_CODE_FORBIDDEN, e.getMessage()); } } catch (ClassNotFoundException e) @@ -474,17 +466,9 @@ class ManagementNode implements MessageSource<ManagementNodeConsumer,ManagementN entity.setDesiredState(entity.getActualState(),State.DELETED); responseHeader.setHeader(STATUS_CODE_HEADER, STATUS_CODE_NO_CONTENT); } - catch(RuntimeException e) + catch(AccessControlException e) { - if (e instanceof AccessControlException || e.getCause() instanceof QpidSecurityException) - { - responseHeader.setHeader(STATUS_CODE_HEADER, STATUS_CODE_FORBIDDEN); - } - else - { - throw e; - } - + responseHeader.setHeader(STATUS_CODE_HEADER, STATUS_CODE_FORBIDDEN); } return InternalMessage.createMapMessage(_virtualHost.getMessageStore(),responseHeader, Collections.emptyMap()); @@ -512,16 +496,9 @@ class ManagementNode implements MessageSource<ManagementNodeConsumer,ManagementN entity.setAttributes((Map)messageBody); return performReadOperation(requestMessage, entity); } - catch(RuntimeException e) + catch(AccessControlException e) { - if (e instanceof AccessControlException || e.getCause() instanceof QpidSecurityException) - { - return createFailureResponse(requestMessage, STATUS_CODE_FORBIDDEN, e.getMessage()); - } - else - { - throw e; - } + return createFailureResponse(requestMessage, STATUS_CODE_FORBIDDEN, e.getMessage()); } } else @@ -615,8 +592,8 @@ class ManagementNode implements MessageSource<ManagementNodeConsumer,ManagementN final InternalMessageHeader requestHeader = msg.getMessageHeader(); final MutableMessageHeader responseHeader = new MutableMessageHeader(); responseHeader.setCorrelationId(requestHeader.getCorrelationId() == null - ? requestHeader.getMessageId() - : requestHeader.getCorrelationId()); + ? requestHeader.getMessageId() + : requestHeader.getCorrelationId()); responseHeader.setMessageId(UUID.randomUUID().toString()); diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java index 9a2f0dd1f6..674ff71232 100644 --- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java +++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java @@ -48,7 +48,6 @@ import org.apache.qpid.server.security.auth.AuthenticatedPrincipal; import org.apache.qpid.server.security.auth.AuthenticationResult.AuthenticationStatus; import org.apache.qpid.server.security.auth.SubjectAuthenticationResult; import org.apache.qpid.server.security.auth.UsernamePrincipal; -import org.apache.qpid.server.security.auth.manager.ExternalAuthenticationManager; import org.apache.qpid.server.security.auth.manager.ExternalAuthenticationManagerFactory; import org.apache.qpid.server.util.ServerScopedRuntimeException; import org.apache.qpid.transport.network.security.ssl.SSLUtil; @@ -108,18 +107,16 @@ public class HttpManagementUtil throw new SecurityException("Only authenticated users can access the management interface"); } LogActor actor = createHttpManagementActor(broker, request); - if (hasAccessToManagement(broker.getSecurityManager(), subject, actor)) - { - saveAuthorisedSubject(session, subject, actor); - } - else - { - throw new AccessControlException("Access to the management interface denied"); - } + + assertManagementAccess(broker.getSecurityManager(), subject, actor); + + saveAuthorisedSubject(session, subject, actor); + + } } - public static boolean hasAccessToManagement(final SecurityManager securityManager, Subject subject, LogActor actor) + public static void assertManagementAccess(final SecurityManager securityManager, Subject subject, LogActor actor) { // TODO: We should eliminate SecurityManager.setThreadSubject in favour of Subject.doAs SecurityManager.setThreadSubject(subject); // Required for accessManagement check @@ -128,12 +125,13 @@ public class HttpManagementUtil { try { - return Subject.doAs(subject, new PrivilegedExceptionAction<Boolean>() + Subject.doAs(subject, new PrivilegedExceptionAction<Void>() { @Override - public Boolean run() throws Exception + public Void run() { - return securityManager.accessManagement(); + securityManager.accessManagement(); + return null; } }); } diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java index 9ca23ce1ce..3eafa7c294 100644 --- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java +++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java @@ -19,6 +19,7 @@ package org.apache.qpid.server.management.plugin.servlet.rest; import java.io.IOException; import java.io.PrintWriter; +import java.security.AccessControlException; import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedHashMap; @@ -426,21 +427,22 @@ public class MessageServlet extends AbstractServlet // FIXME: added temporary authorization check until we introduce management layer // and review current ACL rules to have common rules for all management interfaces String methodName = isMoveTransaction? "moveMessages":"copyMessages"; - if (isQueueUpdateMethodAuthorized(methodName, vhost)) - { - final Queue destinationQueue = getQueueFromVirtualHost(destQueueName, vhost); - final List messageIds = new ArrayList((List) providedObject.get("messages")); - QueueEntryTransaction txn = - isMoveTransaction - ? new MoveTransaction(sourceQueue, messageIds, destinationQueue) - : new CopyTransaction(sourceQueue, messageIds, destinationQueue); - vhost.executeTransaction(txn); - response.setStatus(HttpServletResponse.SC_OK); - } - else - { - response.setStatus(HttpServletResponse.SC_FORBIDDEN); - } + authorizeMethod(methodName, vhost); + + + final Queue destinationQueue = getQueueFromVirtualHost(destQueueName, vhost); + final List messageIds = new ArrayList((List) providedObject.get("messages")); + QueueEntryTransaction txn = + isMoveTransaction + ? new MoveTransaction(sourceQueue, messageIds, destinationQueue) + : new CopyTransaction(sourceQueue, messageIds, destinationQueue); + vhost.executeTransaction(txn); + response.setStatus(HttpServletResponse.SC_OK); + + } + catch(AccessControlException e) + { + response.setStatus(HttpServletResponse.SC_FORBIDDEN); } catch(RuntimeException e) { @@ -470,22 +472,23 @@ public class MessageServlet extends AbstractServlet // FIXME: added temporary authorization check until we introduce management layer // and review current ACL rules to have common rules for all management interfaces - if (isQueueUpdateMethodAuthorized("deleteMessages", vhost)) + try { + authorizeMethod("deleteMessages", vhost); vhost.executeTransaction(new DeleteTransaction(sourceQueue, messageIds)); response.setStatus(HttpServletResponse.SC_OK); } - else + catch (AccessControlException e) { response.setStatus(HttpServletResponse.SC_FORBIDDEN); } } - private boolean isQueueUpdateMethodAuthorized(String methodName, VirtualHost host) + private void authorizeMethod(String methodName, VirtualHost host) { SecurityManager securityManager = host.getSecurityManager(); - return securityManager.authoriseMethod(Operation.UPDATE, "VirtualHost.Queue", methodName); + securityManager.authoriseMethod(Operation.UPDATE, "VirtualHost.Queue", methodName); } } diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java index 45e0c2dab8..e6bc46aa77 100644 --- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java +++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java @@ -27,7 +27,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.log4j.Logger; -import org.apache.qpid.server.security.QpidSecurityException; import org.apache.qpid.server.model.*; import org.codehaus.jackson.map.ObjectMapper; import org.codehaus.jackson.map.SerializationConfig; @@ -498,7 +497,7 @@ public class RestServlet extends AbstractServlet private void setResponseStatus(HttpServletResponse response, RuntimeException e) throws IOException { - if (e instanceof AccessControlException || e.getCause() instanceof QpidSecurityException) + if (e instanceof AccessControlException) { if (LOGGER.isDebugEnabled()) { diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java index a29a875071..5441dc95c4 100644 --- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java +++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java @@ -241,7 +241,11 @@ public class SaslServlet extends AbstractServlet Broker broker = getBroker(); LogActor actor = HttpManagementUtil.getOrCreateAndCacheLogActor(request, broker); - if (!HttpManagementUtil.hasAccessToManagement(broker.getSecurityManager(), subject, actor)) + try + { + HttpManagementUtil.assertManagementAccess(broker.getSecurityManager(), subject, actor); + } + catch(SecurityException e) { sendError(response, HttpServletResponse.SC_FORBIDDEN); return; diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/UserPreferencesServlet.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/UserPreferencesServlet.java index 355b5df177..01657b131d 100644 --- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/UserPreferencesServlet.java +++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/UserPreferencesServlet.java @@ -64,11 +64,16 @@ public class UserPreferencesServlet extends AbstractServlet private void getUserPreferences(String authenticationProviderName, String userId, HttpServletResponse response) throws IOException { - if (!userPreferencesOperationAuthorized(userId)) + try + { + assertUserPreferencesOperationAuthorized(userId); + } + catch (SecurityException e) { response.sendError(HttpServletResponse.SC_FORBIDDEN, "Viewing of preferences is not allowed"); return; } + Map<String, Object> preferences = null; PreferencesProvider preferencesProvider = getPreferencesProvider(authenticationProviderName); if (preferencesProvider == null) @@ -171,11 +176,16 @@ public class UserPreferencesServlet extends AbstractServlet String userId = elements[1]; - if (!userPreferencesOperationAuthorized(userId)) + try + { + assertUserPreferencesOperationAuthorized(userId); + } + catch (SecurityException e) { response.sendError(HttpServletResponse.SC_FORBIDDEN, "Deletion of preferences is not allowed"); return; } + String providerName = elements[0]; Set<String> users = providerUsers.get(providerName); @@ -226,8 +236,8 @@ public class UserPreferencesServlet extends AbstractServlet return provider; } - private boolean userPreferencesOperationAuthorized(String userId) + private void assertUserPreferencesOperationAuthorized(String userId) { - return getBroker().getSecurityManager().authoriseUserOperation(Operation.UPDATE, userId); + getBroker().getSecurityManager().authoriseUserOperation(Operation.UPDATE, userId); } } diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java index 0f963df66f..e9716ab775 100644 --- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java +++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java @@ -227,22 +227,8 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler } methodName = getMethodName(method, args); - if (isAccessMethod(methodName) || impact == MBeanOperationInfo.INFO) - { - // Check for read-only method invocation permission - if (!security.authoriseMethod(Operation.ACCESS, type, methodName)) - { - throw new SecurityException("Permission denied: Access " + methodName); - } - } - else - { - // Check for setting properties permission - if (!security.authoriseMethod(Operation.UPDATE, type, methodName)) - { - throw new SecurityException("Permission denied: Update " + methodName); - } - } + Operation operation = (isAccessMethod(methodName) || impact == MBeanOperationInfo.INFO) ? Operation.ACCESS : Operation.UPDATE; + security.authoriseMethod(operation, type, methodName); boolean oldAccessChecksDisabled = false; if(_managementRightsInferAllAccess) diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalACLJMXTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalACLJMXTest.java index e200741161..389ac5ff4d 100644 --- a/qpid/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalACLJMXTest.java +++ b/qpid/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalACLJMXTest.java @@ -82,7 +82,7 @@ public class ExternalACLJMXTest extends AbstractACLTestCase } catch (SecurityException e) { - assertEquals("Cause message incorrect", "Permission denied: Update resetStatistics", e.getMessage()); + assertEquals("Cause message incorrect", "Permission denied: UPDATE resetStatistics", e.getMessage()); } //try a vhost-level method @@ -93,7 +93,7 @@ public class ExternalACLJMXTest extends AbstractACLTestCase } catch (Exception e) { - assertEquals("Cause message incorrect", "Permission denied: Update createNewQueue", e.getMessage()); + assertEquals("Cause message incorrect", "Permission denied: UPDATE createNewQueue", e.getMessage()); } // Ensure that calls to MBeans outside the Qpid domain are not impeded. @@ -143,7 +143,7 @@ public class ExternalACLJMXTest extends AbstractACLTestCase } catch (SecurityException e) { - assertEquals("Cause message incorrect", "Permission denied: Update createNewQueue", e.getMessage()); + assertEquals("Cause message incorrect", "Permission denied: UPDATE createNewQueue", e.getMessage()); } } @@ -237,7 +237,7 @@ public class ExternalACLJMXTest extends AbstractACLTestCase } catch (SecurityException e) { - assertEquals("Cause message incorrect", "Permission denied: Update createNewQueue", e.getMessage()); + assertEquals("Cause message incorrect", "Permission denied: UPDATE createNewQueue", e.getMessage()); } } @@ -262,7 +262,7 @@ public class ExternalACLJMXTest extends AbstractACLTestCase } catch (SecurityException e) { - assertEquals("Cause message incorrect", "Permission denied: Update resetStatistics", e.getMessage()); + assertEquals("Cause message incorrect", "Permission denied: UPDATE resetStatistics", e.getMessage()); } } @@ -289,7 +289,7 @@ public class ExternalACLJMXTest extends AbstractACLTestCase } catch (SecurityException e) { - assertEquals("Cause message incorrect", "Permission denied: Access getManagementApiMinorVersion", e.getMessage()); + assertEquals("Cause message incorrect", "Permission denied: ACCESS getManagementApiMinorVersion", e.getMessage()); } } |