[Zope-dev] Re: Traversal issue which affects Five

[Florent Guillaume]

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 at 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 at nuxeo.com