QPID-3799 ACL processing - Don't delete rules, keep them all.

Keep original rule allow/deny setting with rule. Get rid of log/logonly bools.



git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/QPID-3799-acl@1291467 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 106 insertions, 165 deletions

diff --git a/cpp/src/qpid/acl/AclData.cpp b/cpp/src/qpid/acl/AclData.cpp
index 06fc223a73..ec004bb785 100644
--- a/cpp/src/qpid/acl/AclData.cpp
+++ b/cpp/src/qpid/acl/AclData.cpp
@@ -29,7 +29,6 @@ namespace acl {
         for (unsigned int cnt=0; cnt< qpid::acl::ACTIONSIZE; cnt++){
             actionList[cnt]=0;
         }
-
     }
 
     void AclData::clear ()
@@ -41,7 +40,6 @@ namespace acl {
             }
             delete[] actionList[cnt];
         }
-
     }
 
     bool AclData::matchProp(const std::string & src, const std::string& src1)
@@ -58,8 +56,8 @@ namespace acl {
         const std::string& name, std::map<Property, std::string>* params) {
 
             QPID_LOG(debug, "ACL: Lookup for id:" << id << " action:" << AclHelper::getActionStr((Action) action)
-                << " objectType:" << AclHelper::getObjectTypeStr((ObjectType) objType) << " name:" << name
-                << " with params " << AclHelper::propertyMapToString(params));
+                     << " objectType:" << AclHelper::getObjectTypeStr((ObjectType) objType) << " name:" << name
+                     << " with params " << AclHelper::propertyMapToString(params));
 
             AclResult aclresult = decisionMode;
             if (actionList[action] && actionList[action][objType]) {
@@ -152,7 +150,7 @@ namespace acl {
                         }
                         if (match)
                         {
-                            aclresult = getACLResult(i->logOnly, i->log);
+                            aclresult = i->ruleMode;
                             QPID_LOG(debug,"Successful match, the decision is:" << AclHelper::getAclResultStr(aclresult));
                             return aclresult;
                         }
@@ -214,7 +212,7 @@ namespace acl {
                         }
                     }
                     if (match){
-                        aclresult = getACLResult(i->logOnly, i->log);
+                        aclresult = i->ruleMode;
                         QPID_LOG(debug,"Successful match, the decision is:" << AclHelper::getAclResultStr(aclresult));
                         return aclresult;
                     }
@@ -227,32 +225,6 @@ namespace acl {
     }
 
 
-    AclResult AclData::getACLResult(bool logOnly, bool log)
-    {
-        switch (decisionMode)
-        {
-        case qpid::acl::ALLOWLOG:
-        case qpid::acl::ALLOW:
-            if (logOnly) return qpid::acl::ALLOWLOG;
-            if (log)
-                return qpid::acl::DENYLOG;
-            else
-                return qpid::acl::DENY;
-
-
-        case qpid::acl::DENYLOG:
-        case qpid::acl::DENY:
-            if (logOnly) return qpid::acl::DENYLOG;
-            if (log)
-                return qpid::acl::ALLOWLOG;
-            else
-                return qpid::acl::ALLOW;
-        }
-
-        QPID_LOG(error, "ACL Decision Failed, setting DENY");
-        return qpid::acl::DENY;
-    }
-
     AclData::~AclData()
     {
         clear();
diff --git a/cpp/src/qpid/acl/AclData.h b/cpp/src/qpid/acl/AclData.h
index 81125fdcbc..5f0c55514d 100644
--- a/cpp/src/qpid/acl/AclData.h
+++ b/cpp/src/qpid/acl/AclData.h
@@ -33,22 +33,24 @@ class AclData {
 public:
 
     typedef std::map<qpid::acl::Property, std::string> propertyMap;
-    typedef propertyMap::const_iterator propertyMapItr;
-    struct rule {
+    typedef propertyMap::const_iterator                propertyMapItr;
 
-        bool log;
-        bool logOnly;  // this is a rule is to log only
+    struct rule {
 
-        // key value map
-        //??
-        propertyMap props;
+        int                   rawRuleNum;   // rule number in ACL file
+        qpid::acl::AclResult  ruleMode;     // combined allow/deny log/nolog
+        propertyMap           props;
 
 
-        rule (propertyMap& p):log(false),logOnly(false),props(p) {};
+        rule (int ruleNum, qpid::acl::AclResult res, propertyMap& p) :
+            rawRuleNum(ruleNum),
+            ruleMode(res),
+            props(p)
+            {};
 
         std::string toString () const {
             std::ostringstream ruleStr;
-            ruleStr << "[log=" << log << ", logOnly=" << logOnly << " props{";
+            ruleStr << "[ruleMode = " << AclHelper::getAclResultStr(ruleMode) << " props{";
             for (propertyMapItr pMItr = props.begin(); pMItr != props.end(); pMItr++) {
                 ruleStr << " " << AclHelper::getPropertyStr((Property) pMItr-> first) << "=" << pMItr->second;
             }
@@ -56,17 +58,17 @@ public:
             return ruleStr.str();
         }
     };
-    typedef  std::vector<rule> ruleSet;
-    typedef  ruleSet::const_iterator ruleSetItr;
+    typedef  std::vector<rule>               ruleSet;
+    typedef  ruleSet::const_iterator         ruleSetItr;
     typedef  std::map<std::string, ruleSet > actionObject; // user 
-    typedef  actionObject::iterator actObjItr;
-    typedef  actionObject* aclAction;
+    typedef  actionObject::iterator          actObjItr;
+    typedef  actionObject*                   aclAction;
 
     // Action*[] -> Object*[] -> map<user -> set<Rule> >
-    aclAction* actionList[qpid::acl::ACTIONSIZE];
-    qpid::acl::AclResult decisionMode;  // determines if the rule set is a deny or allow mode. 
-    bool transferAcl;
-    std::string aclSource; 
+    aclAction*           actionList[qpid::acl::ACTIONSIZE];
+    qpid::acl::AclResult decisionMode;  // allow/deny[-log] if no matching rule found
+    bool                 transferAcl;
+    std::string          aclSource; 
 
     AclResult lookup(const std::string& id, const Action& action, const ObjectType& objType, const std::string& name, std::map<Property, std::string>* params=0);
     AclResult lookup(const std::string& id, const Action& action, const ObjectType& objType, const std::string& ExchangeName, const std::string& RoutingKey);
diff --git a/cpp/src/qpid/acl/AclReader.cpp b/cpp/src/qpid/acl/AclReader.cpp
index 74358a20c1..60e45cfdc2 100644
--- a/cpp/src/qpid/acl/AclReader.cpp
+++ b/cpp/src/qpid/acl/AclReader.cpp
@@ -86,13 +86,13 @@ namespace acl {
     void AclReader::loadDecisionData(boost::shared_ptr<AclData> d) {
         d->clear();
         QPID_LOG(debug, "ACL Load Rules");
-        int cnt = rules.size();
         bool foundmode = false;
 
-        for (rlCitr i = rules.end(); cnt; cnt--) {
+        rlCitr i = rules.end();
+        for (int cnt = rules.size(); cnt; cnt--) {
             i--;
             QPID_LOG(debug, "ACL Processing " << std::setfill(' ') << std::setw(2)
-                << cnt << " " << (*i)->toString());
+                << cnt - 1 << " " << (*i)->toString());
 
             if (!foundmode && (*i)->actionAll && (*i)->names.size() == 1
                 && (*((*i)->names.begin())).compare("*") == 0) {
@@ -101,126 +101,93 @@ namespace acl {
                         << AclHelper::getAclResultStr(d->decisionMode));
                     foundmode = true;
             } else {
-                AclData::rule rule((*i)->props);
-                bool addrule = true;
-
-                switch ((*i)->res) {
-                case qpid::acl::ALLOWLOG:
-                    rule.log = true;
-                    if (d->decisionMode == qpid::acl::ALLOW ||
-                        d->decisionMode == qpid::acl::ALLOWLOG)
-                        rule.logOnly = true;
-                    break;
-                case qpid::acl::ALLOW:
-                    if (d->decisionMode == qpid::acl::ALLOW ||
-                        d->decisionMode == qpid::acl::ALLOWLOG)
-                        addrule = false;
-                    break;
-                case qpid::acl::DENYLOG:
-                    rule.log = true;
-                    if (d->decisionMode == qpid::acl::DENY ||
-                        d->decisionMode == qpid::acl::DENYLOG)
-                        rule.logOnly = true;
-                    break;
-                case qpid::acl::DENY:
-                    if (d->decisionMode == qpid::acl::DENY ||
-                        d->decisionMode == qpid::acl::DENYLOG)
-                        addrule = false;
-                    break;
-                default:
-                    throw Exception("Invalid ACL Result loading rules.");
-                }
+                AclData::rule rule(cnt - 1, (*i)->res, (*i)->props);
 
                 // Action -> Object -> map<user -> set<Rule> >
-                if (addrule) {
-                    std::ostringstream actionstr;
-                    for (int acnt = ((*i)->actionAll ? 0 : (*i)->action);
-                        acnt < acl::ACTIONSIZE;
-                        (*i)->actionAll ? acnt++ : acnt = acl::ACTIONSIZE) {
-
-                            if (acnt == acl::ACT_PUBLISH)
-                                d->transferAcl = true; // we have transfer ACL
-
-                            actionstr << AclHelper::getActionStr((Action) acnt) << ",";
-
-                            //find the Action, create if not exist
-                            if (d->actionList[acnt] == NULL) {
-                                d->actionList[acnt] =
-                                    new AclData::aclAction[qpid::acl::OBJECTSIZE];
-                                for (int j = 0; j < qpid::acl::OBJECTSIZE; j++)
-                                    d->actionList[acnt][j] = NULL;
-                            }
+                std::ostringstream actionstr;
+                for (int acnt = ((*i)->actionAll ? 0 : (*i)->action);
+                    acnt < acl::ACTIONSIZE;
+                    (*i)->actionAll ? acnt++ : acnt = acl::ACTIONSIZE) {
+
+                        if (acnt == acl::ACT_PUBLISH)
+                            d->transferAcl = true; // we have transfer ACL
+
+                        actionstr << AclHelper::getActionStr((Action) acnt) << ",";
+
+                        //find the Action, create if not exist
+                        if (d->actionList[acnt] == NULL) {
+                            d->actionList[acnt] =
+                                new AclData::aclAction[qpid::acl::OBJECTSIZE];
+                            for (int j = 0; j < qpid::acl::OBJECTSIZE; j++)
+                                d->actionList[acnt][j] = NULL;
+                        }
+
+                        // TODO: optimize this loop to limit to valid options only!!
+                        for (int ocnt = ((*i)->objStatus != aclRule::VALUE ? 0
+                            : (*i)->object);
+                            ocnt < acl::OBJECTSIZE;
+                        (*i)->objStatus != aclRule::VALUE ? ocnt++ : ocnt = acl::OBJECTSIZE) {
 
-                            // optimize this loop to limit to valid options only!!
-                            for (int ocnt = ((*i)->objStatus != aclRule::VALUE ? 0
-                                : (*i)->object);
-                                ocnt < acl::OBJECTSIZE;
-                            (*i)->objStatus != aclRule::VALUE ? ocnt++ : ocnt = acl::OBJECTSIZE) {
-
-                                //find the Object, create if not exist
-                                if (d->actionList[acnt][ocnt] == NULL)
-                                    d->actionList[acnt][ocnt] =
-                                    new AclData::actionObject;
-
-                                // add users and Rule to object set
-                                bool allNames = false;
-                                // check to see if names.begin is '*'
-                                if ((*(*i)->names.begin()).compare("*") == 0)
-                                    allNames = true;
-
-                                for (nsCitr itr = (allNames ? names.begin()
-                                    : (*i)->names.begin());
-                                    itr != (allNames ? names.end() : (*i)->names.end());
-                                itr++) {
-
-                                    AclData::actObjItr itrRule =
-                                        d->actionList[acnt][ocnt]->find(*itr);
-
-                                    if (itrRule == d->actionList[acnt][ocnt]->end()) {
-                                        AclData::ruleSet rSet;
-                                        rSet.push_back(rule);
-                                        d->actionList[acnt][ocnt]->insert
-                                            (make_pair(std::string(*itr), rSet));
-                                    } else {
-                                        // TODO add code to check for dead rules
-                                        // allow peter create queue name=tmp <-- dead rule!!
-                                        // allow peter create queue
-
-                                        itrRule->second.push_back(rule);
-                                    }
+                            //find the Object, create if not exist
+                            if (d->actionList[acnt][ocnt] == NULL)
+                                d->actionList[acnt][ocnt] =
+                                new AclData::actionObject;
+
+                            // add users and Rule to object set
+                            bool allNames = false;
+                            // check to see if names.begin is '*'
+                            if ((*(*i)->names.begin()).compare("*") == 0)
+                                allNames = true;
+
+                            for (nsCitr itr = (allNames ? names.begin()
+                                : (*i)->names.begin());
+                                itr != (allNames ? names.end() : (*i)->names.end());
+                            itr++) {
+
+                                AclData::actObjItr itrRule =
+                                    d->actionList[acnt][ocnt]->find(*itr);
+
+                                if (itrRule == d->actionList[acnt][ocnt]->end()) {
+                                    AclData::ruleSet rSet;
+                                    rSet.push_back(rule);
+                                    d->actionList[acnt][ocnt]->insert
+                                        (make_pair(std::string(*itr), rSet));
+                                } else {
+                                    // TODO add code to check for dead rules
+                                    // allow peter create queue name=tmp <-- dead rule!!
+                                    // allow peter create queue
+
+                                    itrRule->second.push_back(rule);
                                 }
-
                             }
-                    }
 
-                    std::ostringstream objstr;
-                    for (int ocnt = ((*i)->objStatus != aclRule::VALUE ? 0 : (*i)->object);
-                        ocnt < acl::OBJECTSIZE;
-                        (*i)->objStatus != aclRule::VALUE ? ocnt++ : ocnt = acl::OBJECTSIZE) {
-                            objstr << AclHelper::getObjectTypeStr((ObjectType) ocnt) << ",";
-                    }
-
-                    bool allNames = ((*(*i)->names.begin()).compare("*") == 0);
-                    std::ostringstream userstr;
-                    for (nsCitr itr = (allNames ? names.begin() : (*i)->names.begin());
-                        itr != (allNames ? names.end() : (*i)->names.end());
-                        itr++) {
-                            userstr << *itr << ",";
-                    }
-
-                    QPID_LOG(debug, "ACL: Adding actions {" <<
-                        actionstr.str().substr(0,actionstr.str().length()-1)
-                        << "} to objects {" <<
-                        objstr.str().substr(0,objstr.str().length()-1)
-                        << "} with props " <<
-                        AclHelper::propertyMapToString(&rule.props)
-                        << " for users {" <<
-                        userstr.str().substr(0,userstr.str().length()-1)
-                        << "}" );
-                } else {
-                    QPID_LOG(debug, "ACL Skipping based on Mode:"
-                        << AclHelper::getAclResultStr(d->decisionMode));
+                        }
                 }
+
+                std::ostringstream objstr;
+                for (int ocnt = ((*i)->objStatus != aclRule::VALUE ? 0 : (*i)->object);
+                    ocnt < acl::OBJECTSIZE;
+                    (*i)->objStatus != aclRule::VALUE ? ocnt++ : ocnt = acl::OBJECTSIZE) {
+                        objstr << AclHelper::getObjectTypeStr((ObjectType) ocnt) << ",";
+                }
+
+                bool allNames = ((*(*i)->names.begin()).compare("*") == 0);
+                std::ostringstream userstr;
+                for (nsCitr itr = (allNames ? names.begin() : (*i)->names.begin());
+                    itr != (allNames ? names.end() : (*i)->names.end());
+                    itr++) {
+                        userstr << *itr << ",";
+                }
+
+                QPID_LOG(debug, "ACL: Adding actions {" <<
+                    actionstr.str().substr(0,actionstr.str().length()-1)
+                    << "} to objects {" <<
+                    objstr.str().substr(0,objstr.str().length()-1)
+                    << "} with props " <<
+                    AclHelper::propertyMapToString(&rule.props)
+                    << " for users {" <<
+                    userstr.str().substr(0,userstr.str().length()-1)
+                    << "}" );
             }
 
         }