Merge pull request #83 from zopefoundation/hotfix-copy-213

Don't copy items the user is not allowed to view. [2.13]

doc/CHANGES.rst

@@ -8,6 +8,9 @@ http://docs.zope.org/zope2/
 2.13.25 (unreleased)
 --------------------
 
+- Don't copy items the user is not allowed to view.
+  From Products.PloneHotfix20161129.  [maurits]
+
 - Quote variables in manage_tabs and manage_container to avoid XSS.
   From Products.PloneHotfix20160830.  [maurits]
 

src/OFS/CopySupport.py

@@ -16,6 +16,7 @@
 from cgi import escape
 from marshal import dumps
 from marshal import loads
+import logging
 import re
 import sys
 import tempfile
@@ -61,8 +62,8 @@ class CopyError(Exception):
     pass
 
 copy_re = re.compile('^copy([0-9]*)_of_(.*)')
-
-_marker=[]
+logger = logging.getLogger('OFS')
+_marker = []
 
 
 class CopyContainer(Base):
@@ -587,7 +588,42 @@ class CopySource(Base):
         f.seek(0)
         ob=container._p_jar.importFile(f)
         f.close()
+        # Cleanup the copy.  It may contain private objects that the current
+        # user is not allowed to see.
+        sm = getSecurityManager()
+        if not sm.checkPermission('View', self):
+            # The user is not allowed to view the object that is currently
+            # being copied, so it makes no sense to check any of its sub
+            # objects.  It probably means we are in a test.
             return ob
+        return self._cleanupCopy(ob, container)
+
+    def _cleanupCopy(self, cp, container):
+        sm = getSecurityManager()
+        ob = aq_base(self)
+        if hasattr(ob, 'objectIds'):
+            for k in self.objectIds():
+                v = self._getOb(k)
+                if not sm.checkPermission('View', v):
+                    # We do not use cp._delObject, because this would fire
+                    # events that are needless for objects that are not even in
+                    # an Acquisition chain yet.
+                    logger.warn(
+                        'While copying %s to %s, removed %s from copy '
+                        'because user is not allowed to view the original.',
+                        '/'.join(self.getPhysicalPath()),
+                        '/'.join(container.getPhysicalPath()),
+                        '/'.join(v.getPhysicalPath())
+                    )
+                    cp._delOb(k)
+                    # We need to cleanup the internal objects list, even when
+                    # in some implementations this is always an empty tuple.
+                    cp._objects = tuple([
+                        i for i in cp._objects if i['id'] != k])
+                else:
+                    # recursively check
+                    v._cleanupCopy(cp._getOb(k), container)
+        return cp
 
     def _postCopy(self, container, op=0):
         # Called after the copy is finished to accomodate special cases.

src/OFS/tests/testCopySupport.py

@@ -428,6 +428,34 @@ class TestCopySupportSecurity( CopySupportTestBase ):
                                    , ce_regex='Not Supported'
                                    )
 
+    def test_copy_cant_copy_invisible_items(self):
+        # User can view folder1.
+        # User cannot view private file in folder1.
+        # When user copies folder1, the private file should not be copied,
+        # because the user would get the Owner role on the copy,
+        # and be able to view it anyway.
+        from AccessControl.Permissions import add_folders
+        from AccessControl.Permissions import view
+
+        folder1, folder2 = self._initFolders()
+        manage_addFile(folder1, 'private',
+                       file='', content_type='text/plain')
+        manage_addFile(folder1, 'public',
+                       file='', content_type='text/plain')
+        folder1.private.manage_permission(view, roles=(), acquire=0)
+        folder2.manage_permission(add_folders, roles=('Anonymous',), acquire=1)
+
+        copy_info = folder2.manage_pasteObjects(
+            self.app.manage_copyObjects('folder1')
+        )
+
+        new_id = copy_info[0]['new_id']
+        new_folder = folder2[new_id]
+        # The private item should not be in the copy.
+        self.assertTrue('private' not in new_folder.objectIds())
+        # There is nothing wrong with copying the public item.
+        self.assertTrue('public' in new_folder.objectIds())
+
     def test_move_baseline( self ):
 
         folder1, folder2 = self._initFolders()