[ZCM] [ZC] 1713/ 6 Resolve "ZCatalog getObject broken"

[Collector: Zope Bugs, Features, and Patches ...]

Issue #1713 Update (Resolve) "ZCatalog getObject broken"
 Status Resolved, Zope/bug critical
To followup, visit:
  http://www.zope.org/Collectors/Zope/1713

==============================================================
= Resolve - Entry #6 by efge on Mar 25, 2005 12:53 pm

 Status: Accepted => Resolved

Fixed on 2.7 branch and trunk.

http://svn.zope.org/Zope/trunk/lib/python/Products/ZCatalog/CatalogBrains.py?rev=29682&r1=24563&r2=29682

________________________________________
= Comment - Entry #5 by shh on Mar 10, 2005 10:36 am

I don't see the benefit of calling validate over validateValue. I mean if you are aiming for Zope 2.8 compatibility you *must* pass the name argument anyway. And it looks to me as if the value argument (the last one) was the most important...

Anyhow, I trust that whoever tackles this will provide sufficient unit tests with it ;-)
________________________________________
= Comment - Entry #4 by efge on Mar 10, 2005 8:47 am

Effects of this patch:
- gives access to more objects than it did during the 2.7.1-2.7.4 timeframe (the objects returned by the new behaviour are a superset of those from the previous behaviour),
- the objects returned are still legitimate candidates (are answers to the catalog query) and are still checked for correct access rights.

________________________________________
= Comment - Entry #3 by ajung on Mar 10, 2005 6:51 am

Before accepting this patch for Zope 2.7.5 I would to know what others think about this patch  concerning backward compatiblity and security. I have no idea what side effects this patch might have.


________________________________________
= Accept - Entry #2 by efge on Mar 10, 2005 6:41 am

 Status: Pending => Accepted

 Supporters added: efge

I intend to fix this based on Roché's patch:

 -        return self.aq_parent.restrictedTraverse(self.getPath(), None)
 +        obj = self.aq_parent.unrestrictedTraverse(self.getPath(), None)
 +        if obj and securityManager.validate(obj, obj, None, None):
 +            return obj
 +        else:
 +            return None
 
adapted as needed to have a proper working validation.

________________________________________
= Request - Entry #1 by roche on Feb 25, 2005 3:01 pm

Last year in March the following checkin was made that changed
ZCatalog's getObject to use restrictedTraverse instead of
unrestrictedTraverse. See:

http://mail.zope.org/pipermail/zope-checkins/2004-March/026846.html

In my opininion this is wrong, and just cost me a good three hours to
figure out why big parts of one of our apps broke after an upgrade to
Zope 2.7.3. What's worse is that there is nowhere a trace of this change
in HISTORY.txt or CHANGES.txt.

Anybody with a site structure that has less restrictive access deeper
into the site would agree that getObject should do an
unrestrictedTraverse. restrictedTraverse returns None as soon as it
traverses an object a user does not have access to, regardless if the
user has access to the object referred to by the full path. To
illustrate imagine the following:

You have a site with a folder containg documents at '/documents'. Inside
that folder you have a whole bunch of documents where users have a local
role of owner to give them permission to access only their own
documents. You use a Catalog query to get the list of documents
belonging to a particular user and want to use getObject to retrieve the
objects found. But, it won't work because restrictedTraverse already
fails when traversing the 'documents' folder.

I would propose that getObject does an unrestrictedTraverse of the path
and then checks if the user has permission to access that the object.
==============================================================