Ok, it looks to me that this was broken way back before 0.6 was branched.
Specifically, it was broken in two separate commits (either of which were enough to break it) - the two commits are:
- r728880 "This is a fix for
- it seems that this commit tried to fix an issue where an ACL group contained a recursive sub-group, eg: group admin admin however, that solution was incorrect. It actually turned an "if" condition into one that can never evaluate to true. It incorrectly compares the name of a found group, with the very same name used to find that group, and insists that they don't match. It seems that what they meant to do, was compare the name of the found group with name of the group being added to, however the latter group name is not available within the AclReader::addName function.
- r732466 "This is related to
QPID-1558. The test case test_group_and_user_with_same_name covers the error condition in QPID-1545"
- this commit was intended to "Reject ACL file if user names does not contain a realm". However, it failed to allow for sub-group names (as documented as valid in the wiki), and so now sub-groups result in the current error message (Username '<group>' must contain a realm).
The patch I'll attach in a minute does two things:
- Revert r728880 - the AclReader::addName can't do what that commit wanted without extended the function signature to either include the name that the groupNameSet parameter came from, or upgrade the groupNameSet argument from a nameSetPtr to the gmCitr from which it was derived. My patch (in step 2 below) performs equivalent work elsewhere.
- Add checks to the AclReader::processGroupLine for valid sub-group names, and if found, defends against:
- recursive sub-groups (eg: group admin admin) in which case a debug message is included, but otherwise the recursion is safely ignored.
- non-existent subgroups (eg: group admin this-group-does-not-exist-yet) in which case an error is returned, and the ACL file rejected.
I believe that my patch restores the original sub-group capability as described at https://cwiki.apache.org/qpid/acl.html, while protecting against the issues raised in both
QPID-1545 and QPID-1558.
Let me know what you think