feat: add "can view object tags" permissions (#94)

openedx_learning/__init__.py

@@ -1,4 +1,4 @@
 """
 Open edX Learning ("Learning Core").
 """
-__version__ = "0.2.4"
+__version__ = "0.2.5"

openedx_tagging/core/tagging/rest_api/v1/views.py

@@ -27,7 +27,7 @@
 from ...import_export.api import export_tags
 from ...import_export.parsers import ParserFormat
 from ...models import Taxonomy
-from ...rules import ChangeObjectTagPermissionItem
+from ...rules import ObjectTagPermissionItem
 from ..paginators import SEARCH_TAGS_THRESHOLD, TAGS_THRESHOLD, DisabledTagsPagination, TagsPagination
 from .permissions import ObjectTagObjectPermissions, TagListPermissions, TaxonomyObjectPermissions
 from .serializers import (
@@ -298,10 +298,30 @@ def get_queryset(self) -> models.QuerySet:
             data=self.request.query_params.dict()
         )
         query_params.is_valid(raise_exception=True)
-        taxonomy_id = query_params.data.get("taxonomy", None)
+        taxonomy = query_params.validated_data.get("taxonomy", None)
+        taxonomy_id = None
+        if taxonomy:
+            taxonomy = taxonomy.cast()
+            taxonomy_id = taxonomy.id
+
+        perm = "oel_tagging.view_objecttag"
+        perm_obj = ObjectTagPermissionItem(
+            taxonomy=taxonomy,
+            object_id=object_id,
+        )
+
+        if not self.request.user.has_perm(
+            perm,
+            # The obj arg expects a model, but we are passing an object
+            perm_obj,  # type: ignore[arg-type]
+        ):
+            raise PermissionDenied(
+                "You do not have permission to view object tags for this taxonomy or object_id."
+            )
+
         return get_object_tags(object_id, taxonomy_id)
 
-    def retrieve(self, request, *args, **kwargs):
+    def retrieve(self, request, *args, **kwargs) -> Response:
         """
         Retrieve ObjectTags that belong to a given object_id
 
@@ -312,11 +332,11 @@ def retrieve(self, request, *args, **kwargs):
         path and returns a it as a single result however that is not
         behavior we want.
         """
-        object_tags = self.get_queryset()
+        object_tags = self.filter_queryset(self.get_queryset())
         serializer = ObjectTagSerializer(object_tags, many=True)
         return Response(serializer.data)
 
-    def update(self, request, *args, **kwargs):
+    def update(self, request, *args, **kwargs) -> Response:
         """
         Update ObjectTags that belong to a given object_id
 
@@ -352,15 +372,19 @@ def update(self, request, *args, **kwargs):
         taxonomy = query_params.validated_data.get("taxonomy", None)
         taxonomy = taxonomy.cast()
 
-        perm = f"{taxonomy._meta.app_label}.change_objecttag"
+        perm = "oel_tagging.change_objecttag"
 
         object_id = kwargs.pop('object_id')
-        perm_obj = ChangeObjectTagPermissionItem(
+        perm_obj = ObjectTagPermissionItem(
             taxonomy=taxonomy,
             object_id=object_id,
         )
 
-        if not request.user.has_perm(perm, perm_obj):
+        if not request.user.has_perm(
+            perm,
+            # The obj arg expects a model, but we are passing an object
+            perm_obj,  # type: ignore[arg-type]
+        ):
             raise PermissionDenied(
                 "You do not have permission to change object tags for this taxonomy or object_id."
             )

openedx_tagging/core/tagging/rules.py

@@ -23,7 +23,7 @@
 
 
 @define
-class ChangeObjectTagPermissionItem:
+class ObjectTagPermissionItem:
     """
     Pair of taxonomy and object_id used for permission checking.
     """
@@ -65,6 +65,58 @@ def can_change_tag(user: UserType, tag: Tag | None = None) -> bool:
     )
 
 
+@rules.predicate
+def can_view_object_tag_taxonomy(user: UserType, taxonomy: Taxonomy) -> bool:
+    """
+    Only enabled taxonomy and users with permission to view this taxonomy can view object tags
+    from that taxonomy.
+
+    This rule is different from can_view_taxonomy because it checks if the taxonomy is enabled.
+    """
+    if not taxonomy:
+        return True
+
+    return taxonomy.cast().enabled and can_view_taxonomy(user, taxonomy)
+
+
+@rules.predicate
+def can_view_object_tag_objectid(_user: UserType, _object_id: str) -> bool:
+    """
+    Everybody can view object tags from any objects.
+
+    This rule could be defined in other apps for proper permission checking.
+    """
+    return True
+
+
+@rules.predicate
+def can_view_object_tag(
+    user: UserType, perm_obj: ObjectTagPermissionItem | None = None
+) -> bool:
+    """
+    Checks if the user has permissions to view tags on the given taxonomy and object_id.
+    """
+
+    # The following code allows METHOD permission (GET) in the viewset for everyone
+    if perm_obj is None:
+        return True
+
+    # Checks the permission for the taxonomy
+    taxonomy_perm = user.has_perm(
+        "oel_tagging.view_objecttag_taxonomy", perm_obj.taxonomy
+    )
+    if not taxonomy_perm:
+        return False
+
+    # Checks the permission for the object_id
+    objectid_perm = user.has_perm(
+        "oel_tagging.view_objecttag_objectid",
+        # The obj arg expects an object, but we are passing a string
+        perm_obj.object_id,  # type: ignore[arg-type]
+    )
+    return objectid_perm
+
+
 @rules.predicate
 def can_change_object_tag_objectid(_user: UserType, _object_id: str) -> bool:
     """
@@ -77,7 +129,7 @@ def can_change_object_tag_objectid(_user: UserType, _object_id: str) -> bool:
 
 @rules.predicate
 def can_change_object_tag(
-    user: UserType, perm_obj: ChangeObjectTagPermissionItem | None = None
+    user: UserType, perm_obj: ObjectTagPermissionItem | None = None
 ) -> bool:
     """
     Checks if the user has permissions to create or modify tags on the given taxonomy and object_id.
@@ -122,8 +174,10 @@ def can_change_object_tag(
 rules.add_perm("oel_tagging.add_objecttag", can_change_object_tag)
 rules.add_perm("oel_tagging.change_objecttag", can_change_object_tag)
 rules.add_perm("oel_tagging.delete_objecttag", can_change_object_tag)
-rules.add_perm("oel_tagging.view_objecttag", rules.always_allow)
+rules.add_perm("oel_tagging.view_objecttag", can_view_object_tag)
 
 # Users can tag objects using tags from any taxonomy that they have permission to view
-rules.add_perm("oel_tagging.change_objecttag_taxonomy", can_view_taxonomy)
+rules.add_perm("oel_tagging.view_objecttag_objectid", can_view_object_tag_objectid)
+rules.add_perm("oel_tagging.view_objecttag_taxonomy", can_view_object_tag_taxonomy)
+rules.add_perm("oel_tagging.change_objecttag_taxonomy", can_view_object_tag_taxonomy)
 rules.add_perm("oel_tagging.change_objecttag_objectid", can_change_object_tag_objectid)

tests/openedx_tagging/core/tagging/test_rules.py

@@ -7,7 +7,7 @@
 from django.test.testcases import TestCase
 
 from openedx_tagging.core.tagging.models import ObjectTag
-from openedx_tagging.core.tagging.rules import ChangeObjectTagPermissionItem
+from openedx_tagging.core.tagging.rules import ObjectTagPermissionItem
 
 from .test_models import TestTagTaxonomyMixin
 
@@ -188,7 +188,7 @@ def test_add_change_object_tag(self, perm):
         """
         Everyone can create/edit an ObjectTag with an enabled Taxonomy
         """
-        obj_perm = ChangeObjectTagPermissionItem(
+        obj_perm = ObjectTagPermissionItem(
             taxonomy=self.object_tag.taxonomy,
             object_id=self.object_tag.object_id,
         )
@@ -210,12 +210,12 @@ def test_object_tag_disabled_taxonomy(self, perm):
         """
         self.taxonomy.enabled = False
         self.taxonomy.save()
-        obj_perm = ChangeObjectTagPermissionItem(
+        obj_perm = ObjectTagPermissionItem(
             taxonomy=self.object_tag.taxonomy,
             object_id=self.object_tag.object_id,
         )
         assert self.superuser.has_perm(perm, obj_perm)
-        assert self.staff.has_perm(perm, obj_perm)
+        assert not self.staff.has_perm(perm, obj_perm)
         assert not self.learner.has_perm(perm, obj_perm)
 
     @ddt.data(
@@ -229,7 +229,7 @@ def test_object_tag_without_object_permission(self, perm):
         """
         self.taxonomy.enabled = False
         self.taxonomy.save()
-        obj_perm = ChangeObjectTagPermissionItem(
+        obj_perm = ObjectTagPermissionItem(
             taxonomy=self.object_tag.taxonomy,
             object_id="not abc",
         )