Merge pull request #84 from zopefoundation/hotfix-copy-master

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

CHANGES.rst

@@ -11,6 +11,8 @@ https://zope.readthedocs.io/en/2.13/CHANGES.html
 Bugs Fixed
 ++++++++++
 
+- Don't copy items the user is not allowed to view.
+  From Products.PloneHotfix20161129.  [maurits]
 
 Features Added
 ++++++++++++++

src/OFS/CopySupport.py

@@ -15,6 +15,7 @@
 
 from json import dumps
 from json import loads
+import logging
 import re
 import tempfile
 import warnings
@@ -56,7 +57,7 @@ class CopyError(Exception):
     pass
 
 copy_re = re.compile('^copy([0-9]*)_of_(.*)')
-
+logger = logging.getLogger('OFS')
 _marker = []
 
 
@@ -531,7 +532,42 @@ def _getCopy(self, container):
         f.seek(0)
         ob = container._p_jar.importFile(f)
         f.close()
-        return ob
+        # 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

@@ -405,6 +405,34 @@ def test_copy_cant_create_target_metatype_not_supported(self):
         self._assertCopyErrorUnauth(
             folder2.manage_pasteObjects, cookie, 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()
         folder2.all_meta_types = FILE_META_TYPES