From a167be5e1f1d28728e3d7476307b874f03e5b764 Mon Sep 17 00:00:00 2001 From: Trenton Holmes <797416+stumpylog@users.noreply.github.com> Date: Sun, 13 Sep 2026 14:26:40 -0700 Subject: [PATCH] Fix: use validate_set_permissions' normalized dict, dedupe action list BulkEditObjectsSerializer._validate_permissions discarded the id-checked/None-normalized dict validate_set_permissions returns, so a literal null for users/groups reached set_permissions_for_objects raw and blew up with an uncaught TypeError, masked by the view's broad except into a vague "check logs" 400. Now stores the returned dict back onto attrs["permissions"]. Also replaced the two independently hardcoded ("view", "change") lists in SetPermissionsSerializer.to_internal_value and validate_set_permissions with one PERMISSION_ACTIONS constant, so adding a new action can't update one and silently miss the other. Co-Authored-By: Claude Sonnet 5 --- src/documents/serialisers.py | 18 ++++++------- src/documents/tests/test_api_permissions.py | 28 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 1561e96f7..c85b62e7a 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -210,6 +210,9 @@ class MatchingModelSerializer(serializers.ModelSerializer[Any]): return match +PERMISSION_ACTIONS = ("view", "change") + + class SetPermissionsMixin: def _validate_user_ids(self, user_ids): users = User.objects.none() @@ -232,12 +235,9 @@ class SetPermissionsMixin: return groups def validate_set_permissions(self, set_permissions=None): - permissions_dict = { - "view": {}, - "change": {}, - } + permissions_dict = {action: {} for action in PERMISSION_ACTIONS} if set_permissions is not None: - for action in ["view", "change"]: + for action in PERMISSION_ACTIONS: if action in set_permissions: if "users" in set_permissions[action]: users = set_permissions[action]["users"] @@ -284,7 +284,7 @@ class SetPermissionsSerializer(serializers.Serializer[dict[str, Any]]): def to_internal_value(self, data): if isinstance(data, dict): - unknown_keys = set(data) - {"view", "change"} + unknown_keys = set(data) - set(PERMISSION_ACTIONS) if unknown_keys: raise serializers.ValidationError( {key: "Unknown permission action." for key in sorted(unknown_keys)}, @@ -3032,8 +3032,8 @@ class BulkEditObjectsSerializer(SerializerWithPerms, SetPermissionsMixin): ) return objects - def _validate_permissions(self, permissions) -> None: - self.validate_set_permissions( + def _validate_permissions(self, permissions) -> dict: + return self.validate_set_permissions( permissions, ) @@ -3061,7 +3061,7 @@ class BulkEditObjectsSerializer(SerializerWithPerms, SetPermissionsMixin): raise serializers.ValidationError( "permissions must not be empty", ) - self._validate_permissions(permissions) + attrs["permissions"] = self._validate_permissions(permissions) return attrs diff --git a/src/documents/tests/test_api_permissions.py b/src/documents/tests/test_api_permissions.py index 6124b6c54..92bad00ee 100644 --- a/src/documents/tests/test_api_permissions.py +++ b/src/documents/tests/test_api_permissions.py @@ -1574,6 +1574,34 @@ class TestBulkEditObjectPermissions(APITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + def test_bulk_object_set_permissions_null_users_is_a_noop(self) -> None: + """ + GIVEN: + - Existing objects + WHEN: + - bulk_edit_objects API endpoint is called with set_permissions + operation and an explicit null for users/groups on an action + THEN: + - Request succeeds and is treated as "no users/groups for this + action", not a crash -- the normalized (id-checked) dict + returned by validate_set_permissions must actually be used, + not discarded in favor of the raw un-normalized input. + """ + response = self.client.post( + "/api/bulk_edit_objects/", + json.dumps( + { + "objects": [self.t1.id], + "object_type": "tags", + "operation": "set_permissions", + "permissions": {"view": {"users": None}}, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + def test_bulk_edit_object_permissions_validation(self) -> None: """ GIVEN: