From 831b12971106e00410dc63b2152f5e57d54ea215 Mon Sep 17 00:00:00 2001 From: Oliver Date: Tue, 13 Aug 2024 15:22:48 +1000 Subject: [PATCH 1/2] [Bug] Attachment permissions (#7863) * Add helper func to check user permission against a given model type * Validate bulk delete of attachments - Check permissions against linked model type(s) * Check permission when creating or editing an attachment instance * Fix typo * Fix AttachmentSerializer to allow editing * Update unit tests accordingly * Remove unused custom permission classs * Bump API version --- .../InvenTree/InvenTree/api_version.py | 6 ++++- src/backend/InvenTree/common/api.py | 21 ++++++++++++++++- src/backend/InvenTree/common/serializers.py | 22 ++++++++++++------ src/backend/InvenTree/common/tests.py | 23 +++++++++++++++++++ src/backend/InvenTree/users/models.py | 12 ++++++++++ 5 files changed, 75 insertions(+), 9 deletions(-) diff --git a/src/backend/InvenTree/InvenTree/api_version.py b/src/backend/InvenTree/InvenTree/api_version.py index 0f0a6a6bde..a8ea357bfd 100644 --- a/src/backend/InvenTree/InvenTree/api_version.py +++ b/src/backend/InvenTree/InvenTree/api_version.py @@ -1,13 +1,17 @@ """InvenTree API version information.""" # InvenTree API version -INVENTREE_API_VERSION = 236 +INVENTREE_API_VERSION = 237 """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" INVENTREE_API_TEXT = """ +v237 - 2024-08-13 : https://github.com/inventree/InvenTree/pull/7863 + - Reimplement "bulk delete" operation for Attachment model + - Fix permission checks for Attachment API endpoints + v236 - 2024-08-10 : https://github.com/inventree/InvenTree/pull/7844 - Adds "supplier_name" to the PurchaseOrder API serializer diff --git a/src/backend/InvenTree/common/api.py b/src/backend/InvenTree/common/api.py index bb2bd8f53d..8701049231 100644 --- a/src/backend/InvenTree/common/api.py +++ b/src/backend/InvenTree/common/api.py @@ -4,6 +4,7 @@ import json from django.conf import settings from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ValidationError from django.db.models import Q from django.http.response import HttpResponse from django.urls import include, path, re_path @@ -706,7 +707,7 @@ class AttachmentFilter(rest_filters.FilterSet): return queryset.filter(Q(attachment=None) | Q(attachment='')).distinct() -class AttachmentList(ListCreateAPI): +class AttachmentList(BulkDeleteMixin, ListCreateAPI): """List API endpoint for Attachment objects.""" queryset = common.models.Attachment.objects.all() @@ -725,6 +726,24 @@ class AttachmentList(ListCreateAPI): attachment.upload_user = self.request.user attachment.save() + def validate_delete(self, queryset, request) -> None: + """Ensure that the user has correct permissions for a bulk-delete. + + - Extract all model types from the provided queryset + - Ensure that the user has correct 'delete' permissions for each model + """ + from common.validators import attachment_model_class_from_label + from users.models import check_user_permission + + model_types = queryset.values_list('model_type', flat=True).distinct() + + for model_type in model_types: + if model_class := attachment_model_class_from_label(model_type): + if not check_user_permission(request.user, model_class, 'delete'): + raise ValidationError( + _('User does not have permission to delete these attachments') + ) + class AttachmentDetail(RetrieveUpdateDestroyAPI): """Detail API endpoint for Attachment objects.""" diff --git a/src/backend/InvenTree/common/serializers.py b/src/backend/InvenTree/common/serializers.py index 8b2e55c014..4c1f6a30dd 100644 --- a/src/backend/InvenTree/common/serializers.py +++ b/src/backend/InvenTree/common/serializers.py @@ -540,12 +540,17 @@ class AttachmentSerializer(InvenTreeModelSerializer): allow_null=False, ) - def save(self): + def save(self, **kwargs): """Override the save method to handle the model_type field.""" from InvenTree.models import InvenTreeAttachmentMixin + from users.models import check_user_permission model_type = self.validated_data.get('model_type', None) + if model_type is None: + if self.instance: + model_type = self.instance.model_type + # Ensure that the user has permission to attach files to the specified model user = self.context.get('request').user @@ -556,15 +561,18 @@ class AttachmentSerializer(InvenTreeModelSerializer): if not issubclass(target_model_class, InvenTreeAttachmentMixin): raise PermissionDenied(_('Invalid model type specified for attachment')) + permission_error_msg = _( + 'User does not have permission to create or edit attachments for this model' + ) + + if not check_user_permission(user, target_model_class, 'change'): + raise PermissionDenied(permission_error_msg) + # Check that the user has the required permissions to attach files to the target model if not target_model_class.check_attachment_permission('change', user): - raise PermissionDenied( - _( - 'User does not have permission to create or edit attachments for this model' - ) - ) + raise PermissionDenied(_(permission_error_msg)) - return super().save() + return super().save(**kwargs) class IconSerializer(serializers.Serializer): diff --git a/src/backend/InvenTree/common/tests.py b/src/backend/InvenTree/common/tests.py index c0d46bd9c2..c76d17f5b9 100644 --- a/src/backend/InvenTree/common/tests.py +++ b/src/backend/InvenTree/common/tests.py @@ -157,6 +157,29 @@ class AttachmentTest(InvenTreeAPITestCase): # Upload should now work! response = self.post(url, data, expected_code=201) + pk = response.data['pk'] + + # Edit the attachment via API + response = self.patch( + reverse('api-attachment-detail', kwargs={'pk': pk}), + {'comment': 'New comment'}, + expected_code=200, + ) + + self.assertEqual(response.data['comment'], 'New comment') + + attachment = Attachment.objects.get(pk=pk) + self.assertEqual(attachment.comment, 'New comment') + + # And check that we cannot edit the attachment without the correct permissions + self.clearRoles() + + self.patch( + reverse('api-attachment-detail', kwargs={'pk': pk}), + {'comment': 'New comment 2'}, + expected_code=403, + ) + # Try to delete the attachment via API (should fail) attachment = part.attachments.first() url = reverse('api-attachment-detail', kwargs={'pk': attachment.pk}) diff --git a/src/backend/InvenTree/users/models.py b/src/backend/InvenTree/users/models.py index 7a0cd636aa..1fcfc9d901 100644 --- a/src/backend/InvenTree/users/models.py +++ b/src/backend/InvenTree/users/models.py @@ -682,6 +682,18 @@ def clear_user_role_cache(user: User): cache.delete(key) +def check_user_permission(user: User, model, permission): + """Check if the user has a particular permission against a given model type. + + Arguments: + user: The user object to check + model: The model class to check (e.g. Part) + permission: The permission to check (e.g. 'view' / 'delete') + """ + permission_name = f'{model._meta.app_label}.{permission}_{model._meta.model_name}' + return user.has_perm(permission_name) + + def check_user_role(user: User, role, permission): """Check if a user has a particular role:permission combination. From 27fba9cd024e9c84ee88f9296d4ced4385997d1f Mon Sep 17 00:00:00 2001 From: Oliver Date: Tue, 13 Aug 2024 15:23:05 +1000 Subject: [PATCH 2/2] [PUI] Search preview enhancements (#7864) - Observe user settings in search preview --- .../src/components/nav/SearchDrawer.tsx | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/src/frontend/src/components/nav/SearchDrawer.tsx b/src/frontend/src/components/nav/SearchDrawer.tsx index 429af00f0c..f21cf55a52 100644 --- a/src/frontend/src/components/nav/SearchDrawer.tsx +++ b/src/frontend/src/components/nav/SearchDrawer.tsx @@ -133,7 +133,11 @@ export function SearchDrawer({ return [ { model: ModelType.part, - parameters: {}, + parameters: { + active: userSettings.isSet('SEARCH_HIDE_INACTIVE_PARTS') + ? true + : undefined + }, enabled: user.hasViewRole(UserRoles.part) && userSettings.isSet('SEARCH_PREVIEW_SHOW_PARTS') @@ -173,7 +177,10 @@ export function SearchDrawer({ model: ModelType.stockitem, parameters: { part_detail: true, - location_detail: true + location_detail: true, + in_stock: userSettings.isSet('SEARCH_PREVIEW_HIDE_UNAVAILABLE_STOCK') + ? true + : undefined }, enabled: user.hasViewRole(UserRoles.stock) && @@ -206,7 +213,12 @@ export function SearchDrawer({ { model: ModelType.purchaseorder, parameters: { - supplier_detail: true + supplier_detail: true, + outstanding: userSettings.isSet( + 'SEARCH_PREVIEW_EXCLUDE_INACTIVE_PURCHASE_ORDERS' + ) + ? true + : undefined }, enabled: user.hasViewRole(UserRoles.purchase_order) && @@ -215,7 +227,12 @@ export function SearchDrawer({ { model: ModelType.salesorder, parameters: { - customer_detail: true + customer_detail: true, + outstanding: userSettings.isSet( + 'SEARCH_PREVIEW_EXCLUDE_INACTIVE_SALES_ORDERS' + ) + ? true + : undefined }, enabled: user.hasViewRole(UserRoles.sales_order) && @@ -224,7 +241,12 @@ export function SearchDrawer({ { model: ModelType.returnorder, parameters: { - customer_detail: true + customer_detail: true, + outstanding: userSettings.isSet( + 'SEARCH_PREVIEW_EXCLUDE_INACTIVE_RETURN_ORDERS' + ) + ? true + : undefined }, enabled: user.hasViewRole(UserRoles.return_order) && @@ -250,7 +272,7 @@ export function SearchDrawer({ let params: any = { offset: 0, - limit: 10, // TODO: Make this configurable (based on settings) + limit: userSettings.getSetting('SEARCH_PREVIEW_RESULTS', '10'), search: searchText, search_regex: searchRegex, search_whole: searchWhole