From fd91a6c5c740d1358f4aba2076d8c695d3902c14 Mon Sep 17 00:00:00 2001 From: Kevyn Suarez Date: Fri, 25 Sep 2026 12:54:19 -0500 Subject: [PATCH 1/2] fix: enforce courses.manage_pages_and_resources on ProctoredExamSettingsView GET and POST on this endpoint only required studio read access, so a user who could view a course in Studio could also read or overwrite its proctored exam settings by calling the API directly, bypassing the UI's staff-only enforcement. Add the same authz check used elsewhere for pages-and-resources management, with a write-level legacy fallback when authz is disabled. Closes openedx-authz#443 --- .../rest_api/v1/views/proctoring.py | 13 ++++++++++- .../v1/views/tests/test_proctoring.py | 22 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py b/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py index 6aa4aa3c17f1..89996cb15ffa 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py @@ -5,8 +5,9 @@ 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 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 @@ -15,6 +16,8 @@ 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 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 @@ -200,6 +203,14 @@ def _get_and_validate_course_access(user, course_id): f'Course with course_id {course_id} does not exist.' ) + if not user_has_course_permission( + user, + COURSES_MANAGE_PAGES_AND_RESOURCES.identifier, + course_key, + LegacyAuthoringPermission.WRITE, + ): + raise PermissionDenied + return course_block diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py index eb09d5f4e6da..eaa71ee19d2f 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py @@ -7,7 +7,7 @@ from django.test.utils import override_settings 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.permissions import COURSES_MANAGE_PAGES_AND_RESOURCES, COURSES_VIEW_ADVANCED_SETTINGS from rest_framework import status from rest_framework.test import APITestCase @@ -42,6 +42,26 @@ def test_404_no_course_block(self): "detail": f"Course with course_id {course_id} does not exist." } + @patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, 'is_enabled', return_value=True) + @patch('openedx.core.djangoapps.authz.decorators.authz_api.is_user_allowed') + def test_authz_user_allowed(self, mock_is_user_allowed, mock_flag): + """A user holding the authz manage_pages_and_resources permission can access.""" + mock_is_user_allowed.return_value = True + self.client.login(username=self.global_staff.username, password=self.password) + response = self.make_request() + assert response.status_code == status.HTTP_200_OK + called_permission = mock_is_user_allowed.call_args[0][1] + assert called_permission == COURSES_MANAGE_PAGES_AND_RESOURCES.identifier + + @patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, 'is_enabled', return_value=True) + @patch('openedx.core.djangoapps.authz.decorators.authz_api.is_user_allowed') + def test_authz_user_not_allowed(self, mock_is_user_allowed, mock_flag): + """Without the authz manage_pages_and_resources permission, access is denied, even for staff.""" + mock_is_user_allowed.return_value = False + self.client.login(username=self.global_staff.username, password=self.password) + response = self.make_request() + assert response.status_code == status.HTTP_403_FORBIDDEN + class ProctoringExamSettingsGetTests( ProctoringExamSettingsTestcase, ModuleStoreTestCase, APITestCase From 0cb8eafbb68b7126ace1b3d5e65710897e3a1e2e Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 5 Oct 2026 15:19:51 -0500 Subject: [PATCH 2/2] fix: use AuthZ view/manage permissions for proctored exam settings Make the AuthZ check the only gate for the proctored exam settings endpoint instead of stacking it on top of the legacy Studio read check: - GET requires courses.view_pages_and_resources and POST requires courses.manage_pages_and_resources. Without AuthZ, both fall back to legacy Studio read access, as before. - POST checks access before validating the payload. - Users with is_staff, or with the manage permission when AuthZ is enabled, can edit the fields reserved to edX staff. Replace the mocked AuthZ tests with tests that assign real roles. Co-Authored-By: Claude Sonnet 5.5 --- .../rest_api/v1/views/proctoring.py | 62 +++++++----- .../v1/views/tests/test_proctoring.py | 94 +++++++++++++++---- 2 files changed, 117 insertions(+), 39 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py b/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py index 89996cb15ffa..3f694395cea8 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py @@ -5,7 +5,7 @@ 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 +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, PermissionDenied from rest_framework.request import Request @@ -13,9 +13,9 @@ 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 @@ -109,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) @@ -135,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 = {} @@ -189,28 +201,34 @@ 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) - - if not course_block: - raise NotFound( - f'Course with course_id {course_id} does not exist.' - ) + authz_permission = ( + COURSES_MANAGE_PAGES_AND_RESOURCES if require_write_access else COURSES_VIEW_PAGES_AND_RESOURCES + ) if not user_has_course_permission( user, - COURSES_MANAGE_PAGES_AND_RESOURCES.identifier, + authz_permission.identifier, course_key, - LegacyAuthoringPermission.WRITE, + 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.' + ) + return course_block diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py index eaa71ee19d2f..5f6abc23e3e5 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py @@ -7,13 +7,15 @@ from django.test.utils import override_settings from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag -from openedx_authz.constants.permissions import COURSES_MANAGE_PAGES_AND_RESOURCES, COURSES_VIEW_ADVANCED_SETTINGS +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 @@ -42,26 +44,84 @@ def test_404_no_course_block(self): "detail": f"Course with course_id {course_id} does not exist." } - @patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, 'is_enabled', return_value=True) - @patch('openedx.core.djangoapps.authz.decorators.authz_api.is_user_allowed') - def test_authz_user_allowed(self, mock_is_user_allowed, mock_flag): - """A user holding the authz manage_pages_and_resources permission can access.""" - mock_is_user_allowed.return_value = True - self.client.login(username=self.global_staff.username, password=self.password) - response = self.make_request() + +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 - called_permission = mock_is_user_allowed.call_args[0][1] - assert called_permission == COURSES_MANAGE_PAGES_AND_RESOURCES.identifier - @patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, 'is_enabled', return_value=True) - @patch('openedx.core.djangoapps.authz.decorators.authz_api.is_user_allowed') - def test_authz_user_not_allowed(self, mock_is_user_allowed, mock_flag): - """Without the authz manage_pages_and_resources permission, access is denied, even for staff.""" - mock_is_user_allowed.return_value = False - self.client.login(username=self.global_staff.username, password=self.password) - response = self.make_request() + 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