fix: let course_id bypass library access when reviewing pending updates

cms/envs/test.py

@@ -240,3 +240,8 @@
 INACTIVE_USER_URL = "http://localhost:18010"
 POLICY_CHANGE_GRADES_ROUTING_KEY = "edx.lms.core.default"
 SINGLE_LEARNER_COURSE_REGRADE_ROUTING_KEY = "edx.lms.core.default"
+
+# lms/envs/test.py sets this; the CMS side never has, but embed_block_view (used to preview
+# library content, including from Studio) reads settings.CORS_ORIGIN_WHITELIST directly with
+# no fallback, so any test that exercises it fails with an AttributeError unless this is set.
+CORS_ORIGIN_WHITELIST = []

cms/static/js/views/modals/preview_v2_library_changes.js

@@ -65,7 +65,7 @@ function($, _, gettext, BaseModal, ViewUtils) {
 
         getContentHtml: function() {
             return `
-                <iframe src="${this.courseAuthoringMfeUrl}/legacy/preview-changes/${this.upstreamBlockId}?old=${this.upstreamBlockVersionSynced}">
+                <iframe src="${this.courseAuthoringMfeUrl}/legacy/preview-changes/${this.upstreamBlockId}?old=${this.upstreamBlockVersionSynced}&downstream_block_id=${this.downstreamBlockId}">
             `;
         },
 

openedx/core/djangoapps/authz/decorators.py

@@ -6,12 +6,15 @@
 from django.contrib.auth.models import AbstractUser
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey, UsageKey
+from opaque_keys.edx.locator import LibraryContainerLocator, LibraryUsageLocatorV2
 from openedx_authz import api as authz_api
 from rest_framework import status
 
 from common.djangoapps.student.roles import enable_authz_course_authoring
 from openedx.core.djangoapps.authz.constants import LEGACY_PERMISSION_HANDLER_MAP, LegacyAuthoringPermission
 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin
+from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.exceptions import ItemNotFoundError
 
 log = logging.getLogger(__name__)
 
@@ -124,3 +127,66 @@ def get_course_key(course_id: str) -> CourseKey:
         # Attempt to parse it as such and extract the course key.
         usage_key = UsageKey.from_string(course_id)
         return usage_key.course_key
+
+
+def user_has_course_permission_for_upstream(
+    request,
+    authz_permission: str,
+    upstream_key,
+    param_name: str = "downstream_block_id",
+) -> bool:
+    """
+    Check an AuthZ course permission for the course of a downstream block, but only if
+    that downstream's upstream link actually points at ``upstream_key``.
+
+    Meant for endpoints that are normally scoped to a library resource (``upstream_key``,
+    a ``LibraryUsageLocatorV2`` or ``LibraryContainerLocator``) but should also grant
+    access to a user reviewing that exact resource from a course they can't otherwise
+    view the library from, e.g. a Course Auditor reviewing pending library updates. The
+    caller is expected to fall back to its regular resource-level permission check when
+    this returns False.
+
+    The query param must be the *downstream block's* usage key, not a bare course id: we
+    resolve its real upstream link and compare it against ``upstream_key`` ourselves,
+    rather than trusting the caller's claim that the two are related. Otherwise, holding
+    the permission in any course would grant access to any library resource, whether or
+    not that course actually uses it.
+
+    Returns False (never raises) for any reason the bypass doesn't apply: the param is
+    absent, not a valid usage key, the downstream doesn't exist, has no upstream link, or
+    that link doesn't match ``upstream_key``.
+
+    Deliberately doesn't use cms.lib.xblock.upstream_sync.UpstreamLink: this module is a
+    dependency of low-level apps like content_libraries and xblock (per the "low-level
+    apps should not depend on high-level apps" import-linter contract), so it can't import
+    from upstream_sync without creating a cycle. We only need the raw upstream reference,
+    so we read the block's ``upstream`` field directly (relying on it having the mixin
+    that provides that field applied elsewhere, not on importing that mixin ourselves) and
+    parse it ourselves, the same way UpstreamLink.get_for_block does internally.
+    """
+    downstream_id = request.GET.get(param_name)
+    if not downstream_id:
+        return False
+    try:
+        downstream_key = UsageKey.from_string(downstream_id)
+    except InvalidKeyError:
+        return False
+    try:
+        downstream = modulestore().get_item(downstream_key)
+    except ItemNotFoundError:
+        return False
+
+    upstream_ref = getattr(downstream, "upstream", None)
+    if not upstream_ref:
+        return False
+    try:
+        downstream_upstream_key = LibraryUsageLocatorV2.from_string(upstream_ref)
+    except InvalidKeyError:
+        try:
+            downstream_upstream_key = LibraryContainerLocator.from_string(upstream_ref)
+        except InvalidKeyError:
+            return False
+
+    if downstream_upstream_key != upstream_key:
+        return False
+    return user_has_course_permission(request.user, authz_permission, downstream_key.course_key)

openedx/core/djangoapps/authz/tests/test_decorators.py

@@ -2,15 +2,23 @@
 from unittest.mock import Mock, patch
 
 from django.test import RequestFactory, TestCase
-from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
+from opaque_keys.edx.locator import (
+    BlockUsageLocator,
+    CourseLocator,
+    LibraryContainerLocator,
+    LibraryLocatorV2,
+    LibraryUsageLocatorV2,
+)
 
 from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
 from openedx.core.djangoapps.authz.decorators import (
     authz_permission_required,
     get_course_key,
     user_has_course_permission,
+    user_has_course_permission_for_upstream,
 )
 from openedx.core.lib.api.view_utils import DeveloperErrorResponseException
+from xmodule.modulestore.exceptions import ItemNotFoundError
 
 
 class AuthzPermissionRequiredDecoratorTests(TestCase):
@@ -240,3 +248,155 @@ def test_usage_key_string(self):
         result = get_course_key(str(usage_key))
 
         self.assertEqual(result, self.course_key)  # noqa: PT009
+
+
+class UserHasCoursePermissionForUpstreamTests(TestCase):
+    """Tests for user_has_course_permission_for_upstream."""
+
+    def setUp(self):
+        self.factory = RequestFactory()
+        self.course_key = CourseLocator("TestX", "TST101", "2025")
+        self.downstream_key = BlockUsageLocator(self.course_key, "html", "downstream1")
+        self.library_key = LibraryLocatorV2(org="TestX", slug="lib1")
+        self.upstream_key = LibraryUsageLocatorV2(self.library_key, block_type="html", usage_id="upstream1")
+        self.user = Mock()
+
+    def test_missing_param_denies_without_loading_anything(self):
+        """No query param at all means the bypass doesn't apply."""
+        request = self.factory.get("/test")
+
+        with patch("openedx.core.djangoapps.authz.decorators.modulestore") as mock_modulestore:
+            result = user_has_course_permission_for_upstream(
+                request, "courses.view_library_updates", self.upstream_key,
+            )
+
+        assert result is False
+        mock_modulestore.assert_not_called()
+
+    def test_invalid_usage_key_denies_without_loading_anything(self):
+        """A malformed usage key is treated as absent, not as an error."""
+        request = self.factory.get("/test", {"downstream_block_id": "not-a-real-key"})
+
+        with patch("openedx.core.djangoapps.authz.decorators.modulestore") as mock_modulestore:
+            result = user_has_course_permission_for_upstream(
+                request, "courses.view_library_updates", self.upstream_key,
+            )
+
+        assert result is False
+        mock_modulestore.assert_not_called()
+
+    def test_downstream_not_found_denies(self):
+        """If the downstream block doesn't exist, the bypass doesn't apply."""
+        request = self.factory.get("/test", {"downstream_block_id": str(self.downstream_key)})
+
+        with patch("openedx.core.djangoapps.authz.decorators.modulestore") as mock_modulestore, patch(
+            "openedx.core.djangoapps.authz.decorators.user_has_course_permission",
+        ) as mock_check:
+            mock_modulestore.return_value.get_item.side_effect = ItemNotFoundError
+            result = user_has_course_permission_for_upstream(
+                request, "courses.view_library_updates", self.upstream_key,
+            )
+
+        assert result is False
+        mock_check.assert_not_called()
+
+    def test_downstream_with_no_upstream_link_denies(self):
+        """If the downstream has no upstream field set at all, the bypass doesn't apply."""
+        request = self.factory.get("/test", {"downstream_block_id": str(self.downstream_key)})
+
+        with patch("openedx.core.djangoapps.authz.decorators.modulestore") as mock_modulestore, patch(
+            "openedx.core.djangoapps.authz.decorators.user_has_course_permission",
+        ) as mock_check:
+            mock_modulestore.return_value.get_item.return_value = Mock(spec=["usage_key"])
+            result = user_has_course_permission_for_upstream(
+                request, "courses.view_library_updates", self.upstream_key,
+            )
+
+        assert result is False
+        mock_check.assert_not_called()
+
+    def test_downstream_with_malformed_upstream_denies(self):
+        """If the downstream's upstream field isn't a parseable library key, deny."""
+        request = self.factory.get("/test", {"downstream_block_id": str(self.downstream_key)})
+
+        with patch("openedx.core.djangoapps.authz.decorators.modulestore") as mock_modulestore, patch(
+            "openedx.core.djangoapps.authz.decorators.user_has_course_permission",
+        ) as mock_check:
+            mock_modulestore.return_value.get_item.return_value = Mock(upstream="not-a-real-key")
+            result = user_has_course_permission_for_upstream(
+                request, "courses.view_library_updates", self.upstream_key,
+            )
+
+        assert result is False
+        mock_check.assert_not_called()
+
+    def test_mismatched_upstream_denies(self):
+        """
+        If the downstream links to a *different* upstream than the one being requested,
+        the bypass doesn't apply. This is the crux of the fix for openedx-authz#441: holding
+        the course permission isn't enough by itself, the linked resource has to match.
+        """
+        request = self.factory.get("/test", {"downstream_block_id": str(self.downstream_key)})
+        unrelated_upstream_key = LibraryUsageLocatorV2(self.library_key, block_type="html", usage_id="other")
+
+        with patch("openedx.core.djangoapps.authz.decorators.modulestore") as mock_modulestore, patch(
+            "openedx.core.djangoapps.authz.decorators.user_has_course_permission",
+        ) as mock_check:
+            mock_modulestore.return_value.get_item.return_value = Mock(upstream=str(unrelated_upstream_key))
+            result = user_has_course_permission_for_upstream(
+                request, "courses.view_library_updates", self.upstream_key,
+            )
+
+        assert result is False
+        mock_check.assert_not_called()
+
+    def test_matching_upstream_delegates_to_permission_check(self):
+        """If the downstream's upstream link matches, we check the course permission."""
+        request = self.factory.get("/test", {"downstream_block_id": str(self.downstream_key)})
+        request.user = self.user
+
+        with patch("openedx.core.djangoapps.authz.decorators.modulestore") as mock_modulestore, patch(
+            "openedx.core.djangoapps.authz.decorators.user_has_course_permission",
+            return_value=True,
+        ) as mock_check:
+            mock_modulestore.return_value.get_item.return_value = Mock(upstream=str(self.upstream_key))
+            result = user_has_course_permission_for_upstream(
+                request, "courses.view_library_updates", self.upstream_key,
+            )
+
+        assert result is True
+        mock_check.assert_called_once_with(self.user, "courses.view_library_updates", self.course_key)
+
+    def test_matching_container_upstream_delegates_to_permission_check(self):
+        """Same as above, but the upstream is a container (unit/subsection/section), not a block."""
+        container_key = LibraryContainerLocator(self.library_key, container_type="unit", container_id="upstream-unit")
+        request = self.factory.get("/test", {"downstream_block_id": str(self.downstream_key)})
+        request.user = self.user
+
+        with patch("openedx.core.djangoapps.authz.decorators.modulestore") as mock_modulestore, patch(
+            "openedx.core.djangoapps.authz.decorators.user_has_course_permission",
+            return_value=True,
+        ) as mock_check:
+            mock_modulestore.return_value.get_item.return_value = Mock(upstream=str(container_key))
+            result = user_has_course_permission_for_upstream(
+                request, "courses.view_library_updates", container_key,
+            )
+
+        assert result is True
+        mock_check.assert_called_once_with(self.user, "courses.view_library_updates", self.course_key)
+
+    def test_custom_param_name(self):
+        """The query param name can be overridden."""
+        request = self.factory.get("/test", {"downstream_id": str(self.downstream_key)})
+        request.user = self.user
+
+        with patch("openedx.core.djangoapps.authz.decorators.modulestore") as mock_modulestore, patch(
+            "openedx.core.djangoapps.authz.decorators.user_has_course_permission",
+            return_value=True,
+        ):
+            mock_modulestore.return_value.get_item.return_value = Mock(upstream=str(self.upstream_key))
+            result = user_has_course_permission_for_upstream(
+                request, "courses.view_library_updates", self.upstream_key, param_name="downstream_id",
+            )
+
+        assert result is True

openedx/core/djangoapps/content_libraries/rest_api/blocks.py

@@ -22,6 +22,7 @@
 from rest_framework.views import APIView
 
 import openedx.core.djangoapps.site_configuration.helpers as configuration_helpers
+from openedx.core.djangoapps.authz.decorators import user_has_course_permission_for_upstream
 from openedx.core.djangoapps.content_libraries import api, permissions
 from openedx.core.djangoapps.content_libraries.rest_api import serializers
 from openedx.core.djangoapps.xblock import api as xblock_api
@@ -122,7 +123,10 @@ def get(self, request, usage_key_str):
         versioning.
         """
         key = LibraryUsageLocatorV2.from_string(usage_key_str)
-        api.require_permission_for_library_key(key.lib_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY)
+        if not user_has_course_permission_for_upstream(
+            request, authz_permissions.COURSES_VIEW_LIBRARY_UPDATES.identifier, key
+        ):
+            api.require_permission_for_library_key(key.lib_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY)
         result = api.get_library_block(key, include_collections=True)
 
         return Response(self.serializer_class(result).data)

openedx/core/djangoapps/content_libraries/rest_api/containers.py

@@ -20,6 +20,7 @@
 from rest_framework.status import HTTP_200_OK, HTTP_204_NO_CONTENT
 from rest_framework.views import APIView
 
+from openedx.core.djangoapps.authz.decorators import user_has_course_permission_for_upstream
 from openedx.core.djangoapps.content_libraries import api, permissions
 from openedx.core.lib.api.view_utils import view_auth_classes
 from openedx.core.types.http import RestRequest
@@ -81,11 +82,14 @@ def get(self, request, container_key: LibraryContainerLocator):
         """
         Get information about a container
         """
-        api.require_permission_for_library_key(
-            container_key.lib_key,
-            request.user,
-            permissions.CAN_VIEW_THIS_CONTENT_LIBRARY,
-        )
+        if not user_has_course_permission_for_upstream(
+            request, authz_permissions.COURSES_VIEW_LIBRARY_UPDATES.identifier, container_key
+        ):
+            api.require_permission_for_library_key(
+                container_key.lib_key,
+                request.user,
+                permissions.CAN_VIEW_THIS_CONTENT_LIBRARY,
+            )
         container = api.get_container(container_key, include_collections=True)
         return Response(serializers.LibraryContainerMetadataSerializer(container).data)
 
@@ -186,11 +190,14 @@ def get(self, request, container_key: LibraryContainerLocator):
         ]
         """
         published = request.GET.get('published', 'false').lower() == 'true'
-        api.require_permission_for_library_key(
-            container_key.lib_key,
-            request.user,
-            permissions.CAN_VIEW_THIS_CONTENT_LIBRARY,
-        )
+        if not user_has_course_permission_for_upstream(
+            request, authz_permissions.COURSES_VIEW_LIBRARY_UPDATES.identifier, container_key
+        ):
+            api.require_permission_for_library_key(
+                container_key.lib_key,
+                request.user,
+                permissions.CAN_VIEW_THIS_CONTENT_LIBRARY,
+            )
         child_entities = api.get_container_children(container_key, published=published)
         if container_key.container_type == content_models.Unit.type_code:
             data = serializers.LibraryXBlockMetadataSerializer(child_entities, many=True).data