From b26d3187cae7bf08ce4ab0ac15150bdc6b0dd065 Mon Sep 17 00:00:00 2001 From: Kevyn Suarez Date: Thu, 13 Aug 2026 12:11:57 -0500 Subject: [PATCH 1/3] fix: Course Auditor gets 403 navigating to a course unit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xblock_outline_handler (the course outline tree) was already migrated to the AuthZ-aware user_has_course_permission(..., COURSES_VIEW_COURSE, ..., LegacyAuthoringPermission.READ) check, so it correctly recognizes AuthZ- native roles that have no legacy equivalent, like course_auditor and course_editor. xblock_container_handler (the unit/container page — what's hit when navigating to a unit) and xblock_view_handler (renders each child block's preview fragment on that page), plus xblock_edit_view, were never migrated the same way: they still called the legacy-only has_studio_read_access directly, which only recognizes roles with a legacy equivalent (staff/instructor/limited_staff). A Course Auditor has none, so get_user_permissions() returned no permissions and these handlers raised PermissionDenied, even though the outline (using the correct pattern) let the same user in. Migrate the three remaining read checks in block.py to the same user_has_course_permission pattern xblock_outline_handler already uses. The xblock_handler/handle_xblock CRUD endpoint was already correctly AuthZ-aware (via _check_xblock_permission) and needed no change. Fixes https://github.com/openedx/openedx-authz/issues/384 --- cms/djangoapps/contentstore/views/block.py | 21 +++- .../contentstore/views/tests/test_block.py | 95 +++++++++++++++++++ 2 files changed, 113 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index b2c0e39df77c..b1f1ab8f5056 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -142,7 +142,12 @@ def xblock_view_handler(request, usage_key_string, view_name): the second is the resource description """ 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() accept_header = request.META.get("HTTP_ACCEPT", "application/json") @@ -299,7 +304,12 @@ def xblock_edit_view(request, usage_key_string): Allows editing of an XBlock specified by the usage key. """ 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() store = modulestore() @@ -371,7 +381,12 @@ def xblock_container_handler(request, usage_key_string): """ 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") diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 9e53a3aa0195..dd087fc1418f 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -3627,6 +3627,101 @@ def test_unauthorized_chapter_outline(self): assert resp.status_code == 403 +@ddt.ddt +class TestXBlockContainerAndViewHandlerAuthz(CourseAuthoringAuthzTestMixin, ItemTest): + """ + Unit tests for xblock_container_handler and xblock_view_handler authorization. + + Regression test for openedx-authz#384: navigating to a unit (the container page, + which in turn renders each child block via the view handler) returned 403 for + roles like course_auditor that have no legacy role equivalent, because these two + handlers checked the legacy-only has_studio_read_access instead of the AuthZ-aware + user_has_course_permission already used by xblock_outline_handler. + """ + + 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, + ) + + @ddt.data( + COURSE_STAFF.external_key, + COURSE_ADMIN.external_key, + COURSE_AUDITOR.external_key, + COURSE_EDITOR.external_key, + ) + def test_course_roles_can_view_unit_container(self, role_key): + """ + Any role with COURSES_VIEW_COURSE, including the legacy-less course_auditor + and course_editor roles, can open the unit (container) page. + """ + role_user = UserFactory(password=self.password) + self.add_user_to_role_in_course(role_user, role_key, self.course.id) + + container_url = reverse_usage_url("xblock_container_handler", self.vertical.location) + self.client.login(username=role_user.username, password=self.password) + resp = self.client.get(container_url, HTTP_ACCEPT="application/json") + + assert resp.status_code == 200 + + @ddt.data( + COURSE_STAFF.external_key, + COURSE_ADMIN.external_key, + COURSE_AUDITOR.external_key, + COURSE_EDITOR.external_key, + ) + def test_course_roles_can_render_unit_preview(self, role_key): + """ + Any role with COURSES_VIEW_COURSE can render the unit's preview fragment, + which is what the frontend fetches for each block shown on the unit page. + """ + role_user = UserFactory(password=self.password) + self.add_user_to_role_in_course(role_user, role_key, self.course.id) + + preview_url = reverse_usage_url( + "xblock_view_handler", self.vertical.location, {"view_name": "container_preview"} + ) + self.client.login(username=role_user.username, password=self.password) + resp = self.client.get(preview_url, HTTP_ACCEPT="application/json") + + assert resp.status_code == 200 + + def test_unauthorized_user_gets_permission_denied_on_unit_container(self): + container_url = reverse_usage_url("xblock_container_handler", self.vertical.location) + + self.client.login(username=self.unauthorized_user.username, password=self.password) + resp = self.client.get(container_url, HTTP_ACCEPT="application/json") + + assert resp.status_code == 403 + + def test_unauthorized_user_gets_permission_denied_on_unit_preview(self): + preview_url = reverse_usage_url( + "xblock_view_handler", self.vertical.location, {"view_name": "container_preview"} + ) + + self.client.login(username=self.unauthorized_user.username, password=self.password) + resp = self.client.get(preview_url, HTTP_ACCEPT="application/json") + + assert resp.status_code == 403 + + class TestGetMetadataWithProblemDefaults(ModuleStoreTestCase): """ Unit tests for _get_metadata_with_problem_defaults. From 32f7163547bd4db6eff798971184b5607273a7ae Mon Sep 17 00:00:00 2001 From: Kevyn Suarez Date: Thu, 13 Aug 2026 14:26:19 -0500 Subject: [PATCH 2/3] fix: also fix the REST API v1 container view that the Authoring MFE actually uses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual testing against a real devstack found that the block.py fix alone wasn't enough: the modern Authoring MFE calls the REST API v1 ContainerHandlerView (/api/contentstore/v1/container_handler/...), which still 403'd for course_auditor. That view (and container_handler/container_embed_handler/xblock_edit_view in the legacy views) all route through the shared _get_item_in_course() helper in component.py, which gated on has_course_author_access — a legacy-only *write* check — even though all four callers only need read access to render a view. course_auditor has no legacy role equivalent, so it never had write access and always got PermissionDenied here, regardless of the block.py fix. Migrate _get_item_in_course() to the same user_has_course_permission read check, fixing all four callers (including the REST API v1 view) at their single shared choke point instead of patching each call site. Verified locally end-to-end: mounted this branch into a real devstack, assigned course_auditor to a test user, confirmed the unit page 403'd before this commit and loads correctly after it. Also ran the full test_block.py + test_vertical_block.py suites against the same devstack (186 passed). --- .../v1/views/tests/test_vertical_block.py | 37 +++++++++++++++++++ .../contentstore/views/component.py | 17 ++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py index 4c4c15bf2022..b38bfb76d73a 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py @@ -4,14 +4,18 @@ from urllib.parse import quote +import ddt from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag +from openedx_authz.constants.roles import COURSE_ADMIN, COURSE_AUDITOR, COURSE_EDITOR, COURSE_STAFF from rest_framework import status from xblock.core import XBlock from xblock.utils.studio_editable import NestedXBlockSpec, StudioContainerWithNestedXBlocksMixin from xblock.validation import ValidationMessage from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin from openedx.core.djangoapps.content_libraries.tests import ContentLibrariesRestApiTest from openedx.core.djangoapps.content_tagging.toggles import DISABLE_TAGGING_FEATURE from xmodule.modulestore import ModuleStoreEnum # pylint: disable=wrong-import-order @@ -275,6 +279,39 @@ def test_component_templates_for_non_mixin_xblock(self): self.assertIn('video', group_types) # noqa: PT009 +@ddt.ddt +class ContainerHandlerViewAuthzTest(CourseAuthoringAuthzTestMixin, BaseXBlockContainer): + """ + Regression test for openedx-authz#384: ContainerHandlerView (the endpoint the + Authoring MFE's unit page calls to render a unit) required legacy write access + via _get_item_in_course(), so AuthZ-native roles with no legacy equivalent + (course_auditor, course_editor) got a 403 despite holding COURSES_VIEW_COURSE. + """ + + view_name = "container_handler" + + @ddt.data( + COURSE_STAFF.external_key, + COURSE_ADMIN.external_key, + COURSE_AUDITOR.external_key, + COURSE_EDITOR.external_key, + ) + def test_course_roles_can_view_unit_container(self, role_key): + role_user = UserFactory(password=self.password) + self.add_user_to_role_in_course(role_user, role_key, self.course.id) + + self.client.login(username=role_user.username, password=self.password) + response = self.client.get(self.get_reverse_url(self.vertical.location)) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + + def test_unauthorized_user_gets_permission_denied(self): + self.client.login(username=self.unauthorized_user.username, password=self.password) + response = self.client.get(self.get_reverse_url(self.vertical.location)) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) # noqa: PT009 + + class ContainerVerticalViewTest(BaseXBlockContainer): """ Unit tests for the ContainerVerticalViewTest. diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 05a9fc12d291..46c83dcd19d6 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -21,6 +21,8 @@ from xblock.plugin import PluginMissingError from xblock.runtime import Mixologist +from openedx_authz.constants.permissions import COURSES_VIEW_COURSE + from cms.djangoapps.contentstore.helpers import get_parent_if_split_test, is_library_content, is_unit from cms.djangoapps.contentstore.toggles import libraries_v2_enabled, use_new_unit_page from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import load_services_for_studio @@ -28,6 +30,8 @@ from common.djangoapps.student.auth import has_course_author_access from common.djangoapps.xblock_django.api import authorable_xblocks, disabled_xblocks from common.djangoapps.xblock_django.models import XBlockStudioConfigurationFlag +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.api import get_object_tags from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration from openedx.core.lib.xblock_utils import get_aside_from_xblock, is_xblock_aside @@ -491,7 +495,11 @@ def _get_item_in_course(request, usage_key): Helper method for getting the old location, containing course, item, lms_link, and preview_lms_link for a given locator. - Verifies that the caller has permission to access this item. + Verifies that the caller has permission to view this item. All current callers + (container_handler, container_embed_handler, xblock_edit_view, and the REST API v1 + ContainerHandlerView) are read-only, so this only requires view access, not write + access — actual mutations are gated separately (e.g. component_handler's own + has_course_author_access check before persisting). """ from ..utils import get_lms_link_for_item @@ -501,7 +509,12 @@ def _get_item_in_course(request, usage_key): course_key = usage_key.course_key - if not has_course_author_access(request.user, course_key): + if not user_has_course_permission( + request.user, + COURSES_VIEW_COURSE.identifier, + course_key, + LegacyAuthoringPermission.READ, + ): raise PermissionDenied() course = modulestore().get_course(course_key) From aa2034a48019bfb57aece325c5361b348d8d111d Mon Sep 17 00:00:00 2001 From: Kevyn Suarez Date: Thu, 13 Aug 2026 14:33:40 -0500 Subject: [PATCH 3/3] fix: sort imports (ruff I001) --- cms/djangoapps/contentstore/views/component.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 46c83dcd19d6..2ab410392272 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -15,14 +15,13 @@ from django.views.decorators.http import require_GET from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey +from openedx_authz.constants.permissions import COURSES_VIEW_COURSE from xblock.core import XBlock from xblock.django.request import django_to_webob_request, webob_to_django_response from xblock.exceptions import NoSuchHandlerError from xblock.plugin import PluginMissingError from xblock.runtime import Mixologist -from openedx_authz.constants.permissions import COURSES_VIEW_COURSE - from cms.djangoapps.contentstore.helpers import get_parent_if_split_test, is_library_content, is_unit from cms.djangoapps.contentstore.toggles import libraries_v2_enabled, use_new_unit_page from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import load_services_for_studio