Commit 64135744 authored by Jérome Perrin's avatar Jérome Perrin

stack/erp5: wip AccessControl patch

parent 67a9971a
From 27d88c40e251b370f4dd2fcc7ae03c2967c68e4c Mon Sep 17 00:00:00 2001
From b2477d41ee40cb17b1a2832115ce3a7f9fb6190a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9rome=20Perrin?= <jerome@nexedi.com> From: =?UTF-8?q?J=C3=A9rome=20Perrin?= <jerome@nexedi.com>
Date: Mon, 2 Sep 2024 04:41:13 +0000 Date: Mon, 30 Sep 2024 00:54:03 +0000
Subject: [PATCH] checkPermission: align behavior with objects raising in Subject: [PATCH] Align behavior with objects raising in `__getattr__`
__getattr__
The observed problem was a behavior different between C and python The observed problem was a behavior different between C and python
implementation on python 3, happening with Zope python script. When the implementation on python 3, happening with Zope python script. When the
...@@ -10,27 +10,31 @@ context can not be accessed by the current user, Zope binds a ...@@ -10,27 +10,31 @@ context can not be accessed by the current user, Zope binds a
`Shared.DC.Scripts.Bindings.UnauthorizedBinding`, a class that raises an `Shared.DC.Scripts.Bindings.UnauthorizedBinding`, a class that raises an
Unauthorized error when the context is actually accessed, in order to Unauthorized error when the context is actually accessed, in order to
postpone the Unauthorized if something is actually accessed. This class postpone the Unauthorized if something is actually accessed. This class
does implements this by raising Unauthorized in __getattr__. does implements this by raising Unauthorized in `__getattr__`.
The python implementation of `checkPermission` uses `hasattr` and The python implementation of `rolesForPermissionOn` used `hasattr` and
`hasattr` has changed between python2 and python3, on python2 it was `hasattr` has changed between python2 and python3, on python2 it was
ignoring all exceptions, including potential Unauthorized errors and ignoring all exceptions, including potential Unauthorized errors and
just returning False, but on python3 these errors are raised. just returning False, but on python3 these errors are now raised.
This change of behavior of python causes checkPermission to behave This change of behavior of python causes `rolesForPermissionOn` to
differently: when using python implementation on python2 or when using behave differently: when using python implementation on python2 or when
C implementation, such Unauthorized errors were gracefully handled and using C implementation, such Unauthorized errors were gracefully handled
caused checkPermission to return False, but on python3 checkPermission and caused `checkPermission` to return False, but on python3 the
raises. Unauthorized is raised.
The C implementation of `rolesForPermissionOn` uses a construct
equivalent to the python2 version of `hasattr`. For consistency - and
because ignoring errors is usually not good - we also want to change it
to be have like the python3 implementation.
This change make this scenario behave the same between python2, python3 This change make this scenario behave the same between python and C
and C implementation: Unauthorized errors raised in __getattr__ are implementations:
supported. The code is also micro-simplified by doing only one getattr - `Unauthorized` errors raised in `__getattr__` are supported on py3.
instead of hasattr and then getattr. - Other errors than `AttributeError` and `Unauthorized` raised in
--- `__getattr__` are no longer ignored in the C implementation.
src/AccessControl/ImplPython.py | 6 +++++-
src/AccessControl/cAccessControl.c | 7 +++++-- ----
src/AccessControl/tests/testZopeSecurityPolicy.py | 15 +++++++++++++++ WIP patch not submitted - for testing
4 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/src/AccessControl/ImplPython.py b/src/AccessControl/ImplPython.py diff --git a/src/AccessControl/ImplPython.py b/src/AccessControl/ImplPython.py
index 1a7788b..0a9326b 100644 index 1a7788b..0a9326b 100644
...@@ -58,10 +62,18 @@ index 1a7788b..0a9326b 100644 ...@@ -58,10 +62,18 @@ index 1a7788b..0a9326b 100644
if _embed_permission_in_roles: if _embed_permission_in_roles:
return (('Anonymous',), n) return (('Anonymous',), n)
diff --git a/src/AccessControl/cAccessControl.c b/src/AccessControl/cAccessControl.c diff --git a/src/AccessControl/cAccessControl.c b/src/AccessControl/cAccessControl.c
index 403ed67..1a109fa 100644 index 403ed67..bb849fe 100644
--- a/src/AccessControl/cAccessControl.c --- a/src/AccessControl/cAccessControl.c
+++ b/src/AccessControl/cAccessControl.c +++ b/src/AccessControl/cAccessControl.c
@@ -1847,13 +1847,16 @@ c_rolesForPermissionOn(PyObject *perm, PyObject *object, @@ -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); Py_INCREF(r);
/* /*
...@@ -79,44 +91,38 @@ index 403ed67..1a109fa 100644 ...@@ -79,44 +91,38 @@ index 403ed67..1a109fa 100644
+ else: + else:
*/ */
PyObject *roles = PyObject_GetAttr(object, n); 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) if (roles != NULL)
diff --git a/src/AccessControl/tests/testZopeSecurityPolicy.py b/src/AccessControl/tests/testZopeSecurityPolicy.py {
index 9b12a0f..ee74bad 100644
--- a/src/AccessControl/tests/testZopeSecurityPolicy.py
+++ b/src/AccessControl/tests/testZopeSecurityPolicy.py
@@ -157,6 +157,15 @@ class PartlyProtectedSimpleItem3 (PartlyProtectedSimpleItem1):
__roles__ = sysadmin_roles
@@ -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;
+class DynamicallyUnauthorized(SimpleItemish): static struct PyModuleDef moduledef =
+ # This class raises an Unauthorized on attribute access, {
+ # similar to Zope's Shared.DC.Scripts.Bindings.UnauthorizedBinding @@ -2400,6 +2414,14 @@ module_init(void) {
+ __ac_local_roles__ = {} Py_DECREF(tmp);
+ tmp = NULL;
+ def __getattr__(self, name):
+ raise Unauthorized('Not authorized to access: %s' % name) + /*| from zExceptions import Unauthorized as zExceptions_Unauthorized
+ */
+ +
+ IMPORT(tmp, "zExceptions");
+ GETATTR_AS(tmp, "Unauthorized", zExceptions_Unauthorized);
+ Py_DECREF(tmp);
+ tmp = NULL;
+ +
class SimpleClass: /*| from AccessControl.SecurityManagement import getSecurityManager
attr = 1 */
@@ -173,6 +182,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
@@ -351,6 +361,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
...@@ -662,7 +662,7 @@ SOAPpy-py3-patches = ${:_profile_base_location_}/../../component/egg-patch/SOAPp ...@@ -662,7 +662,7 @@ SOAPpy-py3-patches = ${:_profile_base_location_}/../../component/egg-patch/SOAPp
SOAPpy-py3-patch-options = -p1 SOAPpy-py3-patch-options = -p1
[eggs:python3] [eggs:python3]
AccessControl-patches = ${:_profile_base_location_}/../../component/egg-patch/AccessControl/157.patch#9b01341bd4271555c9caa66cb9d0f098 AccessControl-patches = ${:_profile_base_location_}/../../component/egg-patch/AccessControl/157.patch#4ada8082272b773ad66dbaaae44ba7a7
AccessControl-patch-options = -p1 AccessControl-patch-options = -p1
interval-patches = ${:_profile_base_location_}/../../component/egg-patch/interval/0001-python3-support.patch#66ac345f0a6d73e0bd29e394b7646311 interval-patches = ${:_profile_base_location_}/../../component/egg-patch/interval/0001-python3-support.patch#66ac345f0a6d73e0bd29e394b7646311
interval-patch-options = -p1 interval-patch-options = -p1
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment