mirror of
https://github.com/inventree/InvenTree.git
synced 2026-08-10 15:36:17 +00:00
Fix permissions for group and RuleSet endpoints (#12563)
- Require StaffRole in addition to is_staff
This commit is contained in:
@@ -308,7 +308,7 @@ class GroupMixin(SerializerContextMixin):
|
|||||||
|
|
||||||
queryset = Group.objects.all()
|
queryset = Group.objects.all()
|
||||||
serializer_class = GroupSerializer
|
serializer_class = GroupSerializer
|
||||||
permission_classes = [InvenTree.permissions.IsStaffOrReadOnlyScope]
|
permission_classes = [InvenTree.permissions.StaffRolePermissionOrReadOnly]
|
||||||
|
|
||||||
|
|
||||||
class GroupOutputOptions(OutputConfiguration):
|
class GroupOutputOptions(OutputConfiguration):
|
||||||
@@ -345,7 +345,7 @@ class RuleSetMixin:
|
|||||||
|
|
||||||
queryset = RuleSet.objects.all()
|
queryset = RuleSet.objects.all()
|
||||||
serializer_class = RuleSetSerializer
|
serializer_class = RuleSetSerializer
|
||||||
permission_classes = [InvenTree.permissions.IsStaffOrReadOnlyScope]
|
permission_classes = [InvenTree.permissions.StaffRolePermissionOrReadOnly]
|
||||||
|
|
||||||
|
|
||||||
class RuleSetList(RuleSetMixin, ListAPI):
|
class RuleSetList(RuleSetMixin, ListAPI):
|
||||||
|
|||||||
@@ -586,3 +586,82 @@ class GroupDetailTests(InvenTreeAPITestCase):
|
|||||||
|
|
||||||
response = self.get(url, {'permission_detail': 'false'}, expected_code=200)
|
response = self.get(url, {'permission_detail': 'false'}, expected_code=200)
|
||||||
self.assertNotIn('permissions', response.data)
|
self.assertNotIn('permissions', response.data)
|
||||||
|
|
||||||
|
|
||||||
|
class RuleSetPermissionTests(InvenTreeAPITestCase):
|
||||||
|
"""Tests for permission enforcement on the Group / RuleSet API endpoints.
|
||||||
|
|
||||||
|
Regression tests for a privilege-escalation bug where a *staff* user
|
||||||
|
(without the 'admin' role) was able to write to these endpoints - in
|
||||||
|
particular, granting themselves 'admin' RuleSet permissions via the
|
||||||
|
RuleSet API, despite not holding the 'admin' role themselves.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def test_group_write_requires_admin_role(self):
|
||||||
|
"""A staff-only user (without the 'admin' role) cannot write to the Group API."""
|
||||||
|
url = reverse('api-group-detail', kwargs={'pk': self.group.pk})
|
||||||
|
|
||||||
|
# Sanity check - the default test user is staff, but has no assigned roles
|
||||||
|
self.assertTrue(self.user.is_staff)
|
||||||
|
|
||||||
|
# Read access is still permitted
|
||||||
|
self.get(url, expected_code=200)
|
||||||
|
|
||||||
|
# Write access is rejected, as the user does not have the 'admin' role
|
||||||
|
self.patch(url, {'name': 'renamed-group'}, expected_code=403)
|
||||||
|
|
||||||
|
self.group.refresh_from_db()
|
||||||
|
self.assertNotEqual(self.group.name, 'renamed-group')
|
||||||
|
|
||||||
|
# Once the 'admin' role is granted, the write succeeds
|
||||||
|
self.assignRole('admin.change')
|
||||||
|
self.patch(url, {'name': 'renamed-group'}, expected_code=200)
|
||||||
|
|
||||||
|
self.group.refresh_from_db()
|
||||||
|
self.assertEqual(self.group.name, 'renamed-group')
|
||||||
|
|
||||||
|
def test_ruleset_write_requires_admin_role(self):
|
||||||
|
"""A staff-only user cannot self-escalate permissions via the RuleSet API."""
|
||||||
|
admin_ruleset = self.group.rule_sets.get(name='admin')
|
||||||
|
url = reverse('api-ruleset-detail', kwargs={'pk': admin_ruleset.pk})
|
||||||
|
|
||||||
|
# Sanity check - the default test user is staff, but has no assigned roles
|
||||||
|
self.assertTrue(self.user.is_staff)
|
||||||
|
self.assertFalse(admin_ruleset.can_change)
|
||||||
|
|
||||||
|
# Read access is still permitted
|
||||||
|
self.get(url, expected_code=200)
|
||||||
|
|
||||||
|
# Attempt to self-escalate to full 'admin' ruleset permissions
|
||||||
|
self.patch(
|
||||||
|
url,
|
||||||
|
{'can_view': True, 'can_add': True, 'can_change': True, 'can_delete': True},
|
||||||
|
expected_code=403,
|
||||||
|
)
|
||||||
|
|
||||||
|
admin_ruleset.refresh_from_db()
|
||||||
|
self.assertFalse(admin_ruleset.can_change)
|
||||||
|
|
||||||
|
# Granting the 'admin' role directly (not via the API) allows the write
|
||||||
|
self.assignRole('admin.change')
|
||||||
|
self.patch(url, {'can_delete': True}, expected_code=200)
|
||||||
|
|
||||||
|
admin_ruleset.refresh_from_db()
|
||||||
|
self.assertTrue(admin_ruleset.can_delete)
|
||||||
|
|
||||||
|
def test_non_staff_user_read_only(self):
|
||||||
|
"""A non-staff, non-admin authenticated user retains read-only access."""
|
||||||
|
self.user.is_staff = False
|
||||||
|
self.user.save()
|
||||||
|
|
||||||
|
group_url = reverse('api-group-detail', kwargs={'pk': self.group.pk})
|
||||||
|
ruleset_url = reverse(
|
||||||
|
'api-ruleset-detail',
|
||||||
|
kwargs={'pk': self.group.rule_sets.get(name='admin').pk},
|
||||||
|
)
|
||||||
|
|
||||||
|
self.get(group_url, expected_code=200)
|
||||||
|
self.get(ruleset_url, expected_code=200)
|
||||||
|
|
||||||
|
self.patch(group_url, {'name': 'renamed-group'}, expected_code=403)
|
||||||
|
self.patch(ruleset_url, {'can_change': True}, expected_code=403)
|
||||||
|
|||||||
Reference in New Issue
Block a user