Log message for revision 40437: Incorporate Jim's feedback: - Sync with Python version. Notably, move check of user's roles *below* the test of ownership and proxy roles, which makes the error handling *much* cleaner. - In the case that we have a stack, but the top *doesn't* have proxy roles, fall back to the user roles. - In the case that we have a stack, and the top *does* have proxy roles, skip checking the user roles altogether (proxy rolse replace user roles).
Changed: U Zope/branches/tseaver-collector_1774/lib/python/AccessControl/ImplPython.py U Zope/branches/tseaver-collector_1774/lib/python/AccessControl/cAccessControl.c -=- Modified: Zope/branches/tseaver-collector_1774/lib/python/AccessControl/ImplPython.py =================================================================== --- Zope/branches/tseaver-collector_1774/lib/python/AccessControl/ImplPython.py 2005-11-30 23:35:15 UTC (rev 40436) +++ Zope/branches/tseaver-collector_1774/lib/python/AccessControl/ImplPython.py 2005-11-30 23:52:43 UTC (rev 40437) @@ -490,6 +490,7 @@ # object is higher up than the owner, # deny access return 0 + for r in proxy_roles: if r in roles: return 1 Modified: Zope/branches/tseaver-collector_1774/lib/python/AccessControl/cAccessControl.c =================================================================== --- Zope/branches/tseaver-collector_1774/lib/python/AccessControl/cAccessControl.c 2005-11-30 23:35:15 UTC (rev 40436) +++ Zope/branches/tseaver-collector_1774/lib/python/AccessControl/cAccessControl.c 2005-11-30 23:52:43 UTC (rev 40437) @@ -1295,7 +1295,7 @@ PyObject *args) { /* return value */ - PyObject *rval = NULL; + PyObject *result = NULL; /* arguments, not increfe'd */ PyObject *permission = NULL; @@ -1346,17 +1346,6 @@ roles = role_list; } - /*| result = context.user.allowed(object, roles) - */ - - user = PyObject_GetAttr(context, user_str); - if (user != NULL) { - ASSIGN(user, PyObject_GetAttr(user, allowed_str)); - if (user != NULL) { - rval = callfunction2(user, object, roles); - } - } - /*| # Check executable security **| stack = context.stack @@ -1391,7 +1380,7 @@ if (owner == NULL) goto cP_done; if (! PyObject_IsTrue(owner)) { - rval = 0; + result = PyInt_FromLong(0); goto cP_done; } } @@ -1405,7 +1394,6 @@ **| # should not be able to use proxy roles to access items **| # above their subfolder! **| owner = eo.getWrappedOwner() - **| **| if owner is not None: **| if object is not aq_base(object): **| if not owner._check_context(object): @@ -1416,19 +1404,18 @@ **| for r in proxy_roles: **| if r in roles: **| return 1 - **| - **| return result + **| return 0 */ proxy_roles = PyObject_GetAttr(eo, _proxy_roles_str); if (proxy_roles == NULL) { PyErr_Clear(); - goto cP_done; + goto cP_check_user; } if (PyObject_IsTrue(proxy_roles)) { method = PyObject_GetAttr(eo, getWrappedOwner_str); - if (method == NULL) goto cP_done; + if (method == NULL) goto cP_done; wrappedowner = PyObject_CallObject(method, NULL); if (wrappedowner == NULL) goto cP_done; @@ -1441,9 +1428,12 @@ incontext = callmethod1(wrappedowner, _check_context_str, object); if (incontext == NULL) goto cP_done; + + if ( ! PyObject_IsTrue(incontext)) { + ASSIGN(result, PyInt_FromLong(0)); + goto cP_done; + } } - - if ( ! PyObject_IsTrue(incontext)) goto cP_done; } } @@ -1459,7 +1449,7 @@ } else { PyObject *proxy_role; length = PySequence_Size(proxy_roles); - if (length < 0) contains = -1; + if (length < 0) goto cP_done; for (iRole=0; contains == 0 && iRole < length; iRole++) { proxy_role = PySequence_GetItem(proxy_roles, iRole); if (proxy_role == NULL) goto cP_done; @@ -1469,10 +1459,27 @@ } } - if (contains > 0) - rval = PyInt_FromLong(contains); + /* If we have proxy roles, and they don't match, then lose + * (don't even check user roles at that point) + */ + if (contains < 0) contains = 0; + ASSIGN(result, PyInt_FromLong(contains)); + goto cP_done; } /* End of stack check */ +cP_check_user: + + /*| return context.user.allowed(object, roles) + */ + user = PyObject_GetAttr(context, user_str); + if (user != NULL) { + ASSIGN(user, PyObject_GetAttr(user, allowed_str)); + if (user != NULL) { + result = callfunction2(user, object, roles); + if (result == NULL) goto cP_done; + } + } + cP_done: Py_XDECREF(roles); Py_XDECREF(user); @@ -1485,7 +1492,7 @@ Py_XDECREF(objectbase); Py_XDECREF(incontext); - return rval; + return result; } /* _______________________________________________ Zope-Checkins maillist - Zope-Checkins@zope.org http://mail.zope.org/mailman/listinfo/zope-checkins