From d6f465d6a10b4d1d9ced48a10ae980c98697ff5b Mon Sep 17 00:00:00 2001 From: Robert Godfrey Date: Tue, 18 Feb 2014 23:07:41 +0000 Subject: 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 --- .../protocol/v0_10/ServerConnectionDelegate.java | 13 +++++-- .../protocol/v0_10/ServerSessionDelegate.java | 28 ++++++++------- .../qpid/server/protocol/v0_8/AMQChannel.java | 17 ++++----- .../v0_8/handler/BasicConsumeMethodHandler.java | 6 ++-- .../v0_8/handler/BasicGetMethodHandler.java | 7 ++-- .../v0_8/handler/BasicPublishMethodHandler.java | 6 ++-- .../v0_8/handler/ConnectionOpenMethodHandler.java | 13 +++++-- .../v0_8/handler/ExchangeDeclareHandler.java | 5 +-- .../v0_8/handler/ExchangeDeleteHandler.java | 5 +-- .../protocol/v0_8/handler/QueueBindHandler.java | 8 ++--- .../protocol/v0_8/handler/QueueDeclareHandler.java | 7 ++-- .../protocol/v0_8/handler/QueueDeleteHandler.java | 8 ++--- .../protocol/v0_8/handler/QueuePurgeHandler.java | 7 ++-- .../protocol/v0_8/handler/QueueUnbindHandler.java | 6 ++-- .../server/protocol/v0_8/BrokerTestHelper_0_8.java | 3 +- .../qpid/server/protocol/v1_0/SendingLink_1_0.java | 16 ++------- .../qpid/server/protocol/v1_0/Session_1_0.java | 5 ++- .../server/management/amqp/ManagementNode.java | 39 +++++--------------- .../management/plugin/HttpManagementUtil.java | 24 ++++++------- .../plugin/servlet/rest/MessageServlet.java | 41 ++++++++++++---------- .../plugin/servlet/rest/RestServlet.java | 3 +- .../plugin/servlet/rest/SaslServlet.java | 6 +++- .../servlet/rest/UserPreferencesServlet.java | 18 +++++++--- .../server/jmx/MBeanInvocationHandlerImpl.java | 18 ++-------- 24 files changed, 139 insertions(+), 170 deletions(-) (limited to 'qpid/java/broker-plugins') 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> 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> */ 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> 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 { private static final Logger _logger = Logger.getLogger(BasicConsumeMethodHandler.class); @@ -167,7 +167,7 @@ public class BasicConsumeMethodHandler implements StateAwareMethodListener @@ -111,7 +110,7 @@ public class BasicGetMethodHandler implements StateAwareMethodListener { private static final Logger _logger = Logger.getLogger(BasicPublishMethodHandler.class); @@ -93,7 +93,7 @@ public class BasicPublishMethodHandler implements StateAwareMethodListener { private static final Logger _logger = Logger.getLogger(ConnectionOpenMethodHandler.class); @@ -79,11 +81,16 @@ public class ConnectionOpenMethodHandler implements StateAwareMethodListener { private static final Logger _logger = Logger.getLogger(ExchangeDeclareHandler.class); @@ -126,7 +127,7 @@ public class ExchangeDeclareHandler implements StateAwareMethodListener { private static final ExchangeDeleteHandler _instance = new ExchangeDeleteHandler(); @@ -83,7 +84,7 @@ public class ExchangeDeleteHandler implements StateAwareMethodListener @@ -135,7 +131,7 @@ public class QueueBindHandler implements StateAwareMethodListener } } } - 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 { private static final QueueDeleteHandler _instance = new QueueDeleteHandler(); @@ -116,7 +114,7 @@ public class QueueDeleteHandler implements StateAwareMethodListener { private static final QueuePurgeHandler _instance = new QueuePurgeHandler(); @@ -107,7 +106,7 @@ public class QueuePurgeHandler implements StateAwareMethodListener { private static final Logger _log = Logger.getLogger(QueueUnbindHandler.class); @@ -110,7 +110,7 @@ public class QueueUnbindHandler implements StateAwareMethodListener() + Subject.doAs(subject, new PrivilegedExceptionAction() { @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 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 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) -- cgit v1.2.1