Hi Alec, Five traversal is definitely very touchy, and the interactions with Zope numerous. So I'm sure the problem you observed is real and that a solution must be found. The unit tests you propose would go a long way to having a fix committed asap, so yes please, provide one. Florent Alec Mitchell wrote:
It seems that the way OFS.Traversable.restrictedTraverse() handles security checking on objects with __bobo_traverse__ methods is considerably different from the way it normally checks security. The result is that traversal cannot obtain attributes using acquisition from objects that are marked <five:traversable>. In the normal case, security is checked using guarded_getattr, which gets an attribute and checks the permissions on the retrieved object in its original context. For __bobo_traverse__methods which return simple properties (say strings), it is impossible to determine the container from which the returned attribute originates, so unless the attribute was not acquired, an Unauthorized error will always be raised.
Objects that are Five Traversable always have __bobo_traverse__ methods which attempt to mimic normal traversal, which effectively means that the security checks end up preventing acquisition of simple properties using traversal from ever working on these objects (say using a TAL path expression 'context/attribute' which you expect to be acquired). Unfortunately, because Five has no control over the security checks done during traversal, this cannot be fixed directly in Five. However, IMHO fixing this makes sense for Zope itself, provided there aren't any undesirable consequences. I propose that if the validation of a __bobo_traverse result raises Unauthorized, that we make one last check to see if the result o 'guarded_getattr(obj, name)' is identical to the result of the __bobo_traverse__ call and allow it if that's the case. Here is my proposed patch against Zope 2.9 trunk:
--- Traversable.py (revision 66323) +++ Traversable.py (working copy) @@ -201,9 +201,18 @@ else: # Can't determine container container = _none - if not securityManager.validate( - obj, container, name, next): - raise Unauthorized, name + try: + if not securityManager.validate( + obj, container, name, next): + raise Unauthorized, name + except Unauthorized: + # If next is a simple unwrapped property, it's + # parentage is indeterminate, but it may have been + # acquired safely. In this case validate will raise + # an error, and we can check that our value was + # acquired safely. + if guarded_getattr(obj, name, marker) is not next: + raise Unauthorized, name else: if restricted: next = guarded_getattr(obj, name, marker)
At the moment Plone 2.5 is really struggling with this issue, and it would be wonderful if a fix for this could go into Zope 2.8 and 2.9 soon. I'm happy to write tests for this, I just want to make sure that I'm not proposing something really wrong/inappropriate here. Generally, the validate() call should return a True or False value, so this change should have little performance impact except in the case where 'container == _none' and validate would otherwise raise a very unhelpful unauthorized error. Your feedback is much appreciated.
Alec Mitchell _______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
-- Florent Guillaume, Nuxeo (Paris, France) Director of R&D +33 1 40 33 71 59 http://nuxeo.com fg@nuxeo.com