grant xblock edit access via authz edit_course_content permission

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.
 
@@ -203,7 +231,14 @@ def xblock_view_handler(request, usage_key_string, view_name):
             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
@@ -247,6 +282,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,

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")

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,217 @@ 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"
+    )
+    # 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):
+        """
+        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 _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
+        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
+
+    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):
     """Tests for '/xblock' DELETE url."""

cms/templates/studio_xblock_wrapper.html

@@ -163,6 +163,7 @@
                 % endif
             </div>
         </div>
+        % if not is_authz_authoring_enabled or authz_can_edit_course_content:
         <div class="header-actions">
             <ul class="actions-list nav-dd ui-right">
                 % if can_edit_title and not can_edit:
@@ -257,6 +258,7 @@
                 % endif
             </ul>
         </div>
+        % endif
     </div>
     % if not is_root:
         <div class="wrapper-xblock-message xblock-validation-messages" data-locator="${xblock.location}"/>