feat: use authz permission checks for tagging in course authoring

.github/workflows/unit-tests.yml

@@ -20,6 +20,7 @@ jobs:
     name: ${{ matrix.shard_name }}(py=${{ matrix.python-version }},dj=${{ matrix.django-version }},mongo=${{ matrix.mongo-version }})
     runs-on: ${{ matrix.os-version }}
     strategy:
+      fail-fast: false
       matrix:
         python-version:
           - "3.12"

cms/djangoapps/contentstore/models.py

@@ -17,7 +17,7 @@
 from opaque_keys.edx.locator import LibraryContainerLocator
 from openedx_content.api import get_published_version
 from openedx_content.models_api import Component, Container
-from openedx_django_lib.fields import immutable_uuid_field, key_field, manual_date_time_field
+from openedx_django_lib.fields import immutable_uuid_field, manual_date_time_field, ref_field
 
 logger = logging.getLogger(__name__)
 
@@ -87,7 +87,7 @@ class EntityLinkBase(models.Model):
     """
     uuid = immutable_uuid_field()
     # Search by library/upstream context key
-    upstream_context_key = key_field(
+    upstream_context_key = ref_field(
         help_text=_("Upstream context key i.e., learning_package/library key"),
         db_index=True,
     )

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py

@@ -274,7 +274,7 @@ def setUp(self):
         collection_key = "test-collection"
         content_api.create_collection(
             learning_package_id=learning_package.id,
-            key=collection_key,
+            collection_code=collection_key,
             title="Test Collection",
             created_by=self.user.id,
         )

cms/djangoapps/contentstore/views/block.py

@@ -12,6 +12,7 @@
 from django.views.decorators.clickjacking import xframe_options_exempt
 from django.views.decorators.http import require_http_methods
 from opaque_keys.edx.keys import CourseKey
+from openedx_authz.constants.permissions import COURSES_VIEW_COURSE
 from web_fragments.fragment import Fragment
 
 from cms.djangoapps.contentstore.utils import load_services_for_studio
@@ -27,6 +28,8 @@
 from common.djangoapps.edxmako.shortcuts import render_to_response, render_to_string
 from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access
 from common.djangoapps.util.json_request import JsonResponse, expect_json
+from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
+from openedx.core.djangoapps.authz.decorators import user_has_course_permission
 from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled
 from openedx.core.lib.xblock_utils import hash_resource, request_token, wrap_xblock, wrap_xblock_aside
 from xmodule.modulestore.django import modulestore  # lint-amnesty, pylint: disable=wrong-import-order
@@ -329,7 +332,7 @@ def xblock_outline_handler(request, usage_key_string):
     a course.
     """
     usage_key = usage_key_with_run(usage_key_string)
-    if not has_studio_read_access(request.user, usage_key.course_key):
+    if not user_has_course_permission(request.user, COURSES_VIEW_COURSE.identifier, usage_key.course_key, LegacyAuthoringPermission.READ):
         raise PermissionDenied()
 
     response_format = request.GET.get("format", "html")

cms/djangoapps/contentstore/views/tests/test_block.py

@@ -19,6 +19,7 @@
 from opaque_keys.edx.asides import AsideUsageKeyV2
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
+from openedx_authz.constants.roles import COURSE_STAFF
 from openedx_events.content_authoring.data import DuplicatedXBlockData
 from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED
 from openedx_events.testing import OpenEdxEventsTestMixin
@@ -54,6 +55,7 @@
 from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
 from common.test.utils import assert_dict_contains_subset
 from lms.djangoapps.lms_xblock.mixin import NONSENSICAL_ACCESS_RESTRICTION
+from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin
 from openedx.core.djangoapps.content_tagging import api as tagging_api
 from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration
 from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE
@@ -3486,6 +3488,125 @@ def validate_xblock_info_consistency(
             self.assertIsNone(xblock_info.get("child_info", None))  # noqa: PT009
 
 
+class TestXBlockOutlineHandlerAuthz(CourseAuthoringAuthzTestMixin, ItemTest):
+    """
+    Unit tests for xblock_outline_handler authorization functionality.
+    """
+
+    def setUp(self):
+        super().setUp()
+        user_id = self.user.id
+        self.chapter = BlockFactory.create(
+            parent_location=self.course.location,
+            category="chapter",
+            display_name="Week 1",
+            user_id=user_id,
+        )
+        self.sequential = BlockFactory.create(
+            parent_location=self.chapter.location,
+            category="sequential",
+            display_name="Lesson 1",
+            user_id=user_id,
+        )
+        self.vertical = BlockFactory.create(
+            parent_location=self.sequential.location,
+            category="vertical",
+            display_name="Unit 1",
+            user_id=user_id,
+        )
+        # Assign COURSE_STAFF role to authorized_user for the course
+        self.add_user_to_role_in_course(
+            self.authorized_user,
+            COURSE_STAFF.external_key,
+            self.course.id
+        )
+
+    def test_authorized_user_gets_json_response(self):
+        """
+        Test that authorized user gets JSON response from xblock_outline_handler.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
+
+        self.client.login(username=self.authorized_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 200
+        json_response = json.loads(resp.content.decode("utf-8"))
+        assert "id" in json_response
+        assert "display_name" in json_response
+        assert "child_info" in json_response
+
+    def test_unauthorized_user_gets_permission_denied(self):
+        """
+        Test that unauthorized user gets 403 response from xblock_outline_handler.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
+
+        self.client.login(username=self.unauthorized_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 403
+
+    def test_superuser_gets_json_response(self):
+        """
+        Test that superuser gets JSON response from xblock_outline_handler.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
+
+        self.client.login(username=self.super_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 200
+        json_response = json.loads(resp.content.decode("utf-8"))
+        assert "id" in json_response
+        assert "display_name" in json_response
+        assert "child_info" in json_response
+
+    def test_staff_user_gets_json_response(self):
+        """
+        Test that staff user gets JSON response from xblock_outline_handler.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
+
+        self.client.login(username=self.staff_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 200
+        json_response = json.loads(resp.content.decode("utf-8"))
+        assert "id" in json_response
+        assert "display_name" in json_response
+        assert "child_info" in json_response
+
+    def test_authorized_chapter_outline(self):
+        """
+        Test that authorized user can access chapter-level outline.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.chapter.location)
+
+        self.client.login(username=self.authorized_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 200
+        json_response = json.loads(resp.content.decode("utf-8"))
+        assert json_response["display_name"] == "Week 1"
+        assert "child_info" in json_response
+        # Verify that children are included (should have the sequential)
+        children = json_response["child_info"]["children"]
+        assert len(children) > 0
+        assert children[0]["display_name"] == "Lesson 1"
+
+    def test_unauthorized_chapter_outline(self):
+        """
+        Test that unauthorized user cannot access chapter-level outline.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.chapter.location)
+
+        self.client.login(username=self.unauthorized_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 403
+
+
 class TestGetMetadataWithProblemDefaults(ModuleStoreTestCase):
     """
     Unit tests for _get_metadata_with_problem_defaults.

cms/djangoapps/modulestore_migrator/api/read_api.py

@@ -135,9 +135,9 @@ def get_migrations(
     if source_key:
         migrations = migrations.filter(source__key=source_key)
     if target_key:
-        migrations = migrations.filter(target__key=str(target_key))
+        migrations = migrations.filter(target__package_ref=str(target_key))
     if target_collection_slug:
-        migrations = migrations.filter(target_collection__key=target_collection_slug)
+        migrations = migrations.filter(target_collection__collection_code=target_collection_slug)
     if task_uuid:
         migrations = migrations.filter(task_status__uuid=task_uuid)
     if is_failed is not None:
@@ -176,9 +176,9 @@ def _migration(m: models.ModulestoreMigration) -> ModulestoreMigration:
     return ModulestoreMigration(
         pk=m.id,
         source_key=m.source.key,
-        target_key=LibraryLocatorV2.from_string(m.target.key),
+        target_key=LibraryLocatorV2.from_string(m.target.package_ref),
         target_title=m.target.title,
-        target_collection_slug=(m.target_collection.key if m.target_collection else None),
+        target_collection_slug=(m.target_collection.collection_code if m.target_collection else None),
         target_collection_title=(m.target_collection.title if m.target_collection else None),
         is_failed=m.is_failed,
         task_uuid=m.task_status.uuid,
@@ -209,7 +209,7 @@ def _block_migration_success(
     """
     Build an instance of the migration success dataclass
     """
-    target_library_key = LibraryLocatorV2.from_string(target.learning_package.key)
+    target_library_key = LibraryLocatorV2.from_string(target.learning_package.package_ref)
     target_key: LibraryUsageLocatorV2 | LibraryContainerLocator
     if hasattr(target, "component"):
         target_key = library_component_usage_key(target_library_key, target.component)

cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py

@@ -18,6 +18,10 @@ class LibraryMigrationCollectionSerializer(serializers.ModelSerializer):
     """
     Serializer for the target collection of a library migration.
     """
+    # Expose Collection.collection_code as "key" to preserve the REST API field name.
+    # This is temporary: https://github.com/openedx/openedx-platform/issues/38406
+    key = serializers.CharField(source='collection_code')
+
     class Meta:
         model = Collection
         fields = ["key", "title"]

cms/djangoapps/modulestore_migrator/rest_api/v1/views.py

@@ -540,7 +540,7 @@ def get_queryset(self):
             self.request.user,
             lib_api.permissions.CAN_VIEW_THIS_CONTENT_LIBRARY
         )
-        queryset = queryset.filter(target__key=library_key, source__key__startswith='course-v1')
+        queryset = queryset.filter(target__package_ref=str(library_key), source__key__startswith='course-v1')
 
         return queryset
 

cms/djangoapps/modulestore_migrator/tasks.py

@@ -346,13 +346,13 @@ def _import_structure(
             LibraryUsageLocatorV2(target_library.key, block_type, block_id)  # type: ignore[abstract]
             for block_type, block_id
             in content_api.get_components(migration.target.id).values_list(
-                "component_type__name", "local_key"
+                "component_type__name", "component_code"
             )
         ),
         used_container_slugs=set(
             content_api.get_containers(
                 migration.target.id
-            ).values_list("publishable_entity__key", flat=True)
+            ).values_list("publishable_entity__entity_ref", flat=True)
         ),
         previous_block_migrations=(
             get_migration_blocks(source_data.previous_migration.pk)
@@ -409,7 +409,7 @@ def _populate_collection(user_id: int, migration: models.ModulestoreMigration) -
     if block_target_pks:
         content_api.add_to_collection(
             learning_package_id=migration.target.pk,
-            key=migration.target_collection.key,
+            collection_code=migration.target_collection.collection_code,
             entities_qset=PublishableEntity.objects.filter(id__in=block_target_pks),
             created_by=user_id,
         )
@@ -867,7 +867,7 @@ def _migrate_container(
         container_exists = False
         if PublishableEntity.objects.filter(
             learning_package_id=context.target_package_id,
-            key=target_key.container_id,
+            entity_ref=target_key.container_id,
         ).exists():
             libraries_api.restore_container(container_key=target_key)
             container = libraries_api.get_container(target_key)
@@ -932,7 +932,7 @@ def _migrate_component(
     try:
         component = content_api.get_components(context.target_package_id).get(
             component_type=component_type,
-            local_key=target_key.block_id,
+            component_code=target_key.block_id,
         )
         component_existed = True
         # Do we have a specific method for this?
@@ -953,7 +953,7 @@ def _migrate_component(
         component = content_api.create_component(
             context.target_package_id,
             component_type=component_type,
-            local_key=target_key.block_id,
+            component_code=target_key.block_id,
             created=context.created_at,
             created_by=context.created_by,
         )
@@ -971,7 +971,7 @@ def _migrate_component(
             continue
         new_path = f"static/{filename}"
         content_api.create_component_version_media(
-            component_version.pk, media_pk, key=new_path
+            component_version.pk, media_pk, path=new_path
         )
 
     # Publish the component

cms/djangoapps/modulestore_migrator/tests/test_api.py

@@ -232,7 +232,7 @@ def test_start_migration_to_library_with_collection(self):
         collection_key = "test-collection"
         content_api.create_collection(
             learning_package_id=self.learning_package.id,
-            key=collection_key,
+            collection_code=collection_key,
             title="Test Collection",
             created_by=user.id,
         )
@@ -249,7 +249,7 @@ def test_start_migration_to_library_with_collection(self):
         )
 
         modulestoremigration = ModulestoreMigration.objects.get()
-        assert modulestoremigration.target_collection.key == collection_key
+        assert modulestoremigration.target_collection.collection_code == collection_key
 
     def test_start_migration_to_library_with_strategy_skip(self):
         """
@@ -487,19 +487,19 @@ def test_migration_api_for_various_scenarios(self):
         # Lib 2 has Collection C
         content_api.create_collection(
             learning_package_id=self.learning_package.id,
-            key="test-collection-1a",
+            collection_code="test-collection-1a",
             title="Test Collection A in Lib 1",
             created_by=user.id,
         )
         content_api.create_collection(
             learning_package_id=self.learning_package.id,
-            key="test-collection-1b",
+            collection_code="test-collection-1b",
             title="Test Collection B in Lib 1",
             created_by=user.id,
         )
         content_api.create_collection(
             learning_package_id=self.learning_package_2.id,
-            key="test-collection-2c",
+            collection_code="test-collection-2c",
             title="Test Collection C in Lib 2",
             created_by=user.id,
         )