From e7af9705cb4aadca4c02f1e8d70adaf4ea9af693 Mon Sep 17 00:00:00 2001 From: Rohan Shaw Date: Wed, 14 May 2025 21:15:29 +0530 Subject: [PATCH 1/7] feat(recyclebin): update permissions for Recycle Bin management and add tests for permission checks --- Products/CMFPlone/browser/configure.zcml | 4 +- .../controlpanel/browser/configure.zcml | 2 +- .../CMFPlone/controlpanel/permissions.zcml | 5 +++ .../CMFPlone/profiles/default/actions.xml | 2 +- .../profiles/default/controlpanel.xml | 2 +- .../CMFPlone/profiles/default/rolemap.xml | 6 +++ Products/CMFPlone/tests/test_recyclebin.py | 40 +++++++++++++++++++ 7 files changed, 56 insertions(+), 5 deletions(-) diff --git a/Products/CMFPlone/browser/configure.zcml b/Products/CMFPlone/browser/configure.zcml index 8dbe1e4320..8ddf4fdc82 100644 --- a/Products/CMFPlone/browser/configure.zcml +++ b/Products/CMFPlone/browser/configure.zcml @@ -316,14 +316,14 @@ name="recyclebin" for="plone.base.interfaces.siteroot.IPloneSiteRoot" class=".recyclebin.RecycleBinView" - permission="cmf.ManagePortal" + permission="Products.CMFPlone.ManageRecycleBin" /> diff --git a/Products/CMFPlone/controlpanel/browser/configure.zcml b/Products/CMFPlone/controlpanel/browser/configure.zcml index 1a7dc5a9c8..ccbc3d46b3 100644 --- a/Products/CMFPlone/controlpanel/browser/configure.zcml +++ b/Products/CMFPlone/controlpanel/browser/configure.zcml @@ -354,7 +354,7 @@ name="plone-recyclebin" for="Products.CMFPlone.interfaces.IPloneSiteRoot" class="Products.CMFPlone.controlpanel.browser.recyclebin.RecyclebinControlPanelView" - permission="cmf.ManagePortal" + permission="Products.CMFPlone.ManageRecycleBin" /> diff --git a/Products/CMFPlone/controlpanel/permissions.zcml b/Products/CMFPlone/controlpanel/permissions.zcml index 7f57dd164f..998b6f5187 100644 --- a/Products/CMFPlone/controlpanel/permissions.zcml +++ b/Products/CMFPlone/controlpanel/permissions.zcml @@ -89,4 +89,9 @@ title="Manage Context Aliases" /> + + diff --git a/Products/CMFPlone/profiles/default/actions.xml b/Products/CMFPlone/profiles/default/actions.xml index c857d46f0c..4728c9ead4 100644 --- a/Products/CMFPlone/profiles/default/actions.xml +++ b/Products/CMFPlone/profiles/default/actions.xml @@ -508,7 +508,7 @@ string:plone-delete portal/@@recyclebin-enabled|nothing - + True diff --git a/Products/CMFPlone/profiles/default/controlpanel.xml b/Products/CMFPlone/profiles/default/controlpanel.xml index b137266cdf..162ca958d9 100644 --- a/Products/CMFPlone/profiles/default/controlpanel.xml +++ b/Products/CMFPlone/profiles/default/controlpanel.xml @@ -362,6 +362,6 @@ url_expr="string:${portal_url}/@@plone-recyclebin" visible="True" > - Manage portal + Manage Recycle Bin diff --git a/Products/CMFPlone/profiles/default/rolemap.xml b/Products/CMFPlone/profiles/default/rolemap.xml index 1336b14e06..ac9d4f0a35 100644 --- a/Products/CMFPlone/profiles/default/rolemap.xml +++ b/Products/CMFPlone/profiles/default/rolemap.xml @@ -378,5 +378,11 @@ + + + + diff --git a/Products/CMFPlone/tests/test_recyclebin.py b/Products/CMFPlone/tests/test_recyclebin.py index bc85796c1a..bc4946780c 100644 --- a/Products/CMFPlone/tests/test_recyclebin.py +++ b/Products/CMFPlone/tests/test_recyclebin.py @@ -514,3 +514,43 @@ def test_restore_with_name_conflict(self): with self.assertRaises(ValueError): # Restore the item self.recyclebin.restore_item(recycle_id) + + +class RecycleBinPermissionTests(RecycleBinTestCase): + """Tests for RecycleBin permission checks""" + + def test_recyclebin_permission(self): + """Test permission checks for the recycle bin""" + # As Manager role, should have access + self.assertTrue(self.recyclebin.check_permission()) + + # Create a test user with limited permissions + from AccessControl import getSecurityManager + from AccessControl.SecurityManagement import newSecurityManager + from AccessControl.SecurityManagement import setSecurityManager + + # Store the current security manager + old_security_manager = getSecurityManager() + + try: + # Create a regular Member user + self.portal.acl_users._doAddUser('testuser', 'password', ['Member'], []) + + # Log in as the test user + user = self.portal.acl_users.getUser('testuser') + user = user.__of__(self.portal.acl_users) + newSecurityManager(None, user) + + # Check permission - should be False for a regular member + self.assertFalse(self.recyclebin.check_permission()) + + # Grant the permission to the Member role + from Products.CMFCore.permissions import setDefaultRoles + setDefaultRoles('Products.CMFPlone.ManageRecycleBin', ('Manager', 'Site Administrator', 'Member',)) + + # Now the test user should have permission + self.assertTrue(self.recyclebin.check_permission()) + + finally: + # Restore the security manager + setSecurityManager(old_security_manager) From ea8eb7be6a98c48ef03e15f6ea333730215a7a67 Mon Sep 17 00:00:00 2001 From: Rohan Shaw Date: Wed, 14 May 2025 21:21:55 +0530 Subject: [PATCH 2/7] changelog --- news/4172.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/4172.feature diff --git a/news/4172.feature b/news/4172.feature new file mode 100644 index 0000000000..582645463e --- /dev/null +++ b/news/4172.feature @@ -0,0 +1 @@ +Introduces custom permission to manage recyclebin @rohnsha0 \ No newline at end of file From 0b57eaf9c05a3ebc4835f94987c0c19ca0f9bc3a Mon Sep 17 00:00:00 2001 From: Rohan Shaw Date: Wed, 14 May 2025 22:30:36 +0530 Subject: [PATCH 3/7] feat(recyclebin): enhance permission checks for managing and restoring items in the recycle bin --- Products/CMFPlone/browser/recyclebin.py | 29 ++++++- Products/CMFPlone/recyclebin.py | 101 ++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 1 deletion(-) diff --git a/Products/CMFPlone/browser/recyclebin.py b/Products/CMFPlone/browser/recyclebin.py index f702ddb861..0189221c01 100644 --- a/Products/CMFPlone/browser/recyclebin.py +++ b/Products/CMFPlone/browser/recyclebin.py @@ -1,3 +1,4 @@ +from AccessControl import getSecurityManager from datetime import datetime from plone.base import PloneMessageFactory as _ from plone.base.interfaces.recyclebin import IRecycleBin @@ -34,7 +35,11 @@ class RecycleBinView(form.Form): def __init__(self, context, request): super().__init__(context, request) self.recycle_bin = getUtility(IRecycleBin) - + + def _get_context(self): + """Get the context (Plone site)""" + return self.context + def update(self): super().update() @@ -317,6 +322,10 @@ def get_items(self): # Get filters early to avoid multiple lookups during the loop filter_type = self.get_filter_type() search_query = self.get_search_query().lower() + + # Check if the user has full access to the recycle bin + # (includes permissions check and Manager/Site Administrator roles) + has_full_access = self.recycle_bin.check_permission(check_roles=True) # Create a list of all items that are children of a parent in the recycle bin child_items_to_exclude = [] @@ -334,6 +343,11 @@ def get_items(self): for item in items: if item.get("id") not in child_items_to_exclude: + # If user doesn't have full access, show only items they can restore + if not has_full_access: + if not self.recycle_bin.can_restore(item["recycle_id"]): + continue + # Apply type filtering if filter_type and item.get("type") != filter_type: continue @@ -410,6 +424,19 @@ def update(self): f"{self.context.absolute_url()}/@@recyclebin" ) return + + # Check if the user has permission to access this item + if not self.recycle_bin.can_restore(self.item_id): + logger.debug(f"User does not have permission to view item {self.item_id}") + message = translate( + _("You don't have permission to access this item in the recycle bin."), + context=self.request, + ) + IStatusMessage(self.request).addStatusMessage(message, type="error") + self.request.response.redirect( + f"{self.context.absolute_url()}/@@recyclebin" + ) + return # Handle restoration of children if "restore.child" in self.request.form: diff --git a/Products/CMFPlone/recyclebin.py b/Products/CMFPlone/recyclebin.py index 90d81863d0..0b72c6a9f8 100644 --- a/Products/CMFPlone/recyclebin.py +++ b/Products/CMFPlone/recyclebin.py @@ -125,6 +125,9 @@ def get_items_sorted_by_date(self, reverse=True): class RecycleBin: """Stores deleted content items""" + # Permission for managing recycle bin + MANAGE_RECYCLEBIN = "Products.CMFPlone.ManageRecycleBin" + def __init__(self): """Initialize the recycle bin utility @@ -165,6 +168,104 @@ def is_enabled(self): return settings.recycling_enabled except (KeyError, AttributeError): return False + + def check_permission(self, check_roles=True): + """Check if the current user has permission to manage the recycle bin + + Args: + check_roles: If True, also check for Manager or Site Administrator roles + + Returns: + Boolean indicating permission + """ + context = self._get_context() + sm = getSecurityManager() + + # Check for explicit permission first + has_permission = sm.checkPermission(self.MANAGE_RECYCLEBIN, context) + if has_permission: + return True + + # If no explicit permission and we're not checking roles, return False + if not check_roles: + return False + + # Additional check for Manager or Site Administrator roles + user = sm.getUser() + user_roles = user.getRolesInContext(context) + return 'Manager' in user_roles or 'Site Administrator' in user_roles + + def can_restore(self, item_id): + """Check if the current user can restore an item + + A user can restore an item if: + 1. They have the ManageRecycleBin permission, or + 2. They are a Manager or Site Administrator, or + 3. They are the owner of the item, or + 4. They deleted the item + + Args: + item_id: The ID of the item in the recycle bin + + Returns: + Boolean indicating if the user can restore the item + """ + # Managers and Site Administrators can always restore items + if self.check_permission(check_roles=True): + return True + + # Check if user is the owner of the item + item_data = self.get_item(item_id) + if not item_data: + return False + + # Get the object + obj = item_data.get("object") + if not obj: + return False + + # Get the current user ID + current_user_id = getSecurityManager().getUser().getId() + if not current_user_id: + return False + + # Check if current user is the content owner + try: + if hasattr(obj, "getOwnerTuple"): + owner_tuple = obj.getOwnerTuple() + if owner_tuple and owner_tuple[1] == current_user_id: + return True + elif hasattr(obj, "getOwner"): + owner = obj.getOwner() + if owner and owner.getId() == current_user_id: + return True + except Exception: + # If there's an error getting the owner, fall back to checking workflow history + pass + + # If not owner, check workflow history to see who deleted the item + if not hasattr(obj, "workflow_history"): + return False + + workflow_tool = getToolByName(self._get_context(), "portal_workflow") + chains = workflow_tool.getChainFor(obj) + + if not chains: + return False + + workflow_id = chains[0] + history = obj.workflow_history.get(workflow_id, ()) + + if not history: + return False + + # Look for the deletion entry + for entry in reversed(history): + if entry.get("action") == "Moved to recycle bin": + # If the current user is the one who deleted it, they can restore it + return entry.get("actor") == current_user_id + + return False def _get_item_title(self, obj, item_type=None): """Helper method to get a meaningful title for an item""" From 21ff8ce109f5c33d55c8ebfe859b1db513c9c612 Mon Sep 17 00:00:00 2001 From: Rohan Shaw Date: Wed, 14 May 2025 22:35:37 +0530 Subject: [PATCH 4/7] refactor(recyclebin): lint --- Products/CMFPlone/browser/recyclebin.py | 11 ++- Products/CMFPlone/recyclebin.py | 46 ++++++------ Products/CMFPlone/tests/test_recyclebin.py | 84 +++++++++++----------- 3 files changed, 72 insertions(+), 69 deletions(-) diff --git a/Products/CMFPlone/browser/recyclebin.py b/Products/CMFPlone/browser/recyclebin.py index 0189221c01..83f34e1a48 100644 --- a/Products/CMFPlone/browser/recyclebin.py +++ b/Products/CMFPlone/browser/recyclebin.py @@ -1,4 +1,3 @@ -from AccessControl import getSecurityManager from datetime import datetime from plone.base import PloneMessageFactory as _ from plone.base.interfaces.recyclebin import IRecycleBin @@ -35,11 +34,11 @@ class RecycleBinView(form.Form): def __init__(self, context, request): super().__init__(context, request) self.recycle_bin = getUtility(IRecycleBin) - + def _get_context(self): """Get the context (Plone site)""" return self.context - + def update(self): super().update() @@ -322,7 +321,7 @@ def get_items(self): # Get filters early to avoid multiple lookups during the loop filter_type = self.get_filter_type() search_query = self.get_search_query().lower() - + # Check if the user has full access to the recycle bin # (includes permissions check and Manager/Site Administrator roles) has_full_access = self.recycle_bin.check_permission(check_roles=True) @@ -347,7 +346,7 @@ def get_items(self): if not has_full_access: if not self.recycle_bin.can_restore(item["recycle_id"]): continue - + # Apply type filtering if filter_type and item.get("type") != filter_type: continue @@ -424,7 +423,7 @@ def update(self): f"{self.context.absolute_url()}/@@recyclebin" ) return - + # Check if the user has permission to access this item if not self.recycle_bin.can_restore(self.item_id): logger.debug(f"User does not have permission to view item {self.item_id}") diff --git a/Products/CMFPlone/recyclebin.py b/Products/CMFPlone/recyclebin.py index 0b72c6a9f8..60cadb08ed 100644 --- a/Products/CMFPlone/recyclebin.py +++ b/Products/CMFPlone/recyclebin.py @@ -168,67 +168,67 @@ def is_enabled(self): return settings.recycling_enabled except (KeyError, AttributeError): return False - + def check_permission(self, check_roles=True): """Check if the current user has permission to manage the recycle bin - + Args: check_roles: If True, also check for Manager or Site Administrator roles - + Returns: Boolean indicating permission """ context = self._get_context() sm = getSecurityManager() - + # Check for explicit permission first has_permission = sm.checkPermission(self.MANAGE_RECYCLEBIN, context) if has_permission: return True - + # If no explicit permission and we're not checking roles, return False if not check_roles: return False - + # Additional check for Manager or Site Administrator roles user = sm.getUser() user_roles = user.getRolesInContext(context) - return 'Manager' in user_roles or 'Site Administrator' in user_roles - + return "Manager" in user_roles or "Site Administrator" in user_roles + def can_restore(self, item_id): """Check if the current user can restore an item - + A user can restore an item if: - 1. They have the ManageRecycleBin permission, or + 1. They have the ManageRecycleBin permission, or 2. They are a Manager or Site Administrator, or 3. They are the owner of the item, or 4. They deleted the item - + Args: item_id: The ID of the item in the recycle bin - + Returns: Boolean indicating if the user can restore the item """ # Managers and Site Administrators can always restore items if self.check_permission(check_roles=True): return True - + # Check if user is the owner of the item item_data = self.get_item(item_id) if not item_data: return False - + # Get the object obj = item_data.get("object") if not obj: return False - + # Get the current user ID current_user_id = getSecurityManager().getUser().getId() if not current_user_id: return False - + # Check if current user is the content owner try: if hasattr(obj, "getOwnerTuple"): @@ -242,29 +242,29 @@ def can_restore(self, item_id): except Exception: # If there's an error getting the owner, fall back to checking workflow history pass - + # If not owner, check workflow history to see who deleted the item if not hasattr(obj, "workflow_history"): return False - + workflow_tool = getToolByName(self._get_context(), "portal_workflow") chains = workflow_tool.getChainFor(obj) - + if not chains: return False - + workflow_id = chains[0] history = obj.workflow_history.get(workflow_id, ()) - + if not history: return False - + # Look for the deletion entry for entry in reversed(history): if entry.get("action") == "Moved to recycle bin": # If the current user is the one who deleted it, they can restore it return entry.get("actor") == current_user_id - + return False def _get_item_title(self, obj, item_type=None): diff --git a/Products/CMFPlone/tests/test_recyclebin.py b/Products/CMFPlone/tests/test_recyclebin.py index bc4946780c..72cbe1660a 100644 --- a/Products/CMFPlone/tests/test_recyclebin.py +++ b/Products/CMFPlone/tests/test_recyclebin.py @@ -89,6 +89,50 @@ def test_recyclebin_settings(self): self.assertEqual(settings.retention_period, 30) self.assertEqual(settings.maximum_size, 100) + def test_recyclebin_permission(self): + """Test permission checks for the recycle bin""" + # As Manager role, should have access + self.assertTrue(self.recyclebin.check_permission()) + + # Create a test user with limited permissions + from AccessControl import getSecurityManager + from AccessControl.SecurityManagement import newSecurityManager + from AccessControl.SecurityManagement import setSecurityManager + + # Store the current security manager + old_security_manager = getSecurityManager() + + try: + # Create a regular Member user + self.portal.acl_users._doAddUser("testuser", "password", ["Member"], []) + + # Log in as the test user + user = self.portal.acl_users.getUser("testuser") + user = user.__of__(self.portal.acl_users) + newSecurityManager(None, user) + + # Check permission - should be False for a regular member + self.assertFalse(self.recyclebin.check_permission()) + + # Grant the permission to the Member role + from Products.CMFCore.permissions import setDefaultRoles + + setDefaultRoles( + "Products.CMFPlone.ManageRecycleBin", + ( + "Manager", + "Site Administrator", + "Member", + ), + ) + + # Now the test user should have permission + self.assertTrue(self.recyclebin.check_permission()) + + finally: + # Restore the security manager + setSecurityManager(old_security_manager) + class RecycleBinContentTests(RecycleBinTestCase): """Tests for deleting and restoring basic content items""" @@ -514,43 +558,3 @@ def test_restore_with_name_conflict(self): with self.assertRaises(ValueError): # Restore the item self.recyclebin.restore_item(recycle_id) - - -class RecycleBinPermissionTests(RecycleBinTestCase): - """Tests for RecycleBin permission checks""" - - def test_recyclebin_permission(self): - """Test permission checks for the recycle bin""" - # As Manager role, should have access - self.assertTrue(self.recyclebin.check_permission()) - - # Create a test user with limited permissions - from AccessControl import getSecurityManager - from AccessControl.SecurityManagement import newSecurityManager - from AccessControl.SecurityManagement import setSecurityManager - - # Store the current security manager - old_security_manager = getSecurityManager() - - try: - # Create a regular Member user - self.portal.acl_users._doAddUser('testuser', 'password', ['Member'], []) - - # Log in as the test user - user = self.portal.acl_users.getUser('testuser') - user = user.__of__(self.portal.acl_users) - newSecurityManager(None, user) - - # Check permission - should be False for a regular member - self.assertFalse(self.recyclebin.check_permission()) - - # Grant the permission to the Member role - from Products.CMFCore.permissions import setDefaultRoles - setDefaultRoles('Products.CMFPlone.ManageRecycleBin', ('Manager', 'Site Administrator', 'Member',)) - - # Now the test user should have permission - self.assertTrue(self.recyclebin.check_permission()) - - finally: - # Restore the security manager - setSecurityManager(old_security_manager) From 82b0107e5d315932ef78dec8d0f88bb953fa76d3 Mon Sep 17 00:00:00 2001 From: Rohan Shaw Date: Sun, 18 May 2025 09:14:30 +0530 Subject: [PATCH 5/7] fix(permissions): standardize "Manage Recycle Bin" title casing to "Manage recycle bin" --- Products/CMFPlone/controlpanel/permissions.zcml | 2 +- Products/CMFPlone/profiles/default/actions.xml | 2 +- Products/CMFPlone/profiles/default/controlpanel.xml | 2 +- Products/CMFPlone/profiles/default/rolemap.xml | 2 +- news/4172.feature | 1 - news/4173.feature | 1 + 6 files changed, 5 insertions(+), 5 deletions(-) delete mode 100644 news/4172.feature create mode 100644 news/4173.feature diff --git a/Products/CMFPlone/controlpanel/permissions.zcml b/Products/CMFPlone/controlpanel/permissions.zcml index 998b6f5187..f8786d6bf3 100644 --- a/Products/CMFPlone/controlpanel/permissions.zcml +++ b/Products/CMFPlone/controlpanel/permissions.zcml @@ -91,7 +91,7 @@ diff --git a/Products/CMFPlone/profiles/default/actions.xml b/Products/CMFPlone/profiles/default/actions.xml index 4728c9ead4..aded4c340a 100644 --- a/Products/CMFPlone/profiles/default/actions.xml +++ b/Products/CMFPlone/profiles/default/actions.xml @@ -508,7 +508,7 @@ string:plone-delete portal/@@recyclebin-enabled|nothing - + True diff --git a/Products/CMFPlone/profiles/default/controlpanel.xml b/Products/CMFPlone/profiles/default/controlpanel.xml index 162ca958d9..07e994cadc 100644 --- a/Products/CMFPlone/profiles/default/controlpanel.xml +++ b/Products/CMFPlone/profiles/default/controlpanel.xml @@ -362,6 +362,6 @@ url_expr="string:${portal_url}/@@plone-recyclebin" visible="True" > - Manage Recycle Bin + Manage recycle bin diff --git a/Products/CMFPlone/profiles/default/rolemap.xml b/Products/CMFPlone/profiles/default/rolemap.xml index ac9d4f0a35..8571ceb103 100644 --- a/Products/CMFPlone/profiles/default/rolemap.xml +++ b/Products/CMFPlone/profiles/default/rolemap.xml @@ -379,7 +379,7 @@ diff --git a/news/4172.feature b/news/4172.feature deleted file mode 100644 index 582645463e..0000000000 --- a/news/4172.feature +++ /dev/null @@ -1 +0,0 @@ -Introduces custom permission to manage recyclebin @rohnsha0 \ No newline at end of file diff --git a/news/4173.feature b/news/4173.feature new file mode 100644 index 0000000000..30b64489a0 --- /dev/null +++ b/news/4173.feature @@ -0,0 +1 @@ +Introduces custom permission to manage recycle bin. @rohnsha0 \ No newline at end of file From 3252f0ae5899bbfc1496be2e815d8481261c690c Mon Sep 17 00:00:00 2001 From: Rohan Shaw Date: Fri, 23 May 2025 10:45:12 +0530 Subject: [PATCH 6/7] refactor(recyclebin): simplify permission checks and update tests for user roles --- Products/CMFPlone/browser/recyclebin.py | 8 +- Products/CMFPlone/recyclebin.py | 95 +--------------------- Products/CMFPlone/tests/test_recyclebin.py | 45 ++-------- 3 files changed, 12 insertions(+), 136 deletions(-) diff --git a/Products/CMFPlone/browser/recyclebin.py b/Products/CMFPlone/browser/recyclebin.py index 83f34e1a48..c7c3a260bf 100644 --- a/Products/CMFPlone/browser/recyclebin.py +++ b/Products/CMFPlone/browser/recyclebin.py @@ -35,10 +35,6 @@ def __init__(self, context, request): super().__init__(context, request) self.recycle_bin = getUtility(IRecycleBin) - def _get_context(self): - """Get the context (Plone site)""" - return self.context - def update(self): super().update() @@ -344,7 +340,7 @@ def get_items(self): if item.get("id") not in child_items_to_exclude: # If user doesn't have full access, show only items they can restore if not has_full_access: - if not self.recycle_bin.can_restore(item["recycle_id"]): + if not self.recycle_bin.check_permission(): continue # Apply type filtering @@ -425,7 +421,7 @@ def update(self): return # Check if the user has permission to access this item - if not self.recycle_bin.can_restore(self.item_id): + if not self.recycle_bin.check_permission(): logger.debug(f"User does not have permission to view item {self.item_id}") message = translate( _("You don't have permission to access this item in the recycle bin."), diff --git a/Products/CMFPlone/recyclebin.py b/Products/CMFPlone/recyclebin.py index 60cadb08ed..79b938dbda 100644 --- a/Products/CMFPlone/recyclebin.py +++ b/Products/CMFPlone/recyclebin.py @@ -126,7 +126,7 @@ class RecycleBin: """Stores deleted content items""" # Permission for managing recycle bin - MANAGE_RECYCLEBIN = "Products.CMFPlone.ManageRecycleBin" + MANAGE_RECYCLEBIN = "Manage recycle bin" def __init__(self): """Initialize the recycle bin utility @@ -169,103 +169,14 @@ def is_enabled(self): except (KeyError, AttributeError): return False - def check_permission(self, check_roles=True): + def check_permission(self): """Check if the current user has permission to manage the recycle bin - Args: - check_roles: If True, also check for Manager or Site Administrator roles - Returns: Boolean indicating permission """ context = self._get_context() - sm = getSecurityManager() - - # Check for explicit permission first - has_permission = sm.checkPermission(self.MANAGE_RECYCLEBIN, context) - if has_permission: - return True - - # If no explicit permission and we're not checking roles, return False - if not check_roles: - return False - - # Additional check for Manager or Site Administrator roles - user = sm.getUser() - user_roles = user.getRolesInContext(context) - return "Manager" in user_roles or "Site Administrator" in user_roles - - def can_restore(self, item_id): - """Check if the current user can restore an item - - A user can restore an item if: - 1. They have the ManageRecycleBin permission, or - 2. They are a Manager or Site Administrator, or - 3. They are the owner of the item, or - 4. They deleted the item - - Args: - item_id: The ID of the item in the recycle bin - - Returns: - Boolean indicating if the user can restore the item - """ - # Managers and Site Administrators can always restore items - if self.check_permission(check_roles=True): - return True - - # Check if user is the owner of the item - item_data = self.get_item(item_id) - if not item_data: - return False - - # Get the object - obj = item_data.get("object") - if not obj: - return False - - # Get the current user ID - current_user_id = getSecurityManager().getUser().getId() - if not current_user_id: - return False - - # Check if current user is the content owner - try: - if hasattr(obj, "getOwnerTuple"): - owner_tuple = obj.getOwnerTuple() - if owner_tuple and owner_tuple[1] == current_user_id: - return True - elif hasattr(obj, "getOwner"): - owner = obj.getOwner() - if owner and owner.getId() == current_user_id: - return True - except Exception: - # If there's an error getting the owner, fall back to checking workflow history - pass - - # If not owner, check workflow history to see who deleted the item - if not hasattr(obj, "workflow_history"): - return False - - workflow_tool = getToolByName(self._get_context(), "portal_workflow") - chains = workflow_tool.getChainFor(obj) - - if not chains: - return False - - workflow_id = chains[0] - history = obj.workflow_history.get(workflow_id, ()) - - if not history: - return False - - # Look for the deletion entry - for entry in reversed(history): - if entry.get("action") == "Moved to recycle bin": - # If the current user is the one who deleted it, they can restore it - return entry.get("actor") == current_user_id - - return False + return getSecurityManager().checkPermission(self.MANAGE_RECYCLEBIN, context) def _get_item_title(self, obj, item_type=None): """Helper method to get a meaningful title for an item""" diff --git a/Products/CMFPlone/tests/test_recyclebin.py b/Products/CMFPlone/tests/test_recyclebin.py index 72cbe1660a..f9b6b8bf2f 100644 --- a/Products/CMFPlone/tests/test_recyclebin.py +++ b/Products/CMFPlone/tests/test_recyclebin.py @@ -94,44 +94,13 @@ def test_recyclebin_permission(self): # As Manager role, should have access self.assertTrue(self.recyclebin.check_permission()) - # Create a test user with limited permissions - from AccessControl import getSecurityManager - from AccessControl.SecurityManagement import newSecurityManager - from AccessControl.SecurityManagement import setSecurityManager - - # Store the current security manager - old_security_manager = getSecurityManager() - - try: - # Create a regular Member user - self.portal.acl_users._doAddUser("testuser", "password", ["Member"], []) - - # Log in as the test user - user = self.portal.acl_users.getUser("testuser") - user = user.__of__(self.portal.acl_users) - newSecurityManager(None, user) - - # Check permission - should be False for a regular member - self.assertFalse(self.recyclebin.check_permission()) - - # Grant the permission to the Member role - from Products.CMFCore.permissions import setDefaultRoles - - setDefaultRoles( - "Products.CMFPlone.ManageRecycleBin", - ( - "Manager", - "Site Administrator", - "Member", - ), - ) - - # Now the test user should have permission - self.assertTrue(self.recyclebin.check_permission()) - - finally: - # Restore the security manager - setSecurityManager(old_security_manager) + self.portal.acl_users._doAddUser("testuser", "password", ["Member"], []) + + # Log in as the test user using plone.app.testing login function + login(self.portal, "testuser") + + # Check permission - should be False for a regular member + self.assertFalse(self.recyclebin.check_permission()) class RecycleBinContentTests(RecycleBinTestCase): From 77a092cc2d4d018233e1a65f486335ffb96fcf8d Mon Sep 17 00:00:00 2001 From: Rohan Shaw Date: Fri, 23 May 2025 10:52:46 +0530 Subject: [PATCH 7/7] remove news entry --- news/4173.feature | 1 - 1 file changed, 1 deletion(-) delete mode 100644 news/4173.feature diff --git a/news/4173.feature b/news/4173.feature deleted file mode 100644 index 30b64489a0..0000000000 --- a/news/4173.feature +++ /dev/null @@ -1 +0,0 @@ -Introduces custom permission to manage recycle bin. @rohnsha0 \ No newline at end of file