From 00d30103507baa1735a21f9579ed54e4f6ba5937 Mon Sep 17 00:00:00 2001 From: jacobo-dominguez-wgu Date: Fri, 21 Aug 2026 12:20:03 -0600 Subject: [PATCH 1/2] feat: adding authz edit permission check on xblock wrapper --- cms/djangoapps/contentstore/views/block.py | 40 +++++- cms/djangoapps/contentstore/views/preview.py | 5 + .../contentstore/views/tests/test_block.py | 127 ++++++++++++++++++ cms/templates/studio_xblock_wrapper.html | 2 + 4 files changed, 172 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index b1f1ab8f5056..76c5fe0ee8b8 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -12,7 +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 openedx_authz.constants.permissions import COURSES_EDIT_COURSE_CONTENT, COURSES_VIEW_COURSE from web_fragments.fragment import Fragment from cms.djangoapps.contentstore.utils import load_services_for_studio @@ -27,6 +27,7 @@ from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW 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.student.roles import enable_authz_course_authoring 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 @@ -129,10 +130,37 @@ def xblock_handler(request, usage_key_string=None): return handle_xblock(request, usage_key_string) +def _get_authz_permissions_flags(user, course_key): + """ + Return the two RBAC-authoring flags used to gate the header-actions div + in the XBlock component card template. + + When ``authz.enable_course_authoring`` is off for the course both flags + default to values that preserve existing (pre-RBAC) behaviour: + - ``is_authz_authoring_enabled = False`` → template always shows the div. + - ``authz_can_edit_course_content = False`` → unused while flag is off. + + When the flag is on, ``authz_can_edit_course_content`` reflects whether + the requesting user holds the ``courses.edit_course_content`` permission. + + Returns: + tuple[bool, bool]: (is_authz_authoring_enabled, authz_can_edit_course_content) + """ + if not enable_authz_course_authoring(course_key): + return False, False + can_edit = user_has_course_permission( + user, + COURSES_EDIT_COURSE_CONTENT.identifier, + course_key, + legacy_permission=LegacyAuthoringPermission.WRITE, + ) + return True, can_edit + + @require_http_methods("GET") @login_required @expect_json -def xblock_view_handler(request, usage_key_string, view_name): +def xblock_view_handler(request, usage_key_string, view_name): # pylint: disable=too-many-statements """ The restful handler for requests for rendered xblock views. @@ -205,6 +233,12 @@ def xblock_view_handler(request, usage_key_string, view_name): ) # Only the "Pages" view uses student view in Studio can_edit = has_studio_write_access(request.user, usage_key.course_key) + # Gate the header-actions div on courses.edit_course_content when + # the authz flag is on. See _get_authz_preview_flags for details. + is_authz_authoring_enabled, authz_can_edit_course_content = ( + _get_authz_permissions_flags(request.user, usage_key.course_key) + ) + # Determine the items to be shown as reorderable. Note that the view # 'reorderable_container_child_preview' is only rendered for xblocks that # are being shown in a reorderable container, so the xblock is automatically @@ -247,6 +281,8 @@ def xblock_view_handler(request, usage_key_string, view_name): "is_pages_view": is_pages_view or view_name == AUTHOR_VIEW, "is_unit_page": is_unit(xblock), "can_edit": can_edit, + "is_authz_authoring_enabled": is_authz_authoring_enabled, + "authz_can_edit_course_content": authz_can_edit_course_content, "root_xblock": xblock if (view_name == "container_preview") else None, diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 408c5649c8d5..4c290516e0b6 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -318,6 +318,9 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): can_edit = context.get('can_edit', True) can_add = context.get('can_add', True) can_move = context.get('can_move', True) + # Set by block.py; default False so callers that don't set it are unaffected. + is_authz_authoring_enabled = context.get('is_authz_authoring_enabled', False) + authz_can_edit_course_content = context.get('authz_can_edit_course_content', True) root_upstream_link = UpstreamLink.try_get_for_block(root_xblock, log_error=False) upstream_link = UpstreamLink.try_get_for_block(xblock, log_error=False) if ( @@ -363,6 +366,8 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'is_course': is_course, 'tags_count': tags_count, 'can_edit_title': True, # This is always true even for imported components + 'is_authz_authoring_enabled': is_authz_authoring_enabled, + 'authz_can_edit_course_content': authz_can_edit_course_content, } add_webpack_js_to_fragment(frag, "js/factories/xblock_validation") diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index dd087fc1418f..9fbac78e3c28 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/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.permissions import COURSES_EDIT_COURSE_CONTENT, COURSES_VIEW_COURSE from openedx_authz.constants.roles import COURSE_ADMIN, COURSE_AUDITOR, COURSE_EDITOR, COURSE_STAFF from openedx_events.content_authoring.data import DuplicatedXBlockData from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED @@ -545,6 +546,132 @@ def assert_xblock_info(xblock, xblock_info): self.assertEqual(xblock_info, response) # noqa: PT009 +class TestXBlockViewHandlerHeaderActionsAuthz(ItemTest): + """ + Regression tests for the ``header-actions`` div gating introduced to + conditionally render the component card action menu based on the RBAC + ``courses.edit_course_content`` permission. + + The gate uses two independent context flags: + - ``is_authz_authoring_enabled``: True when enable_authz_course_authoring + is on for the course. + - ``authz_can_edit_course_content``: True when the user holds + courses.edit_course_content (only evaluated when the flag is on). + + The template condition is: + ``not is_authz_authoring_enabled or authz_can_edit_course_content`` + + So the div is shown when the flag is off (preserving existing behaviour) + or when the flag is on and the user has the permission. + """ + + AUTHZ_FLAG_PATH = ( + "cms.djangoapps.contentstore.views.block.enable_authz_course_authoring" + ) + # Patch user_has_course_permission at the block.py binding so the + # authz_can_edit_course_content value is fully controlled by the test. + # + # NOTE: xblock_view_handler gates the *whole* request on + # ``courses.view_course`` via this same binding before the template is ever + # rendered. A blanket return_value=False would make that view-access gate + # fail and the handler would raise PermissionDenied (403) before reaching + # the header-actions logic. We therefore drive the mock with a + # permission-aware side effect that always grants view access and only + # toggles the ``courses.edit_course_content`` permission. + AUTHZ_PERMISSION_PATH = ( + "cms.djangoapps.contentstore.views.block.user_has_course_permission" + ) + HEADER_ACTIONS_DIV = 'class="header-actions"' + + @staticmethod + def _permission_side_effect(*, can_edit_course_content): + """ + Build a ``user_has_course_permission`` side effect that always grants + ``courses.view_course`` (so the handler returns 200) and returns + ``can_edit_course_content`` for ``courses.edit_course_content``. + + The permission identifier is passed as the second positional argument + by every call site in ``block.py``. + """ + def _side_effect(_user, permission_identifier, *_args, **_kwargs): + if permission_identifier == COURSES_VIEW_COURSE.identifier: + return True + if permission_identifier == COURSES_EDIT_COURSE_CONTENT.identifier: + return can_edit_course_content + return False + + return _side_effect + + def _get_container_preview_html(self): + """ + Return the rendered HTML for a child vertical card inside a parent vertical. + + ``header-actions`` only appears on non-root blocks (``is_root=False``). + We replicate the setup used by ``test_draft_container_preview_html`` in + ``test_container_page.py``: create a parent vertical, add a child + vertical inside it, then request ``reorderable_container_child_preview`` + for that child. A vertical renders cleanly in the test environment + without needing any external services, and its card includes the full + ``header-actions`` section. + """ + parent_usage_key = self._create_vertical() + child_usage_key = self._create_vertical(parent_usage_key=parent_usage_key) + + preview_url = reverse_usage_url( + "xblock_view_handler", + child_usage_key, + {"view_name": "reorderable_container_child_preview"}, + ) + resp = self.client.get(preview_url, HTTP_ACCEPT="application/json") + self.assertEqual(resp.status_code, 200) # noqa: PT009 + return json.loads(resp.content.decode("utf-8"))["html"] + + def test_header_actions_visible_when_flag_off(self): + """ + When enable_authz_course_authoring is off, is_authz_authoring_enabled + is False and the template condition ``not False or *`` is always True, + so the div must be present regardless of any permission value. + Preserves existing behaviour for courses not yet on the authz rollout. + """ + with patch(self.AUTHZ_FLAG_PATH, return_value=False): + html = self._get_container_preview_html() + + self.assertIn(self.HEADER_ACTIONS_DIV, html) # noqa: PT009 + + def test_header_actions_visible_when_flag_on_and_user_allowed(self): + """ + When the flag is on and the user holds courses.edit_course_content, + is_authz_authoring_enabled=True and authz_can_edit_course_content=True, + so the template condition is True and the div must be rendered. + """ + with patch(self.AUTHZ_FLAG_PATH, return_value=True), \ + patch( + self.AUTHZ_PERMISSION_PATH, + side_effect=self._permission_side_effect(can_edit_course_content=True), + ): + html = self._get_container_preview_html() + + self.assertIn(self.HEADER_ACTIONS_DIV, html) # noqa: PT009 + + def test_header_actions_hidden_when_flag_on_and_user_denied(self): + """ + When the flag is on and the user does NOT hold courses.edit_course_content, + is_authz_authoring_enabled=True and authz_can_edit_course_content=False, + so the template condition is False and the entire header-actions div + must be absent from the rendered HTML. + This is the core regression test: without the fix the div would always + render even for read-only users when the authz flag is on. + """ + with patch(self.AUTHZ_FLAG_PATH, return_value=True), \ + patch( + self.AUTHZ_PERMISSION_PATH, + side_effect=self._permission_side_effect(can_edit_course_content=False), + ): + html = self._get_container_preview_html() + + self.assertNotIn(self.HEADER_ACTIONS_DIV, html) # noqa: PT009 + + @ddt.ddt class DeleteItem(ItemTest): """Tests for '/xblock' DELETE url.""" diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index e05bee6017bc..a9b96f92ef6a 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -163,6 +163,7 @@ % endif + % if not is_authz_authoring_enabled or authz_can_edit_course_content:
+ % endif % if not is_root:
From b36a145242d4e0ac9098abc5c26b08569f664421 Mon Sep 17 00:00:00 2001 From: jacobo-dominguez-wgu Date: Mon, 31 Aug 2026 15:05:56 -0600 Subject: [PATCH 2/2] fix: grant xblock edit access via authz edit_course_content permission --- cms/djangoapps/contentstore/views/block.py | 5 +- .../contentstore/views/tests/test_block.py | 85 +++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 76c5fe0ee8b8..3233fb24ee3f 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -231,13 +231,14 @@ def xblock_view_handler(request, usage_key_string, view_name): # pylint: disable is_pages_view = ( view_name == STUDENT_VIEW ) # Only the "Pages" view uses student view in Studio - can_edit = has_studio_write_access(request.user, usage_key.course_key) - # Gate the header-actions div on courses.edit_course_content when # the authz flag is on. See _get_authz_preview_flags for details. is_authz_authoring_enabled, authz_can_edit_course_content = ( _get_authz_permissions_flags(request.user, usage_key.course_key) ) + can_edit = has_studio_write_access(request.user, usage_key.course_key) or ( + is_authz_authoring_enabled and authz_can_edit_course_content + ) # Determine the items to be shown as reorderable. Note that the view # 'reorderable_container_child_preview' is only rendered for xblocks that diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 9fbac78e3c28..4cc9aaaf7d4c 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -581,7 +581,19 @@ class TestXBlockViewHandlerHeaderActionsAuthz(ItemTest): AUTHZ_PERMISSION_PATH = ( "cms.djangoapps.contentstore.views.block.user_has_course_permission" ) + # Patch has_studio_write_access at the block.py binding so the legacy + # authoring path can be toggled independently of the authz path. + STUDIO_WRITE_ACCESS_PATH = ( + "cms.djangoapps.contentstore.views.block.has_studio_write_access" + ) HEADER_ACTIONS_DIV = 'class="header-actions"' + # The component (content) "Edit" button is rendered only when + # ``not show_inline and can_edit`` in studio_xblock_wrapper.html. Match on + # its full class string so this does NOT collide with the separate + # "Edit Title" button (``title-edit-button``), which is rendered on the + # opposite condition (``can_edit_title and not can_edit``) and would + # otherwise match a bare ``edit-button`` substring. + CONTENT_EDIT_BUTTON = 'class="btn-default edit-button action-button"' @staticmethod def _permission_side_effect(*, can_edit_course_content): @@ -626,6 +638,35 @@ def _get_container_preview_html(self): self.assertEqual(resp.status_code, 200) # noqa: PT009 return json.loads(resp.content.decode("utf-8"))["html"] + def _get_leaf_component_preview_html(self): + """ + Return the rendered HTML for a leaf ``html`` component card. + + The component (content) "Edit" button is gated on + ``not show_inline and can_edit`` in studio_xblock_wrapper.html, where + ``show_inline = xblock.has_children and not xblock_url``. A vertical has + children, so its card is rendered inline and never shows that edit + button regardless of ``can_edit``. We therefore create a leaf ``html`` + component (no children -> ``show_inline`` is False) inside a vertical and + request ``container_child_preview`` for it, which is the branch that the + ``can_edit`` fix actually controls. + """ + parent_usage_key = self._create_vertical() + resp = self.create_xblock( + parent_usage_key=parent_usage_key, category="html" + ) + self.assertEqual(resp.status_code, 200) # noqa: PT009 + child_usage_key = self.response_usage_key(resp) + + preview_url = reverse_usage_url( + "xblock_view_handler", + child_usage_key, + {"view_name": "container_child_preview"}, + ) + resp = self.client.get(preview_url, HTTP_ACCEPT="application/json") + self.assertEqual(resp.status_code, 200) # noqa: PT009 + return json.loads(resp.content.decode("utf-8"))["html"] + def test_header_actions_visible_when_flag_off(self): """ When enable_authz_course_authoring is off, is_authz_authoring_enabled @@ -671,6 +712,50 @@ def test_header_actions_hidden_when_flag_on_and_user_denied(self): self.assertNotIn(self.HEADER_ACTIONS_DIV, html) # noqa: PT009 + def test_can_edit_true_via_authz_without_legacy_write_access(self): + """ + Regression test for the ``can_edit`` fix in xblock_view_handler. + + ``can_edit`` is computed as:: + + has_studio_write_access(...) or ( + is_authz_authoring_enabled and authz_can_edit_course_content + ) + + A user granted courses.edit_course_content through the authz rollout may + not hold legacy studio write access. Before the fix ``can_edit`` was + derived solely from ``has_studio_write_access`` and such a user would + lose the per-block "Edit" button. With the flag on and the permission + granted, ``can_edit`` must be True even when legacy write access is + False, so the edit button must be rendered. + """ + with patch(self.STUDIO_WRITE_ACCESS_PATH, return_value=False), \ + patch(self.AUTHZ_FLAG_PATH, return_value=True), \ + patch( + self.AUTHZ_PERMISSION_PATH, + side_effect=self._permission_side_effect(can_edit_course_content=True), + ): + html = self._get_leaf_component_preview_html() + + self.assertIn(self.CONTENT_EDIT_BUTTON, html) # noqa: PT009 + + def test_can_edit_false_without_legacy_write_access_or_authz_permission(self): + """ + When the user lacks legacy studio write access and, with the authz flag + on, is not granted courses.edit_course_content, both operands of the + ``can_edit`` expression are False. ``can_edit`` must therefore be False + and the per-block "Edit" button must be absent. + """ + with patch(self.STUDIO_WRITE_ACCESS_PATH, return_value=False), \ + patch(self.AUTHZ_FLAG_PATH, return_value=True), \ + patch( + self.AUTHZ_PERMISSION_PATH, + side_effect=self._permission_side_effect(can_edit_course_content=False), + ): + html = self._get_leaf_component_preview_html() + + self.assertNotIn(self.CONTENT_EDIT_BUTTON, html) # noqa: PT009 + @ddt.ddt class DeleteItem(ItemTest):