Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cms/djangoapps/contentstore/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,12 +473,12 @@ def sync_discussion_settings(course_key, user):

if (
ENABLE_NEW_STRUCTURE_DISCUSSIONS.is_enabled()
and not course.discussions_settings['provider_type'] == Provider.OPEN_EDX
and not course.discussions_settings.get('provider_type', None) == Provider.OPEN_EDX
and not course.discussions_settings.get('provider', None) == Provider.OPEN_EDX
):
LOGGER.info(f"New structure is enabled, also updating {course_key} to use new provider")
course.discussions_settings['enable_graded_units'] = False
course.discussions_settings['unit_level_visibility'] = True
course.discussions_settings['provider'] = Provider.OPEN_EDX
course.discussions_settings['provider_type'] = Provider.OPEN_EDX
modulestore().update_item(course, user.id)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,11 @@ def _save_xblock(
for metadata_key, value in metadata.items():
field = xblock.fields[metadata_key]

# Prevent setting release date to null
# remove field and inherit from parent instead
if metadata_key == "start" and value == "":
value = None

if value is None:
field.delete_from(xblock)
else:
Expand Down
11 changes: 11 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,17 @@
# .. toggle_creation_date: 2024-04-10
'BADGES_ENABLED': False,

# .. toggle_name: FEATURES['ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID']
# .. toggle_implementation: DjangoSetting
# .. toggle_default: False
# .. toggle_description: Whether to enable the legacy MD5 hashing algorithm to generate anonymous user id
# instead of the newer SHAKE128 hashing algorithm
# .. toggle_use_cases: open_edx
# .. toggle_creation_date: 2022-08-08
# .. toggle_target_removal_date: None
# .. toggle_tickets: 'https://github.com/openedx/edx-platform/pull/30832'
'ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID': False,

# .. toggle_name: FEATURES['IN_CONTEXT_DISCUSSION_ENABLED_DEFAULT']
# .. toggle_implementation: DjangoSetting
# .. toggle_default: True
Expand Down
13 changes: 11 additions & 2 deletions common/djangoapps/student/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,21 @@ def anonymous_id_for_user(user, course_id):
# function: Rotate at will, since the hashes are stored and
# will not change.
# include the secret key as a salt, and to make the ids unique across different LMS installs.
hasher = hashlib.shake_128()
legacy_hash_enabled = settings.FEATURES.get('ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID', False)
if legacy_hash_enabled:
# Use legacy MD5 algorithm if flag enabled
hasher = hashlib.md5()
else:
hasher = hashlib.shake_128()
hasher.update(settings.SECRET_KEY.encode('utf8'))
hasher.update(str(user.id).encode('utf8'))
if course_id:
hasher.update(str(course_id).encode('utf-8'))
anonymous_user_id = hasher.hexdigest(16)

if legacy_hash_enabled:
anonymous_user_id = hasher.hexdigest()
else:
anonymous_user_id = hasher.hexdigest(16) # pylint: disable=too-many-function-args

try:
AnonymousUserId.objects.create(
Expand Down
21 changes: 20 additions & 1 deletion common/djangoapps/student/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class GlobalStaff(AccessRole):
The global staff role
"""
def has_user(self, user):
return bool(user and user.is_staff)
return bool(user and (user.is_superuser or user.is_staff))

def add_users(self, *users):
for user in users:
Expand Down Expand Up @@ -361,6 +361,25 @@ class CourseLimitedStaffRole(CourseStaffRole):
BASE_ROLE = CourseStaffRole.ROLE


@register_access_role
class eSHEInstructorRole(CourseStaffRole):
"""A Staff member of a course without access to the membership tab and enrollment-related operations."""

ROLE = 'eshe_instructor'
BASE_ROLE = CourseStaffRole.ROLE


@register_access_role
class TeachingAssistantRole(CourseStaffRole):
"""
A Staff member of a course without access to the membership tab, enrollment-related operations and
grade-related operations.
"""

ROLE = 'teaching_assistant'
BASE_ROLE = CourseStaffRole.ROLE


@register_access_role
class CourseInstructorRole(CourseRole):
"""A course Instructor"""
Expand Down
4 changes: 4 additions & 0 deletions common/djangoapps/student/tests/test_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
CourseStaffRole,
CourseFinanceAdminRole,
CourseSalesAdminRole,
eSHEInstructorRole,
TeachingAssistantRole,
LibraryUserRole,
CourseDataResearcherRole,
GlobalStaff,
Expand Down Expand Up @@ -199,6 +201,8 @@ class RoleCacheTestCase(TestCase): # lint-amnesty, pylint: disable=missing-clas
ROLES = (
(CourseStaffRole(IN_KEY), ('staff', IN_KEY, 'edX')),
(CourseLimitedStaffRole(IN_KEY), ('limited_staff', IN_KEY, 'edX')),
(eSHEInstructorRole(IN_KEY), ('eshe_instructor', IN_KEY, 'edX')),
(TeachingAssistantRole(IN_KEY), ('teaching_assistant', IN_KEY, 'edX')),
(CourseInstructorRole(IN_KEY), ('instructor', IN_KEY, 'edX')),
(OrgStaffRole(IN_KEY.org), ('staff', None, 'edX')),
(CourseFinanceAdminRole(IN_KEY), ('finance_admin', IN_KEY, 'edX')),
Expand Down
11 changes: 11 additions & 0 deletions common/djangoapps/student/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,17 @@ def test_anonymous_id_secret_key_changes_result_in_diff_values_for_same_new_user
assert anonymous_id != new_anonymous_id
assert self.user == user_by_anonymous_id(new_anonymous_id)

def test_enable_legacy_hash_flag(self):
"""Test that different anonymous id returned if ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID enabled."""
CourseEnrollment.enroll(self.user, self.course.id)
anonymous_id = anonymous_id_for_user(self.user, self.course.id)
with patch.dict(settings.FEATURES, ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID=True):
# Recreate user object to clear cached anonymous id.
self.user = User.objects.get(pk=self.user.id)
AnonymousUserId.objects.filter(user=self.user).filter(course_id=self.course.id).delete()
new_anonymous_id = anonymous_id_for_user(self.user, self.course.id)
assert anonymous_id != new_anonymous_id


@skip_unless_lms
@patch('openedx.core.djangoapps.programs.utils.get_programs')
Expand Down
6 changes: 6 additions & 0 deletions lms/djangoapps/courseware/tabs.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,12 @@ def link_func(course, _reverse_func):
tab_dict['link_func'] = link_func
super().__init__(tab_dict)

@classmethod
def is_enabled(cls, course, user=None):
if settings.FEATURES.get('DISABLE_DATES_TAB'):
return False
return super().is_enabled(course, user)


def get_course_tab_list(user, course):
"""
Expand Down
13 changes: 13 additions & 0 deletions lms/djangoapps/courseware/tests/test_tabs.py
Original file line number Diff line number Diff line change
Expand Up @@ -885,3 +885,16 @@ def test_singular_dates_tab(self):
if tab.type == 'dates':
num_dates_tabs += 1
assert num_dates_tabs == 1

def test_dates_tab_is_enabled_by_default(self):
"""Test dates tab is enabled by default."""
tab = DatesTab({'type': DatesTab.type, 'name': 'dates'})
user = self.create_mock_user()
assert self.is_tab_enabled(tab, self.course, user)

@patch.dict("django.conf.settings.FEATURES", {"DISABLE_DATES_TAB": True})
def test_dates_tab_disabled_by_feature_flag(self):
"""Test dates tab is disabled by the feature flag."""
tab = DatesTab({'type': DatesTab.type, 'name': 'dates'})
user = self.create_mock_user()
assert not self.is_tab_enabled(tab, self.course, user)
4 changes: 4 additions & 0 deletions lms/djangoapps/instructor/access.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
CourseInstructorRole,
CourseLimitedStaffRole,
CourseStaffRole,
eSHEInstructorRole,
TeachingAssistantRole,
)
from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params
from openedx.core.djangoapps.django_comment_common.models import Role
Expand All @@ -30,6 +32,8 @@
'instructor': CourseInstructorRole,
'staff': CourseStaffRole,
'limited_staff': CourseLimitedStaffRole,
'eshe_instructor': eSHEInstructorRole,
'teaching_assistant': TeachingAssistantRole,
'ccx_coach': CourseCcxCoachRole,
'data_researcher': CourseDataResearcherRole,
}
Expand Down
21 changes: 18 additions & 3 deletions lms/djangoapps/instructor/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@
VIEW_ENROLLMENTS = 'instructor.view_enrollments'
VIEW_FORUM_MEMBERS = 'instructor.view_forum_members'

# Due to how the roles iheritance is implemented currently, eshe_instructor and teaching_assistant have implicit
# staff access, but unlike staff, they shouldn't be able to enroll and do grade-related operations as per client's
# requirements. At the same time, all other staff-derived roles, like Limited Staff, should be able to enroll students.
# This solution is far from perfect, but it's probably the best we can do untill the roles system is reworked.
_is_teaching_assistant = HasRolesRule('teaching_assistant')
_is_eshe_instructor = HasRolesRule('eshe_instructor')
_is_eshe_instructor_or_teaching_assistant = _is_teaching_assistant | _is_eshe_instructor
is_staff_but_not_teaching_assistant = (
(_is_teaching_assistant & HasAccessRule('staff', strict=True)) |
(~_is_teaching_assistant & HasAccessRule('staff'))
)
is_staff_but_not_eshe_instructor_or_teaching_assistant = (
(_is_eshe_instructor_or_teaching_assistant & HasAccessRule('staff', strict=True)) |
(~_is_eshe_instructor_or_teaching_assistant & HasAccessRule('staff'))
)

perms[ALLOW_STUDENT_TO_BYPASS_ENTRANCE_EXAM] = HasAccessRule('staff')
perms[ASSIGN_TO_COHORTS] = HasAccessRule('staff')
Expand All @@ -51,17 +66,17 @@
perms[START_CERTIFICATE_REGENERATION] = is_staff | HasAccessRule('instructor')
perms[CERTIFICATE_EXCEPTION_VIEW] = is_staff | HasAccessRule('instructor')
perms[CERTIFICATE_INVALIDATION_VIEW] = is_staff | HasAccessRule('instructor')
perms[GIVE_STUDENT_EXTENSION] = HasAccessRule('staff')
perms[GIVE_STUDENT_EXTENSION] = is_staff_but_not_teaching_assistant
perms[VIEW_ISSUED_CERTIFICATES] = HasAccessRule('staff') | HasRolesRule('data_researcher')
# only global staff or those with the data_researcher role can access the data download tab
# HasAccessRule('staff') also includes course staff
perms[CAN_RESEARCH] = is_staff | HasRolesRule('data_researcher')
perms[CAN_ENROLL] = HasAccessRule('staff')
perms[CAN_ENROLL] = is_staff_but_not_eshe_instructor_or_teaching_assistant
perms[CAN_BETATEST] = HasAccessRule('instructor')
perms[ENROLLMENT_REPORT] = HasAccessRule('staff') | HasRolesRule('data_researcher')
perms[VIEW_COUPONS] = HasAccessRule('staff') | HasRolesRule('data_researcher')
perms[EXAM_RESULTS] = HasAccessRule('staff')
perms[OVERRIDE_GRADES] = HasAccessRule('staff')
perms[OVERRIDE_GRADES] = is_staff_but_not_teaching_assistant
perms[SHOW_TASKS] = HasAccessRule('staff') | HasRolesRule('data_researcher')
perms[EMAIL] = HasAccessRule('staff')
perms[RESCORE_EXAMS] = HasAccessRule('instructor')
Expand Down
99 changes: 99 additions & 0 deletions lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from lms.djangoapps.courseware.tabs import get_course_tab_list
from lms.djangoapps.courseware.tests.factories import StudentModuleFactory
from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase
from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory
from lms.djangoapps.grades.config.waffle import WRITABLE_GRADEBOOK
from lms.djangoapps.instructor.toggles import DATA_DOWNLOAD_V2
from lms.djangoapps.instructor.views.gradebook_api import calculate_page_info
Expand All @@ -38,6 +39,7 @@
ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND,
OVERRIDE_DISCUSSION_LEGACY_SETTINGS_FLAG
)
from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_ADMINISTRATOR
from openedx.core.djangoapps.site_configuration.models import SiteConfiguration


Expand Down Expand Up @@ -315,6 +317,103 @@ def test_membership_reason_field_visibility(self, enbale_reason_field):
else:
self.assertNotContains(response, reason_field)

@ddt.data('eshe_instructor', 'teaching_assistant')
def test_membership_tab_content(self, role):
"""
Verify that eSHE Instructors and Teaching Assistants don't have access to membership tab and
work correctly with other roles.
"""

membership_section = (
'<li class="nav-item">'
'<button type="button" class="btn-link membership" data-section="membership">'
'Membership'
'</button>'
'</li>'
)
batch_enrollment = (
'<fieldset class="batch-enrollment membership-section">'
)

user = UserFactory.create()
self.client.login(username=user.username, password=self.TEST_PASSWORD)

# eSHE Instructors / Teaching Assistants shouldn't have access to membership tab
CourseAccessRoleFactory(
course_id=self.course.id,
user=user,
role=role,
org=self.course.id.org
)
response = self.client.get(self.url)
self.assertNotContains(response, membership_section)

# However if combined with forum_admin, they should have access to this
# tab, but not to batch enrollment
forum_admin_role = RoleFactory(name=FORUM_ROLE_ADMINISTRATOR, course_id=self.course.id)
forum_admin_role.users.add(user)
response = self.client.get(self.url)
self.assertContains(response, membership_section)
self.assertNotContains(response, batch_enrollment)

# Combined with course staff, should have union of all three roles
# permissions sets
CourseAccessRoleFactory(
course_id=self.course.id,
user=user,
role='staff',
org=self.course.id.org
)
response = self.client.get(self.url)
self.assertContains(response, membership_section)
self.assertContains(response, batch_enrollment)

def test_student_admin_tab_content(self):
"""
Verify that Teaching Assistants don't have access to the gradebook-related sections
of the student admin tab.
"""

# Should be visible to Teaching Assistants
view_enrollment_status = '<div class="student-enrollment-status-container action-type-container">'
view_progress = '<div class="student-progress-container action-type-container">'

# Should not be visible to Teaching Assistants
view_gradebook = '<div class="action-type-container ">'
adjust_learner_grade = '<div class="student-grade-container action-type-container ">'
adjust_all_learners_grades = '<div class="course-specific-container action-type-container ">'

user = UserFactory.create()
self.client.login(username=user.username, password=self.TEST_PASSWORD)

# Teaching Assistants shouldn't have access to the gradebook-related sections
CourseAccessRoleFactory(
course_id=self.course.id,
user=user,
role='teaching_assistant',
org=self.course.id.org
)
response = self.client.get(self.url)
self.assertContains(response, view_enrollment_status)
self.assertContains(response, view_progress)
self.assertNotContains(response, view_gradebook)
self.assertNotContains(response, adjust_learner_grade)
self.assertNotContains(response, adjust_all_learners_grades)

# However if combined with instructor, they should have access to all sections
CourseAccessRoleFactory(
course_id=self.course.id,
user=user,
role='instructor',
org=self.course.id.org
)
response = self.client.get(self.url)
self.assertContains(response, view_enrollment_status)
self.assertContains(response, view_progress)
self.assertContains(response, view_gradebook)
self.assertContains(response, adjust_learner_grade)
self.assertContains(response, adjust_all_learners_grades)

def test_student_admin_staff_instructor(self):
"""
Verify that staff users are not able to see course-wide options, while still
Expand Down
Loading
Loading