changeset 1770:c64d3d8d16cb

security fix: fix virtual group bug in ACL evaluation, add a test for it Note: same issue has been fixed in moin/1.9 repo also, see cs 7b9f39289e16. This changeset was just ported to moin/2.0. Issue description: We have code that checks whether a group has special members "All" or "Known" or "Trusted", but there was a bug that checked whether these are present in the group NAME (not, as intended, in the group MEMBERS). a) If you have group MEMBERS like "All" or "Known" or "Trusted", they did not work until now, but will start working with this changeset. E.g. SomeGroup: * JoeDoe * Trusted SomeGroup will now (correctly) include JoeDoe and also all trusted users. It (erroneously) contained only "JoeDoe" and "Trusted" (as a username, not as a virtual group) before. b) If you have group NAMES containing "All" or "Known" or "Trusted", they behaved wrong until now (they erroneously included All/Known/Trusted users even if you did not list them as members), but will start working correctly with this changeset. E.g. AllFriendsGroup: * JoeDoe AllFriendsGroup will now (correctly) include only JoeDoe. It (erroneously) contained all users (including JoeDoe) before. E.g. MyTrustedFriendsGroup: * JoeDoe MyTrustedFriendsGroup will now (correctly) include only JoeDoe. It (erroneously) contained all trusted users and JoeDoe before.
author Thomas Waldmann <tw AT waldmann-edv DOT de>
date Mon, 03 Sep 2012 20:05:30 +0200
parents a759603dd5d4
children dbfad38a4cdc
files MoinMoin/security/__init__.py MoinMoin/security/_tests/test_security.py
diffstat 2 files changed, 49 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/security/__init__.py	Sun Sep 02 23:58:06 2012 +0200
+++ b/MoinMoin/security/__init__.py	Mon Sep 03 20:05:30 2012 +0200
@@ -235,11 +235,12 @@
                 handler = getattr(self, "_special_"+entry, None)
                 allowed = handler(name, dowhat, rightsdict)
             elif entry in groups:
-                if name in groups[entry]:
+                this_group = groups[entry]
+                if name in this_group:
                     allowed = rightsdict.get(dowhat)
                 else:
                     for special in self.special_users:
-                        if special in entry:
+                        if special in this_group:
                             handler = getattr(self, "_special_" + special, None)
                             allowed = handler(name, dowhat, rightsdict)
                             break # order of self.special_users is important
--- a/MoinMoin/security/_tests/test_security.py	Sun Sep 02 23:58:06 2012 +0200
+++ b/MoinMoin/security/_tests/test_security.py	Mon Sep 03 20:05:30 2012 +0200
@@ -16,6 +16,7 @@
 
 from MoinMoin.user import User
 from MoinMoin.config import ACL
+from MoinMoin.datastruct import ConfigGroups
 
 from MoinMoin._tests import update_item
 from MoinMoin._tests import become_trusted
@@ -257,6 +258,51 @@
                 assert not acl.may(user, right)
 
 
+class TestGroupACL(object):
+
+    from MoinMoin._tests import wikiconfig
+    class Config(wikiconfig.Config):
+        def groups(cfg):
+            groups = {
+                u'PGroup': frozenset([u'Antony', u'Beatrice', ]),
+                u'AGroup': frozenset([u'All', ]),
+                # note: the next line is a INTENDED misnomer, there is "All" in
+                # the group NAME, but not in the group members. This makes
+                # sure that a bug that erroneously checked "in groupname" (instead
+                # of "in groupmembers") does not reappear.
+                u'AllGroup': frozenset([]), # note: intended misnomer
+            }
+            return ConfigGroups(groups)
+
+    def testApplyACLByGroup(self):
+        """ security: applying acl by group name"""
+        # This acl string...
+        acl_rights = [
+            "PGroup,AllGroup:read,write,admin "
+            "AGroup:read "
+            ]
+        acl = AccessControlList(acl_rights, valid=app.cfg.acl_rights_contents)
+
+        # Should apply these rights:
+        users = (
+            # user, rights
+            ('Antony', ('read', 'write', 'admin', )),  # in PGroup
+            ('Beatrice', ('read', 'write', 'admin', )),  # in PGroup
+            ('Charles', ('read', )),  # virtually in AGroup
+            )
+
+        # Check rights
+        for user, may in users:
+            mayNot = [right for right in app.cfg.acl_rights_contents
+                      if right not in may]
+            # User should have these rights...
+            for right in may:
+                assert acl.may(user, right)
+            # But NOT these:
+            for right in mayNot:
+                assert not acl.may(user, right)
+
+
 class TestItemAcls(object):
     """ security: real-life access control list on items testing
     """