Author: chug
Date: Fri Jun 1 14:19:34 2012
New Revision: 1345190
URL: http://svn.apache.org/viewvc?rev=1345190&view=rev
Log:
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.
Modified:
qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.cpp
qpid/trunk/qpid/cpp/src/tests/acl.py
Modified: qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.cpp
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.cpp?rev=1345190&r1=1345189&r2=1345190&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.cpp Fri Jun 1 14:19:34 2012
@@ -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 {
Modified: qpid/trunk/qpid/cpp/src/tests/acl.py
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/tests/acl.py?rev=1345190&r1=1345189&r2=1345190&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/tests/acl.py (original)
+++ qpid/trunk/qpid/cpp/src/tests/acl.py Fri Jun 1 14:19:34 2012
@@ -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):
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org
For additional commands, e-mail: commits-h...@qpid.apache.org
