feat: Add permission checks to role checks - contentstore, student, x…

…block, instructor (#34011)

* feat: Add permission checks to role checks - contentstore, student, xblock, instructor

cms/djangoapps/contentstore/views/access.py

@@ -3,6 +3,7 @@
 
 from common.djangoapps.student import auth
 from common.djangoapps.student.roles import CourseInstructorRole
+from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
 
 
 def get_user_role(user, course_id):
@@ -17,7 +18,11 @@ def get_user_role(user, course_id):
     :param course_id: the course_id of the course we're interested in
     """
     # afaik, this is only used in lti
-    if auth.user_has_role(user, CourseInstructorRole(course_id)):
+    # TODO: remove role checks once course_roles is fully implemented and data is migrated
+    if (
+        auth.user_has_role(user, CourseInstructorRole(course_id)) or
+        user.has_perm(CourseRolesPermission.MANAGE_ALL_USERS.perm_name, course_id)
+    ):
         return 'instructor'
     else:
         return 'staff'

cms/djangoapps/contentstore/views/user.py

@@ -19,6 +19,7 @@
 from common.djangoapps.student.models import CourseEnrollment
 from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, LibraryUserRole
 from common.djangoapps.util.json_request import JsonResponse, expect_json
+from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
 
 from ..toggles import use_new_course_team_page
 from ..utils import get_course_team_url, get_course_team
@@ -127,7 +128,11 @@ def _course_team_user(request, course_key, email):
         }
         # what's the highest role that this user has? (How should this report global staff?)
         for role in role_hierarchy:
-            if role(course_key).has_user(user):
+            # TODO: remove role checks once course_roles is fully implemented and data is migrated
+            if (
+                role(course_key).has_user(user) or
+                user.has_perm(CourseRolesPermission.MANAGE_ALL_USERS.perm_name, course_key)
+            ):
                 msg["role"] = role.ROLE
                 break
         return JsonResponse(msg)

common/djangoapps/student/api.py

@@ -30,7 +30,7 @@
     GlobalStaff,
     REGISTERED_ACCESS_ROLES as _REGISTERED_ACCESS_ROLES,
 )
-from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
+from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
 
 
 # This is done so that if these strings change within the app, we can keep exported constants the same
@@ -131,8 +131,10 @@ def is_user_staff_or_instructor_in_course(user, course_key):
     if not isinstance(course_key, CourseKey):
         course_key = CourseKey.from_string(course_key)
 
+    # TODO: remove role checks once course_roles is fully implemented and data is migrated
     return (
         GlobalStaff().has_user(user) or
         CourseStaffRole(course_key).has_user(user) or
-        CourseInstructorRole(course_key).has_user(user)
+        CourseInstructorRole(course_key).has_user(user) or
+        user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name, course_key)
     )

common/djangoapps/student/auth.py

@@ -25,6 +25,7 @@
     OrgLibraryUserRole,
     OrgStaffRole
 )
+from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
 
 # Studio permissions:
 STUDIO_EDIT_ROLES = 8
@@ -79,6 +80,21 @@ def get_user_permissions(user, course_key, org=None):
     Can also set course_key=None and pass in an org to get the user's
     permissions for that organization as a whole.
     """
+    # TODO: remove role checks once course_roles is fully implemented and data is migrated
+    COURSE_INSTRUCTOR_ROLE_PERMISSIONS = [
+        CourseRolesPermission.MANAGE_CONTENT.perm_name,
+        CourseRolesPermission.MANAGE_COURSE_SETTINGS.perm_name,
+        CourseRolesPermission.MANAGE_ADVANCED_SETTINGS.perm_name,
+        CourseRolesPermission.VIEW_COURSE_SETTINGS.perm_name,
+        CourseRolesPermission.MANAGE_ALL_USERS.perm_name,
+    ]
+    STAFF_ROLE_PERMISSIONS = [
+        CourseRolesPermission.MANAGE_CONTENT.perm_name,
+        CourseRolesPermission.MANAGE_COURSE_SETTINGS.perm_name,
+        CourseRolesPermission.MANAGE_ADVANCED_SETTINGS.perm_name,
+        CourseRolesPermission.VIEW_COURSE_SETTINGS.perm_name,
+        CourseRolesPermission.MANAGE_USERS_EXCEPT_ADMIN_AND_STAFF.perm_name,
+    ]
     if org is None:
         org = course_key.org
         course_key = course_key.for_branch(None)
@@ -91,7 +107,11 @@ def get_user_permissions(user, course_key, org=None):
     # global staff, org instructors, and course instructors have all permissions:
     if GlobalStaff().has_user(user) or OrgInstructorRole(org=org).has_user(user):
         return all_perms
-    if course_key and user_has_role(user, CourseInstructorRole(course_key)):
+    if (
+        course_key and (
+            user_has_role(user, CourseInstructorRole(course_key)) or
+            user.has_perms(COURSE_INSTRUCTOR_ROLE_PERMISSIONS, course_key))
+    ):
         return all_perms
     # HACK: Limited Staff should not have studio read access. However, since many LMS views depend on the
     #  `has_course_author_access` check and `course_author_access_required` decorator, we have to allow write access
@@ -104,7 +124,11 @@ def get_user_permissions(user, course_key, org=None):
     if course_key and user_has_role(user, CourseLimitedStaffRole(course_key)):
         return STUDIO_EDIT_CONTENT
     # Staff have all permissions except EDIT_ROLES:
-    if OrgStaffRole(org=org).has_user(user) or (course_key and user_has_role(user, CourseStaffRole(course_key))):
+    if (
+        OrgStaffRole(org=org).has_user(user) or
+        (course_key and user_has_role(user, CourseStaffRole(course_key))) or
+        user.has_perms(STAFF_ROLE_PERMISSIONS, course_key)
+    ):
         return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT
     # Otherwise, for libraries, users can view only:
     if course_key and isinstance(course_key, LibraryLocator):
@@ -234,5 +258,9 @@ def _check_caller_authority(caller, role):
     if isinstance(role, (GlobalStaff, CourseCreatorRole, OrgContentCreatorRole)):  # lint-amnesty, pylint: disable=no-else-raise
         raise PermissionDenied
     elif isinstance(role, CourseRole):  # instructors can change the roles w/in their course
-        if not user_has_role(caller, CourseInstructorRole(role.course_key)):
+        # TODO: remove role checks once course_roles is fully implemented and data is migrated
+        if not (
+            user_has_role(caller, CourseInstructorRole(role.course_key)) or
+            caller.has_perm(CourseRolesPermission.MANAGE_ALL_USERS.perm_name, role.course_key)
+        ):
             raise PermissionDenied

common/djangoapps/student/role_helpers.py

@@ -10,6 +10,7 @@
     FORUM_ROLE_MODERATOR,
     Role
 )
+from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
 from openedx.core.lib.cache_utils import request_cached
 from common.djangoapps.student.roles import (
     CourseBetaTesterRole,
@@ -27,16 +28,24 @@ def has_staff_roles(user, course_key):
     Return true if a user has any of the following roles
     Staff, Instructor, Beta Tester, Forum Community TA, Forum Group Moderator, Forum Moderator, Forum Administrator
     """
+    # TODO: remove role checks once course_roles is fully implemented and data is migrated
     forum_roles = [FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_GROUP_MODERATOR,
                    FORUM_ROLE_MODERATOR, FORUM_ROLE_ADMINISTRATOR]
+    permissions = [
+        CourseRolesPermission.MODERATE_DISCUSSION_FORUMS.value,
+        CourseRolesPermission.MODERATE_DISCUSSION_FORUMS_FOR_A_COHORT.value,
+    ]
     is_staff = CourseStaffRole(course_key).has_user(user)
     is_instructor = CourseInstructorRole(course_key).has_user(user)
     is_beta_tester = CourseBetaTesterRole(course_key).has_user(user)
     is_org_staff = OrgStaffRole(course_key.org).has_user(user)
     is_org_instructor = OrgInstructorRole(course_key.org).has_user(user)
     is_global_staff = GlobalStaff().has_user(user)
     has_forum_role = Role.user_has_role_for_course(user, course_key, forum_roles)
-    if any([is_staff, is_instructor, is_beta_tester, is_org_staff,
-            is_org_instructor, is_global_staff, has_forum_role]):
+    has_discussion_perms = any(user.has_perm(permission, course_key) for permission in permissions)
+    if (
+        any([is_staff, is_instructor, is_beta_tester, is_org_staff,
+            is_org_instructor, is_global_staff, has_forum_role, has_discussion_perms])
+    ):
         return True
     return False

common/djangoapps/xblock_django/constants.py

@@ -26,3 +26,13 @@
 ATTR_KEY_USER_PREFERENCES = 'edx-platform.user_preferences'
 # The user's role in the course ('staff', 'instructor', or 'student').
 ATTR_KEY_USER_ROLE = 'edx-platform.user_role'
+# Permissions attributes (4) have been added, but the role attributes cannot be remove
+# until all x-blocks have been updated to use permissions in place of roles
+# Whether the user has the manage content permission
+ATTR_KEY_USER_HAS_MANAGE_CONTENT = 'edx-platform.user_has_manage_content'
+# Whether the user has the manage grades permission
+ATTR_KEY_USER_HAS_MANAGE_GRADES = 'edx-platform.user_has_manage_grades'
+# Whether the user has the access data downloads permission
+ATTR_KEY_USER_HAS_ACCESS_DATA_DOWNLOADS = 'edx-platform.user_has_access_data_downloads'
+# Whether the user has the view all content permission
+ATTR_KEY_USER_HAS_VIEW_ALL_CONTENT = 'edx-platform.user_has_view_all_content'

common/djangoapps/xblock_django/user_service.py

@@ -16,6 +16,10 @@
     ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID,
     ATTR_KEY_IS_AUTHENTICATED,
     ATTR_KEY_REQUEST_COUNTRY_CODE,
+    ATTR_KEY_USER_HAS_ACCESS_DATA_DOWNLOADS,
+    ATTR_KEY_USER_HAS_MANAGE_CONTENT,
+    ATTR_KEY_USER_HAS_MANAGE_GRADES,
+    ATTR_KEY_USER_HAS_VIEW_ALL_CONTENT,
     ATTR_KEY_USER_ID,
     ATTR_KEY_USERNAME,
     ATTR_KEY_USER_IS_BETA_TESTER,
@@ -48,6 +52,14 @@ def __init__(self, django_user, **kwargs):
                 (course-agnostic) anonymized ID. To preserve backward compatibility, we will continue to provide it.
                 Using course-specific anonymous user ID (`anonymous_user_id`) is a preferred approach.
             request_country_code(str): optional -- country code determined from the user's request IP address.
+            user_has_manage_content(bool): optional - whether the user has the manage_content permission
+                on any of their roles for the course or org
+            user_has_manage_grades(bool): optional - whether the user has the manage_grades permission
+                on any of their roles for the course or org
+            user_has_access_data_downloads(bool): optional - whether the user has the access_data_downloads permission
+                on any of their roles for the course or org
+            user_has_view_all_content(bool): optional - whether the user has the view_all_content permission
+                on any of their roles for the course or org
         """
         super().__init__(**kwargs)
         self._django_user = django_user
@@ -58,6 +70,10 @@ def __init__(self, django_user, **kwargs):
         self._anonymous_user_id = kwargs.get('anonymous_user_id', None)
         self._deprecated_anonymous_user_id = kwargs.get('deprecated_anonymous_user_id', None)
         self._request_country_code = kwargs.get('request_country_code', None)
+        self._user_has_manage_content = kwargs.get('user_has_manage_content', False)
+        self._user_has_manage_grades = kwargs.get('user_has_manage_grades', False)
+        self._user_has_access_data_downloads = kwargs.get('user_has_access_data_downloads', False)
+        self._user_has_view_all_content = kwargs.get('user_has_view_all_content', False)
 
     def get_current_user(self):
         """
@@ -138,6 +154,10 @@ def _convert_django_user_to_xblock_user(self, django_user):
                 for pref in USER_PREFERENCES_WHITE_LIST
                 if pref in user_preferences
             }
+            xblock_user.opt_attrs[ATTR_KEY_USER_HAS_MANAGE_CONTENT] = self._user_has_manage_content
+            xblock_user.opt_attrs[ATTR_KEY_USER_HAS_MANAGE_GRADES] = self._user_has_manage_grades
+            xblock_user.opt_attrs[ATTR_KEY_USER_HAS_ACCESS_DATA_DOWNLOADS] = self._user_has_access_data_downloads
+            xblock_user.opt_attrs[ATTR_KEY_USER_HAS_VIEW_ALL_CONTENT] = self._user_has_view_all_content
         else:
             xblock_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED] = False
             xblock_user.opt_attrs[ATTR_KEY_REQUEST_COUNTRY_CODE] = self._request_country_code

lms/djangoapps/instructor/permissions.py

@@ -6,6 +6,8 @@
 from bridgekeeper.rules import is_staff
 
 from lms.djangoapps.courseware.rules import HasAccessRule, HasRolesRule
+from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
+from openedx.core.djangoapps.course_roles.rules import HasPermissionRule
 
 ALLOW_STUDENT_TO_BYPASS_ENTRANCE_EXAM = 'instructor.allow_student_to_bypass_entrance_exam'
 ASSIGN_TO_COHORTS = 'instructor.assign_to_cohorts'
@@ -42,25 +44,54 @@
 perms[GENERATE_CERTIFICATE_EXCEPTIONS] = is_staff
 perms[GENERATE_BULK_CERTIFICATE_EXCEPTIONS] = is_staff
 perms[GIVE_STUDENT_EXTENSION] = HasAccessRule('staff')
-perms[VIEW_ISSUED_CERTIFICATES] = HasAccessRule('staff') | HasRolesRule('data_researcher')
+# TODO: remove role checks once course_roles is fully implemented and data is migrated
+perms[VIEW_ISSUED_CERTIFICATES] = (
+    HasAccessRule('staff') |
+    HasRolesRule('data_researcher') |
+    HasPermissionRule(CourseRolesPermission.ACCESS_DATA_DOWNLOADS.perm_name)
+)
 # 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')
+# TODO: remove role checks once course_roles is fully implemented and data is migrated
+perms[CAN_RESEARCH] = (
+    is_staff |
+    HasRolesRule('data_researcher') |
+    HasPermissionRule(CourseRolesPermission.ACCESS_DATA_DOWNLOADS.perm_name)
+)
 perms[CAN_ENROLL] = HasAccessRule('staff')
 perms[CAN_BETATEST] = HasAccessRule('instructor')
-perms[ENROLLMENT_REPORT] = HasAccessRule('staff') | HasRolesRule('data_researcher')
-perms[VIEW_COUPONS] = HasAccessRule('staff') | HasRolesRule('data_researcher')
+# TODO: remove role checks once course_roles is fully implemented and data is migrated
+perms[ENROLLMENT_REPORT] = (
+    HasAccessRule('staff') |
+    HasRolesRule('data_researcher') |
+    HasPermissionRule(CourseRolesPermission.ACCESS_DATA_DOWNLOADS.perm_name)
+)
+# TODO: remove role checks once course_roles is fully implemented and data is migrated
+perms[VIEW_COUPONS] = (
+    HasAccessRule('staff') |
+    HasRolesRule('data_researcher') |
+    HasPermissionRule(CourseRolesPermission.ACCESS_DATA_DOWNLOADS.perm_name)
+)
 perms[EXAM_RESULTS] = HasAccessRule('staff')
 perms[OVERRIDE_GRADES] = HasAccessRule('staff')
-perms[SHOW_TASKS] = HasAccessRule('staff') | HasRolesRule('data_researcher')
+# TODO: remove role checks once course_roles is fully implemented and data is migrated
+perms[SHOW_TASKS] = (
+    HasAccessRule('staff') |
+    HasRolesRule('data_researcher') | (
+        HasPermissionRule(CourseRolesPermission.MANAGE_STUDENTS.perm_name) &
+        HasPermissionRule(CourseRolesPermission.ACCESS_DATA_DOWNLOADS.perm_name) &
+        HasPermissionRule(CourseRolesPermission.ACCESS_INSTRUCTOR_DASHBOARD.perm_name)
+    )
+)
 perms[EMAIL] = HasAccessRule('staff')
 perms[RESCORE_EXAMS] = HasAccessRule('instructor')
 perms[VIEW_REGISTRATION] = HasAccessRule('staff')
-perms[VIEW_DASHBOARD] = \
-    HasRolesRule(
-        'staff',
-        'instructor',
-        'data_researcher'
-) | HasAccessRule('staff') | HasAccessRule('instructor')
+# TODO: remove role checks once course_roles is fully implemented and data is migrated
+perms[VIEW_DASHBOARD] = (
+    HasRolesRule('staff', 'instructor', 'data_researcher') |
+    HasAccessRule('staff') |
+    HasAccessRule('instructor') |
+    HasPermissionRule(CourseRolesPermission.ACCESS_INSTRUCTOR_DASHBOARD.perm_name)
+)
 perms[VIEW_ENROLLMENTS] = HasAccessRule('staff')
 perms[VIEW_FORUM_MEMBERS] = HasAccessRule('staff')

lms/djangoapps/instructor/services.py

@@ -17,6 +17,7 @@
 from lms.djangoapps.commerce.utils import create_zendesk_ticket
 from lms.djangoapps.courseware.models import StudentModule
 from lms.djangoapps.instructor.tasks import update_exam_completion_task
+from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
 from xmodule.modulestore.django import modulestore  # lint-amnesty, pylint: disable=wrong-import-order
 
 log = logging.getLogger(__name__)
@@ -107,7 +108,11 @@ def is_course_staff(self, user, course_id):
         Returns True if the user is the course staff
         else Returns False
         """
-        return auth.user_has_role(user, CourseStaffRole(CourseKey.from_string(course_id)))
+        # TODO: remove role checks once course_roles is fully implemented and data is migrated
+        return (
+            auth.user_has_role(user, CourseStaffRole(CourseKey.from_string(course_id))) or
+            user.has_perm(CourseRolesPermission.MANAGE_COURSE_SETTINGS.perm_name, course_id)
+        )
 
     def send_support_notification(self, course_id, exam_name, student_username, review_status, review_url=None):
         """

lms/djangoapps/instructor/views/instructor_dashboard.py

@@ -52,6 +52,7 @@
 from lms.djangoapps.grades.api import is_writable_gradebook_enabled
 from lms.djangoapps.instructor.constants import INSTRUCTOR_DASHBOARD_PLUGIN_VIEW_NAME
 from openedx.core.djangoapps.course_groups.cohorts import DEFAULT_COHORT_NAME, get_course_cohorts, is_course_cohorted
+from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
 from openedx.core.djangoapps.discussions.config.waffle_utils import legacy_discussion_experience_enabled
 from openedx.core.djangoapps.discussions.utils import available_division_schemes
 from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, CourseDiscussionSettings
@@ -119,14 +120,21 @@ def instructor_dashboard_2(request, course_id):  # lint-amnesty, pylint: disable
 
     course = get_course_by_id(course_key, depth=None)
 
+    # TODO: remove role checks once course_roles is fully implemented and data is migrated
     access = {
         'admin': request.user.is_staff,
         'instructor': bool(has_access(request.user, 'instructor', course)),
         'finance_admin': CourseFinanceAdminRole(course_key).has_user(request.user),
         'sales_admin': CourseSalesAdminRole(course_key).has_user(request.user),
         'staff': bool(has_access(request.user, 'staff', course)),
-        'forum_admin': has_forum_access(request.user, course_key, FORUM_ROLE_ADMINISTRATOR),
-        'data_researcher': request.user.has_perm(permissions.CAN_RESEARCH, course_key),
+        'forum_admin': (
+            has_forum_access(request.user, course_key, FORUM_ROLE_ADMINISTRATOR) or
+            request.user.has_perm(CourseRolesPermission.MANAGE_DISCUSSION_MODERATORS.perm_name, course_key)
+        ),
+        'data_researcher': (
+            request.user.has_perm(permissions.CAN_RESEARCH, course_key) or
+            request.user.has_perm(CourseRolesPermission.ACCESS_DATA_DOWNLOADS.perm_name, course_key)
+        ),
     }
 
     if not request.user.has_perm(permissions.VIEW_DASHBOARD, course_key):
@@ -189,10 +197,12 @@ def instructor_dashboard_2(request, course_id):  # lint-amnesty, pylint: disable
     # Gate access to Special Exam tab depending if either timed exams or proctored exams
     # are enabled in the course
 
+    # TODO: remove role checks once course_roles is fully implemented and data is migrated
     user_has_access = any([
         request.user.is_staff,
         CourseStaffRole(course_key).has_user(request.user),
-        CourseInstructorRole(course_key).has_user(request.user)
+        CourseInstructorRole(course_key).has_user(request.user),
+        request.user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name, course_key)
     ])
     course_has_special_exams = course.enable_proctored_exams or course.enable_timed_exams
     can_see_special_exams = course_has_special_exams and user_has_access and settings.FEATURES.get(