QPID-4032 Broker ACL does not accept sub-groups in group declaration

Patch from Paul Colby and new self test demonstrating the fix.

Note that this patch broke the user_realm self test. That is, a naked name
like 'bob' has changed from being a username missing a domain to being
a group name. The self test used to fail and still fails but now for a 
different reason.


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

2 files changed, 52 insertions, 4 deletions

diff --git a/qpid/cpp/src/qpid/acl/AclReader.cpp b/qpid/cpp/src/qpid/acl/AclReader.cpp
index 80debf1bd1..0830f3f4f4 100644
--- a/qpid/cpp/src/qpid/acl/AclReader.cpp
+++ b/qpid/cpp/src/qpid/acl/AclReader.cpp
@@ -314,7 +314,17 @@ namespace acl {
         if (contFlag) {
             gmCitr citr = groups.find(groupName);
             for (unsigned i = 0; i < toksSize; i++) {
-                if (!isValidUserName(toks[i])) return false;
+                if (isValidGroupName(toks[i])) {
+                    if (toks[i] == groupName) {
+                        QPID_LOG(debug, "ACL: Line: " << lineNumber
+                            << ", Ignoring recursive sub-group \"" << toks[i] << "\".");
+                        continue;
+                    } else if (groups.find(toks[i]) == groups.end()) {
+                        errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " << lineNumber
+                            << ", Sub-group \"" << toks[i] << "\" not defined yet.";
+                        return false;
+                    }
+                } else if (!isValidUserName(toks[i])) return false;
                 addName(toks[i], citr->second);
             }
         } else {
@@ -332,7 +342,17 @@ namespace acl {
             gmCitr citr = addGroup(toks[1]);
             if (citr == groups.end()) return false;
             for (unsigned i = 2; i < toksSize; i++) {
-                if (!isValidUserName(toks[i])) return false;
+                if (isValidGroupName(toks[i])) {
+                    if (toks[i] == groupName) {
+                        QPID_LOG(debug, "ACL: Line: " << lineNumber
+                            << ", Ignoring recursive sub-group \"" << toks[i] << "\".");
+                        continue;
+                    } else if (groups.find(toks[i]) == groups.end()) {
+                        errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " << lineNumber
+                            << ", Sub-group \"" << toks[i] << "\" not defined yet.";
+                        return false;
+                    }
+                } else if (!isValidUserName(toks[i])) return false;
                 addName(toks[i], citr->second);
             }
         }
@@ -356,7 +376,7 @@ namespace acl {
 
     void AclReader::addName(const std::string& name, nameSetPtr groupNameSet) {
         gmCitr citr = groups.find(name);
-        if (citr != groups.end() && citr->first != name){
+        if (citr != groups.end()) {
             // This is a previously defined group: add all the names in that group to this group
             groupNameSet->insert(citr->second->begin(), citr->second->end());
         } else {
diff --git a/qpid/cpp/src/tests/acl.py b/qpid/cpp/src/tests/acl.py
index 720b3b4216..663af3e8ac 100755
--- a/qpid/cpp/src/tests/acl.py
+++ b/qpid/cpp/src/tests/acl.py
@@ -285,10 +285,38 @@ class ACLTests(TestBase010):
         if (result):
             self.fail(result)
 
+    def test_nested_groups(self):
+        """
+        Test nested groups
+        """
+
+        aclf = self.get_acl_file()
+        aclf.write('group user-consume martin@QPID ted@QPID\n')
+        aclf.write('group group2 kim@QPID user-consume rob@QPID \n')
+        aclf.write('acl allow anonymous all all \n')
+        aclf.write('acl allow group2 create queue \n')
+        aclf.write('acl deny all all')
+        aclf.close()
+
+        result = self.reload_acl()
+        if (result):
+            self.fail(result)
+
+        session = self.get_session('rob','rob')
+        try:
+            session.queue_declare(queue="rob_queue")
+        except qpid.session.SessionException, e:
+            if (403 == e.args[0].error_code):
+                self.fail("ACL should allow queue create request");
+            self.fail("Error during queue create request");
+
+        
+
     def test_user_realm(self):
         """
         Test a user defined without a realm
         Ex. group admin rajith
+        Note: a user name without a realm is interpreted as a group name
         """
         aclf = self.get_acl_file()
         aclf.write('group admin bob\n') # shouldn't be allowed
@@ -297,7 +325,7 @@ class ACLTests(TestBase010):
         aclf.close()
 
         result = self.reload_acl()
-        if (result.find("Username 'bob' must contain a realm",0,len(result)) == -1):
+        if (result.find("not defined yet.",0,len(result)) == -1):
             self.fail(result)
 
     def test_allowed_chars_for_username(self):