Merge pull request #666 from open-craft/0x29a/bb8914/redwood-code-drift

feat: port code drift for open-craft/edx-platform [BB-8914]

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -1372,6 +1372,16 @@ def test_create_course_with_unicode_in_id_disabled(self):
             self.course_data['run'] = '����������'
             self.assert_create_course_failed(error_message)
 
+    @override_settings(DEFAULT_COURSE_INVITATION_ONLY=True)
+    def test_create_course_invitation_only(self):
+        """
+        Test new course creation with setting: DEFAULT_COURSE_INVITATION_ONLY=True.
+        """
+        test_course_data = self.assert_created_course()
+        course_id = _get_course_id(self.store, test_course_data)
+        course = self.store.get_course(course_id)
+        self.assertEqual(course.invitation_only, True)
+
     def assert_course_permission_denied(self):
         """
         Checks that the course did not get created due to a PermissionError.

cms/djangoapps/contentstore/views/course.py

@@ -990,8 +990,9 @@ def create_new_course_in_store(store, user, org, number, run, fields):
 
     # Set default language from settings and enable web certs
     fields.update({
-        'language': getattr(settings, 'DEFAULT_COURSE_LANGUAGE', 'en'),
         'cert_html_view_enabled': True,
+        'invitation_only': getattr(settings, 'DEFAULT_COURSE_INVITATION_ONLY', False),
+        'language': getattr(settings, 'DEFAULT_COURSE_LANGUAGE', 'en'),
     })
 
     with modulestore().default_store(store):

cms/envs/common.py

@@ -591,6 +591,17 @@
     # .. toggle_use_cases: open_edx
     # .. 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: ENABLE_COPPA_COMPLIANCE
@@ -3003,3 +3014,7 @@ def _should_send_learning_badge_events(settings):
 # See https://www.meilisearch.com/docs/learn/security/tenant_tokens
 MEILISEARCH_INDEX_PREFIX = ""
 MEILISEARCH_API_KEY = "devkey"
+
+############## Default value for invitation_only when creating courses ##############
+
+DEFAULT_COURSE_INVITATION_ONLY = False

common/djangoapps/student/models/user.py

@@ -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(

common/djangoapps/student/roles.py

@@ -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:
@@ -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"""

common/djangoapps/student/tests/test_roles.py

@@ -17,6 +17,8 @@
     CourseStaffRole,
     CourseFinanceAdminRole,
     CourseSalesAdminRole,
+    eSHEInstructorRole,
+    TeachingAssistantRole,
     LibraryUserRole,
     CourseDataResearcherRole,
     GlobalStaff,
@@ -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')),

common/djangoapps/student/tests/tests.py

@@ -1064,6 +1064,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')

lms/djangoapps/courseware/tabs.py

@@ -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):
     """

lms/djangoapps/courseware/tests/test_tabs.py

@@ -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)

lms/djangoapps/instructor/access.py

@@ -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
@@ -30,6 +32,8 @@
     'instructor': CourseInstructorRole,
     'staff': CourseStaffRole,
     'limited_staff': CourseLimitedStaffRole,
+    'eshe_instructor': eSHEInstructorRole,
+    'teaching_assistant': TeachingAssistantRole,
     'ccx_coach': CourseCcxCoachRole,
     'data_researcher': CourseDataResearcherRole,
 }

lms/djangoapps/instructor/permissions.py

@@ -36,6 +36,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')
@@ -49,17 +64,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')

lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py

@@ -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
@@ -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
 
 
@@ -314,6 +316,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