[Zope-Checkins] SVN: Zope/trunk/src/webdav/ - fixed permission check and error handling in DeleteCollection

Yvo Schubbe y.2010 at wcm-solutions.de
Tue Dec 28 07:16:03 EST 2010


Log message for revision 119195:
  - fixed permission check and error handling in DeleteCollection

Changed:
  U   Zope/trunk/src/webdav/Collection.py
  U   Zope/trunk/src/webdav/davcmds.py
  U   Zope/trunk/src/webdav/tests/test_davcmds.py

-=-
Modified: Zope/trunk/src/webdav/Collection.py
===================================================================
--- Zope/trunk/src/webdav/Collection.py	2010-12-28 12:14:38 UTC (rev 119194)
+++ Zope/trunk/src/webdav/Collection.py	2010-12-28 12:16:02 UTC (rev 119195)
@@ -89,7 +89,7 @@
         url = urlfix(REQUEST['URL'], 'DELETE')
         name = unquote(filter(None, url.split( '/'))[-1])
         parent = self.aq_parent
-        user = getSecurityManager().getUser()
+        sm = getSecurityManager()
         token = None
 
 #        if re.match("/Control_Panel",REQUEST['PATH_INFO']):
@@ -119,7 +119,7 @@
                 if ifhdr.find(tok) > -1:
                     token = tok
         cmd = DeleteCollection()
-        result = cmd.apply(self, token, user, REQUEST['URL'])
+        result = cmd.apply(self, token, sm, REQUEST['URL'])
 
         if result:
             # There were conflicts, so we need to report them

Modified: Zope/trunk/src/webdav/davcmds.py
===================================================================
--- Zope/trunk/src/webdav/davcmds.py	2010-12-28 12:14:38 UTC (rev 119194)
+++ Zope/trunk/src/webdav/davcmds.py	2010-12-28 12:16:02 UTC (rev 119195)
@@ -18,6 +18,7 @@
 from urllib import quote
 
 import transaction
+from AccessControl.Permissions import delete_objects
 from AccessControl.SecurityManagement import getSecurityManager
 from Acquisition import aq_base
 from Acquisition import aq_parent
@@ -26,11 +27,12 @@
 from zExceptions import Forbidden
 
 from webdav.common import absattr
+from webdav.common import isDavCollection
+from webdav.common import Locked
+from webdav.common import PreconditionFailed
 from webdav.common import urlbase
 from webdav.common import urlfix
 from webdav.common import urljoin
-from webdav.common import isDavCollection
-from webdav.common import PreconditionFailed
 from webdav.interfaces import IWriteLock
 from webdav.LockItem import LockItem
 from webdav.xmltools import XmlParser
@@ -492,7 +494,7 @@
     checking *all* descendents (deletes on collections are always of depth
     infinite) for locks and if the locks match. """
 
-    def apply(self, obj, token, user, url=None, result=None, top=1):
+    def apply(self, obj, token, sm, url=None, result=None, top=1):
         if result is None:
             result = StringIO()
             url = urlfix(url, 'DELETE')
@@ -502,7 +504,7 @@
         parent = aq_parent(obj)
 
         islockable = IWriteLock.providedBy(obj)
-        if parent and (not user.has_permission('Delete objects', parent)):
+        if parent and (not sm.checkPermission(delete_objects, parent)):
             # User doesn't have permission to delete this object
             errmsg = "403 Forbidden"
         elif islockable and obj.wl_isLocked():
@@ -514,8 +516,10 @@
 
         if errmsg:
             if top and (not iscol):
-                err = errmsg[4:]
-                raise err
+                if errmsg == "403 Forbidden":
+                    raise Forbidden()
+                if errmsg == "423 Locked":
+                    raise Locked()
             elif not result.getvalue():
                 # We haven't had any errors yet, so our result is empty
                 # and we need to set up the XML header
@@ -530,7 +534,7 @@
                 dflag = hasattr(ob,'_p_changed') and (ob._p_changed == None)
                 if hasattr(ob, '__dav_resource__'):
                     uri = urljoin(url, absattr(ob.getId()))
-                    self.apply(ob, token, user, uri, result, top=0)
+                    self.apply(ob, token, sm, uri, result, top=0)
                     if dflag:
                         ob._p_deactivate()
         if not top:

Modified: Zope/trunk/src/webdav/tests/test_davcmds.py
===================================================================
--- Zope/trunk/src/webdav/tests/test_davcmds.py	2010-12-28 12:14:38 UTC (rev 119194)
+++ Zope/trunk/src/webdav/tests/test_davcmds.py	2010-12-28 12:16:02 UTC (rev 119195)
@@ -1,26 +1,44 @@
 import unittest
 
+from AccessControl.SecurityManagement import getSecurityManager
+from AccessControl.SecurityManagement import newSecurityManager
+from AccessControl.SecurityManagement import noSecurityManager
+from AccessControl.SecurityManager import setSecurityPolicy
+from zExceptions import Forbidden
+from zope.interface import implements
+
+
+class _DummySecurityPolicy(object):
+
+    def checkPermission(self, permission, object, context):
+        return False
+
+
+class _DummyContent(object):
+
+    from webdav.interfaces import IWriteLock
+    implements(IWriteLock)
+
+    def __init__(self, token=None):
+        self.token = token
+
+    def wl_hasLock(self, token):
+        return self.token == token
+
+    def wl_isLocked(self):
+        return bool(self.token)
+
+
 class TestUnlock(unittest.TestCase):
 
     def _getTargetClass(self):
         from webdav.davcmds import Unlock
+
         return Unlock
 
-    def _makeOne(self):
-        klass = self._getTargetClass()
-        return klass()
+    def _makeOne(self, *args, **kw):
+        return self._getTargetClass()(*args, **kw)
 
-    def _makeLockable(self, locktoken):
-        from webdav.interfaces import IWriteLock
-        from zope.interface import implements
-        class Lockable:
-            implements(IWriteLock)
-            def __init__(self, token):
-                self.token = token
-            def wl_hasLock(self, token):
-                return self.token == token
-        return Lockable(locktoken)
-
     def test_apply_bogus_lock(self):
         """
         When attempting to unlock a resource with a token that the
@@ -36,7 +54,7 @@
         This was caught by litmus locks.notowner_lock test #10.
         """
         inst = self._makeOne()
-        lockable = self._makeLockable(None)
+        lockable = _DummyContent()
         result = inst.apply(lockable, 'bogus',
                             url='http://example.com/foo/UNLOCK', top=0)
         result = result.getvalue()
@@ -44,15 +62,16 @@
             result.find('<d:status>HTTP/1.1 400 Bad Request</d:status>'),
             -1)
 
+
 class TestPropPatch(unittest.TestCase):
 
     def _getTargetClass(self):
         from webdav.davcmds import PropPatch
+
         return PropPatch
 
-    def _makeOne(self, request):
-        klass = self._getTargetClass()
-        return klass(request)
+    def _makeOne(self, *args, **kw):
+        return self._getTargetClass()(*args, **kw)
 
     def test_parse_xml_property_values_with_namespaces(self):
         """
@@ -79,8 +98,50 @@
         self.assertEqual(len(inst.values), 1)
         self.assertEqual(inst.values[0][3]['__xml_attrs__'], {})
 
+
+class TestDeleteCollection(unittest.TestCase):
+
+    def _getTargetClass(self):
+        from webdav.davcmds import DeleteCollection
+
+        return DeleteCollection
+
+    def _makeOne(self, *args, **kw):
+        return self._getTargetClass()(*args, **kw)
+
+    def setUp(self):
+        self._oldPolicy = setSecurityPolicy(_DummySecurityPolicy())
+        newSecurityManager(None, object())
+
+    def tearDown(self):
+        noSecurityManager()
+        setSecurityPolicy(self._oldPolicy)
+
+    def test_apply_no_parent(self):
+        cmd = self._makeOne()
+        obj = _DummyContent()
+        sm = getSecurityManager()
+        self.assertEqual(cmd.apply(obj, None, sm, '/foo/DELETE'), '')
+
+    def test_apply_no_col_Forbidden(self):
+        cmd = self._makeOne()
+        obj = _DummyContent()
+        obj.__parent__ = _DummyContent()
+        sm = getSecurityManager()
+        self.assertRaises(Forbidden, cmd.apply, obj, None, sm, '/foo/DELETE')
+
+    def test_apply_no_col_Locked(self):
+        from webdav.common import Locked
+
+        cmd = self._makeOne()
+        obj = _DummyContent('LOCKED')
+        sm = getSecurityManager()
+        self.assertRaises(Locked, cmd.apply, obj, None, sm, '/foo/DELETE')
+
+
 def test_suite():
     return unittest.TestSuite((
         unittest.makeSuite(TestUnlock),
         unittest.makeSuite(TestPropPatch),
+        unittest.makeSuite(TestDeleteCollection),
         ))



More information about the Zope-Checkins mailing list