diff --git a/cms/djangoapps/contentstore/rest_api/v0/tests/test_advanced_settings.py b/cms/djangoapps/contentstore/rest_api/v0/tests/test_advanced_settings.py index ff66c1d9f0c4..228fac2fb737 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/tests/test_advanced_settings.py +++ b/cms/djangoapps/contentstore/rest_api/v0/tests/test_advanced_settings.py @@ -11,7 +11,7 @@ from django.urls import reverse from milestones.tests.utils import MilestonesTestCaseMixin from openedx_authz.api.users import assign_role_to_user_in_scope -from openedx_authz.constants.roles import COURSE_STAFF +from openedx_authz.constants.roles import COURSE_AUDITOR, COURSE_EDITOR, COURSE_STAFF from openedx_authz.engine.enforcer import AuthzEnforcer from openedx_authz.engine.utils import migrate_policy_between_enforcers from rest_framework.test import APIClient @@ -103,6 +103,7 @@ def test_disabled_fetch_all_query_param(self, setting, excluded_field): assert excluded_field not in resp.data +@ddt.ddt @patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, 'is_enabled', return_value=True) class AdvancedSettingsAuthzTest(CourseTestCase): """ @@ -203,3 +204,37 @@ def test_patch_unauthorized_for_specific_course(self, mock_flag): content_type="application/json" ) self.assertEqual(response.status_code, 403) # noqa: PT009 + + @ddt.data(COURSE_EDITOR, COURSE_AUDITOR) + def test_view_only_role_can_read(self, role, mock_flag): + """Course editor/auditor (view-only for advanced settings) can GET.""" + view_only_user = UserFactory() + assign_role_to_user_in_scope( + view_only_user.username, + role.external_key, + str(self.course.id), + ) + + view_only_client = APIClient() + view_only_client.force_authenticate(user=view_only_user) + response = view_only_client.get(self.url) + assert response.status_code == 200 + + @ddt.data(COURSE_EDITOR, COURSE_AUDITOR) + def test_view_only_role_cannot_write(self, role, mock_flag): + """Course editor/auditor cannot PATCH advanced settings (requires manage permission).""" + view_only_user = UserFactory() + assign_role_to_user_in_scope( + view_only_user.username, + role.external_key, + str(self.course.id), + ) + + view_only_client = APIClient() + view_only_client.force_authenticate(user=view_only_user) + response = view_only_client.patch( + self.url, + {"display_name": {"value": "Test"}}, + content_type="application/json", + ) + assert response.status_code == 403 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 17dad8136a44..eb09d5f4e6da 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,6 +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 rest_framework import status from rest_framework.test import APITestCase @@ -480,3 +481,18 @@ def test_authz_with_disable_advanced_settings_non_staff_denied(self, mock_is_use response = self.non_staff_client.get(self.url) self.assertEqual(response.status_code, 403) # noqa: PT009 mock_is_user_allowed.assert_not_called() + + @patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, 'is_enabled', return_value=True) + @patch('common.djangoapps.student.auth.authz_api.is_user_allowed') + def test_authz_feature_restricted_checks_view_permission(self, mock_is_user_allowed, mock_flag): + """ + The proctoring errors endpoint uses the 'feature_restricted' access type, which should + check the view (read) permission so that view-only roles (editor/auditor) can access it. + """ + mock_is_user_allowed.return_value = True + response = self.non_staff_client.get(self.url) + assert response.status_code == 200 + mock_is_user_allowed.assert_called_once() + # The permission identifier is the second positional arg to is_user_allowed. + called_permission = mock_is_user_allowed.call_args[0][1] + assert called_permission == COURSES_VIEW_ADVANCED_SETTINGS.identifier diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index 30e543c2f4b1..cb617f367096 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -11,7 +11,10 @@ from django.core.exceptions import PermissionDenied from opaque_keys.edx.locator import LibraryLocator from openedx_authz import api as authz_api -from openedx_authz.constants.permissions import COURSES_MANAGE_ADVANCED_SETTINGS +from openedx_authz.constants.permissions import ( + COURSES_MANAGE_ADVANCED_SETTINGS, + COURSES_VIEW_ADVANCED_SETTINGS, +) from common.djangoapps.student.roles import ( CourseBetaTesterRole, @@ -209,8 +212,15 @@ def check_course_advanced_settings_access(user, course_key, access_type='read'): ): # When feature is disabled, only staff/superuser can access (bypass authz) return user.is_staff or user.is_superuser - # Otherwise check authz permission - return authz_api.is_user_allowed(user.username, COURSES_MANAGE_ADVANCED_SETTINGS.identifier, str(course_key)) + + # Read and feature_restricted require only the view permission; + # write operations require the manage permission. + if access_type == 'write': + permission = COURSES_MANAGE_ADVANCED_SETTINGS.identifier + else: + permission = COURSES_VIEW_ADVANCED_SETTINGS.identifier + + return authz_api.is_user_allowed(user.username, permission, str(course_key)) # Legacy permission checks if access_type == 'read':