Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Jul 22, 2024
10 changes: 10 additions & 0 deletions cms/djangoapps/contentstore/tests/test_contentstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
15 changes: 15 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
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 @@ -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')
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 @@ -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')
Expand All @@ -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')
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 @@ -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
Expand Down
Loading
Loading