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/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/component.py b/cms/djangoapps/contentstore/views/component.py index 05a9fc12d291..2ab410392272 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -15,6 +15,7 @@ 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 @@ -28,6 +29,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 +494,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 +508,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) 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.