Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions openedx/core/djangoapps/content_libraries/api/libraries.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,15 +336,24 @@ def get_metadata(queryset: QuerySet[ContentLibrary], text_search: str | None = N
return libraries


def require_permission_for_library_key(library_key: LibraryLocatorV2, user: UserType, permission) -> ContentLibrary:
def require_permission_for_library_key(
library_key: LibraryLocatorV2, user: UserType, permission: str | authz_api.data.PermissionData
) -> ContentLibrary:
"""
Given any of the content library permission strings defined in
openedx.core.djangoapps.content_libraries.permissions,
check if the given user has that permission for the library with the
specified library ID.
Check if the user has the specified permission for a content library.

Raises django.core.exceptions.PermissionDenied if the user doesn't have
permission.
Args:
library_key: The library key identifying the content library
user: The user whose permissions are being checked
permission: Either a permission string from content_libraries.permissions
or a PermissionData instance from the authz API

Returns:
ContentLibrary: The library object if permission check passes

Raises:
ContentLibraryNotFound: If the library with the given key doesn't exist
PermissionDenied: If the user doesn't have the required permission
"""
library_obj = ContentLibrary.objects.get_by_key(library_key)
# obj should be able to read any valid model object but mypy thinks it can only be
Expand Down Expand Up @@ -828,6 +837,8 @@ def _transform_authz_permission_to_legacy_lib_permission(permission: str) -> str
authz_permissions.CREATE_LIBRARY_COLLECTION.identifier: permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
authz_permissions.EDIT_LIBRARY_COLLECTION.identifier: permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
authz_permissions.DELETE_LIBRARY_COLLECTION.identifier: permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
authz_permissions.MANAGE_LIBRARY_TAGS.identifier: permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
authz_permissions.REUSE_LIBRARY_CONTENT.identifier: permissions.CAN_VIEW_THIS_CONTENT_LIBRARY,
}.get(permission, permission)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ def post(self, request, lib_key_str):
Import the contents of the user's clipboard and paste them into the Library
"""
library_key = LibraryLocatorV2.from_string(lib_key_str)
api.require_permission_for_library_key(library_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY)
api.require_permission_for_library_key(library_key, request.user, authz_permissions.EDIT_LIBRARY_CONTENT)

try:
result = api.import_staged_content_from_user_clipboard(library_key, request.user)
Expand Down
3 changes: 2 additions & 1 deletion openedx/core/djangoapps/content_staging/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locator import CourseLocator, LibraryLocatorV2
from openedx_authz.constants import permissions as authz_permissions
from rest_framework.exceptions import NotFound, PermissionDenied, ValidationError
from rest_framework.response import Response
from rest_framework.views import APIView
Expand Down Expand Up @@ -114,7 +115,7 @@ def post(self, request):
lib_api.require_permission_for_library_key(
course_key,
request.user,
lib_api.permissions.CAN_VIEW_THIS_CONTENT_LIBRARY
authz_permissions.REUSE_LIBRARY_CONTENT
)
block = xblock_api.load_block(usage_key, user=None)
version_num = lib_api.get_library_block(usage_key).draft_version_num
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import abc
import json
from io import BytesIO
from unittest.mock import MagicMock
from unittest.mock import MagicMock, patch
from urllib.parse import parse_qs, urlparse

import ddt
Expand Down Expand Up @@ -36,6 +36,7 @@
from openedx.core.djangoapps.content_tagging import api as tagging_api
from openedx.core.djangoapps.content_tagging.models import TaxonomyOrg
from openedx.core.djangolib.testing.utils import skip_unless_cms
from openedx_authz.constants import permissions as authz_permissions

from ....tests.test_objecttag_export_helpers import TaggedCourseMixin

Expand Down Expand Up @@ -1624,6 +1625,35 @@ def test_tag_library(self, user_attr, taxonomy_attr, tag_values, expected_status
assert status.is_success(new_response.status_code)
assert new_response.data == response.data

@ddt.data("libraryA", "collection_key", "container_key")
@patch("openedx_authz.api.is_user_allowed")
@patch("openedx.core.djangoapps.content_tagging.rules.has_studio_write_access")
def test_tag_library_objects_with_manage_library_tags_permission(
self,
object_attr,
mock_has_studio_write_access,
mock_is_user_allowed,
):
"""
Users with MANAGE_LIBRARY_TAGS permission should be able to tag:
- the library itself
- collections in the library
- containers in the library
"""
mock_is_user_allowed.return_value = True
object_id = getattr(self, object_attr)

self.client.force_authenticate(user=self.library_userA)
response = self._call_put_request(object_id, self.tA1.pk, ["Tag 1"])

self.assertEqual(response.status_code, status.HTTP_200_OK)
mock_is_user_allowed.assert_called_with(
self.library_userA.username,
authz_permissions.MANAGE_LIBRARY_TAGS.identifier,
self.libraryA,
)
mock_has_studio_write_access.assert_not_called()

@ddt.data(
"staffA",
"staff",
Expand Down Expand Up @@ -1945,15 +1975,15 @@ def test_get_copied_tags(self):

@ddt.data(
('staff', 'courseA', 8),
('staff', 'libraryA', 8),
('staff', 'collection_key', 8),
('staff', 'libraryA', 17),
('staff', 'collection_key', 17),
("content_creatorA", 'courseA', 18, False),
("content_creatorA", 'libraryA', 18, False),
("content_creatorA", 'collection_key', 18, False),
("library_staffA", 'libraryA', 18, False), # Library users can only view objecttags, not change them?
("library_staffA", 'collection_key', 18, False),
("library_userA", 'libraryA', 18, False),
("library_userA", 'collection_key', 18, False),
("content_creatorA", 'libraryA', 23, False),
("content_creatorA", 'collection_key', 23, False),
("library_staffA", 'libraryA', 23, False), # Library users can only view objecttags, not change them?
("library_staffA", 'collection_key', 23, False),
("library_userA", 'libraryA', 23, False),
("library_userA", 'collection_key', 23, False),
("instructorA", 'courseA', 18),
("course_instructorA", 'courseA', 18),
("course_staffA", 'courseA', 18),
Expand Down
42 changes: 38 additions & 4 deletions openedx/core/djangoapps/content_tagging/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
import django.contrib.auth.models
import openedx_tagging.rules as oel_tagging
import rules
from opaque_keys.edx.locator import LibraryLocatorV2
from openedx_authz import api as authz_api
from openedx_authz.constants import permissions as authz_permissions
from organizations.models import Organization

from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access
Expand All @@ -17,13 +20,12 @@
OrgContentCreatorRole,
OrgInstructorRole,
OrgLibraryUserRole,
OrgStaffRole
OrgStaffRole,
)

from .models import TaxonomyOrg
from .utils import check_taxonomy_context_key_org, get_context_key_from_key_string, rules_cache


UserType = Union[django.contrib.auth.models.User, django.contrib.auth.models.AnonymousUser]


Expand Down Expand Up @@ -218,7 +220,13 @@ def can_change_taxonomy(user: UserType, taxonomy: oel_tagging.Taxonomy) -> bool:
@rules.predicate
def can_change_object_tag_objectid(user: UserType, object_id: str) -> bool:
"""
Everyone that has permission to edit the object should be able to tag it.
Return True if the user may add or modify tags on the given object.

For Content Libraries V2, this requires either explicit library tagging permission
(MANAGE_LIBRARY_TAGS) or org-level admin access for the library's org.

For other contexts (courses, xblocks, etc.), this requires studio write access or
org-level admin access for the object's org.
"""
if not object_id:
return True
Expand All @@ -229,6 +237,16 @@ def can_change_object_tag_objectid(user: UserType, object_id: str) -> bool:
except (ValueError, AssertionError):
return False

# For Content Libraries V2, prefer explicit library tagging permission,
# however, org-level admins are also allowed to perform these operations.
if isinstance(context_key, LibraryLocatorV2) and authz_api.is_user_allowed(
user.username,
authz_permissions.MANAGE_LIBRARY_TAGS.identifier,
str(context_key),
):
return True

# For other contexts (courses, xblocks, etc.), use general write or org-admin access
if has_studio_write_access(user, context_key):
return True

Expand Down Expand Up @@ -276,7 +294,13 @@ def can_view_object_tag_objectid(user: UserType, object_id: str) -> bool:
@rules.predicate
def can_remove_object_tag_objectid(user: UserType, object_id: str) -> bool:
"""
Everyone that has permission to edit the object should be able remove tags from it.
Return True if the user may remove tags from the given object.

For Content Libraries V2, this requires either explicit library tagging permission
(MANAGE_LIBRARY_TAGS) or org-level admin access for the library's org.

For other contexts (courses, xblocks, etc.), this requires studio write access or
org-level admin access for the object's org.
"""
if not object_id:
raise ValueError("object_id must be provided")
Expand All @@ -290,6 +314,16 @@ def can_remove_object_tag_objectid(user: UserType, object_id: str) -> bool:
except (ValueError, AssertionError):
return False

# For Content Libraries V2, prefer explicit library tagging permission,
# however, org-level admins are also allowed to perform these operations.
if isinstance(context_key, LibraryLocatorV2) and authz_api.is_user_allowed(
user.username,
authz_permissions.MANAGE_LIBRARY_TAGS.identifier,
str(context_key),
):
return True

# For other contexts (courses, xblocks, etc.), use general write or org-admin access
if has_studio_write_access(user, context_key):
return True

Expand Down
140 changes: 139 additions & 1 deletion openedx/core/djangoapps/content_tagging/tests/test_rules.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
"""Tests content_tagging rules-based permissions"""

from unittest.mock import patch

import ddt
from django.contrib.auth import get_user_model
from django.test import TestCase
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator, LibraryLocatorV2
from openedx_authz.constants import permissions as authz_permissions
from openedx_tagging.models import (
Tag,
UserSystemDefinedTaxonomy,
Expand All @@ -14,6 +17,7 @@
from common.djangoapps.student.roles import CourseStaffRole, OrgStaffRole

from .. import api
from ..rules import can_change_object_tag_objectid, can_remove_object_tag_objectid
from .test_api import TestTaxonomyMixin

User = get_user_model()
Expand Down Expand Up @@ -608,3 +612,137 @@ def test_view_object_tag_diabled(self):
assert not self.user_both_orgs.has_perm(perm, self.disabled_course_tag)
assert not self.user_org2.has_perm(perm, self.disabled_course_tag)
assert not self.learner.has_perm(perm, self.disabled_course_tag)


class TestRulesLibraryV2Permissions(TestTaxonomyMixin, TestCase):
"""
Tests for Content Library V2 permissions with MANAGE_LIBRARY_TAGS.
"""

def setUp(self):
super().setUp()

self.library_user = User.objects.create(
username="library_user",
email="library_user@example.com",
)
self.org_admin = User.objects.create(
username="org_admin",
email="org_admin@example.com",
)
self.regular_user = User.objects.create(
username="regular_user",
email="regular_user@example.com",
)
self.superuser = User.objects.create(
username="superuser",
email="superuser@example.com",
is_superuser=True,
)

# Make org_admin an OrgStaffRole for org1
update_org_role(
self.superuser,
OrgStaffRole,
self.org_admin,
[self.org1.short_name],
)

self.library_key = LibraryLocatorV2.from_string(f"lib:{self.org1.short_name}:test_library")

@patch("openedx_authz.api.is_user_allowed")
def test_change_objecttag_objectid_with_manage_library_tags_permission(self, mock_is_user_allowed):
"""
Test that a user with MANAGE_LIBRARY_TAGS permission can change tags on a library.
"""
mock_is_user_allowed.return_value = True

result = can_change_object_tag_objectid(self.library_user, str(self.library_key))

self.assertTrue(result)
mock_is_user_allowed.assert_called_once_with(
self.library_user.username,
authz_permissions.MANAGE_LIBRARY_TAGS.identifier,
str(self.library_key),
)

@patch("openedx_authz.api.is_user_allowed")
def test_change_objecttag_objectid_without_manage_library_tags_but_org_admin(self, mock_is_user_allowed):
"""
Test that an org admin can change tags on a library even without
explicit MANAGE_LIBRARY_TAGS permission.
"""
mock_is_user_allowed.return_value = False

result = can_change_object_tag_objectid(self.org_admin, str(self.library_key))

self.assertTrue(result)

@patch("openedx_authz.api.is_user_allowed")
def test_change_objecttag_objectid_without_permissions(self, mock_is_user_allowed):
"""
Test that a regular user without MANAGE_LIBRARY_TAGS permission and
without org admin access cannot change tags.
"""
mock_is_user_allowed.return_value = False

result = can_change_object_tag_objectid(self.regular_user, str(self.library_key))

self.assertFalse(result)

@patch("openedx_authz.api.is_user_allowed")
def test_remove_objecttag_objectid_with_manage_library_tags_permission(self, mock_is_user_allowed):
"""
Test that a user with MANAGE_LIBRARY_TAGS permission can remove tags
from a library.
"""
mock_is_user_allowed.return_value = True

result = can_remove_object_tag_objectid(self.library_user, str(self.library_key))

self.assertTrue(result)
mock_is_user_allowed.assert_called_once_with(
self.library_user.username,
authz_permissions.MANAGE_LIBRARY_TAGS.identifier,
str(self.library_key),
)

@patch("openedx_authz.api.is_user_allowed")
def test_remove_objecttag_objectid_without_manage_library_tags_but_org_admin(self, mock_is_user_allowed):
"""
Test that an org admin can remove tags from a library even without
explicit MANAGE_LIBRARY_TAGS permission.
"""
mock_is_user_allowed.return_value = False

result = can_remove_object_tag_objectid(self.org_admin, str(self.library_key))

self.assertTrue(result)

@patch("openedx_authz.api.is_user_allowed")
def test_remove_objecttag_objectid_without_permissions(self, mock_is_user_allowed):
"""
Test that a regular user without MANAGE_LIBRARY_TAGS permission and
without org admin access cannot remove tags.
"""
mock_is_user_allowed.return_value = False

result = can_remove_object_tag_objectid(self.regular_user, str(self.library_key))

self.assertFalse(result)

def test_invalid_library_key(self):
"""
Test that invalid library keys return False.
"""
self.assertFalse(can_change_object_tag_objectid(self.library_user, "invalid_key"))
self.assertFalse(can_remove_object_tag_objectid(self.library_user, "invalid_key"))

def test_empty_object_id(self):
"""
Test behavior with empty object_id.
"""
self.assertTrue(can_change_object_tag_objectid(self.library_user, ""))

with self.assertRaises(ValueError):
can_remove_object_tag_objectid(self.library_user, "")
Loading