diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index f8e45ce6ba6c..66bd6806a6ef 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -15,6 +15,7 @@ CourseBetaTesterRole, CourseCreatorRole, CourseInstructorRole, + CourseLimitedStaffRole, CourseRole, CourseStaffRole, GlobalStaff, @@ -92,6 +93,9 @@ def get_user_permissions(user, course_key, org=None): return all_perms if course_key and user_has_role(user, CourseInstructorRole(course_key)): return all_perms + # Limited Course Staff does not have access to Studio. + if course_key and user_has_role(user, CourseLimitedStaffRole(course_key)): + return STUDIO_NO_PERMISSIONS # 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))): return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 3749bc05035a..fe2b6c91d689 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -19,13 +19,17 @@ # A list of registered access roles. REGISTERED_ACCESS_ROLES = {} +# A mapping of roles to the roles that they inherit permissions from. +ACCESS_ROLES_INHERITANCE = {} + def register_access_role(cls): """ Decorator that allows access roles to be registered within the roles module and referenced by their string values. - Assumes that the decorated class has a "ROLE" attribute, defining its type. + Assumes that the decorated class has a "ROLE" attribute, defining its type and an optional "BASE_ROLE" attribute, + defining the role that it inherits permissions from. """ try: @@ -33,6 +37,10 @@ def register_access_role(cls): REGISTERED_ACCESS_ROLES[role_name] = cls except AttributeError: log.exception("Unable to register Access Role with attribute 'ROLE'.") + + if base_role := getattr(cls, "BASE_ROLE", None): + ACCESS_ROLES_INHERITANCE.setdefault(base_role, set()).add(cls.ROLE) + return cls @@ -69,12 +77,20 @@ def __init__(self, user): CourseAccessRole.objects.filter(user=user).all() ) + @staticmethod + def _get_roles(role): + """ + Return the roles that should have the same permissions as the specified role. + """ + return ACCESS_ROLES_INHERITANCE.get(role, set()) | {role} + def has_role(self, role, course_id, org): """ - Return whether this RoleCache contains a role with the specified role, course_id, and org + Return whether this RoleCache contains a role with the specified role + or a role that inherits from the specified role, course_id and org. """ return any( - access_role.role == role and + access_role.role in self._get_roles(role) and access_role.course_id == course_id and access_role.org == org for access_role in self._roles @@ -186,9 +202,10 @@ def add_users(self, *users): # legit get updated. from common.djangoapps.student.models import CourseAccessRole # lint-amnesty, pylint: disable=redefined-outer-name, reimported for user in users: - if user.is_authenticated and user.is_active and not self.has_user(user): - entry = CourseAccessRole(user=user, role=self._role_name, course_id=self.course_key, org=self.org) - entry.save() + if user.is_authenticated and user.is_active: + CourseAccessRole.objects.get_or_create( + user=user, role=self._role_name, course_id=self.course_key, org=self.org + ) if hasattr(user, '_roles'): del user._roles @@ -261,6 +278,14 @@ def __init__(self, *args, **kwargs): super().__init__(self.ROLE, *args, **kwargs) +@register_access_role +class CourseLimitedStaffRole(CourseStaffRole): + """A Staff member of a course without access to Studio.""" + + ROLE = 'limited_staff' + BASE_ROLE = CourseStaffRole.ROLE + + @register_access_role class CourseInstructorRole(CourseRole): """A course Instructor""" diff --git a/common/djangoapps/student/tests/test_authz.py b/common/djangoapps/student/tests/test_authz.py index 78257750cdea..a7a3694d3489 100644 --- a/common/djangoapps/student/tests/test_authz.py +++ b/common/djangoapps/student/tests/test_authz.py @@ -22,6 +22,7 @@ from common.djangoapps.student.roles import ( CourseCreatorRole, CourseInstructorRole, + CourseLimitedStaffRole, CourseStaffRole, OrgContentCreatorRole ) @@ -200,7 +201,7 @@ def test_no_staff_read_access(self): class CourseGroupTest(TestCase): """ - Tests for instructor and staff groups for a particular course. + Tests for instructor, staff and limited staff groups for a particular course. """ def setUp(self): @@ -213,6 +214,9 @@ def setUp(self): self.staff = UserFactory.create( username='teststaff', email='teststaff+courses@edx.org', password='foo', ) + self.limited_staff = UserFactory.create( + username='testlimitedstaff', email='testlimitedstaff+courses@edx.org', password='foo', + ) self.course_key = CourseLocator('mitX', '101', 'test') def test_add_user_to_course_group(self): @@ -230,6 +234,14 @@ def test_add_user_to_course_group(self): add_users(self.creator, CourseStaffRole(self.course_key), self.staff) assert user_has_role(self.staff, CourseStaffRole(self.course_key)) + # Add another user to the limited staff role. + assert not user_has_role(self.limited_staff, CourseLimitedStaffRole(self.course_key)) + add_users(self.creator, CourseLimitedStaffRole(self.course_key), self.limited_staff) + assert user_has_role(self.limited_staff, CourseLimitedStaffRole(self.course_key)) + + # Verify that limited staff inherits from staff. + assert user_has_role(self.limited_staff, CourseStaffRole(self.course_key)) + def test_add_user_to_course_group_permission_denied(self): """ Verifies PermissionDenied if caller of add_user_to_course_group is not instructor role. @@ -249,6 +261,12 @@ def test_remove_user_from_course_group(self): add_users(self.creator, CourseStaffRole(self.course_key), self.staff) assert user_has_role(self.staff, CourseStaffRole(self.course_key)) + add_users(self.creator, CourseLimitedStaffRole(self.course_key), self.limited_staff) + assert user_has_role(self.limited_staff, CourseLimitedStaffRole(self.course_key)) + + remove_users(self.creator, CourseLimitedStaffRole(self.course_key), self.limited_staff) + assert not user_has_role(self.limited_staff, CourseLimitedStaffRole(self.course_key)) + remove_users(self.creator, CourseStaffRole(self.course_key), self.staff) assert not user_has_role(self.staff, CourseStaffRole(self.course_key)) @@ -267,6 +285,15 @@ def test_remove_user_from_course_group_permission_denied(self): with pytest.raises(PermissionDenied): remove_users(self.staff, CourseStaffRole(self.course_key), another_staff) + def test_no_limited_staff_read_or_write_access(self): + """ + Test that course limited staff have no read or write access. + """ + add_users(self.global_admin, CourseLimitedStaffRole(self.course_key), self.limited_staff) + + assert not has_studio_read_access(self.limited_staff, self.course_key) + assert not has_studio_write_access(self.limited_staff, self.course_key) + class CourseOrgGroupTest(TestCase): """ diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 6c344352a142..c670c7c2b75b 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -12,7 +12,12 @@ CourseBetaTesterRole, CourseInstructorRole, CourseRole, + CourseLimitedStaffRole, CourseStaffRole, + CourseFinanceAdminRole, + CourseSalesAdminRole, + LibraryUserRole, + CourseDataResearcherRole, GlobalStaff, OrgContentCreatorRole, OrgInstructorRole, @@ -162,8 +167,13 @@ 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')), (CourseInstructorRole(IN_KEY), ('instructor', IN_KEY, 'edX')), (OrgStaffRole(IN_KEY.org), ('staff', None, 'edX')), + (CourseFinanceAdminRole(IN_KEY), ('finance_admin', IN_KEY, 'edX')), + (CourseSalesAdminRole(IN_KEY), ('sales_admin', IN_KEY, 'edX')), + (LibraryUserRole(IN_KEY), ('library_user', IN_KEY, 'edX')), + (CourseDataResearcherRole(IN_KEY), ('data_researcher', IN_KEY, 'edX')), (OrgInstructorRole(IN_KEY.org), ('instructor', None, 'edX')), (CourseBetaTesterRole(IN_KEY), ('beta_testers', IN_KEY, 'edX')), ) @@ -183,7 +193,13 @@ def test_only_in_role(self, role, target): if other_role == role: continue - assert not cache.has_role(*other_target) + role_base_id = getattr(role, "BASE_ROLE", None) + other_role_id = getattr(other_role, "ROLE", None) + + if other_role_id and role_base_id == other_role_id: + assert cache.has_role(*other_target) + else: + assert not cache.has_role(*other_target) @ddt.data(*ROLES) @ddt.unpack diff --git a/lms/djangoapps/instructor/access.py b/lms/djangoapps/instructor/access.py index 85f659d6d0b0..9255d113f038 100644 --- a/lms/djangoapps/instructor/access.py +++ b/lms/djangoapps/instructor/access.py @@ -17,7 +17,8 @@ CourseCcxCoachRole, CourseDataResearcherRole, CourseInstructorRole, - CourseStaffRole + CourseLimitedStaffRole, + CourseStaffRole, ) from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params from openedx.core.djangoapps.django_comment_common.models import Role @@ -28,6 +29,7 @@ 'beta': CourseBetaTesterRole, 'instructor': CourseInstructorRole, 'staff': CourseStaffRole, + 'limited_staff': CourseLimitedStaffRole, 'ccx_coach': CourseCcxCoachRole, 'data_researcher': CourseDataResearcherRole, } diff --git a/lms/static/js/fixtures/instructor_dashboard/membership.html b/lms/static/js/fixtures/instructor_dashboard/membership.html index 791a0a8c2a8b..962bfac5b4f1 100644 --- a/lms/static/js/fixtures/instructor_dashboard/membership.html +++ b/lms/static/js/fixtures/instructor_dashboard/membership.html @@ -10,6 +10,7 @@