Skip to content

Commit 691d65e

Browse files
committed
Address PR #1384 review: add CreateLabelForLabelsetMutation tests
Reviewer flagged that the behavioral change to CreateLabelForLabelsetMutation (creator-only -> guardian UPDATE) lacked test coverage matching the RemoveLabelsFromLabelsetMutation suite. Adds CreateLabelForLabelsetMutationTestCase covering: creator happy path, non-owner without permission rejected, is_public alone insufficient, and non-owner with explicit UPDATE grant succeeds. Also fixes the minor style nit on the IDOR-deny path - raise the exception with parentheses for consistency with idiomatic Python.
1 parent 32703db commit 691d65e

2 files changed

Lines changed: 138 additions & 2 deletions

File tree

config/graphql/label_mutations.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ def mutate(root, info, labelset_id, text, description, color, icon, label_type):
240240
include_group_permissions=True,
241241
):
242242
# Generic deny path — same message and code path as not-found
243-
raise LabelSet.DoesNotExist
243+
raise LabelSet.DoesNotExist()
244244
logger.debug("CreateLabelForLabelsetMutation - mutate / Labelset", labelset)
245245
obj = AnnotationLabel.objects.create(
246246
text=text,
@@ -305,7 +305,7 @@ def mutate(root, info, label_ids, labelset_id):
305305
include_group_permissions=True,
306306
):
307307
# Generic deny path — same message and code path as not-found
308-
raise LabelSet.DoesNotExist
308+
raise LabelSet.DoesNotExist()
309309
labelset.annotation_labels.remove(*label_pks)
310310
ok = True
311311
message = "Success"

opencontractserver/tests/test_label_mutations.py

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,27 @@ def __init__(self, user):
3737
"""
3838

3939

40+
CREATE_LABEL_FOR_LABELSET_MUTATION = """
41+
mutation CreateLabelForLabelset(
42+
$labelsetId: String!
43+
$text: String
44+
$color: String
45+
$labelType: String
46+
) {
47+
createAnnotationLabelForLabelset(
48+
labelsetId: $labelsetId
49+
text: $text
50+
color: $color
51+
labelType: $labelType
52+
) {
53+
ok
54+
message
55+
objId
56+
}
57+
}
58+
"""
59+
60+
4061
class RemoveLabelsFromLabelsetMutationTestCase(TestCase):
4162
"""Regression coverage for issue #1359."""
4263

@@ -226,3 +247,118 @@ def test_remove_labels_allows_non_owner_with_explicit_update_permission(
226247
)
227248
remaining = set(self.labelset.annotation_labels.values_list("pk", flat=True))
228249
self.assertEqual(remaining, {self.label_b.pk, self.label_c.pk})
250+
251+
252+
class CreateLabelForLabelsetMutationTestCase(TestCase):
253+
"""Coverage for ``CreateLabelForLabelsetMutation`` permission gating.
254+
255+
The mutation moved from creator-only to guardian-permission-based, so we
256+
pin the four cases reviewers asked for: creator happy path, non-creator
257+
rejection, ``is_public`` is not enough, and explicit ``UPDATE`` works.
258+
"""
259+
260+
def setUp(self) -> None:
261+
self.user = User.objects.create_user(
262+
username="testuser", password="testpassword"
263+
)
264+
self.other_user = User.objects.create_user(
265+
username="otheruser", password="otherpassword"
266+
)
267+
268+
self.client = Client(schema, context_value=TestContext(self.user))
269+
self.other_client = Client(schema, context_value=TestContext(self.other_user))
270+
271+
self.labelset = LabelSet.objects.create(
272+
title="Test Labelset",
273+
description="Test labelset",
274+
creator=self.user,
275+
)
276+
set_permissions_for_obj_to_user(
277+
self.user, self.labelset, [PermissionTypes.CRUD]
278+
)
279+
280+
def _create_variables(self, text: str = "New Label") -> dict:
281+
return {
282+
"labelsetId": to_global_id("LabelSetType", self.labelset.id),
283+
"text": text,
284+
"color": "#ABCDEF",
285+
"labelType": LabelType.SPAN_LABEL,
286+
}
287+
288+
def test_creator_can_create_label(self) -> None:
289+
"""Regression guard: the creator's happy path still works."""
290+
291+
result = self.client.execute(
292+
CREATE_LABEL_FOR_LABELSET_MUTATION,
293+
variables=self._create_variables("Creator Label"),
294+
)
295+
296+
self.assertIsNone(result.get("errors"))
297+
data = result["data"]["createAnnotationLabelForLabelset"]
298+
self.assertTrue(
299+
data["ok"],
300+
msg=f"Mutation did not succeed. Message: {data['message']}",
301+
)
302+
self.assertEqual(data["message"], "SUCCESS")
303+
self.assertIsNotNone(data["objId"])
304+
self.assertEqual(self.labelset.annotation_labels.count(), 1)
305+
self.assertEqual(self.labelset.annotation_labels.first().text, "Creator Label")
306+
307+
def test_non_owner_without_permission_is_rejected(self) -> None:
308+
"""A user with no permission on the labelset cannot add labels."""
309+
310+
result = self.other_client.execute(
311+
CREATE_LABEL_FOR_LABELSET_MUTATION,
312+
variables=self._create_variables("Should Not Exist"),
313+
)
314+
315+
self.assertIsNone(result.get("errors"))
316+
data = result["data"]["createAnnotationLabelForLabelset"]
317+
self.assertFalse(data["ok"])
318+
self.assertEqual(self.labelset.annotation_labels.count(), 0)
319+
# No orphan AnnotationLabel should leak when the caller is unauthorized
320+
self.assertFalse(
321+
AnnotationLabel.objects.filter(text="Should Not Exist").exists()
322+
)
323+
324+
def test_non_owner_rejected_even_when_labelset_is_public(self) -> None:
325+
"""``is_public`` is READ-only and must not grant CREATE rights."""
326+
327+
self.labelset.is_public = True
328+
self.labelset.save()
329+
330+
result = self.other_client.execute(
331+
CREATE_LABEL_FOR_LABELSET_MUTATION,
332+
variables=self._create_variables("Public Should Not Help"),
333+
)
334+
335+
self.assertIsNone(result.get("errors"))
336+
data = result["data"]["createAnnotationLabelForLabelset"]
337+
self.assertFalse(data["ok"])
338+
self.assertEqual(self.labelset.annotation_labels.count(), 0)
339+
self.assertFalse(
340+
AnnotationLabel.objects.filter(text="Public Should Not Help").exists()
341+
)
342+
343+
def test_non_owner_with_explicit_update_permission_can_create(self) -> None:
344+
"""A collaborator with guardian UPDATE on the labelset may add labels."""
345+
346+
set_permissions_for_obj_to_user(
347+
self.other_user, self.labelset, [PermissionTypes.UPDATE]
348+
)
349+
350+
result = self.other_client.execute(
351+
CREATE_LABEL_FOR_LABELSET_MUTATION,
352+
variables=self._create_variables("Collaborator Label"),
353+
)
354+
355+
self.assertIsNone(result.get("errors"))
356+
data = result["data"]["createAnnotationLabelForLabelset"]
357+
self.assertTrue(
358+
data["ok"],
359+
msg=f"Mutation did not succeed. Message: {data['message']}",
360+
)
361+
self.assertEqual(self.labelset.annotation_labels.count(), 1)
362+
new_label = self.labelset.annotation_labels.first()
363+
self.assertEqual(new_label.text, "Collaborator Label")
364+
self.assertEqual(new_label.creator_id, self.other_user.id)

0 commit comments

Comments
 (0)