diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 8b817b3f2..1561e96f7 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -265,41 +265,31 @@ class SerializerWithPerms(serializers.Serializer[dict[str, Any]]): super().__init__(*args, **kwargs) -@extend_schema_field( - field={ - "type": "object", - "properties": { - "view": { - "type": "object", - "properties": { - "users": { - "type": "array", - "items": {"type": "integer"}, - }, - "groups": { - "type": "array", - "items": {"type": "integer"}, - }, - }, - }, - "change": { - "type": "object", - "properties": { - "users": { - "type": "array", - "items": {"type": "integer"}, - }, - "groups": { - "type": "array", - "items": {"type": "integer"}, - }, - }, - }, - }, - }, -) -class SetPermissionsSerializer(serializers.DictField): - pass +class _PermissionSetSerializer(serializers.Serializer[dict[str, Any]]): + users = serializers.ListField( + child=serializers.IntegerField(), + required=False, + allow_null=True, + ) + groups = serializers.ListField( + child=serializers.IntegerField(), + required=False, + allow_null=True, + ) + + +class SetPermissionsSerializer(serializers.Serializer[dict[str, Any]]): + view = _PermissionSetSerializer(required=False) + change = _PermissionSetSerializer(required=False) + + def to_internal_value(self, data): + if isinstance(data, dict): + unknown_keys = set(data) - {"view", "change"} + if unknown_keys: + raise serializers.ValidationError( + {key: "Unknown permission action." for key in sorted(unknown_keys)}, + ) + return super().to_internal_value(data) class OwnedObjectSerializer( @@ -470,7 +460,6 @@ class OwnedObjectSerializer( set_permissions = SetPermissionsSerializer( label="Set permissions", - allow_empty=True, required=False, write_only=True, ) @@ -2045,8 +2034,13 @@ class BulkEditSerializer( def _validate_parameters_set_permissions(self, parameters) -> None: if "set_permissions" not in parameters: raise serializers.ValidationError("set_permissions not specified") + set_permissions = parameters["set_permissions"] + if set_permissions is not None: + set_permissions = SetPermissionsSerializer().run_validation( + set_permissions, + ) parameters["set_permissions"] = self.validate_set_permissions( - parameters["set_permissions"], + set_permissions, ) if "owner" in parameters and parameters["owner"] is not None: self._validate_owner(parameters["owner"]) @@ -3001,9 +2995,8 @@ class BulkEditObjectsSerializer(SerializerWithPerms, SetPermissionsMixin): allow_null=True, ) - permissions = serializers.DictField( + permissions = SetPermissionsSerializer( label="Set permissions", - allow_empty=False, required=False, write_only=True, ) @@ -3064,6 +3057,10 @@ class BulkEditObjectsSerializer(SerializerWithPerms, SetPermissionsMixin): if operation == "set_permissions": permissions = attrs.get("permissions") if permissions is not None: + if not permissions: + raise serializers.ValidationError( + "permissions must not be empty", + ) self._validate_permissions(permissions) return attrs diff --git a/src/documents/tests/test_api_bulk_edit.py b/src/documents/tests/test_api_bulk_edit.py index 714e1bcfd..c591bad9a 100644 --- a/src/documents/tests/test_api_bulk_edit.py +++ b/src/documents/tests/test_api_bulk_edit.py @@ -1165,6 +1165,98 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): self.assertIn(b"set_permissions not specified", response.content) m.assert_not_called() + @mock.patch("documents.serialisers.bulk_edit.set_permissions") + def test_set_permissions_rejects_non_dict_value(self, m) -> None: + self.setup_mock(m, "set_permissions") + + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "set_permissions", + "parameters": {"set_permissions": False}, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"Expected a dictionary", response.content) + m.assert_not_called() + + @mock.patch("documents.serialisers.bulk_edit.set_permissions") + def test_set_permissions_rejects_non_list_users(self, m) -> None: + self.setup_mock(m, "set_permissions") + + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "set_permissions", + "parameters": { + "set_permissions": {"view": {"users": False}}, + }, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"Expected a list", response.content) + m.assert_not_called() + + @mock.patch("documents.serialisers.bulk_edit.set_permissions") + def test_set_permissions_rejects_unknown_action(self, m) -> None: + self.setup_mock(m, "set_permissions") + + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "set_permissions", + "parameters": { + "set_permissions": {"not_a_real_action": {"users": [1]}}, + }, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"Unknown permission action", response.content) + m.assert_not_called() + + @mock.patch("documents.serialisers.bulk_edit.set_permissions") + def test_set_permissions_null_is_a_noop(self, m) -> None: + """ + A `set_permissions: null` value is a deliberate no-op (e.g. a + request that only updates `owner`), not a validation error -- + this must keep working even though every other non-dict value is + now rejected. + """ + self.setup_mock(m, "set_permissions") + + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "set_permissions", + "parameters": { + "set_permissions": None, + "owner": self.user.id, + }, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + m.assert_called_once() + @mock.patch("documents.serialisers.bulk_edit.set_permissions") def test_set_permissions_merge(self, m) -> None: self.setup_mock(m, "set_permissions") @@ -1300,7 +1392,7 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): self.client.force_authenticate(user=user1) permissions = { - "owner": user1.id, + "view": {"users": [user1.id]}, } response = self.client.post( diff --git a/src/documents/tests/test_api_permissions.py b/src/documents/tests/test_api_permissions.py index 29eb8f496..6124b6c54 100644 --- a/src/documents/tests/test_api_permissions.py +++ b/src/documents/tests/test_api_permissions.py @@ -1499,6 +1499,81 @@ class TestBulkEditObjectPermissions(APITestCase): self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertTrue(Tag.objects.filter(pk=self.t1.id).exists()) + def test_bulk_object_set_permissions_rejects_empty_permissions(self) -> None: + """ + GIVEN: + - Existing objects + WHEN: + - bulk_edit_objects API endpoint is called with set_permissions + operation and an empty permissions dict + THEN: + - Validation fails rather than silently applying a no-op + """ + response = self.client.post( + "/api/bulk_edit_objects/", + json.dumps( + { + "objects": [self.t1.id], + "object_type": "tags", + "operation": "set_permissions", + "permissions": {}, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_bulk_object_set_permissions_rejects_non_dict_permissions(self) -> None: + """ + GIVEN: + - Existing objects + WHEN: + - bulk_edit_objects API endpoint is called with set_permissions + operation and a non-dict permissions value + THEN: + - Validation fails rather than crashing + """ + response = self.client.post( + "/api/bulk_edit_objects/", + json.dumps( + { + "objects": [self.t1.id], + "object_type": "tags", + "operation": "set_permissions", + "permissions": False, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_bulk_object_set_permissions_rejects_unknown_action(self) -> None: + """ + GIVEN: + - Existing objects + WHEN: + - bulk_edit_objects API endpoint is called with set_permissions + operation and an unrecognized permission action name + THEN: + - Validation fails rather than silently no-oping + """ + response = self.client.post( + "/api/bulk_edit_objects/", + json.dumps( + { + "objects": [self.t1.id], + "object_type": "tags", + "operation": "set_permissions", + "permissions": {"not_a_real_action": {"users": [self.user1.id]}}, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + def test_bulk_edit_object_permissions_validation(self) -> None: """ GIVEN: