diff --git a/CHANGES.rst b/CHANGES.rst index edacea0..2bd593e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -8,6 +8,8 @@ For changes before version 3.0, see ``HISTORY.rst``. - Respect ``PURE_PYTHON`` environment variable set to ``0`` when running tests. +- Let the roles access in ``rolesForPermissionOn`` interpret ``AttributeError`` and ``Unauthorized`` as "no roles definition for this permission at this object" and report any other exception (for the Python and C implementation). We have to treat ``Unauthorized`` like ``AttributeError`` to support ``Shared.DC.Scripts.Bindings.UnauthorizedBinding`` which raises ``Unauthorized`` for any access. + 7.0 (2024-05-30) ---------------- diff --git a/src/AccessControl/ImplPython.py b/src/AccessControl/ImplPython.py index 1a7788b..0a9326b 100644 --- a/src/AccessControl/ImplPython.py +++ b/src/AccessControl/ImplPython.py @@ -31,6 +31,7 @@ from Acquisition import aq_parent from ExtensionClass import Base from zope.interface import implementer +from zExceptions import Unauthorized as zExceptions_Unauthorized PURE_PYTHON = int(os.environ.get('PURE_PYTHON', '0')) if PURE_PYTHON: @@ -71,8 +72,11 @@ def rolesForPermissionOn(perm, object, default=_default_roles, n=None): r = None while True: - if hasattr(object, n): + try: roles = getattr(object, n) + except (AttributeError, zExceptions_Unauthorized): + pass + else: if roles is None: if _embed_permission_in_roles: return (('Anonymous',), n) diff --git a/src/AccessControl/cAccessControl.c b/src/AccessControl/cAccessControl.c index 403ed67..bb849fe 100644 --- a/src/AccessControl/cAccessControl.c +++ b/src/AccessControl/cAccessControl.c @@ -652,6 +652,7 @@ static PyExtensionClass imPermissionRoleType = { static PyObject *Containers = NULL; static PyObject *ContainerAssertions = NULL; static PyObject *Unauthorized = NULL; +static PyObject *zExceptions_Unauthorized = NULL; static PyObject *warn= NULL; static PyObject *NoSequenceFormat = NULL; static PyObject *_what_not_even_god_should_do = NULL; @@ -1847,15 +1848,27 @@ c_rolesForPermissionOn(PyObject *perm, PyObject *object, Py_INCREF(r); /* - while 1: + while True: */ while (1) { /* - if hasattr(object, n): + try: roles = getattr(object, n) + except (AttributeError, zExceptions_Unauthorized): + pass + else: */ PyObject *roles = PyObject_GetAttr(object, n); + if (roles == NULL) + { + if (! (PyErr_ExceptionMatches(PyExc_AttributeError) + || PyErr_ExceptionMatches(zExceptions_Unauthorized))) + { + /* re-raise */ + return NULL; + } + } if (roles != NULL) { @@ -2313,6 +2326,7 @@ static struct PyMethodDef dtml_methods[] = { */ #define IMPORT(module, name) if ((module = PyImport_ImportModule(name)) == NULL) return NULL; #define GETATTR(module, name) if ((name = PyObject_GetAttrString(module, #name)) == NULL) return NULL; +#define GETATTR_AS(module, name, as_name) if ((as_name = PyObject_GetAttrString(module, name)) == NULL) return NULL; static struct PyModuleDef moduledef = { @@ -2400,6 +2414,14 @@ module_init(void) { Py_DECREF(tmp); tmp = NULL; + /*| from zExceptions import Unauthorized as zExceptions_Unauthorized + */ + + IMPORT(tmp, "zExceptions"); + GETATTR_AS(tmp, "Unauthorized", zExceptions_Unauthorized); + Py_DECREF(tmp); + tmp = NULL; + /*| from AccessControl.SecurityManagement import getSecurityManager */ diff --git a/src/AccessControl/tests/testPermissionRole.py b/src/AccessControl/tests/testPermissionRole.py index 8052b8e..566b18b 100644 --- a/src/AccessControl/tests/testPermissionRole.py +++ b/src/AccessControl/tests/testPermissionRole.py @@ -19,7 +19,7 @@ from Acquisition import Implicit from Acquisition import aq_base -from AccessControl.PermissionRole import PermissionRole +from ..Implementation import PURE_PYTHON ViewPermission = 'View' @@ -50,40 +50,39 @@ class PermissiveObject(Explicit): _Edit_Things__Permission = ['Anonymous'] -def assertPRoles(ob, permission, expect): - """ - Asserts that in the context of ob, the given permission maps to - the given roles. - """ - pr = PermissionRole(permission) - roles = pr.__of__(ob) - roles2 = aq_base(pr).__of__(ob) - assert roles == roles2 or tuple(roles) == tuple(roles2), ( - 'Different methods of checking roles computed unequal results') - same = 0 - if roles: - # When verbose security is enabled, permission names are - # embedded in the computed roles. Remove the permission - # names. - roles = [r for r in roles if not r.endswith('_Permission')] - - if roles is None or expect is None: - if (roles is None or tuple(roles) == ('Anonymous', )) and \ - (expect is None or tuple(expect) == ('Anonymous', )): - same = 1 - else: - got = {} - for r in roles: - got[r] = 1 - expected = {} - for r in expect: - expected[r] = 1 - if got == expected: # Dict compare does the Right Thing. - same = 1 - assert same, f'Expected roles: {expect!r}, got: {roles!r}' - - -class PermissionRoleTests (unittest.TestCase): +class PermissionRoleTestBase: + + def assertPRoles(self, ob, permission, expect): + """ + Asserts that in the context of ob, the given permission maps to + the given roles. + """ + pr = self._getTargetClass()(permission) + roles = pr.__of__(ob) + roles2 = aq_base(pr).__of__(ob) + assert roles == roles2 or tuple(roles) == tuple(roles2), ( + 'Different methods of checking roles computed unequal results') + same = 0 + if roles: + # When verbose security is enabled, permission names are + # embedded in the computed roles. Remove the permission + # names. + roles = [r for r in roles if not r.endswith('_Permission')] + + if roles is None or expect is None: + if (roles is None or tuple(roles) == ('Anonymous', )) and \ + (expect is None or tuple(expect) == ('Anonymous', )): + same = 1 + else: + got = {} + for r in roles: + got[r] = 1 + expected = {} + for r in expect: + expected[r] = 1 + if got == expected: # Dict compare does the Right Thing. + same = 1 + self.assertTrue(same, f'Expected roles: {expect!r}, got: {roles!r}') def testRestrictive(self, explicit=0): app = AppRoot() @@ -93,9 +92,9 @@ def testRestrictive(self, explicit=0): app.c = ImplicitContainer() app.c.o = RestrictiveObject() o = app.c.o - assertPRoles(o, ViewPermission, ('Manager', )) - assertPRoles(o, EditThingsPermission, ('Manager', 'Owner')) - assertPRoles(o, DeletePermission, ()) + self.assertPRoles(o, ViewPermission, ('Manager', )) + self.assertPRoles(o, EditThingsPermission, ('Manager', 'Owner')) + self.assertPRoles(o, DeletePermission, ()) def testPermissive(self, explicit=0): app = AppRoot() @@ -105,11 +104,11 @@ def testPermissive(self, explicit=0): app.c = ImplicitContainer() app.c.o = PermissiveObject() o = app.c.o - assertPRoles(o, ViewPermission, ('Anonymous', )) - assertPRoles(o, EditThingsPermission, ('Anonymous', - 'Manager', - 'Owner')) - assertPRoles(o, DeletePermission, ('Manager', )) + self.assertPRoles(o, ViewPermission, ('Anonymous', )) + self.assertPRoles(o, EditThingsPermission, ('Anonymous', + 'Manager', + 'Owner')) + self.assertPRoles(o, DeletePermission, ('Manager', )) def testExplicit(self): self.testRestrictive(1) @@ -117,13 +116,55 @@ def testExplicit(self): def testAppDefaults(self): o = AppRoot() - assertPRoles(o, ViewPermission, ('Anonymous', )) - assertPRoles(o, EditThingsPermission, ('Manager', 'Owner')) - assertPRoles(o, DeletePermission, ('Manager', )) + self.assertPRoles(o, ViewPermission, ('Anonymous', )) + self.assertPRoles(o, EditThingsPermission, ('Manager', 'Owner')) + self.assertPRoles(o, DeletePermission, ('Manager', )) def testPermissionRoleSupportsGetattr(self): - a = PermissionRole('a') + a = self._getTargetClass()('a') self.assertEqual(getattr(a, '__roles__'), ('Manager', )) self.assertEqual(getattr(a, '_d'), ('Manager', )) self.assertEqual(getattr(a, '__name__'), 'a') self.assertEqual(getattr(a, '_p'), '_a_Permission') + + def testErrorsDuringGetattr(self): + pr = self._getTargetClass()('View') + + class AttributeErrorObject(Implicit): + pass + self.assertEqual( + tuple(pr.__of__(AttributeErrorObject())), ('Manager', )) + + # Unauthorized errors are tolerated and equivalent to no + # permission declaration + class UnauthorizedErrorObject(Implicit): + def __getattr__(self, name): + from zExceptions import Unauthorized + if name == '_View_Permission': + raise Unauthorized(name) + raise AttributeError(name) + self.assertEqual( + tuple(pr.__of__(UnauthorizedErrorObject())), ('Manager', )) + + # other exceptions propagate + class ErrorObject(Implicit): + def __getattr__(self, name): + if name == '_View_Permission': + raise RuntimeError("Error raised during getattr") + raise AttributeError(name) + with self.assertRaisesRegex( + RuntimeError, "Error raised during getattr"): + tuple(pr.__of__(ErrorObject())) + + +class Python_PermissionRoleTests(PermissionRoleTestBase, unittest.TestCase): + def _getTargetClass(self): + from AccessControl.ImplPython import PermissionRole + return PermissionRole + + +@unittest.skipIf(PURE_PYTHON, reason="Test expects C impl.") +class C__PermissionRoleTests(PermissionRoleTestBase, unittest.TestCase): + def _getTargetClass(self): + from AccessControl.ImplC import PermissionRole + return PermissionRole diff --git a/src/AccessControl/tests/testZopeSecurityPolicy.py b/src/AccessControl/tests/testZopeSecurityPolicy.py index 068c49b..b3cd900 100644 --- a/src/AccessControl/tests/testZopeSecurityPolicy.py +++ b/src/AccessControl/tests/testZopeSecurityPolicy.py @@ -158,6 +158,15 @@ class PartlyProtectedSimpleItem3 (PartlyProtectedSimpleItem1): __roles__ = sysadmin_roles +class DynamicallyUnauthorized(SimpleItemish): + # This class raises an Unauthorized on attribute access, + # similar to Zope's Shared.DC.Scripts.Bindings.UnauthorizedBinding + __ac_local_roles__ = {} + + def __getattr__(self, name): + raise Unauthorized('Not authorized to access: %s' % name) + + class SimpleClass: attr = 1 @@ -174,6 +183,7 @@ def setUp(self): a.item1 = PartlyProtectedSimpleItem1() a.item2 = PartlyProtectedSimpleItem2() a.item3 = PartlyProtectedSimpleItem3() + a.d_item = DynamicallyUnauthorized() uf = UserFolder() a.acl_users = uf self.uf = a.acl_users @@ -352,6 +362,11 @@ def test_checkPermission_proxy_role_scope(self): r_subitem, context)) + def test_checkPermission_dynamically_unauthorized(self): + d_item = self.a.d_item + context = self.context + self.assertFalse(self.policy.checkPermission('View', d_item, context)) + def testUnicodeRolesForPermission(self): r_item = self.a.r_item context = self.context