Description

HelpOnAccessControlLists tells us that

• #acl +All:read -SomeUser:admin SomeGroup:read,write,admin

  +All:read means that all users are given read access at that point, and processing continues. -SomeUser:admin removes admin access for SomeUser, and processing continues.

Sounds great, but the code shows that processing does not continue if a right is really found in the rights dict. So either the code or the description is wrong.

Yes. Maybe the description can be misunderstood. But otoh there is no need to continue if you have already found what you are looking for. So this works as intended, no bug.

Example

First we give All users read and admin with '+' modifier. So now the system should know all users have read and admin initially, but it should continue processing. Now we add a rule that removes '-' the admin right from SomeUser. That should leave him with one read access. Wrong: he has admin, cause the '+' in the All rule will not hinder the system to abort processing. True. But it is designed to work like that. See above.

Add this to test_wikiacl.py:

   1     def testApplyModifyACLByUser(self):
   2         """wikiacl: applying modifier acl by user name"""
   3         assert self.request.cfg.acl_enabled
   4 
   5         # This acl string...
   6         acl_rights = [
   7             "+All:read,admin -SomeUser:admin SomeGroup:read,write,admin"
   8             ]
   9         acl = wikiacl.AccessControlList(self.request, acl_rights)
  10 
  11         # Should apply these rights:
  12         users = (
  13             # user,                 rights
  14             # CamelCase names
  15             ('SomeUser',    ('read',)),
  16             ('SomeGroup',   ('read', 'write', 'admin')),
  17             ('All',         ('read','admin')),
  18             )
  19 
  20         # Check rights
  21         for user, may in users:
  22             mayNot = [right for right in self.request.cfg.acl_rights_valid
  23                       if right not in may]
  24             # User should have these rights...
  25             for right in may:
  26                 self.assert_(acl.may(self.request, user, right),
  27                     '"%(user)s" should be allowed to "%(right)s"' % locals())
  28             # But NOT these:
  29             for right in mayNot:
  30                 self.failIf(acl.may(self.request, user, right),
  31                     '"%(user)s" should NOT be allowed to "%(right)s"' % locals())

Details

This Wiki.

Workaround

Discussion

Fix the documentation or fix the implementation.

The rightsdict needs to be extended, so each right has a flag if it is terminal or if processing should continue.

It seems that it was just a doc error...

The problems is you read the docs the code does not say anything about "continuing" processing. It very clear that - and + are just a way to add a single rule to the rightsdict.

We should fix the docs, unless we want different behavior.

Plan

• Priority:
• Assigned to:
• Status: docs in moinmaster have been fixed

CategoryMoinMoinBugFixed