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(palm): create Course Limited Staff role [BB-7503] #547

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading