[Zope-dev] Re: ZCatalog getObject broken
Roché Compaan wrote: I'm unsure about the security check in the patch below - I copied the way restrictedTraverse does it. I read through validate in the default security policy but it is one of those methods where all the security implications doesn't fit in your head all at once. --- CatalogBrains.py~ 2004-03-23 22:27:23.0 +0200 +++ CatalogBrains.py2005-03-03 09:43:48.0 +0200 @@ -47,7 +47,11 @@ (i.e., it was deleted or moved without recataloging), or if the user is not authorized to access an object along the path. -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 There is a method deep down in Zope somewhere called: self.authenticated_has_access(obj) I cannot find the definition on my local Windows install, so I assume it's defined in some c code somewhere. Unfortunately there is no docs on the web either. Though there must have been at some time, as I would otherwise never have found it. Hmm... that is odd. -- hilsen/regards Max M, Denmark http://www.mxm.dk/ IT's Mad Science ___ 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 )
Re: [Zope-dev] Re: ZCatalog getObject broken
On Thu, 2005-03-03 at 09:27 +0100, Max M wrote: Roché Compaan wrote: I'm unsure about the security check in the patch below - I copied the way restrictedTraverse does it. I read through validate in the default security policy but it is one of those methods where all the security implications doesn't fit in your head all at once. --- CatalogBrains.py~ 2004-03-23 22:27:23.0 +0200 +++ CatalogBrains.py2005-03-03 09:43:48.0 +0200 @@ -47,7 +47,11 @@ (i.e., it was deleted or moved without recataloging), or if the user is not authorized to access an object along the path. -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 There is a method deep down in Zope somewhere called: self.authenticated_has_access(obj) I cannot find the definition on my local Windows install, so I assume it's defined in some c code somewhere. Unfortunately there is no docs on the web either. Though there must have been at some time, as I would otherwise never have found it. Hmm... that is odd. In this context the user does not need to be authenticated - Anonymous might have view rights in the context of the object. -- Roché Compaan Upfront Systems http://www.upfrontsystems.co.za ___ 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 )
Re: [Zope-dev] ZCatalog getObject broken
Roché Compaan wrote: +obj = self.aq_parent.unrestrictedTraverse(self.getPath(), None) +if obj and securityManager.validate(obj, obj, None, None): +return obj +else: +return None Urm, Roche, doesn't the above seek to do exactly what... return self.aq_parent.restrictedTraverse(self.getPath(), None) ...does? The problem is that an error should be raised, Unauthorized in my opinion, rather than None being returned. None should never be returned in place of a brain, although I'll soften that to say that if it does, it means something weird has happened (used to mean the object the catalog entry mapped to had gone away) I think: self.aq_parent.restrictedTraverse(self.getPath()) ...should be fine, no? Chris -- Simplistix - Content Management, Zope Python Consulting - http://www.simplistix.co.uk ___ 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 )
Re: [Zope-dev] ZCatalog getObject broken
On Thu, 2005-03-03 at 14:56 +, Chris Withers wrote: Roché Compaan wrote: +obj = self.aq_parent.unrestrictedTraverse(self.getPath(), None) +if obj and securityManager.validate(obj, obj, None, None): +return obj +else: +return None Urm, Roche, doesn't the above seek to do exactly what... return self.aq_parent.restrictedTraverse(self.getPath(), None) ...does? No it doesn't, restrictedTraverse fails along the way. If the path is /a/b and the user doesn't have access to /a/ restrictedTraverse will return None even though the user has access to /a/b/. In my code above we only do a security check on the object that the full path resolves to. The problem is that an error should be raised, Unauthorized in my opinion, rather than None being returned. I would be ok with raising Unauthorized but it is not backwards compatible. I suppose changing to 'unrestrictedTraverse' is also not backward compatible but the current 'getObject' seems to suggest that we do not want to raise an exception when the user does not have permission to access the object. Is there some use case for 'getObject' that we are missing here? None should never be returned in place of a brain, although I'll soften that to say that if it does, it means something weird has happened (used to mean the object the catalog entry mapped to had gone away) I agree. -- Roché Compaan Upfront Systems http://www.upfrontsystems.co.za ___ 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 )
Re: [Zope-dev] ZCatalog getObject broken
Roché Compaan wrote at 2005-3-3 09:53 +0200: ... -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): I think this is not correct: validate needs at least a value parameter (this is the forth parameter). There is a validateValue method (instead of validate) that does what you want. You find it in AccessControl/ImplPython.py. Drawback: it might disappear in Zope 2.8. -- Dieter ___ 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 )
Re: [Zope-dev] ZCatalog getObject broken
On Thu, 2005-03-03 at 19:36 +0100, Dieter Maurer wrote: Roché Compaan wrote at 2005-3-3 09:53 +0200: ... -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): I think this is not correct: validate needs at least a value parameter (this is the forth parameter). I thought this much but what value? And doesn't this make the implementation of restrictedTraverse suspect too? When code is calling getObject on a catalog brain we don't know what attribute or method of that object the calling code will access. Does it then make any sense at all to do security checks in getObject? IMO it doesn't. ___ 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 )
