From a03e6d63959095ab9b8faa6b595d9e9178ca61dc Mon Sep 17 00:00:00 2001 From: hsinkoff <10408711+hsinkoff@users.noreply.github.com> Date: Fri, 22 Dec 2023 16:55:26 +0000 Subject: [PATCH] feat: courseware role checks to permission checks --- lms/djangoapps/courseware/access.py | 27 +++++++++++++++++--- lms/djangoapps/courseware/access_utils.py | 7 ++++- lms/djangoapps/courseware/block_render.py | 18 ++++++++++++- lms/djangoapps/courseware/masquerade.py | 31 ++++++++++++++++++++--- 4 files changed, 74 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 74f1d74f837f..da3988760742 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -42,6 +42,7 @@ from lms.djangoapps.mobile_api.models import IgnoreMobileAvailableFlagConfig from lms.djangoapps.courseware.toggles import course_is_invitation_only from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.course_roles.data import CourseRolesPermission from openedx.features.course_duration_limits.access import check_course_expired from common.djangoapps.student import auth from common.djangoapps.student.models import CourseEnrollmentAllowed @@ -83,7 +84,8 @@ def has_ccx_coach_role(user, course_key): ccx_id = course_key.ccx role = CourseCcxCoachRole(course_key) - if role.has_user(user): + # TODO: remove role check once course_roles is fully impelented and data is migrated + if role.has_user(user) or user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name): list_ccx = CustomCourseForEdX.objects.filter( course_id=course_key.to_course_locator(), coach=user @@ -174,7 +176,12 @@ def has_staff_access_to_preview_mode(user, course_key): """ has_admin_access_to_course = any(administrative_accesses_to_course_for_user(user, course_key)) - return has_admin_access_to_course or is_masquerading_as_student(user, course_key) + # TODO: remove role check once course_roles is fully impelented and data is migrated + return ( + has_admin_access_to_course or + is_masquerading_as_student(user, course_key) or + user.has_perm(CourseRolesPermission.VIEW_ALL_CONTENT.perm_name) + ) def _can_view_courseware_with_prerequisites(user, course): @@ -721,12 +728,12 @@ def _has_access_to_course(user, access_level, course_key): return ACCESS_DENIED global_staff, staff_access, instructor_access = administrative_accesses_to_course_for_user(user, course_key) - + permissions_access = user.has_perm(CourseRolesPermission.VIEW_ALL_CONTENT.perm_name) if global_staff: debug("Allow: user.is_staff") return ACCESS_GRANTED - if access_level not in ('staff', 'instructor'): + if access_level not in ('staff', 'instructor') and not permissions_access: log.debug("Error in access._has_access_to_course access_level=%s unknown", access_level) debug("Deny: unknown access level") return ACCESS_DENIED @@ -738,6 +745,10 @@ def _has_access_to_course(user, access_level, course_key): if instructor_access and access_level in ('staff', 'instructor'): debug("Allow: user has course instructor access") return ACCESS_GRANTED + + if permissions_access: + debug("Allow: user has view all content permission") + return ACCESS_GRANTED debug("Deny: user did not have correct access") return ACCESS_DENIED @@ -875,11 +886,19 @@ def is_mobile_available_for_user(user, block): Checks: mobile_available flag on the course Beta User and staff access overrides the mobile_available flag + Permission to view_all_published_content or + view_only_published_content overrides mobile_available flag Arguments: block (CourseBlock|CourseOverview): course or overview of course in question """ + permissions_access = ( + user.has_perm(CourseRolesPermission.VIEW_LIVE_PUBLISHED_CONTENT.perm_name) and + user.has_perm(CourseRolesPermission.VIEW_ALL_PUBLISHED_CONTENT.perm_name) + ) + # TODO: remove role check once course_roles is fully impelented and data is migrated return ( auth.user_has_role(user, CourseBetaTesterRole(block.id)) + or permissions_access or _has_staff_access_to_block(user, block, block.id) or _is_block_mobile_available(block) ) diff --git a/lms/djangoapps/courseware/access_utils.py b/lms/djangoapps/courseware/access_utils.py index dd2f8d8f6dfe..705c74ff134d 100644 --- a/lms/djangoapps/courseware/access_utils.py +++ b/lms/djangoapps/courseware/access_utils.py @@ -25,6 +25,7 @@ StartDateError ) from lms.djangoapps.courseware.masquerade import get_course_masquerade, is_masquerading_as_student +from openedx.core.djangoapps.course_roles.data import CourseRolesPermission from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, COURSE_PRE_START_ACCESS_FLAG from xmodule.course_block import COURSE_VISIBILITY_PUBLIC # lint-amnesty, pylint: disable=wrong-import-order from xmodule.util.xmodule_django import get_current_request_hostname # lint-amnesty, pylint: disable=wrong-import-order @@ -57,7 +58,11 @@ def adjust_start_date(user, days_early_for_beta, start, course_key): # bail early if no beta testing is set up return start - if CourseBetaTesterRole(course_key).has_user(user): + # TODO: remove role check once course_roles is fully impelented and data is migrated + if ( + CourseBetaTesterRole(course_key).has_user(user) or + user.has_perm(CourseRolesPermission.VIEW_LIVE_PUBLISHED_CONTENT.perm_name) + ): debug("Adjust start time: user in beta role for %s", course_key) delta = timedelta(days_early_for_beta) effective = start - delta diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index fc02e2662e67..7f2a825c1b33 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -68,6 +68,7 @@ from lms.djangoapps.lms_xblock.runtime import UserTagsService, lms_wrappers_aside, lms_applicable_aside_types from lms.djangoapps.verify_student.services import XBlockVerificationService from openedx.core.djangoapps.bookmarks.api import BookmarksService +from openedx.core.djangoapps.course_roles.data import CourseRolesPermission from openedx.core.djangoapps.crawlers.models import CrawlersConfig from openedx.core.djangoapps.credit.services import CreditService from openedx.core.djangoapps.util.user_utils import SystemUser @@ -572,7 +573,15 @@ def inner_get_block(block: XBlock) -> XBlock | None: block_wrappers.append(partial(course_expiration_wrapper, user)) block_wrappers.append(partial(offer_banner_wrapper, user)) - user_is_staff = bool(has_access(user, 'staff', course_id)) + has_permission = ( + user.has_perm(CourseRolesPermission.VIEW_ALL_CONTENT.perm_name) and + user.has_perm(CourseRolesPermission.SPECIFIC_MASQUERADING.perm_name) + ) + # TODO: remove role check once course_roles is fully impelented and data is migrated + user_is_staff = ( + bool(has_access(user, 'staff', course_id)) or + has_permission + ) if settings.FEATURES.get('DISPLAY_DEBUG_INFO_TO_STAFF'): if user_is_staff or is_masquerading_as_specific_student(user, course_id): @@ -582,6 +591,9 @@ def inner_get_block(block: XBlock) -> XBlock | None: block_wrappers.append(partial(add_staff_markup, user, disable_staff_debug_info)) store = modulestore() + # The user_is_staff, user_is_beta_test, and user_role cannot be removed until + # all x-blocks have been updated to use permissions, once they have been updated + # remove role check in favor of permissions checks services = { 'fs': FSService(), @@ -596,6 +608,10 @@ def inner_get_block(block: XBlock) -> XBlock | None: # See the docstring of `DjangoXBlockUserService`. deprecated_anonymous_user_id=anonymous_id_for_user(user, None), request_country_code=user_location, + user_has_manage_content=user.has_perm(CourseRolesPermission.MANAGE_CONTENT.perm_name), + user_has_manage_grades=user.has_perm(CourseRolesPermission.MANAGE_GRADES.perm_name), + user_has_access_data_downloads=user.has_perm(CourseRolesPermission.ACCESS_DATA_DOWNLOADS.perm_name), + user_has_view_all_content=user.has_perm(CourseRolesPermission.VIEW_ALL_CONTENT.perm_name) ), 'verification': XBlockVerificationService(), 'proctoring': ProctoringService(), diff --git a/lms/djangoapps/courseware/masquerade.py b/lms/djangoapps/courseware/masquerade.py index 5ce266c463d6..41ecb2dc32f7 100644 --- a/lms/djangoapps/courseware/masquerade.py +++ b/lms/djangoapps/courseware/masquerade.py @@ -20,6 +20,7 @@ from xblock.runtime import KeyValueStore from common.djangoapps.course_modes.models import CourseMode +from openedx.core.djangoapps.course_roles.data import CourseRolesPermission from openedx.core.djangoapps.util.user_messages import PageLevelMessages from openedx.core.djangolib.markup import HTML from openedx.features.content_type_gating.helpers import CONTENT_GATING_PARTITION_ID @@ -98,8 +99,13 @@ def get(self, request, course_key_string): Retrieve data on the active and available masquerade options """ course_key = CourseKey.from_string(course_key_string) + # TODO: remove role check once course_roles is fully impelented and data is migrated is_staff = has_staff_roles(request.user, course_key) - if not is_staff: + has_permissions = ( + request.user.has_perm(CourseRolesPermission.GENERAL_MASQUERADING.perm_name) or + request.user.has_perm(CourseRolesPermission.SPECIFIC_MASQUERADING.perm_name) + ) + if not is_staff and not has_permissions: return JsonResponse({ 'success': False, }) @@ -165,8 +171,13 @@ def post(self, request, course_key_string): to CourseMasquerade objects. """ course_key = CourseKey.from_string(course_key_string) + # TODO: remove role check once course_roles is fully impelented and data is migrated is_staff = has_staff_roles(request.user, course_key) - if not is_staff: + has_permissions = ( + request.user.has_perm(CourseRolesPermission.GENERAL_MASQUERADING.perm_name) or + request.user.has_perm(CourseRolesPermission.SPECIFIC_MASQUERADING.perm_name) + ) + if not is_staff and not has_permissions: return JsonResponse({ 'success': False, }) @@ -395,8 +406,22 @@ def check_content_start_date_for_masquerade_user(course_key, user, request, cour if now < most_future_date and _is_masquerading: group_masquerade = is_masquerading_as_student(user, course_key) specific_student_masquerade = is_masquerading_as_specific_student(user, course_key) + # TODO: remove role check once course_roles is fully impelented and data is migrated is_staff = has_staff_roles(user, course_key) - if group_masquerade or (specific_student_masquerade and not is_staff): + has_permissions = ( + user.has_perm(CourseRolesPermission.SPECIFIC_MASQUERADING.perm_name) and + ( + user.has_perm(CourseRolesPermission.MANAGE_CONTENT.perm_name) or + user.has_perm(CourseRolesPermission.VIEW_ALL_CONTENT.perm_name) or + user.has_perm(CourseRolesPermission.VIEW_LIVE_AND_PUBLISHED_CONTENT.perm_name) or + user.has_perm(CourseRolesPermission.VIEW_ALL_PUBLISHED_CONTENT.perm_name) + ) + ) + if group_masquerade or ( + specific_student_masquerade and ( + not is_staff and not has_permissions + ) + ): PageLevelMessages.register_warning_message( request, HTML(_('This user does not have access to this content because \