Skip to content

Commit 3b296a9

Browse files
feat: make object tag permission checks modular
1 parent 10eae2d commit 3b296a9

3 files changed

Lines changed: 60 additions & 33 deletions

File tree

src/openedx_core/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
"""
77

88
# The version for the entire repository
9-
__version__ = "0.39.2"
9+
__version__ = "0.40.0"

src/openedx_tagging/rest_api/v1/serializers.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,12 @@ def get_can_delete_objecttag(self, instance) -> bool | None:
147147
"""
148148
Returns True if the current request user may delete object tags on this taxonomy
149149
"""
150+
return self.can_delete_object_tag(instance)
151+
152+
def can_delete_object_tag(self, instance) -> bool | None:
153+
"""
154+
Check if the user is authorized to delete the provided tag.
155+
"""
150156
perm_name = f'{self.app_label}.remove_objecttag_objectid'
151157
return self._can(perm_name, instance.object_id)
152158

@@ -179,7 +185,6 @@ def to_representation(self, instance: list[ObjectTag]) -> dict:
179185
# Allows consumers like edx-platform to override this
180186
ObjectTagViewMinimalSerializer = self.context["view"].minimal_serializer_class
181187

182-
can_tag_object_perm = f"{self.app_label}.can_tag_object"
183188
by_object: dict[str, dict[str, Any]] = {}
184189
for obj_tag in instance:
185190
if obj_tag.object_id not in by_object:
@@ -192,14 +197,21 @@ def to_representation(self, instance: list[ObjectTag]) -> dict:
192197
tax_entry = {
193198
"name": obj_tag.taxonomy.name if obj_tag.taxonomy else None,
194199
"taxonomy_id": obj_tag.taxonomy_id,
195-
"can_tag_object": self._can(can_tag_object_perm, obj_tag),
200+
"can_tag_object": self.can_tag_object(obj_tag),
196201
"tags": [],
197202
"export_id": obj_tag.export_id,
198203
}
199204
taxonomies.append(tax_entry)
200205
tax_entry["tags"].append(ObjectTagViewMinimalSerializer(obj_tag, context=self.context).data)
201206
return by_object
202207

208+
def can_tag_object(self, obj_tag) -> bool | None:
209+
"""
210+
Check if the user is authorized to tag the provided object.
211+
"""
212+
perm_name = f"{self.app_label}.can_tag_object"
213+
return self._can(perm_name, obj_tag)
214+
203215

204216
class ObjectTagUpdateByTaxonomySerializer(serializers.Serializer): # pylint: disable=abstract-method
205217
"""

src/openedx_tagging/rest_api/v1/views.py

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -448,12 +448,15 @@ class ObjectTagView(
448448

449449
# Serializer used in `get_queryset` when getting tags per taxonomy
450450
serializer_class = ObjectTagSerializer
451+
# Serializer used in `retrieve` to represent all tags of a given object
452+
object_tags_serializer_class = ObjectTagsByTaxonomySerializer
451453
# Serializer used in the result in `to_representation` in `ObjectTagsByTaxonomySerializer`
452454
minimal_serializer_class = ObjectTagMinimalSerializer
453455
permission_classes = [ObjectTagObjectPermissions]
454456
lookup_field = "object_id"
455457
lookup_value_regex = r'[\w\.\+\-@:]+'
456458

459+
457460
def get_queryset(self) -> models.QuerySet:
458461
"""
459462
Return a queryset of object tags for a given object.
@@ -471,23 +474,29 @@ def get_queryset(self) -> models.QuerySet:
471474
taxonomy = taxonomy.cast()
472475
taxonomy_id = taxonomy.id
473476

474-
if object_id.endswith("*") or "," in object_id: # pylint: disable=no-else-raise
475-
raise ValidationError("Retrieving tags from multiple objects is not yet supported.")
477+
if object_id.endswith("*") or "," in object_id:
476478
# Note: This API is actually designed so that in the future it can be extended to return tags for multiple
477479
# objects, e.g. if object_id.endswith("*") then it results in a object_id__startswith query. However, for
478480
# now we have no use case for that so we retrieve tags for one object at a time.
479-
else:
480-
if not self.request.user.has_perm(
481-
"oel_tagging.view_objecttag",
482-
# The obj arg expects a model, but we are passing an object
483-
ObjectTagPermissionItem(taxonomy=taxonomy, object_id=object_id), # type: ignore[arg-type]
484-
):
485-
raise PermissionDenied(
486-
"You do not have permission to view object tags for this taxonomy or object_id."
487-
)
481+
raise ValidationError("Retrieving tags from multiple objects is not yet supported.")
482+
483+
self.ensure_has_view_object_tag_permission(self.request.user, taxonomy, object_id)
488484

489485
return get_object_tags(object_id, taxonomy_id)
490486

487+
def ensure_has_view_object_tag_permission(self, user, taxonomy, object_id):
488+
"""
489+
Check if user has permission to view object tags.
490+
"""
491+
if not user.has_perm(
492+
"oel_tagging.view_objecttag",
493+
# The obj arg expects a model, but we are passing an object
494+
ObjectTagPermissionItem(taxonomy=taxonomy, object_id=object_id), # type: ignore[arg-type]
495+
):
496+
raise PermissionDenied(
497+
"You do not have permission to view object tags for this taxonomy or object_id."
498+
)
499+
491500
def retrieve(self, request, *args, **kwargs) -> Response:
492501
"""
493502
Retrieve ObjectTags that belong to a given object_id
@@ -500,7 +509,7 @@ def retrieve(self, request, *args, **kwargs) -> Response:
500509
behavior we want.
501510
"""
502511
object_tags = self.filter_queryset(self.get_queryset())
503-
serializer = ObjectTagsByTaxonomySerializer(list(object_tags), context=self.get_serializer_context())
512+
serializer = self.object_tags_serializer_class(list(object_tags), context=self.get_serializer_context())
504513
response_data = serializer.data
505514
if self.kwargs["object_id"] not in response_data:
506515
# For consistency, the key with the object_id should always be present in the response, even if there
@@ -514,7 +523,7 @@ def update(self, request, *args, **kwargs) -> Response:
514523
515524
Pass a list of Tag ids or Tag values to be applied to an object id in the
516525
body `tag` parameter, by each taxonomy. Passing an empty list will remove all tags from
517-
the object id on an specific taxonomy.
526+
the object id on a specific taxonomy.
518527
519528
**Example Body Requests**
520529
@@ -550,13 +559,13 @@ def update(self, request, *args, **kwargs) -> Response:
550559
},
551560
]
552561
}
562+
```
553563
"""
554564
partial = kwargs.pop('partial', False)
555565
if partial:
556566
raise MethodNotAllowed("PATCH", detail="PATCH not allowed")
557567

558568
object_id = kwargs.pop('object_id')
559-
perm = "oel_tagging.can_tag_object"
560569
body = ObjectTagUpdateBodySerializer(data=request.data)
561570
body.is_valid(raise_exception=True)
562571

@@ -566,14 +575,33 @@ def update(self, request, *args, **kwargs) -> Response:
566575
return self.retrieve(request, object_id)
567576

568577
# Check permissions
569-
for tagsData in data:
570-
taxonomy = tagsData.get("taxonomy")
578+
self.ensure_user_has_can_tag_object_permissions(request.user, data, object_id)
579+
580+
# Tag object_id per taxonomy
581+
for tag_data in data:
582+
taxonomy = tag_data.get("taxonomy")
583+
tags = tag_data.get("tags", [])
584+
try:
585+
tag_object(object_id, taxonomy, tags)
586+
except TagDoesNotExist as e:
587+
raise ValidationError from e
588+
except ValueError as e:
589+
raise ValidationError from e
571590

591+
return self.retrieve(request, object_id)
592+
593+
def ensure_user_has_can_tag_object_permissions(self, user, tags_data, object_id):
594+
"""
595+
Check if user has permission to tag object for each taxonomy.
596+
"""
597+
perm = "oel_tagging.can_tag_object"
598+
for tag_data in tags_data:
599+
taxonomy = tag_data.get("taxonomy")
572600
perm_obj = ObjectTagPermissionItem(
573601
taxonomy=taxonomy,
574602
object_id=object_id,
575603
)
576-
if not request.user.has_perm(
604+
if not user.has_perm(
577605
perm,
578606
# The obj arg expects a model, but we are passing an object
579607
perm_obj, # type: ignore[arg-type]
@@ -583,19 +611,6 @@ def update(self, request, *args, **kwargs) -> Response:
583611
for Taxonomy: {str(taxonomy)} or Object: {object_id}.
584612
""")
585613

586-
# Tag object_id per taxonomy
587-
for tagsData in data:
588-
taxonomy = tagsData.get("taxonomy")
589-
tags = tagsData.get("tags", [])
590-
try:
591-
tag_object(object_id, taxonomy, tags)
592-
except TagDoesNotExist as e:
593-
raise ValidationError from e
594-
except ValueError as e:
595-
raise ValidationError from e
596-
597-
return self.retrieve(request, object_id)
598-
599614

600615
@view_auth_classes
601616
class ObjectTagCountsView(

0 commit comments

Comments
 (0)