fix: split authz view and manage permissions in ProctoredExamSettingsView

cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py

@@ -5,16 +5,19 @@
 from drf_spectacular.types import OpenApiTypes
 from drf_spectacular.utils import OpenApiParameter, OpenApiResponse, extend_schema
 from opaque_keys.edx.keys import CourseKey
+from openedx_authz.constants.permissions import COURSES_MANAGE_PAGES_AND_RESOURCES, COURSES_VIEW_PAGES_AND_RESOURCES
 from rest_framework import status
-from rest_framework.exceptions import NotFound
+from rest_framework.exceptions import NotFound, PermissionDenied
 from rest_framework.request import Request
 from rest_framework.response import Response
 from rest_framework.views import APIView
 
 from cms.djangoapps.contentstore.utils import get_proctored_exam_settings_url
-from cms.djangoapps.contentstore.views.course import get_course_and_check_access
 from cms.djangoapps.models.settings.course_metadata import CourseMetadata
 from common.djangoapps.student.auth import check_course_advanced_settings_access
+from common.djangoapps.student.roles import enable_authz_course_authoring
+from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
+from openedx.core.djangoapps.authz.decorators import user_has_course_permission
 from openedx.core.djangoapps.course_apps.toggles import exams_ida_enabled
 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes
 from xmodule.course_block import (  # pylint: disable=wrong-import-order
@@ -106,7 +109,7 @@ class ProctoredExamSettingsView(APIView):
     def get(self, request, course_id):
         """ GET handler """
         with modulestore().bulk_operations(CourseKey.from_string(course_id)):
-            course_block = self._get_and_validate_course_access(request.user, course_id)
+            course_block = self._get_and_validate_course_access(request.user, course_id, require_write_access=False)
             course_metadata = CourseMetadata().fetch_all(course_block)
             proctored_exam_settings = self._get_proctored_exam_setting_values(course_metadata)
 
@@ -132,17 +135,29 @@ def get(self, request, course_id):
 
     def post(self, request, course_id):
         """ POST handler """
-        serializer = ProctoredExamSettingsSerializer if request.user.is_staff \
-            else LimitedProctoredExamSettingsSerializer
-        exam_config = serializer(data=request.data.get('proctored_exam_settings', {}))
-        valid_request = exam_config.is_valid()
-        if not request.user.is_staff and valid_request and ProctoredExamSettingsSerializer(
-            data=request.data.get('proctored_exam_settings', {})
-        ).is_valid():
-            return Response(status=status.HTTP_403_FORBIDDEN)
-
         with modulestore().bulk_operations(CourseKey.from_string(course_id)):
-            course_block = self._get_and_validate_course_access(request.user, course_id)
+            course_block = self._get_and_validate_course_access(request.user, course_id, require_write_access=True)
+
+            # The access check above already required the manage permission when AuthZ is enabled for
+            # the course, so reaching this point means the user holds it. Without AuthZ the legacy
+            # fallback only checks read access, so staff-only fields stay reserved to global staff.
+            can_edit_staff_fields = request.user.is_staff or enable_authz_course_authoring(
+                CourseKey.from_string(course_id)
+            )
+
+            serializer = (
+                ProctoredExamSettingsSerializer if can_edit_staff_fields else LimitedProctoredExamSettingsSerializer
+            )
+            exam_config = serializer(data=request.data.get("proctored_exam_settings", {}))
+            valid_request = exam_config.is_valid()
+
+            if (
+                not can_edit_staff_fields
+                and valid_request
+                and ProctoredExamSettingsSerializer(data=request.data.get("proctored_exam_settings", {})).is_valid()
+            ):
+                return Response(status=status.HTTP_403_FORBIDDEN)
+
             course_metadata = CourseMetadata().fetch_all(course_block)
 
             models_to_update = {}
@@ -186,15 +201,29 @@ def _get_proctored_exam_setting_values(cls, course_metadata):
         }
 
     @staticmethod
-    def _get_and_validate_course_access(user, course_id):
+    def _get_and_validate_course_access(user, course_id, require_write_access):
         """
         Check if course_id exists and is accessible by the user.
 
+        Reading requires the AuthZ view permission and writing the manage permission.
+        When AuthZ is not enabled for the course, both fall back to legacy Studio read access.
+
         Returns a course_block object
         """
         course_key = CourseKey.from_string(course_id)
-        course_block = get_course_and_check_access(course_key, user)
+        authz_permission = (
+            COURSES_MANAGE_PAGES_AND_RESOURCES if require_write_access else COURSES_VIEW_PAGES_AND_RESOURCES
+        )
+
+        if not user_has_course_permission(
+            user,
+            authz_permission.identifier,
+            course_key,
+            LegacyAuthoringPermission.READ,
+        ):
+            raise PermissionDenied
 
+        course_block = modulestore().get_course(course_key, depth=0)
         if not course_block:
             raise NotFound(
                 f'Course with course_id {course_id} does not exist.'

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py

@@ -8,12 +8,14 @@
 from django.urls import reverse
 from edx_toggles.toggles.testutils import override_waffle_flag
 from openedx_authz.constants.permissions import COURSES_VIEW_ADVANCED_SETTINGS
+from openedx_authz.constants.roles import COURSE_AUDITOR, COURSE_LIMITED_STAFF, COURSE_STAFF
 from rest_framework import status
 from rest_framework.test import APITestCase
 
 from cms.djangoapps.contentstore.tests.test_utils import AuthorizeStaffTestCase
 from cms.djangoapps.contentstore.tests.utils import CourseTestCase
 from openedx.core import toggles as core_toggles
+from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin
 from openedx.core.djangoapps.course_apps.toggles import EXAMS_IDA
 from xmodule.course_metadata_utils import DEFAULT_START_DATE
 from xmodule.modulestore.django import modulestore  # pylint: disable=wrong-import-order
@@ -43,6 +45,84 @@ def test_404_no_course_block(self):
         }
 
 
+class ProctoredExamSettingsAuthzTests(CourseAuthoringAuthzTestMixin, CourseTestCase):
+    """
+    Tests for the proctored exam settings endpoint with AuthZ enabled, using real role assignments.
+
+    GET requires courses.view_pages_and_resources and POST requires courses.manage_pages_and_resources.
+    """
+
+    def setUp(self):
+        super().setUp()
+        self.url = reverse(
+            "cms.djangoapps.contentstore:v1:proctored_exam_settings",
+            kwargs={"course_id": self.course.id},
+        )
+
+    def _get_opt_out(self):
+        return modulestore().get_item(self.course.location).allow_proctoring_opt_out
+
+    def _post_flipping_opt_out(self, client):
+        """POST the settings with allow_proctoring_opt_out (a staff-only field) set to the opposite value."""
+        data = ProctoringExamSettingsPostTests.get_request_data(allow_proctoring_opt_out=not self._get_opt_out())
+        return client.post(self.url, data, format="json")
+
+    def test_staff_role_can_get_settings(self):
+        self.add_user_to_role_in_course(self.authorized_user, COURSE_STAFF.external_key, self.course.id)
+        response = self.authorized_client.get(self.url)
+        assert response.status_code == status.HTTP_200_OK
+
+    def test_auditor_role_can_get_settings(self):
+        self.add_user_to_role_in_course(self.authorized_user, COURSE_AUDITOR.external_key, self.course.id)
+        response = self.authorized_client.get(self.url)
+        assert response.status_code == status.HTTP_200_OK
+
+    def test_limited_staff_role_cannot_get_settings(self):
+        self.add_user_to_role_in_course(self.authorized_user, COURSE_LIMITED_STAFF.external_key, self.course.id)
+        response = self.authorized_client.get(self.url)
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
+    def test_user_without_role_cannot_get_settings(self):
+        response = self.unauthorized_client.get(self.url)
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
+    def test_global_staff_can_get_settings(self):
+        response = self.staff_client.get(self.url)
+        assert response.status_code == status.HTTP_200_OK
+
+    def test_staff_role_can_edit_staff_only_fields(self):
+        """A non-staff user whose role grants manage can save the fields reserved to edX staff."""
+        self.add_user_to_role_in_course(self.authorized_user, COURSE_STAFF.external_key, self.course.id)
+        initial = self._get_opt_out()
+        response = self._post_flipping_opt_out(self.authorized_client)
+        assert response.status_code == status.HTTP_200_OK
+        assert response.data["proctored_exam_settings"]["allow_proctoring_opt_out"] is not initial
+        assert self._get_opt_out() is not initial
+
+    def test_auditor_role_cannot_post_settings(self):
+        """The view permission is not enough to save the settings."""
+        self.add_user_to_role_in_course(self.authorized_user, COURSE_AUDITOR.external_key, self.course.id)
+        initial = self._get_opt_out()
+        response = self._post_flipping_opt_out(self.authorized_client)
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+        assert self._get_opt_out() is initial
+
+    def test_limited_staff_role_cannot_post_settings(self):
+        self.add_user_to_role_in_course(self.authorized_user, COURSE_LIMITED_STAFF.external_key, self.course.id)
+        response = self._post_flipping_opt_out(self.authorized_client)
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
+    def test_user_without_role_cannot_post_settings(self):
+        response = self._post_flipping_opt_out(self.unauthorized_client)
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
+    def test_global_staff_can_post_settings(self):
+        initial = self._get_opt_out()
+        response = self._post_flipping_opt_out(self.staff_client)
+        assert response.status_code == status.HTTP_200_OK
+        assert self._get_opt_out() is not initial
+
+
 class ProctoringExamSettingsGetTests(
     ProctoringExamSettingsTestcase, ModuleStoreTestCase, APITestCase
 ):