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 <noreply@anthropic.com>
This commit is contained in:
Trenton Holmes
2026-09-13 14:26:40 -07:00
co-authored by Claude Sonnet 5
parent 4dc1736bee
commit a167be5e1f
2 changed files with 37 additions and 9 deletions
+9 -9
View File
@@ -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
@@ -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: