From 447c7ba0613012094034f031016f2a4874e66418 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Mon, 9 Oct 2023 10:21:50 -0300 Subject: [PATCH] feat: add ObjectTag view permissions and filters --- openedx_learning/__init__.py | 2 +- .../core/tagging/rest_api/v1/filters.py | 19 +++++++++++++++++++ .../core/tagging/rest_api/v1/views.py | 16 +++++++++++++++- openedx_tagging/core/tagging/rules.py | 10 ++++++++++ .../core/tagging/test_views.py | 7 ++++--- 5 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 openedx_tagging/core/tagging/rest_api/v1/filters.py diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index fe8603ba..fcfa685c 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -1,4 +1,4 @@ """ Open edX Learning ("Learning Core"). """ -__version__ = "0.2.2" +__version__ = "0.2.3" diff --git a/openedx_tagging/core/tagging/rest_api/v1/filters.py b/openedx_tagging/core/tagging/rest_api/v1/filters.py new file mode 100644 index 00000000..a7c745e0 --- /dev/null +++ b/openedx_tagging/core/tagging/rest_api/v1/filters.py @@ -0,0 +1,19 @@ +""" +API Filters for content +""" + +from rest_framework.filters import BaseFilterBackend + +from ... import rules as oel_tagging + + +class ObjectTagTaxonomyFilterBackend(BaseFilterBackend): + """ + Filter for ObjectTagViewSet to only show taxonomies that the user can view. + """ + + def filter_queryset(self, request, queryset, _): + if oel_tagging.is_taxonomy_admin(request.user): + return queryset + + return queryset.filter(taxonomy__enabled=True) diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index f940add1..f6ee3248 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -26,6 +26,7 @@ from ...models import Taxonomy from ...rules import ChangeObjectTagPermissionItem from ..paginators import SEARCH_TAGS_THRESHOLD, TAGS_THRESHOLD, DisabledTagsPagination, TagsPagination +from .filters import ObjectTagTaxonomyFilterBackend from .permissions import ObjectTagObjectPermissions, TagListPermissions, TaxonomyObjectPermissions from .serializers import ( ObjectTagListQueryParamsSerializer, @@ -228,6 +229,7 @@ class ObjectTagView( serializer_class = ObjectTagSerializer permission_classes = [ObjectTagObjectPermissions] + filter_backends = [ObjectTagTaxonomyFilterBackend] lookup_field = "object_id" def get_queryset(self) -> models.QuerySet: @@ -242,6 +244,18 @@ def get_queryset(self) -> models.QuerySet: ) query_params.is_valid(raise_exception=True) taxonomy_id = query_params.data.get("taxonomy", None) + + # Checks the permission for the object_id + objectid_perm = self.request.user.has_perm( + "oel_tagging.view_objecttag_objectid", + # The obj arg expects an object, but we are passing a string + object_id, # type: ignore[arg-type] + ) + + if not objectid_perm: + raise PermissionDenied( + "You do not have permission to view object tags for this object_id." + ) return get_object_tags(object_id, taxonomy_id) def retrieve(self, request, *args, **kwargs): @@ -255,7 +269,7 @@ 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) diff --git a/openedx_tagging/core/tagging/rules.py b/openedx_tagging/core/tagging/rules.py index 00ec8811..7af6fef3 100644 --- a/openedx_tagging/core/tagging/rules.py +++ b/openedx_tagging/core/tagging/rules.py @@ -104,6 +104,16 @@ def can_change_object_tag( return objectid_perm +@rules.predicate +def can_change_view_object_tag(_user: UserType, _object_id: str) -> bool: + """ + Nobody can view object tags without checking the permission for the tagged object. + + This rule should be defined in other apps for proper permission checking. + """ + return False + + # Taxonomy rules.add_perm("oel_tagging.add_taxonomy", can_change_taxonomy) rules.add_perm("oel_tagging.change_taxonomy", can_change_taxonomy) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 9183ee5f..14aaebfc 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -479,14 +479,15 @@ def _object_permission(_user, object_id: str) -> bool: # Override the object permission for the test rules.set_perm("oel_tagging.change_objecttag_objectid", _object_permission) + rules.set_perm("oel_tagging.view_objecttag_objectid", _object_permission) @ddt.data( (None, "abc", status.HTTP_403_FORBIDDEN, None), - ("user", "abc", status.HTTP_200_OK, 81), + ("user", "abc", status.HTTP_200_OK, 51), ("staff", "abc", status.HTTP_200_OK, 81), (None, "non-existing-id", status.HTTP_403_FORBIDDEN, None), - ("user", "non-existing-id", status.HTTP_200_OK, 0), - ("staff", "non-existing-id", status.HTTP_200_OK, 0), + ("user", "non-existing-id", status.HTTP_403_FORBIDDEN, None), + ("staff", "non-existing-id", status.HTTP_403_FORBIDDEN, None), ) @ddt.unpack def test_retrieve_object_tags(self, user_attr, object_id, expected_status, expected_count):