mirror of
https://github.com/inventree/InvenTree.git
synced 2026-11-10 02:47:49 +00:00
Permissions fix for metadata endpoints (#13213)
Metadata API endpoints now observe the user permissions associated with the underlying models
This commit is contained in:
1 parent
08dad19122
commit
0cb1852e0d
4 files changed
+154
No files matched your search
@@ -550,6 +550,17 @@ class ContentTypePermission(OASTokenMixin, permissions.BasePermission):
|
||||
return obj.check_permission(permission, request.user)
|
||||
|
||||
if model_class := obj.__class__:
|
||||
table_name = f'{model_class._meta.app_label}_{model_class._meta.model_name}'
|
||||
|
||||
if table_name in users.ruleset.get_ruleset_ignore():
|
||||
# Ignore-listed models have no RuleSet permissions of their own,
|
||||
# so check_user_permission() would grant access to every active user.
|
||||
# Without an explicit check_permission() on the model, fail safe:
|
||||
# allow read access only, and restrict writes to staff users.
|
||||
return permission == 'view' or bool(
|
||||
request.user.is_staff or request.user.is_superuser
|
||||
)
|
||||
|
||||
return users.permissions.check_user_permission(
|
||||
request.user, model_class, 'change'
|
||||
)
|
||||
|
||||
@@ -2567,6 +2567,14 @@ class SelectionList(InvenTree.models.MetadataMixin, InvenTree.models.InvenTreeMo
|
||||
verbose_name = _('Selection List')
|
||||
verbose_name_plural = _('Selection Lists')
|
||||
|
||||
def check_permission(self, permission, user):
|
||||
"""Check if the user has the required permission for this selection list.
|
||||
|
||||
SelectionList has no RuleSet permissions of its own (it is ignore-listed),
|
||||
so mirror the IsStaffOrReadOnlyScope restriction of its own API endpoint.
|
||||
"""
|
||||
return permission == 'view' or user.is_staff
|
||||
|
||||
name = models.CharField(
|
||||
max_length=100,
|
||||
verbose_name=_('Name'),
|
||||
@@ -2790,6 +2798,14 @@ class ParameterTemplate(
|
||||
MODEL_TYPE = 1, _('Unique for model type')
|
||||
GLOBAL = 2, _('Globally unique')
|
||||
|
||||
def check_permission(self, permission, user):
|
||||
"""Check if the user has the required permission for this parameter template.
|
||||
|
||||
ParameterTemplate has no RuleSet permissions of its own (it is ignore-listed),
|
||||
so mirror the IsStaffOrReadOnlyScope restriction of its own API endpoint.
|
||||
"""
|
||||
return permission == 'view' or user.is_staff
|
||||
|
||||
@staticmethod
|
||||
def get_api_url() -> str:
|
||||
"""Return the API URL associated with the ParameterTemplate model."""
|
||||
|
||||
@@ -3473,3 +3473,122 @@ class GenericMetadataAuthorizationTests(InvenTreeAPITestCase):
|
||||
self.user.is_staff = True
|
||||
self.user.save()
|
||||
self.patch(url, {'metadata': {'x': 1}}, expected_code=200)
|
||||
|
||||
def _check_staff_only_write(self, model, pk, detail_url):
|
||||
"""Metadata for a staff-write model is readable by all, writable only by staff."""
|
||||
url = self._metadata_url(model, pk)
|
||||
|
||||
self.user.is_staff = False
|
||||
self.user.save()
|
||||
|
||||
# The dedicated endpoint refuses writes from a non-staff user...
|
||||
self.patch(detail_url, {'description': 'x'}, expected_code=403)
|
||||
|
||||
# ...and so must the generic metadata endpoint
|
||||
self.get(url, expected_code=200)
|
||||
self.patch(url, {'metadata': {'x': 1}}, expected_code=403)
|
||||
self.put(url, {'metadata': {'x': 1}}, expected_code=403)
|
||||
|
||||
self.user.is_staff = True
|
||||
self.user.save()
|
||||
self.patch(url, {'metadata': {'x': 1}}, expected_code=200)
|
||||
|
||||
def test_parameter_template_metadata_requires_staff_for_write(self):
|
||||
"""ParameterTemplate metadata mirrors the staff-only write restriction of its own endpoint."""
|
||||
template = self.parameter.template
|
||||
self._check_staff_only_write(
|
||||
'parametertemplate',
|
||||
template.pk,
|
||||
reverse('api-parameter-template-detail', kwargs={'pk': template.pk}),
|
||||
)
|
||||
|
||||
def test_selection_list_metadata_requires_staff_for_write(self):
|
||||
"""SelectionList metadata mirrors the staff-only write restriction of its own endpoint."""
|
||||
selection_list = SelectionList.objects.create(name='Metadata Perm Test List')
|
||||
self._check_staff_only_write(
|
||||
'selectionlist',
|
||||
selection_list.pk,
|
||||
reverse('api-selectionlist-detail', kwargs={'pk': selection_list.pk}),
|
||||
)
|
||||
|
||||
def test_tip_result_metadata_owner_only(self):
|
||||
"""TipResult metadata is only accessible to the owning user (mirrors TipResultDetail)."""
|
||||
from django.contrib.auth import get_user_model
|
||||
|
||||
from users.models import TipResult
|
||||
|
||||
other_user = get_user_model().objects.create_user(
|
||||
username='tip-other', password='tip-other-pw'
|
||||
)
|
||||
|
||||
own_tip = TipResult.objects.create(user=self.user, tip_id='own-tip')
|
||||
other_tip = TipResult.objects.create(user=other_user, tip_id='other-tip')
|
||||
|
||||
own_url = self._metadata_url('tipresult', own_tip.pk)
|
||||
other_url = self._metadata_url('tipresult', other_tip.pk)
|
||||
|
||||
self.get(own_url, expected_code=200)
|
||||
self.patch(own_url, {'metadata': {'x': 1}}, expected_code=200)
|
||||
|
||||
# Staff status does not grant access to another user's tip results
|
||||
self.assertTrue(self.user.is_staff)
|
||||
self.get(other_url, expected_code=403)
|
||||
self.patch(other_url, {'metadata': {'x': 1}}, expected_code=403)
|
||||
|
||||
other_tip.refresh_from_db()
|
||||
self.assertNotIn('x', other_tip.metadata or {})
|
||||
|
||||
def test_ignore_listed_metadata_models_define_check_permission(self):
|
||||
"""Every ignore-listed model with metadata must define check_permission().
|
||||
|
||||
Otherwise the generic metadata endpoint falls back to the (read-only for
|
||||
non-staff) default in ContentTypePermission, which may not match the
|
||||
restriction enforced by the model's own API endpoint.
|
||||
"""
|
||||
from django.apps import apps
|
||||
|
||||
from users.ruleset import get_ruleset_ignore
|
||||
|
||||
ignored = set(get_ruleset_ignore())
|
||||
missing = []
|
||||
|
||||
for model in apps.get_models():
|
||||
table = f'{model._meta.app_label}_{model._meta.model_name}'
|
||||
if table not in ignored:
|
||||
continue
|
||||
if not any(f.name == 'metadata' for f in model._meta.get_fields()):
|
||||
continue
|
||||
if not hasattr(model, 'check_permission'):
|
||||
missing.append(table)
|
||||
|
||||
self.assertEqual(missing, [])
|
||||
|
||||
def test_ignore_listed_fallback_denies_non_staff_write(self):
|
||||
"""ContentTypePermission fails safe for ignore-listed models without check_permission()."""
|
||||
from rest_framework.test import APIRequestFactory
|
||||
|
||||
from InvenTree.permissions import ContentTypePermission
|
||||
|
||||
# CustomUnit is ignore-listed and does not define check_permission()
|
||||
unit = common.models.CustomUnit.objects.create(
|
||||
name='metadata_perm_unit', definition='1 m'
|
||||
)
|
||||
self.assertFalse(hasattr(unit, 'check_permission'))
|
||||
|
||||
factory = APIRequestFactory()
|
||||
perm = ContentTypePermission()
|
||||
|
||||
self.user.is_staff = False
|
||||
self.user.save()
|
||||
|
||||
for method, expected in [('get', True), ('patch', False), ('put', False)]:
|
||||
request = getattr(factory, method)('/')
|
||||
request.user = self.user
|
||||
self.assertEqual(perm.has_object_permission(request, None, unit), expected)
|
||||
|
||||
self.user.is_staff = True
|
||||
self.user.save()
|
||||
|
||||
request = factory.patch('/')
|
||||
request.user = self.user
|
||||
self.assertTrue(perm.has_object_permission(request, None, unit))
|
||||
@@ -854,6 +854,14 @@ class TipResult(InvenTree.models.MetadataMixin):
|
||||
help_text=_('The last time this tip result was updated'),
|
||||
)
|
||||
|
||||
def check_permission(self, permission, user):
|
||||
"""Check if the user has the required permission for this tip result.
|
||||
|
||||
TipResult has no RuleSet permissions of its own (it is ignore-listed),
|
||||
so only the owning user may access it (mirrors TipResultDetail).
|
||||
"""
|
||||
return self.user_id == user.pk
|
||||
|
||||
def __str__(self):
|
||||
"""Return string representation of the tip result."""
|
||||
if self.finished:
|
||||
|
||||
Reference in new issue
Block a user