From 11c6f46e4472e076f0b0142d9cf502dff2d0acef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 11 Oct 2023 08:50:50 -0300 Subject: [PATCH] refactor: change can_view_object_tag_objectid to return True --- openedx_tagging/core/tagging/rules.py | 6 ++-- .../core/tagging/test_views.py | 35 +++++++++++++------ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/openedx_tagging/core/tagging/rules.py b/openedx_tagging/core/tagging/rules.py index e9724499..426d80cb 100644 --- a/openedx_tagging/core/tagging/rules.py +++ b/openedx_tagging/core/tagging/rules.py @@ -68,11 +68,11 @@ 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. + Everybody can view object tags from any objects. - This rule should be defined in other apps for proper permission checking. + This rule could be defined in other apps for proper permission checking. """ - return False + return True @rules.predicate diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 0650999d..31c70834 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -418,8 +418,8 @@ 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 + if object_id == "unauthorized_id": + return False return can_view_object_tag_objectid(user, object_id) @@ -495,19 +495,16 @@ def _view_object_permission(user, object_id: str) -> bool: 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_403_FORBIDDEN, None), - ("staff", "non-existing-id", status.HTTP_403_FORBIDDEN, None), + (None, status.HTTP_403_FORBIDDEN, None), + ("user", status.HTTP_200_OK, 81), + ("staff", status.HTTP_200_OK, 81), ) @ddt.unpack - def test_retrieve_object_tags(self, user_attr, object_id, expected_status, expected_count): + def test_retrieve_object_tags(self, user_attr, expected_status, expected_count): """ Test retrieving object tags """ - url = OBJECT_TAGS_RETRIEVE_URL.format(object_id=object_id) + url = OBJECT_TAGS_RETRIEVE_URL.format(object_id="abc") if user_attr: user = getattr(self, user_attr) @@ -519,6 +516,24 @@ def test_retrieve_object_tags(self, user_attr, object_id, expected_status, expec if status.is_success(expected_status): assert len(response.data) == expected_count + @ddt.data( + None, + "user", + "staff" + ) + def test_retrieve_object_tags_unauthorized(self, user_attr): + """ + Test retrieving object tags from an unauthorized object_id + """ + url = OBJECT_TAGS_RETRIEVE_URL.format(object_id="unauthorized_id") + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.get(url) + assert response.status_code == status.HTTP_403_FORBIDDEN + @ddt.data( (None, "abc", status.HTTP_403_FORBIDDEN, None), ("user", "abc", status.HTTP_200_OK, 20),