Skip to content

Commit

Permalink
Do not allow a Site Administrator user to add users to groups that have
Browse files Browse the repository at this point in the history
the Manager role
  • Loading branch information
wesleybl committed Sep 22, 2023
1 parent e5bc9b8 commit 08e5526
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/plone/restapi/services/groups/update.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from AccessControl import getSecurityManager
from plone.restapi.deserializer import json_body
from plone.restapi.services import Service
from Products.CMFCore.permissions import ManagePortal
from Products.CMFCore.utils import getToolByName
from zExceptions import BadRequest
from zope.component.hooks import getSite
Expand Down Expand Up @@ -34,6 +36,10 @@ def __init__(self, context, request):
super().__init__(context, request)
self.params = []

@property
def is_zope_manager(self):
return getSecurityManager().checkPermission(ManagePortal, self.context)

def publishTraverse(self, request, name):
# Consume any path segments after /@groups as parameters
self.params.append(name)
Expand All @@ -54,6 +60,9 @@ def reply(self):
data = json_body(self.request)
group = self._get_group(self._get_group_id)

if not self.is_zope_manager and "Manager" in group.getRoles():
return self.reply_no_content(status=403)

if not group:
raise BadRequest("Trying to update a non-existing group.")

Expand Down
25 changes: 25 additions & 0 deletions src/plone/restapi/tests/test_services_groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@ def setUp(self):
def tearDown(self):
self.api_session.close()

def set_siteadm(self):
siteadm_username = "siteadm"
siteadm_password = "siteadmpassword"
api.user.create(
email="[email protected]",
roles=["Site Administrator"],
username=siteadm_username,
password=siteadm_password,
)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (siteadm_username, siteadm_password)
transaction.commit()

def test_list_groups(self):
response = self.api_session.get("/@groups")

Expand Down Expand Up @@ -173,3 +187,14 @@ def test_delete_non_existing_group(self):
transaction.commit()

self.assertEqual(response.status_code, 404)

def test_siteadm_not_add_user_to_group_with_manager_role(self):
self.set_siteadm()
payload = {
"users": {TEST_USER_ID: True, SITE_OWNER_NAME: False},
}
self.api_session.patch("/@groups/Administrators", json=payload)
transaction.commit()

administrators = self.gtool.getGroupById("Administrators")
self.assertNotIn(TEST_USER_ID, administrators.getGroupMemberIds())

0 comments on commit 08e5526

Please sign in to comment.