Skip to content

Commit

Permalink
fix: missing advance_settings_access template variable
Browse files Browse the repository at this point in the history
Co-authored-by: Farhaan Bukhsh <[email protected]>
  • Loading branch information
0x29a and farhaanbukhsh committed May 5, 2023
1 parent 6307961 commit 065f894
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 26 deletions.
59 changes: 49 additions & 10 deletions cms/djangoapps/contentstore/tests/test_course_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,12 @@ def setUp(self):
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):
Expand Down Expand Up @@ -145,16 +150,50 @@ def test_discussion_fields_available(self, is_pages_and_resources_enabled,
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):
"""
If this feature is enabled, only staff should be able to access the advanced settings page.
"""
response = self.non_staff_client.get_html(self.course_setting_url)
self.assertEqual(response.status_code, 403)
@ddt.data(False, True)
def test_disable_advanced_settings_feature(self, disable_advanced_settings):
"""
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.
"""
advanced_settings_link_html = f"<a href=\"{self.course_setting_url}\">Advanced Settings</a>".encode('utf-8')

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)

response = self.client.get_html(self.course_setting_url)
self.assertEqual(response.status_code, 200)
# 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
Expand Down
21 changes: 7 additions & 14 deletions cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@
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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
})

Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions cms/templates/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -721,7 +722,9 @@ <h3 class="title-3">${_("Other Course Settings")}</h3>
<li class="nav-item"><a href="${grading_config_url}">${_("Grading")}</a></li>
<li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% if has_studio_advanced_settings_access(request.user):
<li class="nav-item"><a href="${advanced_config_url}">${_("Advanced Settings")}</a></li>
% endif
% if mfe_proctored_exam_settings_url:
<li class="nav-item"><a href="${mfe_proctored_exam_settings_url}">${_("Proctored Exam Settings")}</a></li>
% endif
Expand Down
3 changes: 3 additions & 0 deletions cms/templates/settings_graders.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import json
from cms.djangoapps.contentstore import utils
from django.utils.translation import gettext as _
from common.djangoapps.student.auth import has_studio_advanced_settings_access
from cms.djangoapps.models.settings.encoder import CourseSettingsEncoder
from openedx.core.djangolib.js_utils import (
dump_js_escaped_json, js_escaped_string
Expand Down Expand Up @@ -168,7 +169,9 @@ <h3 class="title-3">${_("Other Course Settings")}</h3>
<li class="nav-item"><a href="${detailed_settings_url}">${_("Details & Schedule")}</a></li>
<li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% if has_studio_advanced_settings_access(request.user):
<li class="nav-item"><a href="${advanced_settings_url}">${_("Advanced Settings")}</a></li>
% endif
% if mfe_proctored_exam_settings_url:
<li class="nav-item"><a href="${mfe_proctored_exam_settings_url}">${_("Proctored Exam Settings")}</a></li>
% endif
Expand Down
3 changes: 2 additions & 1 deletion cms/templates/widgets/header.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.urls import reverse
from django.utils.translation import gettext as _
from urllib.parse import quote_plus
from common.djangoapps.student.auth import has_studio_advanced_settings_access
from cms.djangoapps.contentstore import toggles
from cms.djangoapps.contentstore.utils import get_pages_and_resources_url
from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND
Expand Down Expand Up @@ -120,7 +121,7 @@ <h3 class="title"><span class="label"><span class="label-prefix sr">${_("Course"
<a href="${mfe_proctored_exam_settings_url}">${_("Proctored Exam Settings")}</a>
</li>
% endif
% if advance_settings_access:
% if has_studio_advanced_settings_access(request.user):
<li class="nav-item nav-course-settings-advanced">
<a href="${advanced_settings_url}">${_("Advanced Settings")}</a>
</li>
Expand Down
16 changes: 15 additions & 1 deletion common/djangoapps/student/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,23 @@ def has_course_author_access(user, course_key):
return has_studio_write_access(user, course_key)


def has_studio_advanced_settings_access(user):
"""
If DISABLE_ADVANCED_SETTINGS feature is enabled, only Django Superuser
or Django Staff can access "Advanced Settings".
By default, this feature is disabled.
"""
return (
not settings.FEATURES.get('DISABLE_ADVANCED_SETTINGS', False)
or user.is_staff
or user.is_superuser
)


def has_studio_read_access(user, course_key):
"""
Return True iff user is allowed to view this course/library in studio.
Return True if user is allowed to view this course/library in studio.
Will also return True if user has write access in studio (has_course_author_access)
There is currently no such thing as read-only course access in studio, but
Expand Down

0 comments on commit 065f894

Please sign in to comment.