diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 6a1977f375..605e0d2133 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -106,7 +106,12 @@ class CourseAdvanceSettingViewTest(CourseTestCase, MilestonesTestCaseMixin): super().setUp() self.fullcourse = CourseFactory.create() self.course_setting_url = get_url(self.course.id, 'advanced_settings_handler') - self.non_staff_client, _ = self.create_non_staff_authed_user_client() + + self.non_staff_client, self.nonstaff = self.create_non_staff_authed_user_client() + # "nonstaff" means "non Django staff" here. We assign this user to course staff + # role to check that even so they won't have advanced settings access when explicitly + # restricted. + CourseStaffRole(self.course.id).add_users(self.nonstaff) @override_settings(FEATURES={'DISABLE_MOBILE_COURSE_AVAILABLE': True}) def test_mobile_field_available(self): @@ -145,16 +150,50 @@ class CourseAdvanceSettingViewTest(CourseTestCase, MilestonesTestCaseMixin): self.assertEqual('discussion_blackouts' in response, fields_visible) self.assertEqual('discussion_topics' in response, fields_visible) - @override_settings(FEATURES={'DISABLE_ADVANCED_SETTINGS': True}) - def test_disable_advanced_settings_feature(self): + @ddt.data(False, True) + def test_disable_advanced_settings_feature(self, disable_advanced_settings): """ - If this feature is enabled, only staff should be able to access the advanced settings page. + If this feature is enabled, only Django Staff/Superuser should be able to access the "Advanced Settings" page. + For non-staff users the "Advanced Settings" tab link should not be visible. """ - response = self.non_staff_client.get_html(self.course_setting_url) - self.assertEqual(response.status_code, 403) + advanced_settings_link_html = f"Advanced Settings".encode('utf-8') - response = self.client.get_html(self.course_setting_url) - self.assertEqual(response.status_code, 200) + with override_settings(FEATURES={'DISABLE_ADVANCED_SETTINGS': disable_advanced_settings}): + for handler in ( + 'import_handler', + 'export_handler', + 'course_team_handler', + 'course_info_handler', + 'assets_handler', + 'tabs_handler', + 'settings_handler', + 'grading_handler', + 'textbooks_list_handler', + ): + # Test that non-staff users don't see the "Advanced Settings" tab link. + response = self.non_staff_client.get_html( + get_url(self.course.id, handler) + ) + self.assertEqual(response.status_code, 200) + if disable_advanced_settings: + self.assertNotIn(advanced_settings_link_html, response.content) + else: + self.assertIn(advanced_settings_link_html, response.content) + + # Test that staff users see the "Advanced Settings" tab link. + response = self.client.get_html( + get_url(self.course.id, handler) + ) + self.assertEqual(response.status_code, 200) + self.assertIn(advanced_settings_link_html, response.content) + + # Test that non-staff users can't access the "Advanced Settings" page. + response = self.non_staff_client.get_html(self.course_setting_url) + self.assertEqual(response.status_code, 403 if disable_advanced_settings else 200) + + # Test that staff users can access the "Advanced Settings" page. + response = self.client.get_html(self.course_setting_url) + self.assertEqual(response.status_code, 200) @ddt.ddt diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 0cc4a5281c..e15904d640 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -43,7 +43,12 @@ from common.djangoapps.course_action_state.managers import CourseActionStateItem from common.djangoapps.course_action_state.models import CourseRerunState, CourseRerunUIStateManager from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.edxmako.shortcuts import render_to_response -from common.djangoapps.student.auth import has_course_author_access, has_studio_read_access, has_studio_write_access +from common.djangoapps.student.auth import ( + has_course_author_access, + has_studio_read_access, + has_studio_write_access, + has_studio_advanced_settings_access +) from common.djangoapps.student.roles import ( CourseInstructorRole, CourseStaffRole, @@ -149,17 +154,6 @@ class AccessListFallback(Exception): pass # lint-amnesty, pylint: disable=unnecessary-pass -def has_advanced_settings_access(user): - """ - If DISABLE_ADVANCED_SETTINGS feature is enabled, only global staff can access "Advanced Settings". - """ - return ( - not settings.FEATURES.get('DISABLE_ADVANCED_SETTINGS', False) - or user.is_staff - or user.is_superuser - ) - - def get_course_and_check_access(course_key, user, depth=0): """ Function used to calculate and return the locator and course block @@ -765,7 +759,6 @@ def course_index(request, course_key): 'frontend_app_publisher_url': frontend_app_publisher_url, 'mfe_proctored_exam_settings_url': get_proctored_exam_settings_url(course_block.id), 'advance_settings_url': reverse_course_url('advanced_settings_handler', course_block.id), - 'advance_settings_access': has_advanced_settings_access(request.user), 'proctoring_errors': proctoring_errors, }) @@ -1437,7 +1430,7 @@ def advanced_settings_handler(request, course_key_string): json: update the Course's settings. The payload is a json rep of the metadata dicts. """ - if not has_advanced_settings_access(request.user): + if not has_studio_advanced_settings_access(request.user): raise PermissionDenied() course_key = CourseKey.from_string(course_key_string) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index c3ed1afb99..b7ef368a09 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -7,6 +7,7 @@ <%namespace name='static' file='static_content.html'/> <%! from django.utils.translation import gettext as _ + from common.djangoapps.student.auth import has_studio_advanced_settings_access from cms.djangoapps.contentstore import utils from lms.djangoapps.certificates.api import can_show_certificate_available_date_field from openedx.core.djangolib.js_utils import ( @@ -721,7 +722,9 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url | n, js_escaped_string}'