QPID-3964: Enforce 'access' premission rather than 'create' for passive declares; remove the now redundant 'passive' property from ACL model

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

4 files changed, 36 insertions, 35 deletions

diff --git a/qpid/cpp/src/qpid/broker/AclModule.h b/qpid/cpp/src/qpid/broker/AclModule.h
index 09f2701060..ff9281b6fc 100644
--- a/qpid/cpp/src/qpid/broker/AclModule.h
+++ b/qpid/cpp/src/qpid/broker/AclModule.h
@@ -69,7 +69,6 @@ namespace acl {
         PROP_DURABLE,
         PROP_OWNER,
         PROP_ROUTINGKEY,
-        PROP_PASSIVE,
         PROP_AUTODELETE,
         PROP_EXCLUSIVE,
         PROP_TYPE,
@@ -89,7 +88,6 @@ namespace acl {
         SPECPROP_DURABLE         = PROP_DURABLE,
         SPECPROP_OWNER           = PROP_OWNER,
         SPECPROP_ROUTINGKEY      = PROP_ROUTINGKEY,
-        SPECPROP_PASSIVE         = PROP_PASSIVE,
         SPECPROP_AUTODELETE      = PROP_AUTODELETE,
         SPECPROP_EXCLUSIVE       = PROP_EXCLUSIVE,
         SPECPROP_TYPE            = PROP_TYPE,
@@ -202,7 +200,6 @@ namespace acl {
             if (str.compare("durable")       == 0) return PROP_DURABLE;
             if (str.compare("owner")         == 0) return PROP_OWNER;
             if (str.compare("routingkey")    == 0) return PROP_ROUTINGKEY;
-            if (str.compare("passive")       == 0) return PROP_PASSIVE;
             if (str.compare("autodelete")    == 0) return PROP_AUTODELETE;
             if (str.compare("exclusive")     == 0) return PROP_EXCLUSIVE;
             if (str.compare("type")          == 0) return PROP_TYPE;
@@ -221,7 +218,6 @@ namespace acl {
             case PROP_DURABLE:       return "durable";
             case PROP_OWNER:         return "owner";
             case PROP_ROUTINGKEY:    return "routingkey";
-            case PROP_PASSIVE:       return "passive";
             case PROP_AUTODELETE:    return "autodelete";
             case PROP_EXCLUSIVE:     return "exclusive";
             case PROP_TYPE:          return "type";
@@ -241,7 +237,6 @@ namespace acl {
             if (str.compare("durable")       == 0) return SPECPROP_DURABLE;
             if (str.compare("owner")         == 0) return SPECPROP_OWNER;
             if (str.compare("routingkey")    == 0) return SPECPROP_ROUTINGKEY;
-            if (str.compare("passive")       == 0) return SPECPROP_PASSIVE;
             if (str.compare("autodelete")    == 0) return SPECPROP_AUTODELETE;
             if (str.compare("exclusive")     == 0) return SPECPROP_EXCLUSIVE;
             if (str.compare("type")          == 0) return SPECPROP_TYPE;
@@ -265,7 +260,6 @@ namespace acl {
                 case SPECPROP_DURABLE:       return "durable";
                 case SPECPROP_OWNER:         return "owner";
                 case SPECPROP_ROUTINGKEY:    return "routingkey";
-                case SPECPROP_PASSIVE:       return "passive";
                 case SPECPROP_AUTODELETE:    return "autodelete";
                 case SPECPROP_EXCLUSIVE:     return "exclusive";
                 case SPECPROP_TYPE:          return "type";
@@ -326,7 +320,6 @@ namespace acl {
             propSetPtr p1(new propSet);
             p1->insert(PROP_TYPE);
             p1->insert(PROP_ALTERNATE);
-            p1->insert(PROP_PASSIVE);
             p1->insert(PROP_DURABLE);
 
             propSetPtr p2(new propSet);
@@ -351,7 +344,6 @@ namespace acl {
 
             propSetPtr p4(new propSet);
             p4->insert(PROP_ALTERNATE);
-            p4->insert(PROP_PASSIVE);
             p4->insert(PROP_DURABLE);
             p4->insert(PROP_EXCLUSIVE);
             p4->insert(PROP_AUTODELETE);
diff --git a/qpid/cpp/src/qpid/broker/Broker.cpp b/qpid/cpp/src/qpid/broker/Broker.cpp
index 02111b4387..726b47c268 100644
--- a/qpid/cpp/src/qpid/broker/Broker.cpp
+++ b/qpid/cpp/src/qpid/broker/Broker.cpp
@@ -883,7 +883,6 @@ std::pair<boost::shared_ptr<Queue>, bool> Broker::createQueue(
     if (acl) {
         std::map<acl::Property, std::string> params;
         params.insert(make_pair(acl::PROP_ALTERNATE, alternateExchange));
-        params.insert(make_pair(acl::PROP_PASSIVE, _FALSE));
         params.insert(make_pair(acl::PROP_DURABLE, durable ? _TRUE : _FALSE));
         params.insert(make_pair(acl::PROP_EXCLUSIVE, owner ? _TRUE : _FALSE));
         params.insert(make_pair(acl::PROP_AUTODELETE, autodelete ? _TRUE : _FALSE));
@@ -954,7 +953,6 @@ std::pair<Exchange::shared_ptr, bool> Broker::createExchange(
         std::map<acl::Property, std::string> params;
         params.insert(make_pair(acl::PROP_TYPE, type));
         params.insert(make_pair(acl::PROP_ALTERNATE, alternateExchange));
-        params.insert(make_pair(acl::PROP_PASSIVE, _FALSE));
         params.insert(make_pair(acl::PROP_DURABLE, durable ? _TRUE : _FALSE));
         if (!acl->authorise(userId,acl::ACT_CREATE,acl::OBJ_EXCHANGE,name,&params) )
             throw framing::UnauthorizedAccessException(QPID_MSG("ACL denied exchange create request from " << userId));
diff --git a/qpid/cpp/src/qpid/broker/SessionAdapter.cpp b/qpid/cpp/src/qpid/broker/SessionAdapter.cpp
index 69fba58353..78f2e43ce0 100644
--- a/qpid/cpp/src/qpid/broker/SessionAdapter.cpp
+++ b/qpid/cpp/src/qpid/broker/SessionAdapter.cpp
@@ -74,18 +74,12 @@ void SessionAdapter::ExchangeHandlerImpl::declare(const string& exchange, const
     if(passive){
         AclModule* acl = getBroker().getAcl();
         if (acl) {
-            //TODO: why does a passive declare require create
-            //permission? The purpose of the passive flag is to state
-            //that the exchange should *not* created. For
-            //authorisation a passive declare is similar to
-            //exchange-query.
             std::map<acl::Property, std::string> params;
             params.insert(make_pair(acl::PROP_TYPE, type));
             params.insert(make_pair(acl::PROP_ALTERNATE, alternateExchange));
-            params.insert(make_pair(acl::PROP_PASSIVE, _TRUE));
             params.insert(make_pair(acl::PROP_DURABLE, durable ? _TRUE : _FALSE));
-            if (!acl->authorise(getConnection().getUserId(),acl::ACT_CREATE,acl::OBJ_EXCHANGE,exchange,&params) )
-                throw framing::UnauthorizedAccessException(QPID_MSG("ACL denied exchange create request from " << getConnection().getUserId()));
+            if (!acl->authorise(getConnection().getUserId(),acl::ACT_ACCESS,acl::OBJ_EXCHANGE,exchange,&params) )
+                throw framing::UnauthorizedAccessException(QPID_MSG("ACL denied exchange access request from " << getConnection().getUserId()));
         }
         Exchange::shared_ptr actual(getBroker().getExchanges().get(exchange));
         checkType(actual, type);
@@ -275,22 +269,16 @@ void SessionAdapter::QueueHandlerImpl::declare(const string& name, const string&
     if (passive && !name.empty()) {
         AclModule* acl = getBroker().getAcl();
         if (acl) {
-            //TODO: why does a passive declare require create
-            //permission? The purpose of the passive flag is to state
-            //that the queue should *not* created. For
-            //authorisation a passive declare is similar to
-            //queue-query (or indeed a qmf query).
             std::map<acl::Property, std::string> params;
             params.insert(make_pair(acl::PROP_ALTERNATE, alternateExchange));
-            params.insert(make_pair(acl::PROP_PASSIVE, _TRUE));
             params.insert(make_pair(acl::PROP_DURABLE, std::string(durable ? _TRUE : _FALSE)));
             params.insert(make_pair(acl::PROP_EXCLUSIVE, std::string(exclusive ? _TRUE : _FALSE)));
             params.insert(make_pair(acl::PROP_AUTODELETE, std::string(autoDelete ? _TRUE : _FALSE)));
             params.insert(make_pair(acl::PROP_POLICYTYPE, arguments.getAsString("qpid.policy_type")));
             params.insert(make_pair(acl::PROP_MAXQUEUECOUNT, boost::lexical_cast<string>(arguments.getAsInt("qpid.max_count"))));
             params.insert(make_pair(acl::PROP_MAXQUEUESIZE, boost::lexical_cast<string>(arguments.getAsInt64("qpid.max_size"))));
-            if (!acl->authorise(getConnection().getUserId(),acl::ACT_CREATE,acl::OBJ_QUEUE,name,&params) )
-                throw UnauthorizedAccessException(QPID_MSG("ACL denied queue create request from " << getConnection().getUserId()));
+            if (!acl->authorise(getConnection().getUserId(),acl::ACT_ACCESS,acl::OBJ_QUEUE,name,&params) )
+                throw UnauthorizedAccessException(QPID_MSG("ACL denied queue access request from " << getConnection().getUserId()));
         }
         queue = getQueue(name);
         //TODO: check alternate-exchange is as expected
diff --git a/qpid/cpp/src/tests/acl.py b/qpid/cpp/src/tests/acl.py
index 8f14a0332f..63b21059cf 100755
--- a/qpid/cpp/src/tests/acl.py
+++ b/qpid/cpp/src/tests/acl.py
@@ -509,7 +509,8 @@ class ACLTests(TestBase010):
         Test cases for queue acl in allow mode
         """
         aclf = self.get_acl_file()
-        aclf.write('acl deny bob@QPID create queue name=q1 durable=true passive=true\n')
+        aclf.write('acl deny bob@QPID access queue name=q1\n')
+        aclf.write('acl deny bob@QPID create queue name=q1 durable=true\n')
         aclf.write('acl deny bob@QPID create queue name=q2 exclusive=true policytype=ring\n')
         aclf.write('acl deny bob@QPID access queue name=q3\n')
         aclf.write('acl deny bob@QPID purge queue name=q3\n')
@@ -525,8 +526,15 @@ class ACLTests(TestBase010):
         session = self.get_session('bob','bob')
 
         try:
+            session.queue_declare(queue="q1", durable=True)
+            self.fail("ACL should deny queue create request with name=q1 durable=true");
+        except qpid.session.SessionException, e:
+            self.assertEqual(403,e.args[0].error_code)
+            session = self.get_session('bob','bob')
+
+        try:
             session.queue_declare(queue="q1", durable=True, passive=True)
-            self.fail("ACL should deny queue create request with name=q1 durable=true passive=true");
+            self.fail("ACL should deny queue passive declare request with name=q1 durable=true");
         except qpid.session.SessionException, e:
             self.assertEqual(403,e.args[0].error_code)
             session = self.get_session('bob','bob')
@@ -612,7 +620,8 @@ class ACLTests(TestBase010):
         Test cases for queue acl in deny mode
         """
         aclf = self.get_acl_file()
-        aclf.write('acl allow bob@QPID create queue name=q1 durable=true passive=true\n')
+        aclf.write('acl allow bob@QPID access queue name=q1\n')
+        aclf.write('acl allow bob@QPID create queue name=q1 durable=true\n')
         aclf.write('acl allow bob@QPID create queue name=q2 exclusive=true policytype=ring\n')
         aclf.write('acl allow bob@QPID access queue name=q3\n')
         aclf.write('acl allow bob@QPID purge queue name=q3\n')
@@ -632,10 +641,16 @@ class ACLTests(TestBase010):
         session = self.get_session('bob','bob')
 
         try:
+            session.queue_declare(queue="q1", durable=True)
+        except qpid.session.SessionException, e:
+            if (403 == e.args[0].error_code):
+                self.fail("ACL should allow queue create request with name=q1 durable=true");
+
+        try:
             session.queue_declare(queue="q1", durable=True, passive=True)
         except qpid.session.SessionException, e:
             if (403 == e.args[0].error_code):
-                self.fail("ACL should allow queue create request with name=q1 durable=true passive=true");
+                self.fail("ACL should allow queue passive declare request with name=q1 durable=true passive=true");
 
         try:
             session.queue_declare(queue="q1", durable=False, passive=False)
@@ -785,7 +800,8 @@ class ACLTests(TestBase010):
         Test cases for exchange acl in allow mode
         """
         aclf = self.get_acl_file()
-        aclf.write('acl deny bob@QPID create exchange name=testEx durable=true passive=true\n')
+        aclf.write('acl deny bob@QPID access exchange name=testEx\n')
+        aclf.write('acl deny bob@QPID create exchange name=testEx durable=true\n')
         aclf.write('acl deny bob@QPID create exchange name=ex1 type=direct\n')
         aclf.write('acl deny bob@QPID access exchange name=myEx queuename=q1 routingkey=rk1.*\n')
         aclf.write('acl deny bob@QPID bind exchange name=myEx queuename=q1 routingkey=rk1\n')
@@ -804,18 +820,25 @@ class ACLTests(TestBase010):
         session.exchange_declare(exchange='myEx', type='direct')
 
         try:
+            session.exchange_declare(exchange='testEx', durable=True)
+            self.fail("ACL should deny exchange create request with name=testEx durable=true");
+        except qpid.session.SessionException, e:
+            self.assertEqual(403,e.args[0].error_code)
+            session = self.get_session('bob','bob')
+
+        try:
             session.exchange_declare(exchange='testEx', durable=True, passive=True)
-            self.fail("ACL should deny exchange create request with name=testEx durable=true passive=true");
+            self.fail("ACL should deny passive exchange declare request with name=testEx durable=true passive=true");
         except qpid.session.SessionException, e:
             self.assertEqual(403,e.args[0].error_code)
             session = self.get_session('bob','bob')
 
         try:
-            session.exchange_declare(exchange='testEx', type='direct', durable=True, passive=False)
+            session.exchange_declare(exchange='testEx', type='direct', durable=False)
         except qpid.session.SessionException, e:
             print e
             if (403 == e.args[0].error_code):
-                self.fail("ACL should allow exchange create request for testEx with any parameter other than durable=true and passive=true");
+                self.fail("ACL should allow exchange create request for testEx with any parameter other than durable=true");
 
         try:
             session.exchange_declare(exchange='ex1', type='direct')
@@ -916,7 +939,7 @@ class ACLTests(TestBase010):
         Test cases for exchange acl in deny mode
         """
         aclf = self.get_acl_file()
-        aclf.write('acl allow bob@QPID create exchange name=myEx durable=true passive=false\n')
+        aclf.write('acl allow bob@QPID create exchange name=myEx durable=true\n')
         aclf.write('acl allow bob@QPID bind exchange name=amq.topic queuename=bar routingkey=foo.*\n')
         aclf.write('acl allow bob@QPID unbind exchange name=amq.topic queuename=bar routingkey=foo.*\n')
         aclf.write('acl allow bob@QPID access exchange name=myEx queuename=q1 routingkey=rk1.*\n')