[Checkins] SVN: z3c.dav/trunk/ Improve the handling of Unauthorized errors on recursive PROPFIND requests. We

Michael Kerrin michael.kerrin at openapp.ie
Mon Feb 11 15:55:44 EST 2008


Log message for revision 83753:
  Improve the handling of Unauthorized errors on recursive PROPFIND requests. We
  weren't returning this to the user and instead either hiding the error in the
  response or refusing to list folder contents. Now return the unauthorized
  response only if the problem occurs on the requested resource.
  

Changed:
  U   z3c.dav/trunk/CHANGES.txt
  U   z3c.dav/trunk/src/z3c/dav/ftests/test_propfind.py
  U   z3c.dav/trunk/src/z3c/dav/propfind.py
  U   z3c.dav/trunk/src/z3c/dav/tests/test_propfind.py

-=-
Modified: z3c.dav/trunk/CHANGES.txt
===================================================================
--- z3c.dav/trunk/CHANGES.txt	2008-02-11 20:54:20 UTC (rev 83752)
+++ z3c.dav/trunk/CHANGES.txt	2008-02-11 20:55:44 UTC (rev 83753)
@@ -5,6 +5,13 @@
 1.0b2
 =====
 
+- Improved the handling of `Unauthorized' and `Forbidden' errors during the
+  processing of `PROPFIND' requests. Basically we return a `Unauthorized'
+  response requesting the user to log-in when the problem occurs on the
+  requested resource otherwise we render the problem into the `multistatus'
+  response. In the case where we aren't allowed to list a folder we ignore
+  the contents, unless it is the requested resource.
+
 - Register views for the `zope.security.interfaces.Forbidden' exceptions.
 
 1.0b1

Modified: z3c.dav/trunk/src/z3c/dav/ftests/test_propfind.py
===================================================================
--- z3c.dav/trunk/src/z3c/dav/ftests/test_propfind.py	2008-02-11 20:54:20 UTC (rev 83752)
+++ z3c.dav/trunk/src/z3c/dav/ftests/test_propfind.py	2008-02-11 20:55:44 UTC (rev 83753)
@@ -24,6 +24,7 @@
 
 import dav
 from zope import component
+import zope.security.interfaces
 import z3c.dav.interfaces
 import z3c.etree.testing
 
@@ -223,6 +224,17 @@
                                     "http://localhost/b/",
                                     "http://localhost/r1"])
 
+    def test_etcsite(self):
+        # By all means this test is a bit stupid but it does highlight a fact.
+        # We get unauthorized errors when we enter try and access a resource
+        # which doesn't give the admin access to folder listing. But as
+        # the previous test shows we can get the problem resource to display
+        # in the depth infinity folder listing. As long as the problem resource
+        # is not the request resource then no errors should be raised.
+        self.assertRaises(zope.security.interfaces.Unauthorized,
+                          self.publish, "/++etc++site/", basic = "mrg:mgrpw",
+                          env = {"REQUEST_METHOD": "PROPFIND"})
+
     def test_depthone(self):
         self.createCollectionResourceStructure()
 

Modified: z3c.dav/trunk/src/z3c/dav/propfind.py
===================================================================
--- z3c.dav/trunk/src/z3c/dav/propfind.py	2008-02-11 20:54:20 UTC (rev 83752)
+++ z3c.dav/trunk/src/z3c/dav/propfind.py	2008-02-11 20:55:44 UTC (rev 83753)
@@ -42,8 +42,7 @@
 from zope import component
 from zope.filerepresentation.interfaces import IReadDirectory
 from zope.error.interfaces import IErrorReportingUtility
-from zope.security.checker import canAccess
-from zope.security.interfaces import Unauthorized
+import zope.security.interfaces
 
 import z3c.etree
 import z3c.dav.utils
@@ -64,7 +63,6 @@
         self.request = request
 
     def getDepth(self):
-        # default is infinity.
         return self.request.getHeader("depth", "infinity")
 
     def PROPFIND(self):
@@ -122,7 +120,8 @@
         return etree.tostring(multistatus(), encoding = "utf-8")
 
     def handlePropfindResource(self, ob, req, depth, \
-                               propertiesFactory, extraArg):
+                               propertiesFactory, extraArg,
+                               level = 0):
         """
         Recursive method that collects all the `response' XML elements for
         the current PROPFIND request.
@@ -132,16 +131,44 @@
         request and `extraArg' used to pass in specific information about
         the properties we want to return.
         """
-        responses = [propertiesFactory(ob, req, extraArg)]
+        responses = [propertiesFactory(ob, req, extraArg, level)]
         if depth in ("1", "infinity"):
             subdepth = (depth == "1") and "0" or "infinity"
 
             readdir = IReadDirectory(ob, None)
-            if readdir is not None and canAccess(readdir, "values"):
-                for subob in readdir.values():
-                    if subob is not None:
+            if readdir is not None:
+                try:
+                    # We catch any Forbidden or Unauthorized exceptions here.
+                    # We have successfully rendered the property on this
+                    # resource and we are as such trying to render a listing
+                    # of the container object. If we do get an Unauthorized
+                    # exception then we only raise this if level == 0.
+                    # Otherwise the user might never have access to the resource
+                    # and they will never get to see the resources that they
+                    # are interested in.
+                    values = readdir.values()
+                except zope.security.interfaces.Forbidden:
+                    # Since we successfully rendered the properties and the
+                    # user is forbidden to access the folder listing then
+                    # we silently ignore this exception. Allowing them to
+                    # continue with there usage of the system.
+                    errUtility = component.getUtility(IErrorReportingUtility)
+                    errUtility.raising(sys.exc_info(), req)
+                except zope.security.interfaces.Unauthorized:
+                    # Sometimes even the administrator will raise an
+                    # `Unauthorized' exception on the `values' method. If this
+                    # happens on a sub-resource to the requested resource then
+                    # we log the exception and continue - others this request
+                    # fails with the `Unauthorized' exception.
+                    if level == 0:
+                        raise
+                    errUtility = component.getUtility(IErrorReportingUtility)
+                    errUtility.raising(sys.exc_info(), req)
+                else:
+                    for subob in values:
                         responses.extend(self.handlePropfindResource(
-                            subob, req, subdepth, propertiesFactory, extraArg))
+                            subob, req, subdepth,
+                            propertiesFactory, extraArg, level + 1))
 
         return responses
 
@@ -149,22 +176,23 @@
         error_view = component.queryMultiAdapter(
             (exc_info[1], request), z3c.dav.interfaces.IDAVErrorWidget)
         if error_view is None:
-            ## An unexpected error occured here. This error should be
-            ## fixed. In order to easily debug the problem we will
-            ## log the error with the ErrorReportingUtility
-            errUtility = component.getUtility(IErrorReportingUtility)
-            errUtility.raising(exc_info, request)
+            # An unexpected error occured here and should be fixed.
             propstat = response.getPropstat(500) # Internal Server Error
         else:
             propstat = response.getPropstat(error_view.status)
-            ## XXX - needs testing
+            # XXX - needs testing
             propstat.responsedescription += error_view.propstatdescription
             response.responsedescription += error_view.responsedescription
 
+        # In order to easily debug the problem we will log the error with
+        # the ErrorReportingUtility.
+        errUtility = component.getUtility(IErrorReportingUtility)
+        errUtility.raising(exc_info, request)
+
         etree = z3c.etree.getEngine()
         propstat.properties.append(etree.Element(proptag))
 
-    def renderPropnames(self, ob, req, ignore):
+    def renderPropnames(self, ob, req, ignoreExtraArg, ignorelevel = 0):
         """
         See doc string for the renderAllProperties method. Note that we don't
         need to worry about the security in this method has the permissions on
@@ -184,7 +212,7 @@
 
         return response
 
-    def renderAllProperties(self, ob, req, include):
+    def renderAllProperties(self, ob, req, include, level = 0):
         """
         The specification says:
         
@@ -213,7 +241,7 @@
                 davwidget = z3c.dav.properties.getWidget(
                     davprop, adapter, req)
                 response.addProperty(200, davwidget.render())
-            except Unauthorized:
+            except zope.security.interfaces.Unauthorized:
                 # Users don't have the permission to view this property and
                 # if they didn't explicitly ask for the named property
                 # we can silently ignore this property, pretending that it
@@ -228,7 +256,7 @@
                     # just in case this is a problem that needs sorting out.
                     errUtility = component.getUtility(IErrorReportingUtility)
                     errUtility.raising(sys.exc_info(), req)
-            except Exception:
+            except:
                 self.handleException(
                     "{%s}%s" %(davprop.namespace, davprop.__name__),
                     sys.exc_info(), req,
@@ -236,14 +264,14 @@
 
         return response
 
-    def renderSelectedProperties(self, ob, req, props):
+    def renderSelectedProperties(self, ob, req, props, level = 0):
         response = z3c.dav.utils.Response(
             z3c.dav.utils.getObjectURL(ob, req))
 
         for prop in props:
             if z3c.dav.utils.parseEtreeTag(prop.tag)[0] == "":
-                # A namespace which is None corresponds to when no prefix is
-                # set, which I think is fine.
+                # XXX - A namespace which is None corresponds to when no
+                # prefix is set, which I think is fine.
                 raise z3c.dav.interfaces.BadRequest(
                     self.request,
                     u"PROPFIND with invalid namespace declaration in body")
@@ -255,7 +283,16 @@
                     davprop, adapter, req)
                 propstat = response.getPropstat(200)
                 propstat.properties.append(davwidget.render())
-            except Exception:
+            except zope.security.interfaces.Unauthorized:
+                if level == 0:
+                    # When we are rendering properties on the requested
+                    # resource then we must raise any Unauthorized exceptions
+                    # so that user can then log in. But if the Unauthorized
+                    # exception is on a sub resource to the requested resource
+                    # then we can just handle this unauthorized exception.
+                    raise
                 self.handleException(prop.tag, sys.exc_info(), req, response)
+            except:
+                self.handleException(prop.tag, sys.exc_info(), req, response)
 
         return response

Modified: z3c.dav/trunk/src/z3c/dav/tests/test_propfind.py
===================================================================
--- z3c.dav/trunk/src/z3c/dav/tests/test_propfind.py	2008-02-11 20:54:20 UTC (rev 83752)
+++ z3c.dav/trunk/src/z3c/dav/tests/test_propfind.py	2008-02-11 20:55:44 UTC (rev 83753)
@@ -32,8 +32,8 @@
 from zope.app.container.interfaces import IReadContainer
 from zope.error.interfaces import IErrorReportingUtility
 from zope.security.interfaces import Unauthorized, IUnauthorized
-from zope.security.interfaces import IChecker
-from zope.security.checker import CheckerPublic
+import zope.security.checker
+import zope.security.interfaces
 
 import z3c.dav.properties
 import z3c.dav.publisher
@@ -247,16 +247,18 @@
         self.context = context
         self.request = request
 
-    def _getproperty(name, default = None):
+    def _getproperty(name):
         def get(self):
-            return getattr(self.context, name, default)
+            # Don't supply default values to getattr as this hides any
+            # exceptions that I need for the tests to run.
+            return getattr(self.context, name)
         def set(self, value):
             setattr(self.context, name, value)
         return property(get, set)
 
-    exampleintprop = _getproperty("intprop", default = 0)
+    exampleintprop = _getproperty("intprop")
 
-    exampletextprop = _getproperty("text", default = u"")
+    exampletextprop = _getproperty("text")
 
 
 class BrokenPropertyStorage(object):
@@ -382,6 +384,9 @@
     gsm.registerAdapter(z3c.dav.exceptions.UnauthorizedError,
                         (IUnauthorized,
                          z3c.dav.interfaces.IWebDAVRequest))
+    gsm.registerAdapter(z3c.dav.exceptions.ForbiddenError,
+                        (zope.security.interfaces.IForbidden,
+                         z3c.dav.interfaces.IWebDAVRequest))
 
 
 def propfindTearDown():
@@ -432,6 +437,9 @@
     gsm.unregisterAdapter(z3c.dav.exceptions.UnauthorizedError,
                           (IUnauthorized,
                            z3c.dav.interfaces.IWebDAVRequest))
+    gsm.unregisterAdapter(z3c.dav.exceptions.ForbiddenError,
+                          (zope.security.interfaces.IForbidden,
+                           z3c.dav.interfaces.IWebDAVRequest))
     gsm.unregisterAdapter(z3c.dav.widgets.ListDAVWidget,
                           (zope.schema.interfaces.IList,
                            z3c.dav.interfaces.IWebDAVRequest))
@@ -682,7 +690,10 @@
         error = self.errUtility.errors[0]
         self.assertEqual(isinstance(error[0][1], NotImplementedError), True)
 
-    def test_render_selected_unauthorizedProperty(self):
+    def test_render_selected_unauthorizedProperty_toplevel(self):
+        # If during the processing of a PROPFIND request and access to a
+        # property on the requested resource is unauthorized to the current
+        # user then we raise an `Unauthorized' requesting the user to log in.
         resource = Resource("some text", 10)
         request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
         propf = PROPFIND(None, None)
@@ -693,7 +704,30 @@
 <D:exampletextprop />
 </prop>""")
 
-        response = propf.renderSelectedProperties(resource, request, props)
+        self.assertRaises(
+            zope.security.interfaces.Unauthorized,
+            propf.renderSelectedProperties, resource, request, props)
+
+    def test_render_selected_unauthorizedProperty_sublevel(self):
+        # This is the same as the previous test but since we are rendering
+        # a sub-resource to the requested resource - we render the forbidden
+        # error as part of the successful `multistatus' response. This stops
+        # errors from raising to the user when they might not ever have access
+        # to the particular resource but they are still continue using the
+        # system. Where as the previous test shows that they can still get
+        # access to the system.
+        resource = Resource("some text", 10)
+        request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
+        propf = PROPFIND(None, None)
+
+        etree = z3c.etree.getEngine()
+        props = etree.fromstring("""<prop xmlns="DAV:" xmlns:D="DAVtest:">
+<D:unauthprop />
+<D:exampletextprop />
+</prop>""")
+
+        # The current `level' is the last argument to this method.
+        response = propf.renderSelectedProperties(resource, request, props, 1)
         response = response()
 
         # The PROPFIND method should return a 401 when the user is unauthorized
@@ -718,7 +752,7 @@
         # but instead we need to make sure that the renderSelectedProperties
         # does throw the exception.
 
-    def test_renderAllProperties_unauthorized(self):
+    def test_renderAllProperties_unauthorized_toplevel(self):
         # If we request to render all property but we are unauthorized to
         # access one of the propertues then this property should be threated
         # as if it were restricted property and not returned to the user.
@@ -755,6 +789,44 @@
         exc_info = self.errUtility.errors[0]
         self.assertEqual(isinstance(exc_info[0][1], Unauthorized), True)
 
+    def test_renderAllProperties_unauthorized_sublevel(self):
+        # If we request to render all property but we are unauthorized to
+        # access one of the propertues then this property should be threated
+        # as if it were restricted property and not returned to the user.
+        resource = Resource("some text", 10)
+        request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
+        propf = PROPFIND(None, request)
+
+        # Set the unauthproperty as un-restricted so that the
+        # renderAllProperties will render all the properties.
+        unauthProperty.restricted = False
+
+        # PROPFIND does catch all exceptions during the main PROPFIND method
+        # but instead we need to make sure that the renderSelectedProperties
+        # does throw the exception.
+        # The current `level' is the last argument to this method.
+        response = propf.renderAllProperties(resource, request, None, 1)
+        response = response()
+
+        # Note that the unauthprop is not included in the response.
+        assertXMLEqualIgnoreOrdering("""<response xmlns="DAV:">
+<href>/resource</href>
+<propstat>
+  <prop>
+    <ns1:exampletextprop xmlns:ns1="DAVtest:">some text</ns1:exampletextprop>
+    <resourcetype />
+    <ns1:exampleintprop xmlns:ns1="DAVtest:">10</ns1:exampleintprop>
+  </prop>
+  <status>HTTP/1.1 200 Ok</status>
+</propstat>
+</response>""", response)
+
+        # Since we silenty ignored returning a property. We now log the
+        # Unauthorized exception so debuging and logging purposes.
+        self.assertEqual(len(self.errUtility.errors), 1)
+        exc_info = self.errUtility.errors[0]
+        self.assertEqual(isinstance(exc_info[0][1], Unauthorized), True)
+
     def test_renderAllProperties_unauthorized_included(self):
         # If we request to render all properties, and request to render a
         # property we ain't authorized via the 'include' element then we
@@ -830,17 +902,21 @@
         self.assertEqual(isinstance(exc_info[0][1], NotImplementedError), True)
 
 
-class CollectionSecurityChecker(object):
-    # Simple security checker to make the recursive propfind method handle
-    # the canAccess call that is made during the processing of these requests.
-    interface.implements(IChecker)
+class SecurityChecker(object):
+    # Custom security checker to make the recursive propfind method handle
+    # authentication errors properly. We use this checker instead of the
+    # zope.security.checker.Checker object when we want to raise Unauthorized
+    # errors and not Forbidden errors.
+    interface.implements(zope.security.interfaces.IChecker)
 
     def __init__(self, get_permissions = {}):
         self.get_permissions = get_permissions
 
     def check_getattr(self, ob, name):
+        if name in ("__provides__",):
+            return
         permission = self.get_permissions.get(name)
-        if permission is CheckerPublic:
+        if permission is zope.security.checker.CheckerPublic:
             return
         raise Unauthorized(object, name, permission)
 
@@ -851,16 +927,37 @@
         raise NotImplementedError("check(ob, operation) not implemented")
 
     def proxy(self, value):
-        raise NotImplementedError("proxy(value) not implemented")
+        if type(value) is zope.security.checker.Proxy:
+            return value
+        checker = getattr(value, '__Security_checker__', None)
+        if checker is None:
+            checker = zope.security.checker.selectChecker(value)
+            if checker is None:
+                return value
 
+        return zope.security.checker.Proxy(value, checker)
 
-def readDirectory(container):
-    container.__Security_checker__ = CollectionSecurityChecker(
-        {"values": CheckerPublic})
+
+def readDirectoryNoOp(container):
     return container
 
 
-class PROPFINDRecuseTest(unittest.TestCase):
+class PROPFINDSecurityTestCase(unittest.TestCase):
+    # When processing PROPFIND requests with depth `infinity' sometimes we
+    # run into problems. These can include users been unauthorized to certain
+    # items. This test implements a custom security policy in `SecurityChecker'
+    # to test the use case of finding security problems in rendering these
+    # requests.
+    # This really needs to be a doctest:
+    # - test_handlePropfindResource
+    # - test_handlePropfind_forbiddenResourceProperty
+    # - test_handlePropfind_forbiddenRequestedResourceProperty
+    # - test_handlePropfind_unauthorizedRequestedResourceProperty
+    # - test_handlePropfind_forbiddenCollection_listing
+    # - test_handlePropfind_unauthorizedCollection_listing
+    # - test_handlePropfind_forbiddenRootCollection_listing
+    # - test_handlePropfind_unauthorizedRootCollection_listing
+    # - test_handlePropfindResource_unauthorizedResource
 
     def setUp(self):
         propfindSetUp()
@@ -869,19 +966,47 @@
         unauthProperty.restricted = True
 
         component.getGlobalSiteManager().registerAdapter(
-            readDirectory, (IReadContainer,), provided = IReadDirectory)
+            readDirectoryNoOp, (IReadContainer,), provided = IReadDirectory)
 
+        self.errUtility = ErrorReportingUtility()
+        component.getGlobalSiteManager().registerUtility(self.errUtility)
+
+        # Collect all the checkers we define so that we can remove them later.
+        self.undefine_type_checkers = []
+
+        self.collection = Collection()
+        self.collection["r1"] = Resource("some text - r1", 2)
+        self.collection["c"] = Collection()
+        self.collection["c"]["r2"] = Resource("some text - r2", 4)
+
     def tearDown(self):
         propfindTearDown()
 
         component.getGlobalSiteManager().unregisterAdapter(
-            readDirectory, (IReadContainer,), provided = IReadDirectory)
+            readDirectoryNoOp, (IReadContainer,), provided = IReadDirectory)
 
+        component.getGlobalSiteManager().unregisterUtility(self.errUtility)
+        del self.errUtility
+
+        for type_ in self.undefine_type_checkers:
+            zope.security.checker.undefineChecker(type_)
+
+    def addChecker(self, obj, checker):
+        self.undefine_type_checkers.append(obj.__class__)
+        zope.security.checker.defineChecker(obj.__class__, checker)
+        return zope.security.checker.ProxyFactory(obj)
+
     def test_handlePropfindResource(self):
-        collection = Collection()
-        collection["r1"] = Resource("some text - r1", 2)
-        collection["c"] = Collection()
-        collection["c"]["r2"] = Resource("some text - r2", 4)
+        # Just make sure that the custom security checker works by giving
+        # access to all the resources and subcollections.
+        self.addChecker(
+            self.collection["r1"], zope.security.checker.Checker({
+                "text": zope.security.checker.CheckerPublic,
+                "intprop": zope.security.checker.CheckerPublic}))
+        collection = self.addChecker(
+            self.collection, zope.security.checker.Checker({
+                "values": zope.security.checker.CheckerPublic}))
+
         request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
         request.processInputs()
         propf = PROPFIND(collection, request)
@@ -936,10 +1061,406 @@
 </D:response></D:multistatus>
         """)
 
+        self.assertEqual(len(self.errUtility.errors), 0)
 
+    def test_handlePropfind_forbiddenResourceProperty(self):
+        # Remove access to the `exampleintprop' on the collection['r1']
+        # resource. Since this not the requested resource we render the
+        # error and include it in the `{DAV:}response' for the corresponding
+        # resource but with no value and a `403' status.
+        self.collection.data["r1"] = zope.security.checker.ProxyFactory(
+            self.collection["r1"], zope.security.checker.Checker({
+                "text": zope.security.checker.CheckerPublic}))
+        self.addChecker(
+            self.collection["c"]["r2"], zope.security.checker.Checker({
+                "text": zope.security.checker.CheckerPublic,
+                "intprop": zope.security.checker.CheckerPublic}))
+        collection = self.addChecker(
+            self.collection, zope.security.checker.Checker({
+                "values": zope.security.checker.CheckerPublic}))
+
+        request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
+        request.processInputs()
+        propf = PROPFIND(collection, request)
+
+        result = propf.PROPFIND()
+
+        self.assertEqual(request.response.getStatus(), 207)
+        assertXMLEqualIgnoreOrdering(result, """<D:multistatus xmlns:D="DAV:">
+<D:response>
+  <D:href>/collection/</D:href>
+  <D:propstat>
+    <D:prop>
+      <D:resourcetype>
+        <D:collection />
+      </D:resourcetype>
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+</D:response>
+<D:response>
+  <D:href>/collection/c/</D:href>
+  <D:propstat>
+    <D:prop>
+      <D:resourcetype>
+        <D:collection />
+      </D:resourcetype>
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+</D:response>
+<D:response>
+  <D:href>/collection/c/r2</D:href>
+  <D:propstat>
+    <D:prop>
+      <D1:exampletextprop xmlns:D1="DAVtest:">some text - r2</D1:exampletextprop>
+      <D:resourcetype />
+      <D1:exampleintprop xmlns:D1="DAVtest:">4</D1:exampleintprop>
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+</D:response>
+<D:response>
+  <D:href>/collection/r1</D:href>
+  <D:propstat>
+    <D:prop>
+      <D1:exampletextprop xmlns:D1="DAVtest:">some text - r1</D1:exampletextprop>
+      <D:resourcetype />
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+  <D:propstat>
+    <D:prop>
+      <D1:exampleintprop xmlns:D1="DAVtest:" />
+    </D:prop>
+    <D:status>HTTP/1.1 403 Forbidden</D:status>
+  </D:propstat>
+</D:response></D:multistatus>
+        """)
+
+        # Note that the `{DAVtest:}exampleintprop' was not rendered because
+        # we didn't give access to this property is our dummy security proxy.
+        self.assertEqual(len(self.errUtility.errors), 1)
+        self.assert_(
+            isinstance(self.errUtility.errors[0][0][1],
+                       zope.security.interfaces.Forbidden),
+            "We didn't raise an `Forbidden' error.")
+
+    def test_handlePropfind_forbiddenRequestedResourceProperty(self):
+        # Remove access to the `exampleintprop' on the collection['r1']
+        # resource and render this resource as the requested resource. But
+        # since we get a forbidden error we render all the properties but the
+        # `exampleintprop' is still rendered with no value and a `403' status.
+        self.collection.data["r1"] = zope.security.checker.ProxyFactory(
+            self.collection["r1"], zope.security.checker.Checker({
+                "text": zope.security.checker.CheckerPublic}))
+
+        request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
+        request.processInputs()
+        propf = PROPFIND(self.collection["r1"], request)
+
+        result = propf.PROPFIND()
+
+        self.assertEqual(request.response.getStatus(), 207)
+        assertXMLEqualIgnoreOrdering(result, """<D:multistatus xmlns:D="DAV:">
+<D:response>
+  <D:href>/collection/r1</D:href>
+  <D:propstat>
+    <D:prop>
+      <D1:exampletextprop xmlns:D1="DAVtest:">some text - r1</D1:exampletextprop>
+      <D:resourcetype />
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+  <D:propstat>
+    <D:prop>
+      <D1:exampleintprop xmlns:D1="DAVtest:" />
+    </D:prop>
+    <D:status>HTTP/1.1 403 Forbidden</D:status>
+  </D:propstat>
+</D:response></D:multistatus>
+        """)
+
+        # Note that the `{DAVtest:}exampleintprop' was not rendered because
+        # we didn't give access to this property is our dummy security proxy.
+        self.assertEqual(len(self.errUtility.errors), 1)
+        self.assert_(
+            isinstance(self.errUtility.errors[0][0][1],
+                       zope.security.interfaces.Forbidden),
+            "We didn't raise an `Forbidden' error.")
+
+    def test_handlePropfind_unauthorizedRequestedResourceProperty(self):
+        # Remove access to the `exampleintprop' on the collection['r1']
+        # resource and render this resource as the requested resource. Since
+        # we requested to render all properties we silently ignore the
+        # `exampleintprop' property. But we still log this error.
+        self.collection.data["r1"] = zope.security.checker.ProxyFactory(
+            self.collection["r1"], SecurityChecker({
+                "__name__": zope.security.checker.CheckerPublic,
+                "__parent__": zope.security.checker.CheckerPublic,
+                "text": zope.security.checker.CheckerPublic}))
+
+        request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
+        request.processInputs()
+        propf = PROPFIND(self.collection["r1"], request)
+
+        result = propf.PROPFIND()
+
+        self.assertEqual(request.response.getStatus(), 207)
+        assertXMLEqualIgnoreOrdering(result, """<D:multistatus xmlns:D="DAV:">
+<D:response>
+  <D:href>/collection/r1</D:href>
+  <D:propstat>
+    <D:prop>
+      <D1:exampletextprop xmlns:D1="DAVtest:">some text - r1</D1:exampletextprop>
+      <D:resourcetype />
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+</D:response></D:multistatus>
+        """)
+
+        # Note that the `{DAVtest:}exampleintprop' was not rendered because
+        # we didn't give access to this property is our dummy security proxy.
+        self.assertEqual(len(self.errUtility.errors), 1)
+        self.assert_(
+            isinstance(self.errUtility.errors[0][0][1],
+                       zope.security.interfaces.Unauthorized),
+            "We didn't raise an `Unauthorized' error.")
+
+    def test_handlePropfind_forbiddenCollection_listing(self):
+        # Remove permission to the collections `values' method. We get a
+        # `Forbidden' exception in this case. Since this folder isn't the
+        # request resource but a sub-resource of it we ignore the folder
+        # listing on this folder so that users can continue managing the
+        # content the content they do have access to but we still render
+        # the properties of this folder since we do have access to it.
+        self.collection.data["c"] = zope.security.checker.ProxyFactory(
+            self.collection["c"], zope.security.checker.Checker({}))
+        self.addChecker(self.collection["r1"], zope.security.checker.Checker({
+            "text": zope.security.checker.CheckerPublic,
+            "intprop": zope.security.checker.CheckerPublic}))
+        collection = self.addChecker(
+            self.collection, zope.security.checker.Checker({
+                "values": zope.security.checker.CheckerPublic}))
+
+        request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
+        request.processInputs()
+        propf = PROPFIND(collection, request)
+
+        result = propf.PROPFIND()
+
+        self.assertEqual(request.response.getStatus(), 207)
+        assertXMLEqualIgnoreOrdering(result, """<D:multistatus xmlns:D="DAV:">
+<D:response>
+  <D:href>/collection/</D:href>
+  <D:propstat>
+    <D:prop>
+      <D:resourcetype>
+        <D:collection />
+      </D:resourcetype>
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+</D:response>
+<D:response>
+  <D:href>/collection/c/</D:href>
+  <D:propstat>
+    <D:prop>
+      <D:resourcetype>
+        <D:collection />
+      </D:resourcetype>
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+</D:response>
+<D:response>
+  <D:href>/collection/r1</D:href>
+  <D:propstat>
+    <D:prop>
+      <D1:exampletextprop xmlns:D1="DAVtest:">some text - r1</D1:exampletextprop>
+      <D:resourcetype />
+      <D1:exampleintprop xmlns:D1="DAVtest:">2</D1:exampleintprop>
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+</D:response></D:multistatus>
+        """)
+
+        self.assertEqual(len(self.errUtility.errors), 1)
+        self.assert_(
+            isinstance(self.errUtility.errors[0][0][1],
+                       zope.security.interfaces.Forbidden),
+            "We didn't raise an `Forbidden' error.")
+
+    def test_handlePropfind_unauthorizedCollection_listing(self):
+        # Remove permission to the collections `values' method for the `c'
+        # resource. In this case the user isn't presented with a 401 error
+        # but instead we ignore the the listing of this folder and return
+        # the information on the rest of the documents. The reason is that
+        # the user mightnot have access to this folder so we shouldn't
+        # interfere with there access to the folder. This only applies to a
+        # PROPFIND request with depth equal to `0' or `infinity'.
+        self.collection.data["c"] = zope.security.checker.ProxyFactory(
+            self.collection["c"], SecurityChecker({
+                "__name__": zope.security.checker.CheckerPublic,
+                "__parent__": zope.security.checker.CheckerPublic,
+            }))
+        self.addChecker(self.collection["r1"], zope.security.checker.Checker({
+            "text": zope.security.checker.CheckerPublic,
+            "intprop": zope.security.checker.CheckerPublic}))
+        collection = self.addChecker(
+            self.collection, zope.security.checker.Checker({
+                "values": zope.security.checker.CheckerPublic}))
+
+        request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
+        request.processInputs()
+        propf = PROPFIND(collection, request)
+
+        result = propf.PROPFIND()
+
+        self.assertEqual(request.response.getStatus(), 207)
+        assertXMLEqualIgnoreOrdering(result, """<D:multistatus xmlns:D="DAV:">
+<D:response>
+  <D:href>/collection/</D:href>
+  <D:propstat>
+    <D:prop>
+      <D:resourcetype>
+        <D:collection />
+      </D:resourcetype>
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+</D:response>
+<D:response>
+  <D:href>/collection/c/</D:href>
+  <D:propstat>
+    <D:prop>
+      <D:resourcetype>
+        <D:collection />
+      </D:resourcetype>
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+</D:response>
+<D:response>
+  <D:href>/collection/r1</D:href>
+  <D:propstat>
+    <D:prop>
+      <D1:exampletextprop xmlns:D1="DAVtest:">some text - r1</D1:exampletextprop>
+      <D:resourcetype />
+      <D1:exampleintprop xmlns:D1="DAVtest:">2</D1:exampleintprop>
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+</D:response></D:multistatus>
+        """)
+
+        # Make sure that we handled the excepted failure.
+        self.assertEqual(len(self.errUtility.errors), 1)
+        self.assert_(
+            isinstance(self.errUtility.errors[0][0][1],
+                       zope.security.interfaces.Unauthorized),
+            "We didn't raise an `Unauthorized' error.")
+
+    def test_handlePropfind_forbiddenRootCollection_listing(self):
+        # Remove permission to the root collections `values' method. This
+        # raises an Forbidden exception - not sure why not an Unauthorized
+        # exception. In this case we just return the properties of the
+        # resource - since the user can't list the folder and might not have
+        # the permissions to do so.
+        # XXX - this seems correct but ...
+        self.addChecker(self.collection["r1"], zope.security.checker.Checker({
+            "text": zope.security.checker.CheckerPublic,
+            "intprop": zope.security.checker.CheckerPublic}))
+        collection = self.addChecker(
+            self.collection, zope.security.checker.Checker({}))
+
+        request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
+        request.processInputs()
+        propf = PROPFIND(collection, request)
+
+        result = propf.PROPFIND()
+
+        self.assertEqual(request.response.getStatus(), 207)
+        assertXMLEqualIgnoreOrdering(result, """<D:multistatus xmlns:D="DAV:">
+<D:response>
+  <D:href>/collection/</D:href>
+  <D:propstat>
+    <D:prop>
+      <D:resourcetype>
+        <D:collection />
+      </D:resourcetype>
+    </D:prop>
+    <D:status>HTTP/1.1 200 Ok</D:status>
+  </D:propstat>
+</D:response></D:multistatus>
+        """)
+
+        self.assertEqual(len(self.errUtility.errors), 1)
+        self.assert_(
+            isinstance(self.errUtility.errors[0][0][1],
+                       zope.security.interfaces.Forbidden),
+            "We didn't raise an `Forbidden' error.")
+
+    def test_handlePropfind_unauthorizedRootCollection_listing(self):
+        # Remove permission to the root collections `values' method. This
+        # raises an Unauthorized exception. In this case we just return the
+        # properties of the resource - since the user can't list the folder
+        # and might not have the permissions to do so. Even though we could
+        # have just rendered the properties on this folder.
+        self.addChecker(self.collection["r1"], zope.security.checker.Checker({
+            "text": zope.security.checker.CheckerPublic,
+            "intprop": zope.security.checker.CheckerPublic}))
+        collection = zope.security.checker.ProxyFactory(
+            self.collection, SecurityChecker({
+                "__name__": zope.security.checker.CheckerPublic,
+                "__parent__": zope.security.checker.CheckerPublic,
+            }))
+
+        request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
+        request.processInputs()
+        propf = PROPFIND(collection, request)
+
+        self.assertRaises(zope.security.interfaces.Unauthorized, propf.PROPFIND)
+
+        # Since we raise the `Unauthorized' exception the publisher will
+        # automatically log this exception when we handle the exception.
+        self.assertEqual(len(self.errUtility.errors), 0)
+
+    def test_handlePropfindResource_unauthorizedResource(self):
+        # This is an edge case. By removing access to the `__name__' and
+        # `__parent__' attributes on the `collection["r1"]' resource we
+        # can no longer generate a URL for this resource and get an
+        # Unauthorized exception. Can't do much about this but it doesn't
+        # really come up.
+        self.collection.data["r1"] = zope.security.checker.ProxyFactory(
+            self.collection["r1"], SecurityChecker({
+                "__name__": 1, # disable access to the `__name__' attribute
+                "__parent__": 1,
+                "text": zope.security.checker.CheckerPublic,
+                "intprop": zope.security.checker.CheckerPublic}))
+        self.addChecker(
+            self.collection["c"]["r2"], zope.security.checker.Checker({
+                "text": zope.security.checker.CheckerPublic,
+                "intprop": zope.security.checker.CheckerPublic}))
+        collection = self.addChecker(
+            self.collection, zope.security.checker.Checker({
+                "values": zope.security.checker.CheckerPublic}))
+
+        request = z3c.dav.publisher.WebDAVRequest(StringIO(""), {})
+        request.processInputs()
+        propf = PROPFIND(collection, request)
+
+        self.assertRaises(
+            zope.security.interfaces.Unauthorized, propf.PROPFIND)
+
+        self.assertEqual(len(self.errUtility.errors), 0)
+
+
 def test_suite():
     return unittest.TestSuite((
         unittest.makeSuite(PROPFINDBodyTestCase),
         unittest.makeSuite(PROPFINDTestRender),
-        unittest.makeSuite(PROPFINDRecuseTest),
+        unittest.makeSuite(PROPFINDSecurityTestCase),
         ))



More information about the Checkins mailing list