QPID-5299: check access permissions before resolving node

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

3 files changed, 42 insertions, 6 deletions

diff --git a/qpid/cpp/src/qpid/broker/amqp/Authorise.cpp b/qpid/cpp/src/qpid/broker/amqp/Authorise.cpp
index 10b7fa5f7a..28dc3b25ac 100644
--- a/qpid/cpp/src/qpid/broker/amqp/Authorise.cpp
+++ b/qpid/cpp/src/qpid/broker/amqp/Authorise.cpp
@@ -128,4 +128,22 @@ void Authorise::interlink()
     }
 }
 
+void Authorise::access(const std::string& node, bool queueRequested, bool exchangeRequested)
+{
+    if (acl) {
+        std::map<acl::Property, std::string> params;
+        bool checkExchange = true;
+        bool checkQueue = true;
+        if (exchangeRequested) checkQueue = false;
+        else if (queueRequested) checkExchange = false;
+
+        bool allowExchange = !checkExchange || acl->authorise(user, acl::ACT_ACCESS, acl::OBJ_EXCHANGE, node, &params);
+        bool allowQueue = !checkQueue || acl->authorise(user, acl::ACT_ACCESS, acl::OBJ_QUEUE, node, &params);
+
+        if (!allowQueue || !allowExchange) {
+            throw Exception(qpid::amqp::error_conditions::UNAUTHORIZED_ACCESS, QPID_MSG("ACL denied access request to " << node << " from " << user));
+        }
+    }
+}
+
 }}} // namespace qpid::broker::amqp
diff --git a/qpid/cpp/src/qpid/broker/amqp/Authorise.h b/qpid/cpp/src/qpid/broker/amqp/Authorise.h
index e429c60ab8..3714511177 100644
--- a/qpid/cpp/src/qpid/broker/amqp/Authorise.h
+++ b/qpid/cpp/src/qpid/broker/amqp/Authorise.h
@@ -48,6 +48,13 @@ class Authorise
     void outgoing(boost::shared_ptr<Queue>);
     void route(boost::shared_ptr<Exchange>, const Message&);
     void interlink();
+    /**
+     * Used to determine whether the user has access permission for a
+     * given node name. If a specific type of node was requested, only
+     * acces to that type is checked. Otherwise access to either queue
+     * or exchange is required.
+     */
+    void access(const std::string& name, bool queueRequested, bool exchangeRequested);
   private:
     const std::string user;
     AclModule* const acl;
diff --git a/qpid/cpp/src/qpid/broker/amqp/Session.cpp b/qpid/cpp/src/qpid/broker/amqp/Session.cpp
index 8ad95e8075..16aadfc6c8 100644
--- a/qpid/cpp/src/qpid/broker/amqp/Session.cpp
+++ b/qpid/cpp/src/qpid/broker/amqp/Session.cpp
@@ -199,13 +199,25 @@ Session::Session(pn_session_t* s, Connection& c, qpid::sys::OutputControl& o)
 
 Session::ResolvedNode Session::resolve(const std::string name, pn_terminus_t* terminus, bool incoming)
 {
-    ResolvedNode node;
-    node.exchange = connection.getBroker().getExchanges().find(name);
-    node.queue = connection.getBroker().getQueues().find(name);
-    node.topic = connection.getTopics().get(name);
-    bool createOnDemand = is_capability_requested(CREATE_ON_DEMAND, pn_terminus_capabilities(terminus));
     bool isQueueRequested = is_capability_requested(QUEUE, pn_terminus_capabilities(terminus));
     bool isTopicRequested = is_capability_requested(TOPIC, pn_terminus_capabilities(terminus));
+    if (isTopicRequested && isQueueRequested) {
+        //requesting both renders each request meaningless
+        isQueueRequested = false;
+        isTopicRequested = false;
+    }
+    //check whether user is even allowed access to queues/topics before resolving
+    authorise.access(name, isQueueRequested, isTopicRequested);
+    ResolvedNode node;
+    if (isTopicRequested || !isQueueRequested) {
+        node.topic = connection.getTopics().get(name);
+        if (node.topic) node.exchange = node.topic->getExchange();
+        else node.exchange = connection.getBroker().getExchanges().find(name);
+    }
+    if (isQueueRequested || !isTopicRequested) {
+        node.queue = connection.getBroker().getQueues().find(name);
+    }
+    bool createOnDemand = is_capability_requested(CREATE_ON_DEMAND, pn_terminus_capabilities(terminus));
     //Strictly speaking, properties should only be specified when the
     //terminus is dynamic. However we will not enforce that here. If
     //properties are set on the attach request, we will set them on
@@ -213,7 +225,6 @@ Session::ResolvedNode Session::resolve(const std::string name, pn_terminus_t* te
     //qpid messaging API to be implemented over 1.0.
     node.properties.read(pn_terminus_properties(terminus));
 
-    if (node.topic) node.exchange = node.topic->getExchange();
     if (node.exchange && createOnDemand && isTopicRequested) {
         if (!node.properties.getExchangeType().empty() && node.properties.getExchangeType() != node.exchange->getType()) {
             //emulate 0-10 exchange-declare behaviour