Skip to content

Commit

Permalink
refactor: change can_view_object_tag_objectid to return True
Browse files Browse the repository at this point in the history
  • Loading branch information
rpenido committed Oct 11, 2023
1 parent c1502d0 commit 11c6f46
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 13 deletions.
6 changes: 3 additions & 3 deletions openedx_tagging/core/tagging/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 25 additions & 10 deletions tests/openedx_tagging/core/tagging/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand All @@ -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),
Expand Down

0 comments on commit 11c6f46

Please sign in to comment.