From b5076bf815307049bad2974dd2b82f2ec5d5da6a Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 3 Mar 2026 14:01:00 -0500 Subject: [PATCH 01/14] feat: add missing library enforcements points --- common/djangoapps/student/auth.py | 29 ++++++++++++++++++- .../content_libraries/api/libraries.py | 23 ++++++++++----- .../content_libraries/rest_api/libraries.py | 4 ++- .../core/djangoapps/content_staging/views.py | 3 +- .../rest_api/v1/tests/test_views.py | 24 +++++++-------- .../core/djangoapps/content_tagging/rules.py | 13 +++++++-- 6 files changed, 72 insertions(+), 24 deletions(-) diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index b9abbe1efaf2..ce6adec6b314 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -9,8 +9,9 @@ from ccx_keys.locator import CCXBlockUsageLocator, CCXLocator from django.conf import settings from django.core.exceptions import PermissionDenied -from opaque_keys.edx.locator import LibraryLocator +from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 from openedx_authz import api as authz_api +from openedx_authz.constants import permissions as authz_permissions from openedx_authz.constants.permissions import COURSES_MANAGE_ADVANCED_SETTINGS from openedx.core import toggles as core_toggles @@ -180,6 +181,32 @@ def has_studio_read_access(user, course_key): return bool(STUDIO_VIEW_CONTENT & get_user_permissions(user, course_key)) +def has_library_tagging_access(user, library_key: LibraryLocatorV2) -> bool: + """ + Check if user has permission to tag content in the specified library. + + Note: `MANAGE_LIBRARY_TAGS` implies `EDIT_LIBRARY_CONTENT` via authz policies (g2), + so users with tagging permission also have edit permission automatically. + + Args: + user (User): Django user object + library_key (LibraryLocatorV2): Key for the library + + Returns: + bool: True if user has permission to tag content in the library, False otherwise + """ + # Import here to avoid circular import + from openedx.core.djangoapps.content_libraries.api import libraries as lib_api + + try: + library_obj = lib_api.ContentLibrary.objects.get_by_key(library_key) + return lib_api.user_has_permission_across_lib_authz_systems( + user, authz_permissions.MANAGE_LIBRARY_TAGS, library_obj + ) + except lib_api.ContentLibrary.DoesNotExist: + return False + + def check_course_advanced_settings_access(user, course_key, access_type='read'): """ Check if user has access to advanced settings for a course. diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index fce2ce5ec4f6..d3b8b4365251 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -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 diff --git a/openedx/core/djangoapps/content_libraries/rest_api/libraries.py b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py index 3c531d629706..cefe5c43f269 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py @@ -575,7 +575,9 @@ 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.REUSE_LIBRARY_CONTENT + ) try: result = api.import_staged_content_from_user_clipboard(library_key, request.user) diff --git a/openedx/core/djangoapps/content_staging/views.py b/openedx/core/djangoapps/content_staging/views.py index 37bfa54d7df2..79e5d8f05552 100644 --- a/openedx/core/djangoapps/content_staging/views.py +++ b/openedx/core/djangoapps/content_staging/views.py @@ -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 @@ -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 diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index b40ad5d55156..cc42ff8b5ff9 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1945,18 +1945,18 @@ def test_get_copied_tags(self): @ddt.data( ('staff', 'courseA', 8), - ('staff', 'libraryA', 8), - ('staff', 'collection_key', 8), - ("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), - ("instructorA", 'courseA', 18), - ("course_instructorA", 'courseA', 18), - ("course_staffA", 'courseA', 18), + ('staff', 'libraryA', 23), + ('staff', 'collection_key', 23), + ("content_creatorA", 'courseA', 17, False), + ("content_creatorA", 'libraryA', 28, False), + ("content_creatorA", 'collection_key', 28, False), + ("library_staffA", 'libraryA', 28, False), # Library users can only view objecttags, not change them? + ("library_staffA", 'collection_key', 28, False), + ("library_userA", 'libraryA', 28, False), + ("library_userA", 'collection_key', 28, False), + ("instructorA", 'courseA', 17), + ("course_instructorA", 'courseA', 17), + ("course_staffA", 'courseA', 17), ) @ddt.unpack def test_object_tags_query_count( diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index 3f798d4053ba..bc763cd96a1c 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -7,9 +7,10 @@ import django.contrib.auth.models import openedx_tagging.rules as oel_tagging import rules +from opaque_keys.edx.locator import LibraryLocatorV2 from organizations.models import Organization -from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access +from common.djangoapps.student.auth import has_library_tagging_access, has_studio_read_access, has_studio_write_access from common.djangoapps.student.role_helpers import get_course_roles, get_role_cache from common.djangoapps.student.roles import ( CourseInstructorRole, @@ -218,7 +219,10 @@ 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. + Check if user has permission to tag the object. + + For Content Libraries V2: requires MANAGE_LIBRARY_TAGS permission. + For other contexts (courses, etc.): requires edit/write access. """ if not object_id: return True @@ -229,6 +233,11 @@ def can_change_object_tag_objectid(user: UserType, object_id: str) -> bool: except (ValueError, AssertionError): return False + # For Content Libraries V2, check specific tagging permission + if isinstance(context_key, LibraryLocatorV2) and has_library_tagging_access(user, context_key): + return True + + # For other contexts (courses, xblocks, etc.), use general write access if has_studio_write_access(user, context_key): return True From 5ca6bfc104d97858e5f49853d9872b5264f3ad6d Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Thu, 5 Mar 2026 09:52:41 -0500 Subject: [PATCH 02/14] test: update query count --- .../rest_api/v1/tests/test_views.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index cc42ff8b5ff9..c1ff11cf01b7 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1945,15 +1945,15 @@ def test_get_copied_tags(self): @ddt.data( ('staff', 'courseA', 8), - ('staff', 'libraryA', 23), - ('staff', 'collection_key', 23), + ('staff', 'libraryA', 17), + ('staff', 'collection_key', 17), ("content_creatorA", 'courseA', 17, False), - ("content_creatorA", 'libraryA', 28, False), - ("content_creatorA", 'collection_key', 28, False), - ("library_staffA", 'libraryA', 28, False), # Library users can only view objecttags, not change them? - ("library_staffA", 'collection_key', 28, False), - ("library_userA", 'libraryA', 28, False), - ("library_userA", 'collection_key', 28, False), + ("content_creatorA", 'libraryA', 22, False), + ("content_creatorA", 'collection_key', 22, False), + ("library_staffA", 'libraryA', 22, False), # Library users can only view objecttags, not change them? + ("library_staffA", 'collection_key', 22, False), + ("library_userA", 'libraryA', 22, False), + ("library_userA", 'collection_key', 22, False), ("instructorA", 'courseA', 17), ("course_instructorA", 'courseA', 17), ("course_staffA", 'courseA', 17), From cc0108d642be4ec84c6909e09879e9c00359f60f Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Thu, 5 Mar 2026 10:07:32 -0500 Subject: [PATCH 03/14] feat: add permission mapping for reusing library content --- openedx/core/djangoapps/content_libraries/api/libraries.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index d3b8b4365251..bb00cef5a6fe 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -837,6 +837,7 @@ 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.REUSE_LIBRARY_CONTENT.identifier: permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, }.get(permission, permission) From 1945fbec153a072917fa429d3223ae36a9e0911d Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Thu, 5 Mar 2026 10:26:10 -0500 Subject: [PATCH 04/14] feat: add permission mapping for managing library tags --- openedx/core/djangoapps/content_libraries/api/libraries.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index bb00cef5a6fe..1be452869399 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -837,6 +837,7 @@ 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) From 6a346f67dbbc0318ba2e109f302410605f870766 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 6 Mar 2026 11:56:57 -0500 Subject: [PATCH 05/14] test: update object tags query count in test cases --- .../rest_api/v1/tests/test_views.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index c1ff11cf01b7..6983e1fc5b60 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1945,18 +1945,18 @@ def test_get_copied_tags(self): @ddt.data( ('staff', 'courseA', 8), - ('staff', 'libraryA', 17), - ('staff', 'collection_key', 17), - ("content_creatorA", 'courseA', 17, False), - ("content_creatorA", 'libraryA', 22, False), - ("content_creatorA", 'collection_key', 22, False), - ("library_staffA", 'libraryA', 22, False), # Library users can only view objecttags, not change them? - ("library_staffA", 'collection_key', 22, False), - ("library_userA", 'libraryA', 22, False), - ("library_userA", 'collection_key', 22, False), - ("instructorA", 'courseA', 17), - ("course_instructorA", 'courseA', 17), - ("course_staffA", 'courseA', 17), + ('staff', 'libraryA', 9), + ('staff', 'collection_key', 9), + ("content_creatorA", 'courseA', 18, False), + ("content_creatorA", 'libraryA', 24, False), + ("content_creatorA", 'collection_key', 24, False), + ("library_staffA", 'libraryA', 24, False), # Library users can only view objecttags, not change them? + ("library_staffA", 'collection_key', 24, False), + ("library_userA", 'libraryA', 24, False), + ("library_userA", 'collection_key', 24, False), + ("instructorA", 'courseA', 18), + ("course_instructorA", 'courseA', 18), + ("course_staffA", 'courseA', 18), ) @ddt.unpack def test_object_tags_query_count( From 7474c3e8fe48a8ea2a81902ce97a595203ff13d9 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 6 Mar 2026 15:30:45 -0500 Subject: [PATCH 06/14] feat: validate permissions in remove object tag rule --- openedx/core/djangoapps/content_tagging/rules.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index bc763cd96a1c..d4ba43a54f08 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -299,6 +299,11 @@ def can_remove_object_tag_objectid(user: UserType, object_id: str) -> bool: except (ValueError, AssertionError): return False + # For Content Libraries V2, check specific tagging permission + if isinstance(context_key, LibraryLocatorV2) and has_library_tagging_access(user, context_key): + return True + + # For other contexts (courses, xblocks, etc.), use general write access if has_studio_write_access(user, context_key): return True From 373cc5f0fae09ba9c0a85ec470c9ecfb8b08b037 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 6 Mar 2026 15:59:02 -0500 Subject: [PATCH 07/14] feat: enhance tagging permission checks for content libraries v2 --- openedx/core/djangoapps/content_tagging/rules.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index d4ba43a54f08..f35c74b178bc 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -233,11 +233,12 @@ def can_change_object_tag_objectid(user: UserType, object_id: str) -> bool: except (ValueError, AssertionError): return False - # For Content Libraries V2, check specific tagging permission + # For Content Libraries V2, prefer explicit library tagging permission, + # but fall back to org-level admin access for backwards compatibility. if isinstance(context_key, LibraryLocatorV2) and has_library_tagging_access(user, context_key): return True - # For other contexts (courses, xblocks, etc.), use general write access + # For other contexts (courses, xblocks, etc.), use general write or org-admin access if has_studio_write_access(user, context_key): return True @@ -299,11 +300,12 @@ def can_remove_object_tag_objectid(user: UserType, object_id: str) -> bool: except (ValueError, AssertionError): return False - # For Content Libraries V2, check specific tagging permission + # For Content Libraries V2, prefer explicit library tagging permission, + # but fall back to org-level admin access for backwards compatibility. if isinstance(context_key, LibraryLocatorV2) and has_library_tagging_access(user, context_key): return True - # For other contexts (courses, xblocks, etc.), use general write access + # For other contexts (courses, xblocks, etc.), use general write or org-admin access if has_studio_write_access(user, context_key): return True From 26cf5d249284618ea3b6577be8f818915c4c2404 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 6 Mar 2026 15:59:16 -0500 Subject: [PATCH 08/14] test: update object tags query count for library permissions --- .../rest_api/v1/tests/test_views.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 6983e1fc5b60..23cb55c6bd12 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1945,15 +1945,15 @@ def test_get_copied_tags(self): @ddt.data( ('staff', 'courseA', 8), - ('staff', 'libraryA', 9), - ('staff', 'collection_key', 9), + ('staff', 'libraryA', 11), + ('staff', 'collection_key', 11), ("content_creatorA", 'courseA', 18, False), - ("content_creatorA", 'libraryA', 24, False), - ("content_creatorA", 'collection_key', 24, False), - ("library_staffA", 'libraryA', 24, False), # Library users can only view objecttags, not change them? - ("library_staffA", 'collection_key', 24, False), - ("library_userA", 'libraryA', 24, False), - ("library_userA", 'collection_key', 24, False), + ("content_creatorA", 'libraryA', 32, False), + ("content_creatorA", 'collection_key', 32, False), + ("library_staffA", 'libraryA', 32, False), # Library users can only view objecttags, not change them? + ("library_staffA", 'collection_key', 32, False), + ("library_userA", 'libraryA', 32, False), + ("library_userA", 'collection_key', 32, False), ("instructorA", 'courseA', 18), ("course_instructorA", 'courseA', 18), ("course_staffA", 'courseA', 18), From e9fae9b27684f8ace78a5ba1343778dbd134b217 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 6 Mar 2026 16:02:25 -0500 Subject: [PATCH 09/14] docs: clarify permission requirements for tagging and removing tags on objects --- .../core/djangoapps/content_tagging/rules.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index f35c74b178bc..d7fc7a1f8e91 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -219,10 +219,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: """ - Check if user has permission to tag the object. + Return True if the user may add or modify tags on the given object. - For Content Libraries V2: requires MANAGE_LIBRARY_TAGS permission. - For other contexts (courses, etc.): requires edit/write access. + 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 @@ -286,7 +289,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") From f33ea7b34ed747cf074a9b69c03f9ac719e9661f Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 6 Mar 2026 16:13:11 -0500 Subject: [PATCH 10/14] fix: update permission requirement for pasting clipboard content in library --- .../core/djangoapps/content_libraries/rest_api/libraries.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/libraries.py b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py index cefe5c43f269..63a8f5fccd0d 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py @@ -575,9 +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, authz_permissions.REUSE_LIBRARY_CONTENT - ) + 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) From 7317ce1ffa747dd92966ecfb3f1ab7153510caf1 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 10 Mar 2026 12:49:41 -0500 Subject: [PATCH 11/14] refactor: check permissions within the rules --- common/djangoapps/student/auth.py | 29 +------------------ .../rest_api/v1/tests/test_views.py | 16 +++++----- .../core/djangoapps/content_tagging/rules.py | 19 ++++++++---- 3 files changed, 23 insertions(+), 41 deletions(-) diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index ce6adec6b314..b9abbe1efaf2 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -9,9 +9,8 @@ from ccx_keys.locator import CCXBlockUsageLocator, CCXLocator from django.conf import settings from django.core.exceptions import PermissionDenied -from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 +from opaque_keys.edx.locator import LibraryLocator from openedx_authz import api as authz_api -from openedx_authz.constants import permissions as authz_permissions from openedx_authz.constants.permissions import COURSES_MANAGE_ADVANCED_SETTINGS from openedx.core import toggles as core_toggles @@ -181,32 +180,6 @@ def has_studio_read_access(user, course_key): return bool(STUDIO_VIEW_CONTENT & get_user_permissions(user, course_key)) -def has_library_tagging_access(user, library_key: LibraryLocatorV2) -> bool: - """ - Check if user has permission to tag content in the specified library. - - Note: `MANAGE_LIBRARY_TAGS` implies `EDIT_LIBRARY_CONTENT` via authz policies (g2), - so users with tagging permission also have edit permission automatically. - - Args: - user (User): Django user object - library_key (LibraryLocatorV2): Key for the library - - Returns: - bool: True if user has permission to tag content in the library, False otherwise - """ - # Import here to avoid circular import - from openedx.core.djangoapps.content_libraries.api import libraries as lib_api - - try: - library_obj = lib_api.ContentLibrary.objects.get_by_key(library_key) - return lib_api.user_has_permission_across_lib_authz_systems( - user, authz_permissions.MANAGE_LIBRARY_TAGS, library_obj - ) - except lib_api.ContentLibrary.DoesNotExist: - return False - - def check_course_advanced_settings_access(user, course_key, access_type='read'): """ Check if user has access to advanced settings for a course. diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 23cb55c6bd12..23dfe5a6c6e6 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1945,15 +1945,15 @@ def test_get_copied_tags(self): @ddt.data( ('staff', 'courseA', 8), - ('staff', 'libraryA', 11), - ('staff', 'collection_key', 11), + ('staff', 'libraryA', 17), + ('staff', 'collection_key', 17), ("content_creatorA", 'courseA', 18, False), - ("content_creatorA", 'libraryA', 32, False), - ("content_creatorA", 'collection_key', 32, False), - ("library_staffA", 'libraryA', 32, False), # Library users can only view objecttags, not change them? - ("library_staffA", 'collection_key', 32, False), - ("library_userA", 'libraryA', 32, False), - ("library_userA", 'collection_key', 32, 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), diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index d7fc7a1f8e91..bebdefa1e133 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -8,9 +8,11 @@ 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_library_tagging_access, has_studio_read_access, has_studio_write_access +from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access from common.djangoapps.student.role_helpers import get_course_roles, get_role_cache from common.djangoapps.student.roles import ( CourseInstructorRole, @@ -18,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] @@ -238,7 +239,11 @@ def can_change_object_tag_objectid(user: UserType, object_id: str) -> bool: # For Content Libraries V2, prefer explicit library tagging permission, # but fall back to org-level admin access for backwards compatibility. - if isinstance(context_key, LibraryLocatorV2) and has_library_tagging_access(user, context_key): + 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 @@ -311,7 +316,11 @@ def can_remove_object_tag_objectid(user: UserType, object_id: str) -> bool: # For Content Libraries V2, prefer explicit library tagging permission, # but fall back to org-level admin access for backwards compatibility. - if isinstance(context_key, LibraryLocatorV2) and has_library_tagging_access(user, context_key): + 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 From 59285061b2b622daf081f9d242c23ebd32b96b64 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 10 Mar 2026 19:00:04 -0500 Subject: [PATCH 12/14] chore: clarify org-level admin permissions for library tagging operations --- openedx/core/djangoapps/content_tagging/rules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index bebdefa1e133..e20cb401c688 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -238,7 +238,7 @@ def can_change_object_tag_objectid(user: UserType, object_id: str) -> bool: return False # For Content Libraries V2, prefer explicit library tagging permission, - # but fall back to org-level admin access for backwards compatibility. + # 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, @@ -315,7 +315,7 @@ def can_remove_object_tag_objectid(user: UserType, object_id: str) -> bool: return False # For Content Libraries V2, prefer explicit library tagging permission, - # but fall back to org-level admin access for backwards compatibility. + # 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, From f378617dec4f094a8a2d46fe6f1abf192015f75d Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Wed, 11 Mar 2026 17:11:43 -0500 Subject: [PATCH 13/14] test: add unit tests for MANAGE_LIBRARY_TAGS permission --- .../rest_api/v1/tests/test_views.py | 27 +++- .../content_tagging/tests/test_rules.py | 140 +++++++++++++++++- 2 files changed, 165 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 23dfe5a6c6e6..bb0eab839293 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -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 @@ -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 @@ -1624,6 +1625,30 @@ 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", diff --git a/openedx/core/djangoapps/content_tagging/tests/test_rules.py b/openedx/core/djangoapps/content_tagging/tests/test_rules.py index 75926d094103..b94dafa97d80 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_rules.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_rules.py @@ -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, @@ -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() @@ -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, "") From 67749778e5ff1f9fc7637d75e9740030d3d2570c Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Wed, 11 Mar 2026 17:22:21 -0500 Subject: [PATCH 14/14] chore: fix pylint checks --- .../content_tagging/rest_api/v1/tests/test_views.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index bb0eab839293..b5d7e733448e 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1628,7 +1628,12 @@ def test_tag_library(self, user_attr, taxonomy_attr, tag_values, expected_status @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): + 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