Skip to content

Commit

Permalink
feat: create Course Limited Staff role
Browse files Browse the repository at this point in the history
This is an experimental approach to introduce a role which has all Course Staff
permissions, except for the Studio access.

Co-authored-by: 0x29a <[email protected]>
(cherry picked from commit 3ccfe7c)
  • Loading branch information
Agrendalath committed Jul 4, 2023
1 parent 05324a5 commit 6b01531
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 9 deletions.
4 changes: 4 additions & 0 deletions common/djangoapps/student/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
CourseBetaTesterRole,
CourseCreatorRole,
CourseInstructorRole,
CourseLimitedStaffRole,
CourseRole,
CourseStaffRole,
GlobalStaff,
Expand Down Expand Up @@ -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
Expand Down
37 changes: 31 additions & 6 deletions common/djangoapps/student/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,28 @@
# 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:
role_name = cls.ROLE
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


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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"""
Expand Down
29 changes: 28 additions & 1 deletion common/djangoapps/student/tests/test_authz.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from common.djangoapps.student.roles import (
CourseCreatorRole,
CourseInstructorRole,
CourseLimitedStaffRole,
CourseStaffRole,
OrgContentCreatorRole
)
Expand Down Expand Up @@ -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):
Expand All @@ -213,6 +214,9 @@ def setUp(self):
self.staff = UserFactory.create(
username='teststaff', email='[email protected]', password='foo',
)
self.limited_staff = UserFactory.create(
username='testlimitedstaff', email='[email protected]', password='foo',
)
self.course_key = CourseLocator('mitX', '101', 'test')

def test_add_user_to_course_group(self):
Expand All @@ -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.
Expand All @@ -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))

Expand All @@ -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):
"""
Expand Down
18 changes: 17 additions & 1 deletion common/djangoapps/student/tests/test_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@
CourseBetaTesterRole,
CourseInstructorRole,
CourseRole,
CourseLimitedStaffRole,
CourseStaffRole,
CourseFinanceAdminRole,
CourseSalesAdminRole,
LibraryUserRole,
CourseDataResearcherRole,
GlobalStaff,
OrgContentCreatorRole,
OrgInstructorRole,
Expand Down Expand Up @@ -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')),
)
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion lms/djangoapps/instructor/access.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -28,6 +29,7 @@
'beta': CourseBetaTesterRole,
'instructor': CourseInstructorRole,
'staff': CourseStaffRole,
'limited_staff': CourseLimitedStaffRole,
'ccx_coach': CourseCcxCoachRole,
'data_researcher': CourseDataResearcherRole,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ <h3 class="hd hd-3">Course Team Management</h3>
<label>Select a course team role:
<select id="member-lists-selector" class="member-lists-selector">
<option value="staff">Staff</option>
<option value="limited_staff">Limited Staff</option>
<option value="instructor">Admin</option>
<option value="beta">Beta Testers</option>
<option value="Administrator">Discussion Admins</option>
Expand Down
13 changes: 13 additions & 0 deletions lms/templates/instructor/instructor_dashboard_2/membership.html
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,19 @@ <h3 class="hd hd-3">${_("Course Team Management")}</h3>
data-add-button-label="${_("Add Staff")}"
></div>

<div class="auth-list-container"
data-rolename="limited_staff"
data-display-name="${_("Limited Staff")}"
data-info-text="
${_("Course team members with the Limited Staff role help you manage your course. "
"Limited Staff can enroll and unenroll learners, as well as modify their grades and "
"access all course data. Limited Staff don't have access to your course in Studio. "
"You can only give course team roles to enrolled users.")}"
data-list-endpoint="${ section_data['list_course_role_members_url'] }"
data-modify-endpoint="${ section_data['modify_access_url'] }"
data-add-button-label="${_("Add Limited Staff")}"
></div>

## Note that "Admin" is identified as "Instructor" in the Django admin panel.
<div class="auth-list-container"
data-rolename="instructor"
Expand Down

0 comments on commit 6b01531

Please sign in to comment.