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), )) _______________________________________________ Zope-Checkins maillist - Zope-Checkins@zope.org https://mail.zope.org/mailman/listinfo/zope-checkins