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/broker-core | |
| 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/broker-core')
33 files changed, 241 insertions, 455 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()); |