Merge b8acc4a into 9543c8b

CHANGELOG.rst

@@ -1,7 +1,8 @@
 2.3.34 (unreleased)
 -------------------
 
-- Nothing changed yet.
+- Deny permissions take precedence over allow permissions on content
+  [vangheem]
 
 
 2.3.33 (2018-03-03)

guillotina/security/policy.py

@@ -197,10 +197,18 @@ def cached_decision(self, parent, principal, groups, permission):
             # Get the roles from the user
             prin_roles = self.cached_principal_roles(
                 parent, principal, groups, 'o')
+            found = False
             for role, setting in prin_roles.items():
                 if setting and (role in roles):
-                    cache_decision_prin[permission] = decision = True
-                    return decision
+                    cache_decision_prin[permission] = decision = bool(roles[role])
+                    found = True
+                    if roles[role] == 0:
+                        # if it is deny, dive out immediately
+                        # otherwise, it is allow and we need to make sure there
+                        # are no deny roles here
+                        return decision
+            if found:
+                return decision
 
         cache_decision_prin[permission] = decision = False
         return decision
@@ -365,8 +373,14 @@ def cached_roles(self, parent, permission, level):
                     roles[role] = 1
                 elif setting is AllowSingle and level == 'o':
                     roles[role] = 1
-                elif setting is Deny and role in roles:
-                    del roles[role]
+                elif setting is Deny:
+                    if role in roles and roles[role] == 1:
+                        # on role that has been allowed already, delete it
+                        del roles[role]
+                    else:
+                        # not already applied on parent so explicitly
+                        # deny access here
+                        roles[role] = 0
 
         if level != 'o':
             # Only cache on non 1rst level queries needs new way

guillotina/tests/test_security.py

@@ -1,3 +1,4 @@
+from guillotina.interfaces import IRolePermissionManager
 from guillotina.security.utils import get_principals_with_access_content
 from guillotina.security.utils import get_roles_with_access_content
 from guillotina.security.utils import settings_for_object
@@ -169,3 +170,14 @@ async def test_canido(container_requester):
             'GET', '/db/guillotina/@canido?permission=guillotina.ViewContent')
         assert status == 200
         assert response
+
+
+async def test_deny_on_object_permissions(dummy_request):
+    request = dummy_request
+    content = utils.create_content()
+    utils.login(request)
+    request.security.participations[0].principal.roles['guillotina.Editor'] = 1
+    assert request.security.check_permission('guillotina.AddContent', content)
+    manager = IRolePermissionManager(content)
+    manager.deny_permission_to_role('guillotina.AddContent', 'guillotina.Editor')
+    assert not request.security.check_permission('guillotina.AddContent', content)