Skip to content

Commit

Permalink
feat: teams role checks to permission checks
Browse files Browse the repository at this point in the history
  • Loading branch information
hsinkoff committed Jan 12, 2024
1 parent 1ad5236 commit b908907
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 11 deletions.
41 changes: 33 additions & 8 deletions lms/djangoapps/teams/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,10 @@ def has_team_api_access(user, course_key, access_username=None):
bool: True if the user has access, False otherwise.
"""
# TODO: remove role checks once course_roles is fully impelented and data is migrated
if has_course_staff_privileges(user, course_key):
if (
has_course_staff_privileges(user, course_key) or
user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name)
):
return True
if has_discussion_privileges(user, course_key):
return True
Expand All @@ -181,7 +184,11 @@ def user_organization_protection_status(user, course_key):
If the user is a staff of the course, we return the protection_exempt status
else, we return the unprotected status
"""
if has_course_staff_privileges(user, course_key):
# TODO: remove role checks once course_roles is fully impelented and data is migrated
if (
has_course_staff_privileges(user, course_key) or
user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name)
):
return OrganizationProtectionStatus.protection_exempt
enrollment = CourseEnrollment.get_enrollment(user, course_key)
if enrollment and enrollment.is_active:
Expand All @@ -205,8 +212,13 @@ def has_specific_team_access(user, team):
- be in the correct bubble
- be in the team if it is private
"""
return has_course_staff_privileges(user, team.course_id) or (
user_protection_status_matches_team(user, team) and user_on_team_or_team_is_public(user, team)
# TODO: remove role checks once course_roles is fully impelented and data is migrated
user_has_manage_student_permission = user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name)
return (
has_course_staff_privileges(user, team.course_id) or
user_has_manage_student_permission or (
user_protection_status_matches_team(user, team) and user_on_team_or_team_is_public(user, team)
)
)


Expand All @@ -216,8 +228,13 @@ def has_specific_teamset_access(user, course_block, teamset_id):
All non-staff users have access to open and public_managed teamsets.
Non-staff users only have access to a private_managed teamset if they are in a team in that teamset
"""
return has_course_staff_privileges(user, course_block.id) or \
# TODO: remove role checks once course_roles is fully impelented and data is migrated
user_has_manage_student_permission = user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name)
return (
has_course_staff_privileges(user, course_block.id) or
user_has_manage_student_permission or \
teamset_is_public_or_user_is_on_team_in_teamset(user, course_block, teamset_id)
)


def teamset_is_public_or_user_is_on_team_in_teamset(user, course_block, teamset_id):
Expand Down Expand Up @@ -327,9 +344,11 @@ def can_user_modify_team(user, team):
Assumes that user is enrolled in course run.
"""
# TODO: remove role checks once course_roles is fully impelented and data is migrated
return (
(not is_instructor_managed_team(team)) or
has_course_staff_privileges(user, team.course_id)
has_course_staff_privileges(user, team.course_id) or
user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name)
)


Expand All @@ -339,9 +358,11 @@ def can_user_create_team_in_topic(user, course_id, topic_id):
Assumes that user is enrolled in course run.
"""
# TODO: remove role checks once course_roles is fully impelented and data is migrated
return (
(not is_instructor_managed_topic(course_id, topic_id)) or
has_course_staff_privileges(user, course_id)
has_course_staff_privileges(user, course_id) or
user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name)
)


Expand Down Expand Up @@ -406,7 +427,11 @@ def anonymous_user_ids_for_team(user, team):
if not user or not team:
raise Exception("User and team must be provided for ID lookup")

if not has_course_staff_privileges(user, team.course_id) and not user_is_a_team_member(user, team):
# TODO: remove role checks once course_roles is fully impelented and data is migrated
if (
not has_course_staff_privileges(user, team.course_id) and not
user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name)
) and not user_is_a_team_member(user, team):
raise Exception("User {user} is not permitted to access team info for {team}".format(
user=user.username,
team=team.team_id
Expand Down
17 changes: 14 additions & 3 deletions lms/djangoapps/teams/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from lms.djangoapps.courseware.courses import get_course_with_access, has_access
from lms.djangoapps.discussion.django_comment_client.utils import has_discussion_privileges
from lms.djangoapps.teams.models import CourseTeam, CourseTeamMembership
from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser, BearerAuthentication
from openedx.core.lib.api.parsers import MergePatchParser
from openedx.core.lib.api.permissions import IsCourseStaffInstructor, IsStaffOrReadOnly
Expand Down Expand Up @@ -1070,7 +1071,9 @@ def _filter_hidden_private_teamsets(user, teamsets, course_block):
Return a filtered list of teamsets, removing any private teamsets that a user doesn't have access to.
Follows the same logic as `has_specific_teamset_access` but in bulk rather than for one teamset at a time
"""
if has_course_staff_privileges(user, course_block.id):
# TODO: remove role checks once course_roles is fully impelented and data is migrated
if (has_course_staff_privileges(user, course_block.id) or
user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name)):
return teamsets
private_teamset_ids = [teamset.teamset_id for teamset in course_block.teamsets if teamset.is_private_managed]
teamset_ids_user_has_access_to = set(
Expand Down Expand Up @@ -1384,7 +1387,11 @@ def get(self, request): # lint-amnesty, pylint: disable=too-many-statements
status=status.HTTP_404_NOT_FOUND
)
teamset_teams = CourseTeam.objects.filter(course_id=requested_course_key, topic_id=teamset_id)
if has_course_staff_privileges(request.user, requested_course_key):
# TODO: remove role checks once course_roles is fully impelented and data is migrated
if (
has_course_staff_privileges(request.user, requested_course_key) or
request.user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name)
):
teams_with_access = list(teamset_teams)
else:
teams_with_access = [
Expand Down Expand Up @@ -1689,7 +1696,11 @@ def check_access(self):
"""
Raises 403 if user does not have access to this endpoint.
"""
if not has_course_staff_privileges(self.request.user, self.course.id):
# TODO: remove role checks once course_roles is fully impelented and data is migrated
if not (
has_course_staff_privileges(self.request.user, self.course.id) or
self.request.user.has_perm(CourseRolesPermission.MANAGE_STUDENTS.perm_name)
):
raise PermissionDenied(
"To manage team membership of {}, you must be course staff.".format(
self.course.id
Expand Down

0 comments on commit b908907

Please sign in to comment.