From 9011b4ee6a667d0c49702a0d140fbe3b6d9dbd71 Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Tue, 21 Oct 2025 17:56:29 -0500 Subject: [PATCH 1/3] feat: add the authz check to the library api function feat: add the authz publish check in rest_api blocks and containers feat: add the authz checks in libraries and refactor feat: add collections checks feat: update enforcement in serializer file refactor: refactor the permission check functions fix: fix value error fix: calling the queries twice --- .../content_libraries/api/libraries.py | 103 ++++++++++++++++-- .../content_libraries/api/permissions.py | 10 ++ .../content_libraries/rest_api/blocks.py | 2 +- .../content_libraries/rest_api/collections.py | 34 ++++-- .../content_libraries/rest_api/containers.py | 2 +- .../content_libraries/rest_api/libraries.py | 10 +- .../content_libraries/rest_api/serializers.py | 4 +- 7 files changed, 142 insertions(+), 23 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index ee0618898a9a..fe2a55c827b4 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -53,13 +53,10 @@ from django.db import IntegrityError, transaction from django.db.models import Q, QuerySet from django.utils.translation import gettext as _ -from opaque_keys.edx.locator import ( - LibraryLocatorV2, - LibraryUsageLocatorV2, -) -from openedx_events.content_authoring.data import ( - ContentLibraryData, -) +from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 +from openedx_authz import api as authz_api +from openedx_authz.api import assign_role_to_user_in_scope +from openedx_events.content_authoring.data import ContentLibraryData from openedx_events.content_authoring.signals import ( CONTENT_LIBRARY_CREATED, CONTENT_LIBRARY_DELETED, @@ -70,7 +67,6 @@ from organizations.models import Organization from user_tasks.models import UserTaskArtifact, UserTaskStatus from xblock.core import XBlock -from openedx_authz.api import assign_role_to_user_in_scope from openedx.core.types import User as UserType @@ -78,6 +74,7 @@ from ..constants import ALL_RIGHTS_RESERVED from ..models import ContentLibrary, ContentLibraryPermission from .exceptions import LibraryAlreadyExists, LibraryPermissionIntegrityError +from .permissions import LEGACY_LIB_PERMISSIONS log = logging.getLogger(__name__) @@ -109,6 +106,7 @@ "revert_changes", "get_backup_task_status", "assign_library_role_to_user", + "user_has_permission_across_lib_authz_systems", ] @@ -245,7 +243,14 @@ def user_can_create_library(user: AbstractUser) -> bool: """ Check if the user has permission to create a content library. """ - return user.has_perm(permissions.CAN_CREATE_CONTENT_LIBRARY) + library_permission = permissions.CAN_CREATE_CONTENT_LIBRARY + lib_permission_in_authz = _transform_legacy_lib_permission_to_authz_permission(library_permission) + has_perms = user.has_perm(library_permission) or authz_api.is_user_allowed( + user, + lib_permission_in_authz, + 'sc:generic', + ) + return has_perms def get_libraries_for_user(user, org=None, text_search=None, order=None) -> QuerySet[ContentLibrary]: @@ -336,7 +341,7 @@ def require_permission_for_library_key(library_key: LibraryLocatorV2, user: User 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 # "User | AnonymousUser | None" - if not user.has_perm(permission, obj=library_obj): # type:ignore[arg-type] + if not user_has_permission_across_lib_authz_systems(user, permission, library_obj): raise PermissionDenied return library_obj @@ -754,3 +759,81 @@ def get_backup_task_status( result['file'] = artifact.file return result + + +def _transform_legacy_lib_permission_to_authz_permission(permission: str) -> str: + """ + Transform a legacy content library permission to an openedx-authz permission. + """ + mapping = { + permissions.CAN_CREATE_CONTENT_LIBRARY: 'create_library', + permissions.CAN_DELETE_THIS_CONTENT_LIBRARY: 'delete_library', + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY: 'edit_library_content', + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM: 'manage_library_team', + permissions.CAN_VIEW_THIS_CONTENT_LIBRARY: 'view_library', + permissions.CAN_VIEW_THIS_CONTENT_LIBRARY_TEAM: 'view_library_team', + } + return mapping.get(permission, permission) + + +def _transform_authz_permission_to_legacy_lib_permission(permission: str) -> str: + """ + Transform an openedx-authz permission to a legacy content library permission. + """ + mapping = { + 'publish_library_content': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + 'create_library_collection': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + 'edit_library_collection': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + 'delete_library_collection': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + } + return mapping.get(permission, permission) + + +def user_has_permission_across_lib_authz_systems( + user: UserType, + permission: str, + library_obj: ContentLibrary, +) -> bool: + """ + Check whether a user has a given permission on a content library across both the + legacy edx-platform permission system and the newer openedx-authz system. + + The provided permission name is normalized to both systems (legacy and authz), and + authorization is granted if either: + - the user holds the legacy object-level permission on the ContentLibrary instance, or + - the openedx-authz API allows the user for the corresponding permission on the library. + + Args: + user: The Django user (or user-like object) to check. + permission: The permission identifier (either a legacy codename or an openedx-authz name). + library_key: The LibraryLocatorV2 identifying the target content library. + + Returns: + bool: True if the user is authorized by either system; otherwise False. + + Raises: + ContentLibrary.DoesNotExist: If a library does not exist for the given key. + """ + if _is_legacy_permission(permission): + legacy_permission = permission + authz_permission = _transform_legacy_lib_permission_to_authz_permission(permission) + else: + authz_permission = permission + legacy_permission = _transform_authz_permission_to_legacy_lib_permission(permission) + return ( + # Check both the legacy and the new openedx-authz permissions + user.has_perm(perm=legacy_permission, obj=library_obj) + or authz_api.is_user_allowed( + user, + authz_permission, + str(library_obj.library_key), + ) + ) + + +def _is_legacy_permission(permission: str) -> bool: + """ + Determine if the specified library permission is part of the legacy + or the new openedx-authz system. + """ + return permission in LEGACY_LIB_PERMISSIONS diff --git a/openedx/core/djangoapps/content_libraries/api/permissions.py b/openedx/core/djangoapps/content_libraries/api/permissions.py index 6064b80d6f9e..5b8bd4ba7e1a 100644 --- a/openedx/core/djangoapps/content_libraries/api/permissions.py +++ b/openedx/core/djangoapps/content_libraries/api/permissions.py @@ -12,3 +12,13 @@ CAN_VIEW_THIS_CONTENT_LIBRARY, CAN_VIEW_THIS_CONTENT_LIBRARY_TEAM ) + +LEGACY_LIB_PERMISSIONS = frozenset({ + CAN_CREATE_CONTENT_LIBRARY, + CAN_DELETE_THIS_CONTENT_LIBRARY, + CAN_EDIT_THIS_CONTENT_LIBRARY, + CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM, + CAN_LEARN_FROM_THIS_CONTENT_LIBRARY, + CAN_VIEW_THIS_CONTENT_LIBRARY, + CAN_VIEW_THIS_CONTENT_LIBRARY_TEAM, +}) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py index b93b48e5ad86..f7b0a8d89b61 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py @@ -238,7 +238,7 @@ def post(self, request, usage_key_str): api.require_permission_for_library_key( key.lib_key, request.user, - permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + 'publish_library_content' ) api.publish_component_changes(key, request.user) return Response({}) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/collections.py b/openedx/core/djangoapps/content_libraries/rest_api/collections.py index d893d766d80f..df6ae6529ce1 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/collections.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/collections.py @@ -51,16 +51,10 @@ def get_content_library(self) -> ContentLibrary: lib_key_str = self.kwargs["lib_key_str"] library_key = LibraryLocatorV2.from_string(lib_key_str) - permission = ( - permissions.CAN_VIEW_THIS_CONTENT_LIBRARY - if self.request.method in ['OPTIONS', 'GET'] - else permissions.CAN_EDIT_THIS_CONTENT_LIBRARY - ) - self._content_library = api.require_permission_for_library_key( library_key, self.request.user, - permission, + permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, ) return self._content_library @@ -110,6 +104,11 @@ def create(self, request: RestRequest, *args, **kwargs) -> Response: Create a Collection that belongs to a Content Library """ content_library = self.get_content_library() + api.require_permission_for_library_key( + content_library.library_key, + request.user, + 'create_library_collection' + ) create_serializer = ContentLibraryCollectionUpdateSerializer(data=request.data) create_serializer.is_valid(raise_exception=True) @@ -144,6 +143,11 @@ def partial_update(self, request: RestRequest, *args, **kwargs) -> Response: Update a Collection that belongs to a Content Library """ content_library = self.get_content_library() + api.require_permission_for_library_key( + content_library.library_key, + request.user, + 'edit_library_collection' + ) collection_key = kwargs["key"] update_serializer = ContentLibraryCollectionUpdateSerializer( @@ -165,6 +169,12 @@ def destroy(self, request: RestRequest, *args, **kwargs) -> Response: """ Soft-deletes a Collection that belongs to a Content Library """ + content_library = self.get_content_library() + api.require_permission_for_library_key( + content_library.library_key, + request.user, + 'delete_library_collection' + ) collection = super().get_object() assert collection.learning_package_id authoring_api.delete_collection( @@ -181,6 +191,11 @@ def restore(self, request: RestRequest, *args, **kwargs) -> Response: Restores a soft-deleted Collection that belongs to a Content Library """ content_library = self.get_content_library() + api.require_permission_for_library_key( + content_library.library_key, + request.user, + 'edit_library_collection' + ) assert content_library.learning_package_id collection_key = kwargs["key"] authoring_api.restore_collection( @@ -198,6 +213,11 @@ def update_items(self, request: RestRequest, *args, **kwargs) -> Response: Collection and items must all be part of the given library/learning package. """ content_library = self.get_content_library() + api.require_permission_for_library_key( + content_library.library_key, + request.user, + 'edit_library_collection' + ) collection_key = kwargs["key"] serializer = ContentLibraryItemKeysSerializer(data=request.data) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index 67070a0a82f9..2dab7c25e65c 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -379,7 +379,7 @@ def post(self, request: RestRequest, container_key: LibraryContainerLocator) -> api.require_permission_for_library_key( container_key.lib_key, request.user, - permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + 'publish_library_content' ) api.publish_container_changes(container_key, request.user.id) # If we need to in the future, we could return a list of all the child containers/components that were diff --git a/openedx/core/djangoapps/content_libraries/rest_api/libraries.py b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py index 9f6cca19947a..9aa9fc5c2119 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py @@ -219,7 +219,7 @@ def post(self, request): """ Create a new content library. """ - if not request.user.has_perm(permissions.CAN_CREATE_CONTENT_LIBRARY): + if not api.user_can_create_library(request.user): raise PermissionDenied serializer = ContentLibraryMetadataSerializer(data=request.data) serializer.is_valid(raise_exception=True) @@ -479,7 +479,11 @@ def post(self, request, lib_key_str): descendants. """ key = LibraryLocatorV2.from_string(lib_key_str) - api.require_permission_for_library_key(key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY) + api.require_permission_for_library_key( + key, + request.user, + 'publish_library_content' + ) api.publish_changes(key, request.user.id) return Response({}) @@ -838,7 +842,7 @@ def post(self, request): """ Restore a library from a backup file. """ - if not request.user.has_perm(permissions.CAN_CREATE_CONTENT_LIBRARY): + if not api.user_can_create_library(request.user): raise PermissionDenied serializer = LibraryRestoreFileSerializer(data=request.data) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py index a1e24c6a64a4..c0bf07d087fc 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -14,6 +14,7 @@ from user_tasks.models import UserTaskStatus from openedx.core.djangoapps.content_libraries.tasks import LibraryRestoreTask +from openedx.core.djangoapps.content_libraries import api from openedx.core.djangoapps.content_libraries.api.containers import ContainerType from openedx.core.djangoapps.content_libraries.constants import ALL_RIGHTS_RESERVED, LICENSE_OPTIONS from openedx.core.djangoapps.content_libraries.models import ( @@ -75,7 +76,8 @@ def get_can_edit_library(self, obj): return False library_obj = ContentLibrary.objects.get_by_key(obj.key) - return user.has_perm(permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, obj=library_obj) + return api.user_has_permission_across_lib_authz_systems( + user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, library_obj) class ContentLibraryUpdateSerializer(serializers.Serializer): From 5330c1bb12a627729e72eab54288c5273013d305 Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Tue, 28 Oct 2025 10:17:54 -0500 Subject: [PATCH 2/3] test: add structure for test and apply feedback refactor: refactor the tests and apply feedback fix: apply feedback Revert "refactor: refactor the tests and apply feedback" This reverts commit aa0bd527dd7bc7dec4a7ad7adb41a3c932f4a587. refactor: use constants and avoid mapping test: fix the test to have them in order docs: about we rely on bridgekeeper and the old check for two cases docs: update openedx/core/djangoapps/content_libraries/api/libraries.py Co-authored-by: Maria Grimaldi (Majo) refactor: use global scope wildcard instead of * refactor: allow receiving PermissionData objects refactor: do not inherit from BaseRolesTestCase to favor CL setup methods If both BaseRolesTestCase and ContentLibrariesRestApiTest define a method with the same name (e.g., setUp()), Python will use the one found first in the MRO, which is the one in BaseRolesTestCase because it is listed first in the class definition leading to unexpected behavior. refactor: remove unnecessary imports and indent --- .../content_libraries/api/libraries.py | 58 ++-- .../content_libraries/rest_api/blocks.py | 3 +- .../content_libraries/rest_api/collections.py | 18 +- .../content_libraries/rest_api/containers.py | 3 +- .../content_libraries/rest_api/libraries.py | 3 +- .../tests/test_content_libraries.py | 285 +++++++++++++++++- 6 files changed, 338 insertions(+), 32 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index fe2a55c827b4..66281addb643 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -56,6 +56,7 @@ from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 from openedx_authz import api as authz_api from openedx_authz.api import assign_role_to_user_in_scope +from openedx_authz.constants import permissions as authz_permissions from openedx_events.content_authoring.data import ContentLibraryData from openedx_events.content_authoring.signals import ( CONTENT_LIBRARY_CREATED, @@ -245,10 +246,14 @@ def user_can_create_library(user: AbstractUser) -> bool: """ library_permission = permissions.CAN_CREATE_CONTENT_LIBRARY lib_permission_in_authz = _transform_legacy_lib_permission_to_authz_permission(library_permission) + # The authz_api.is_user_allowed check only validates permissions within a specific library context. Since + # creating a library is not tied to an existing one, we use user.has_perm (via Bridgekeeper) to check if the user + # can create libraries, meaning they have the course creator role. In the future, this should rely on a global (*) + # role defined in the Authorization Framework for instance-level resource creation. has_perms = user.has_perm(library_permission) or authz_api.is_user_allowed( user, lib_permission_in_authz, - 'sc:generic', + authz_api.data.GLOBAL_SCOPE_WILDCARD, ) return has_perms @@ -765,33 +770,33 @@ def _transform_legacy_lib_permission_to_authz_permission(permission: str) -> str """ Transform a legacy content library permission to an openedx-authz permission. """ - mapping = { - permissions.CAN_CREATE_CONTENT_LIBRARY: 'create_library', - permissions.CAN_DELETE_THIS_CONTENT_LIBRARY: 'delete_library', - permissions.CAN_EDIT_THIS_CONTENT_LIBRARY: 'edit_library_content', - permissions.CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM: 'manage_library_team', - permissions.CAN_VIEW_THIS_CONTENT_LIBRARY: 'view_library', - permissions.CAN_VIEW_THIS_CONTENT_LIBRARY_TEAM: 'view_library_team', - } - return mapping.get(permission, permission) + # There is no dedicated permission or role for can_create_content_library in openedx-authz yet, + # so we reuse the same permission to rely on user.has_perm via Bridgekeeper. + return { + permissions.CAN_CREATE_CONTENT_LIBRARY: permissions.CAN_CREATE_CONTENT_LIBRARY, + permissions.CAN_DELETE_THIS_CONTENT_LIBRARY: authz_permissions.DELETE_LIBRARY.identifier, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY: authz_permissions.EDIT_LIBRARY_CONTENT.identifier, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM: authz_permissions.MANAGE_LIBRARY_TEAM.identifier, + permissions.CAN_VIEW_THIS_CONTENT_LIBRARY: authz_permissions.VIEW_LIBRARY.identifier, + permissions.CAN_VIEW_THIS_CONTENT_LIBRARY_TEAM: authz_permissions.VIEW_LIBRARY_TEAM.identifier, + }.get(permission, permission) def _transform_authz_permission_to_legacy_lib_permission(permission: str) -> str: """ Transform an openedx-authz permission to a legacy content library permission. """ - mapping = { - 'publish_library_content': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, - 'create_library_collection': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, - 'edit_library_collection': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, - 'delete_library_collection': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, - } - return mapping.get(permission, permission) + return { + authz_permissions.PUBLISH_LIBRARY_CONTENT.identifier: permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + 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, + }.get(permission, permission) def user_has_permission_across_lib_authz_systems( user: UserType, - permission: str, + permission: str | authz_api.data.PermissionData, library_obj: ContentLibrary, ) -> bool: """ @@ -803,17 +808,26 @@ def user_has_permission_across_lib_authz_systems( - the user holds the legacy object-level permission on the ContentLibrary instance, or - the openedx-authz API allows the user for the corresponding permission on the library. + **Note:** + Temporary: this function uses Bridgekeeper-based logic for cases not yet modeled in openedx-authz. + + Current gaps covered here: + - CAN_CREATE_CONTENT_LIBRARY: we call user.has_perm via Bridgekeeper to verify the user is a course creator. + - CAN_VIEW_THIS_CONTENT_LIBRARY: we respect the allow_public_read flag via Bridgekeeper. + + Replace these with authz_api.is_user_allowed once openedx-authz supports + these conditions natively (including global (*) roles). + Args: user: The Django user (or user-like object) to check. permission: The permission identifier (either a legacy codename or an openedx-authz name). - library_key: The LibraryLocatorV2 identifying the target content library. + library_obj: The ContentLibrary instance to check against. Returns: bool: True if the user is authorized by either system; otherwise False. - - Raises: - ContentLibrary.DoesNotExist: If a library does not exist for the given key. """ + if isinstance(permission, authz_api.data.PermissionData): + permission = permission.identifier if _is_legacy_permission(permission): legacy_permission = permission authz_permission = _transform_legacy_lib_permission_to_authz_permission(permission) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py index f7b0a8d89b61..7aa15f6e7834 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py @@ -9,6 +9,7 @@ from django.utils.decorators import method_decorator from drf_yasg.utils import swagger_auto_schema from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 +from openedx_authz.constants import permissions as authz_permissions from openedx_learning.api import authoring as authoring_api from rest_framework import status from rest_framework.exceptions import NotFound, ValidationError @@ -238,7 +239,7 @@ def post(self, request, usage_key_str): api.require_permission_for_library_key( key.lib_key, request.user, - 'publish_library_content' + authz_permissions.PUBLISH_LIBRARY_CONTENT ) api.publish_component_changes(key, request.user) return Response({}) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/collections.py b/openedx/core/djangoapps/content_libraries/rest_api/collections.py index df6ae6529ce1..f4d579aa04a2 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/collections.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/collections.py @@ -13,6 +13,7 @@ from rest_framework.status import HTTP_204_NO_CONTENT from opaque_keys.edx.locator import LibraryLocatorV2 +from openedx_authz.constants import permissions as authz_permissions from openedx_learning.api import authoring as authoring_api from openedx_learning.api.authoring_models import Collection @@ -51,10 +52,15 @@ def get_content_library(self) -> ContentLibrary: lib_key_str = self.kwargs["lib_key_str"] library_key = LibraryLocatorV2.from_string(lib_key_str) + permission = ( + permissions.CAN_VIEW_THIS_CONTENT_LIBRARY + if self.request.method in ['OPTIONS', 'GET'] + else permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + ) self._content_library = api.require_permission_for_library_key( library_key, self.request.user, - permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, + permission, ) return self._content_library @@ -107,7 +113,7 @@ def create(self, request: RestRequest, *args, **kwargs) -> Response: api.require_permission_for_library_key( content_library.library_key, request.user, - 'create_library_collection' + authz_permissions.CREATE_LIBRARY_COLLECTION ) create_serializer = ContentLibraryCollectionUpdateSerializer(data=request.data) create_serializer.is_valid(raise_exception=True) @@ -146,7 +152,7 @@ def partial_update(self, request: RestRequest, *args, **kwargs) -> Response: api.require_permission_for_library_key( content_library.library_key, request.user, - 'edit_library_collection' + authz_permissions.EDIT_LIBRARY_COLLECTION ) collection_key = kwargs["key"] @@ -173,7 +179,7 @@ def destroy(self, request: RestRequest, *args, **kwargs) -> Response: api.require_permission_for_library_key( content_library.library_key, request.user, - 'delete_library_collection' + authz_permissions.DELETE_LIBRARY_COLLECTION ) collection = super().get_object() assert collection.learning_package_id @@ -194,7 +200,7 @@ def restore(self, request: RestRequest, *args, **kwargs) -> Response: api.require_permission_for_library_key( content_library.library_key, request.user, - 'edit_library_collection' + authz_permissions.EDIT_LIBRARY_COLLECTION ) assert content_library.learning_package_id collection_key = kwargs["key"] @@ -216,7 +222,7 @@ def update_items(self, request: RestRequest, *args, **kwargs) -> Response: api.require_permission_for_library_key( content_library.library_key, request.user, - 'edit_library_collection' + authz_permissions.EDIT_LIBRARY_COLLECTION ) collection_key = kwargs["key"] diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index 2dab7c25e65c..c60c40b9802d 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -12,6 +12,7 @@ from drf_yasg import openapi from opaque_keys.edx.locator import LibraryLocatorV2, LibraryContainerLocator +from openedx_authz.constants import permissions as authz_permissions from openedx_learning.api import authoring as authoring_api from rest_framework.generics import GenericAPIView from rest_framework.response import Response @@ -379,7 +380,7 @@ def post(self, request: RestRequest, container_key: LibraryContainerLocator) -> api.require_permission_for_library_key( container_key.lib_key, request.user, - 'publish_library_content' + authz_permissions.PUBLISH_LIBRARY_CONTENT ) api.publish_container_changes(container_key, request.user.id) # If we need to in the future, we could return a list of all the child containers/components that were diff --git a/openedx/core/djangoapps/content_libraries/rest_api/libraries.py b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py index 9aa9fc5c2119..2d50fa6c8644 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py @@ -82,6 +82,7 @@ from user_tasks.models import UserTaskStatus from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 +from openedx_authz.constants import permissions as authz_permissions from organizations.api import ensure_organization from organizations.exceptions import InvalidOrganizationException from organizations.models import Organization @@ -482,7 +483,7 @@ def post(self, request, lib_key_str): api.require_permission_for_library_key( key, request.user, - 'publish_library_content' + authz_permissions.PUBLISH_LIBRARY_CONTENT ) api.publish_changes(key, request.user.id) return Response({}) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 61bc8012640f..91a9c29a3754 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -19,9 +19,10 @@ from django.test import override_settings from django.test.client import Client from freezegun import freeze_time -from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 +from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2, LibraryCollectionLocator from organizations.models import Organization from rest_framework.test import APITestCase +from rest_framework import status from openedx_learning.api.authoring_models import LearningPackage from user_tasks.models import UserTaskStatus, UserTaskArtifact @@ -35,6 +36,9 @@ URL_BLOCK_XBLOCK_HANDLER, ContentLibrariesRestApiTest, ) +from openedx_authz import api as authz_api +from openedx_authz.constants import roles +from openedx_authz.engine.enforcer import AuthzEnforcer from openedx.core.djangoapps.xblock import api as xblock_api from openedx.core.djangolib.testing.utils import skip_unless_cms from openedx_authz.constants.permissions import VIEW_LIBRARY @@ -1704,3 +1708,282 @@ def test_xblock_handler_invalid_key(self): secure_token='random', ))) self.assertEqual(response.status_code, 404) + + +@skip_unless_cms +class ContentLibrariesRestAPIAuthzIntegrationTestCase(ContentLibrariesRestApiTest): + """ + Test that Content Libraries REST API endpoints respect AuthZ roles and permissions. + + Roles tested: + 1. Library Admin: Full access to all library operations. + 2. Library Author: Can view and edit library content, but cannot delete the library. + 3. Library Contributor: Can view and edit library content, but cannot delete or publish the library. + 4. Library User: Can only view library content. + """ + + def setUp(self): + super().setUp() + self._seed_database_with_policies() + + self.library_admin = UserFactory.create( + username="library_admin", + email="libadmin@example.com") + self.library_author = UserFactory.create( + username="library_author", + email="libauthor@example.com") + self.library_contributor = UserFactory.create( + username="library_contributor", + email="libcontributor@example.com") + self.library_user = UserFactory.create( + username="library_user", + email="libuser@example.com") + self.random_user = UserFactory.create( + username="random_user", + email="random@example.com") + + # Define user groups by permission level + self.list_of_all_users = [ + self.library_admin, + self.library_author, + self.library_contributor, + self.library_user, + self.random_user, + ] + self.library_viewers = [self.library_admin, self.library_author, self.library_contributor, self.library_user] + self.library_editors = [self.library_admin, self.library_author, self.library_contributor] + self.library_publishers = [self.library_admin, self.library_author] + self.library_collection_editors = [self.library_admin, self.library_author, self.library_contributor] + self.library_deleters = [self.library_admin] + + # Create library and assign roles + library = self._create_library( + slug="authzlib", + title="AuthZ Test Library", + description="Testing AuthZ", + ) + self.lib_id = library["id"] + + authz_api.assign_role_to_user_in_scope( + self.library_admin.username, + roles.LIBRARY_ADMIN.external_key, self.lib_id) + authz_api.assign_role_to_user_in_scope( + self.library_author.username, + roles.LIBRARY_AUTHOR.external_key, self.lib_id) + authz_api.assign_role_to_user_in_scope( + self.library_contributor.username, + roles.LIBRARY_CONTRIBUTOR.external_key, self.lib_id) + authz_api.assign_role_to_user_in_scope( + self.library_user.username, + roles.LIBRARY_USER.external_key, self.lib_id) + AuthzEnforcer.get_enforcer().load_policy() # Load policies to simulate fresh start + + def tearDown(self): + """Clean up after each test to ensure isolation.""" + super().tearDown() + AuthzEnforcer.get_enforcer().clear_policy() # Clear policies after each test to ensure isolation + + @classmethod + def _seed_database_with_policies(cls): + """Seed the database with policies from the policy file. + + This simulates the one-time database seeding that would happen + during application deployment, separate from the runtime policy loading. + """ + import pkg_resources + from openedx_authz.engine.utils import migrate_policy_between_enforcers + import casbin + + global_enforcer = AuthzEnforcer.get_enforcer() + global_enforcer.load_policy() + model_path = pkg_resources.resource_filename("openedx_authz.engine", "config/model.conf") + policy_path = pkg_resources.resource_filename("openedx_authz.engine", "config/authz.policy") + + migrate_policy_between_enforcers( + source_enforcer=casbin.Enforcer(model_path, policy_path), + target_enforcer=global_enforcer, + ) + global_enforcer.clear_policy() # Clear to simulate fresh start for each test + + def _all_users_excluding(self, excluded_users): + return set(self.list_of_all_users) - set(excluded_users) + + def test_view_permissions(self): + """ + Verify that only users with view permissions can view. + """ + # Test library view access + for user in self.library_viewers: + with self.as_user(user): + self._get_library(self.lib_id, expect_response=status.HTTP_200_OK) + for user in self._all_users_excluding(self.library_viewers): + with self.as_user(user): + self._get_library(self.lib_id, expect_response=status.HTTP_403_FORBIDDEN) + + def test_edit_permissions(self): + """ + Verify that only users with edit permissions can edit. + """ + # Test library edit access + for user in self.library_editors: + with self.as_user(user): + self._update_library( + self.lib_id, + description=f"Description by {user.username}", + expect_response=status.HTTP_200_OK, + ) + #Verify the permitted changes were made + data = self._get_library(self.lib_id) + assert data['description'] == f"Description by {user.username}" + + for user in self._all_users_excluding(self.library_editors): + with self.as_user(user): + self._update_library( + self.lib_id, + description="I can't edit this.", expect_response=status.HTTP_403_FORBIDDEN) + + # Verify the no permitted changes weren't made: + data = self._get_library(self.lib_id) + assert data['description'] != "I can't edit this." + + # Library XBlock editing + for user in self.library_editors: + with self.as_user(user): + # They can create blocks + block_data = self._add_block_to_library(self.lib_id, "problem", f"problem_{user.username}") + # They can modify blocks + self._set_library_block_olx( + block_data["id"], + "", + expect_response=status.HTTP_200_OK) + self._set_library_block_fields( + block_data["id"], + {"data": "", "metadata": {}}, + expect_response=status.HTTP_200_OK) + self._set_library_block_asset( + block_data["id"], + "static/test.txt", + b"data", + expect_response=status.HTTP_200_OK) + # They can remove blocks + self._delete_library_block(block_data["id"], expect_response=status.HTTP_200_OK) + # Verify deletion + self._get_library_block(block_data["id"], expect_response=404) + + # Recreate blocks for further tests + block_data = self._add_block_to_library(self.lib_id, "problem", "new_problem") + + for user in self._all_users_excluding(self.library_editors): + with self.as_user(user): + self._add_block_to_library( + self.lib_id, + "problem", + "problem1", + expect_response=status.HTTP_403_FORBIDDEN) + # They can't modify blocks + self._set_library_block_olx( + block_data["id"], + "", + expect_response=status.HTTP_403_FORBIDDEN) + self._set_library_block_fields( + block_data["id"], + {"data": "", "metadata": {}}, + expect_response=status.HTTP_403_FORBIDDEN) + self._set_library_block_asset( + block_data["id"], + "static/test.txt", + b"data", + expect_response=status.HTTP_403_FORBIDDEN) + # They can't remove blocks + self._delete_library_block(block_data["id"], expect_response=status.HTTP_403_FORBIDDEN) + + def test_publish_permissions(self): + """ + Verify that only users with publish permissions can publish. + """ + # Test publish access + for user in self.library_publishers: + with self.as_user(user): + block_data = self._add_block_to_library(self.lib_id, "problem", f"problem_{user.username}_1") + self._publish_library_block(block_data["id"], expect_response=status.HTTP_200_OK) + block_data = self._add_block_to_library(self.lib_id, "problem", f"problem_{user.username}_2") + assert self._get_library(self.lib_id)['has_unpublished_changes'] is True + self._commit_library_changes(self.lib_id, expect_response=status.HTTP_200_OK) + assert self._get_library(self.lib_id)['has_unpublished_changes'] is False + + block_data = self._add_block_to_library(self.lib_id, "problem", "draft_problem") + assert self._get_library(self.lib_id)['has_unpublished_changes'] is True + + for user in self._all_users_excluding(self.library_publishers): + with self.as_user(user): + self._publish_library_block(block_data["id"], expect_response=status.HTTP_403_FORBIDDEN) + self._commit_library_changes(self.lib_id, expect_response=status.HTTP_403_FORBIDDEN) + # Verify that no changes were published + assert self._get_library(self.lib_id)['has_unpublished_changes'] is True + + def test_collection_permissions(self): + """ + Verify that only users with collection permissions can perform collection actions. + """ + library_key = LibraryLocatorV2.from_string(self.lib_id) + block_data = self._add_block_to_library(self.lib_id, "problem", "collection_problem") + # Test library collection access + for user in self.library_collection_editors: + with self.as_user(user): + # Create collection + collection_data = self._create_collection( + self.lib_id, + title=f"Temp Collection {user.username}", + expect_response=status.HTTP_200_OK) + collection_id = collection_data["key"] + collection_key = LibraryCollectionLocator(lib_key=library_key, collection_id=collection_id) + # Update collection + self._update_collection(collection_key, title="Updated Collection", expect_response=status.HTTP_200_OK) + self._add_items_to_collection( + collection_key, + item_keys=[block_data["id"]], + expect_response=status.HTTP_200_OK) + # Delete collection + self._soft_delete_collection(collection_key, expect_response=status.HTTP_204_NO_CONTENT) + + collection_data = self._create_collection( + self.lib_id, + title="New Temp Collection", + expect_response=status.HTTP_200_OK) + collection_id = collection_data["key"] + collection_key = LibraryCollectionLocator(lib_key=library_key, collection_id=collection_id) + + for user in self._all_users_excluding(self.library_collection_editors): + with self.as_user(user): + # Attempt to create collection + self._create_collection( + self.lib_id, + title="Unauthorized Collection", + expect_response=status.HTTP_403_FORBIDDEN) + # Attempt to update collection + self._update_collection( + collection_key, + title="Unauthorized Change", + expect_response=status.HTTP_403_FORBIDDEN) + self._add_items_to_collection( + collection_key, + item_keys=[block_data["id"]], + expect_response=status.HTTP_403_FORBIDDEN) + # Attempt to delete collection + self._soft_delete_collection(collection_key, expect_response=status.HTTP_403_FORBIDDEN) + + def test_delete_library_permissions(self): + """ + Verify that only users with delete permissions can delete a library. + """ + # Test library delete access + for user in self._all_users_excluding(self.library_deleters): + with self.as_user(user): + result = self._delete_library(self.lib_id, expect_response=status.HTTP_403_FORBIDDEN) + assert 'detail' in result # Error message + assert 'permission' in result['detail'].lower() + + for user in self.library_deleters: + with self.as_user(user): + result = self._delete_library(self.lib_id, expect_response=status.HTTP_200_OK) + assert result == {} From 5104ea1d8f9b03e4755b7ca48bb9b38a2300c308 Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Thu, 13 Nov 2025 11:33:05 -0500 Subject: [PATCH 3/3] chore: bump openedx-authz version --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f516c1b647d3..e5495b08f405 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -826,7 +826,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==0.13.0 +openedx-authz==0.15.0 # via -r requirements/edx/kernel.in openedx-calc==4.0.2 # via -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 086fd64690a9..c37b9eb45409 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1376,7 +1376,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==0.13.0 +openedx-authz==0.15.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 79c0fd08c0bf..9371f16e6c67 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -1003,7 +1003,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==0.13.0 +openedx-authz==0.15.0 # via -r requirements/edx/base.txt openedx-calc==4.0.2 # via -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 2b3f60018375..23655adbdcfe 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1049,7 +1049,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==0.13.0 +openedx-authz==0.15.0 # via -r requirements/edx/base.txt openedx-calc==4.0.2 # via -r requirements/edx/base.txt