From c1502d01068559e3aae76ad16aa7cea7ef2e13fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Tue, 10 Oct 2023 11:23:42 -0300 Subject: [PATCH] feat: add ObjectTag view permissions --- openedx_learning/__init__.py | 2 +- .../core/tagging/rest_api/v1/views.py | 14 +++++++++- openedx_tagging/core/tagging/rules.py | 11 ++++++++ .../core/tagging/test_views.py | 28 ++++++++++++++----- 4 files changed, 46 insertions(+), 9 deletions(-) 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/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index f940add1..c7181842 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -242,6 +242,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 +267,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..e9724499 100644 --- a/openedx_tagging/core/tagging/rules.py +++ b/openedx_tagging/core/tagging/rules.py @@ -65,6 +65,16 @@ def can_change_tag(user: UserType, tag: Tag | None = None) -> bool: ) +@rules.predicate +def can_view_object_tag_objectid(_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 + + @rules.predicate def can_change_object_tag_objectid(_user: UserType, _object_id: str) -> bool: """ @@ -124,5 +134,6 @@ def can_change_object_tag( rules.add_perm("oel_tagging.view_objecttag", rules.always_allow) # Users can tag objects using tags from any taxonomy that they have permission to view +rules.add_perm("oel_tagging.view_objecttag_objectid", can_view_object_tag_objectid) rules.add_perm("oel_tagging.change_objecttag_taxonomy", can_view_taxonomy) rules.add_perm("oel_tagging.change_objecttag_objectid", can_change_object_tag_objectid) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 9183ee5f..0650999d 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -15,6 +15,7 @@ from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy from openedx_tagging.core.tagging.rest_api.paginators import TagsPagination +from openedx_tagging.core.tagging.rules import can_change_object_tag_objectid, can_view_object_tag_objectid User = get_user_model() @@ -404,11 +405,23 @@ class TestObjectTagViewSet(APITestCase): def setUp(self): - def _object_permission(_user, object_id: str) -> bool: + def _change_object_permission(user, object_id: str) -> bool: """ - Everyone have object permission on object_id "abc" + Everyone have object permission on object_id "abc" and "limit_tag_count" """ - return object_id in ("abc", "limit_tag_count") + if object_id in ("abc", "limit_tag_count"): + return True + + return can_change_object_tag_objectid(user, object_id) + + def _view_object_permission(user, object_id: str) -> bool: + """ + Everyone have object permission on object_id "abc", "limit_tag_count" and "view_only" + """ + if object_id in ("abc", "limit_tag_count", "view_only"): + return True + + return can_view_object_tag_objectid(user, object_id) super().setUp() @@ -478,15 +491,16 @@ def _object_permission(_user, object_id: str) -> bool: self.dummy_taxonomies.append(taxonomy) # Override the object permission for the test - rules.set_perm("oel_tagging.change_objecttag_objectid", _object_permission) + rules.set_perm("oel_tagging.change_objecttag_objectid", _change_object_permission) + rules.set_perm("oel_tagging.view_objecttag_objectid", _view_object_permission) @ddt.data( (None, "abc", status.HTTP_403_FORBIDDEN, None), ("user", "abc", status.HTTP_200_OK, 81), ("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): @@ -744,7 +758,7 @@ def test_tag_object_without_permission(self, user_attr, expected_status): user = getattr(self, user_attr) self.client.force_authenticate(user=user) - url = OBJECT_TAGS_UPDATE_URL.format(object_id="not abc", taxonomy_id=self.enabled_taxonomy.pk) + url = OBJECT_TAGS_UPDATE_URL.format(object_id="view_only", taxonomy_id=self.enabled_taxonomy.pk) response = self.client.put(url, {"tags": ["Tag 1"]}, format="json") assert response.status_code == expected_status