Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

create team view teamset privacy fixes (#23607)

Browse Source
This commit is contained in:
Jansen Kantor
2020-04-07 22:04:23 -04:00
committed by GitHub
parent 1b2e10d96e
commit 90885e4193
3 changed files with 98 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
lms/djangoapps/teams/api.py
View File

@@ -193,6 +193,33 @@ def has_specific_team_access(user, team):
)
def has_specific_teamset_access(user, course_module, teamset_id):
"""
Staff have access to all teamsets.
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_module.id) or \
teamset_is_public_or_user_is_on_team_in_teamset(user, course_module, teamset_id)
def teamset_is_public_or_user_is_on_team_in_teamset(user, course_module, teamset_id):
"""
The only users who should be able to see private_managed teamsets
or recieve any information about them at all from the API are:
- Course staff
- Users who are enrolled in a team in a private_managed teamset
course_module is passed in because almost universally where we'll be calling this, we will already
need to have looked up the course from modulestore to make sure that the topic we're interested in
exists in the course.
"""
teamset = course_module.teams_configuration.teamsets_by_id[teamset_id]
if teamset.teamset_type != TeamsetType.private_managed:
return True
return CourseTeamMembership.user_in_team_for_course(user, course_module.id, topic_id=teamset_id)
def user_on_team_or_team_is_public(user, team):
"""
The only users who should be able to see private_managed teams

46
lms/djangoapps/teams/tests/test_views.py
View File

@@ -594,7 +594,14 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
team_list = self.get_teams_list(user=user, expected_status=200, data=course_two_data)
self.assertEqual(team_list['count'], 0)
def build_team_data(self, name="Test team", course=None, description="Filler description", **kwargs):
def build_team_data(
self,
name="Test team",
course=None,
description="Filler description",
topic_id="topic_0",
**kwargs
):
"""Creates the payload for creating a team. kwargs can be used to specify additional fields."""
data = kwargs
course = course if course else self.test_course_1
@@ -602,6 +609,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
'name': name,
'course_id': str(course.id),
'description': description,
'topic_id': topic_id,
})
return data
@@ -1008,7 +1016,19 @@ class TestCreateTeamAPI(EventTestMixin, TeamAPITestCase):
def test_bad_course_data(self, status, data):
self.post_create_team(status, data)
def test_student_in_team(self):
def test_bad_topic_id(self):
self.post_create_team(
404,
data=self.build_team_data(topic_id='asdfasdfasdfa'),
user='staff'
)
def test_missing_topic_id(self):
data = self.build_team_data()
data.pop('topic_id')
self.post_create_team(400, data=data, user='staff')
def test_student_in_teamset(self):
response = self.post_create_team(
400,
data=self.build_team_data(
@@ -1019,11 +1039,12 @@ class TestCreateTeamAPI(EventTestMixin, TeamAPITestCase):
user='student_enrolled'
)
self.assertEqual(
"You are already in a team in this course.",
"You are already in a team in this teamset.",
json.loads(response.content.decode('utf-8'))["user_message"]
)
@patch('lms.djangoapps.teams.views.can_user_create_team_in_topic', return_value=False)
@patch('lms.djangoapps.teams.views.has_specific_teamset_access', return_value=True)
def test_student_create_team_instructor_managed_topic(self, *args): # pylint: disable=unused-argument
response = self.post_create_team(
403,
@@ -1031,7 +1052,7 @@ class TestCreateTeamAPI(EventTestMixin, TeamAPITestCase):
name="student create team in instructor managed topic",
course=self.test_course_1,
description="student cannot create team in instructor-managed topic",
topic_id='great-topic'
topic_id='private_topic_1_id'
),
user='student_enrolled_not_on_team'
)
@@ -1054,7 +1075,8 @@ class TestCreateTeamAPI(EventTestMixin, TeamAPITestCase):
data=self.build_team_data(
name="Another team",
course=self.test_course_1,
description="Privileged users are the best"
description="Privileged users are the best",
topic_id=self.solar_team.topic_id
),
user=user
)
@@ -1139,12 +1161,12 @@ class TestCreateTeamAPI(EventTestMixin, TeamAPITestCase):
@ddt.unpack
@ddt.data(
('student_enrolled', 400, 'You are already in a team in this course.'),
('student_enrolled', 404, None),
('student_unenrolled', 403, None),
('student_enrolled_not_on_team', 403, "You can't create a team in an instructor managed topic."),
('student_masters', 400, 'You are already in a team in this course.'),
('student_on_team_1_private_set_1', 400, 'You are already in a team in this course.'),
('student_on_team_2_private_set_1', 400, 'You are already in a team in this course.'),
('student_enrolled_not_on_team', 404, None),
('student_masters', 404, None),
('student_on_team_1_private_set_1', 403, "You can't create a team in an instructor managed topic."),
('student_on_team_2_private_set_1', 403, "You can't create a team in an instructor managed topic."),
('staff', 200, None)
)
def test_private_managed_access(self, user, expected_response, msg):
@@ -1153,10 +1175,6 @@ class TestCreateTeamAPI(EventTestMixin, TeamAPITestCase):
Only staff should be able to create teams in managed teamsets, but they're also
the only ones who should know that private_managed teamsets exist. If the team hasn't been created yet,
no one can be in it, so no non-staff should get any info at all from this endpoint.
TODO: This endpoint should check for teamset enrollment rather than course enrollment.
I'm not sure of the severity of the 'user_message' info, but it seems like we might not want to
return that for student_enrolled_not_on_team (they should just get 403, like student_unenrolled)
"""
response = self.post_create_team(
expected_response,

54
lms/djangoapps/teams/views.py
View File

@@ -53,6 +53,7 @@ from .api import (
can_user_create_team_in_topic,
has_course_staff_privileges,
has_specific_team_access,
has_specific_teamset_access,
has_team_api_access,
user_organization_protection_status
)
@@ -363,7 +364,7 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
**Response Values for POST**
Any logged in user who has verified their email address can create
a team. The format mirrors that of a GET for an individual team,
a team in an open teamset. The format mirrors that of a GET for an individual team,
but does not include the id, date_created, or membership fields.
id is automatically computed based on name.
@@ -373,10 +374,13 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
global staff, or does not have discussion privileges a 403 error
is returned.
If the course_id is not valid or extra fields are included in the
request, a 400 error is returned.
If the course_id is not valid, or the topic_id is missing, or extra fields
are included in the request, a 400 error is returned.
If the specified course does not exist, a 404 error is returned.
If the specified teamset does not exist, a 404 error is returned.
If the specified teamset does exist, but the requesting user shouldn't be
able to see it, a 404 is returned.
"""
# BearerAuthentication must come first to return a 401 for unauthenticated users
@@ -547,12 +551,13 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
"""POST /api/team/v0/teams/"""
field_errors = {}
course_key = None
course_id = request.data.get('course_id')
#Handle field errors and check that the course exists
try:
course_key = CourseKey.from_string(course_id)
# Ensure the course exists
if not modulestore().has_course(course_key):
course_module = modulestore().get_course(course_key)
if not course_module:
return Response(status=status.HTTP_404_NOT_FOUND)
except InvalidKeyError:
field_errors['course_id'] = build_api_error(
@@ -563,27 +568,46 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
'field_errors': field_errors,
}, status=status.HTTP_400_BAD_REQUEST)
# Course and global staff, as well as discussion "privileged" users, will not automatically
# be added to a team when they create it. They are allowed to create multiple teams.
team_administrator = (has_access(request.user, 'staff', course_key)
or has_discussion_privileges(request.user, course_key))
if not team_administrator and CourseTeamMembership.user_in_team_for_course(request.user, course_key):
error_message = build_api_error(
ugettext_noop('You are already in a team in this course.'),
topic_id = request.data.get('topic_id')
if not topic_id:
field_errors['topic_id'] = build_api_error(
ugettext_noop(u'topic_id is required'),
course_id=course_id
)
return Response(error_message, status=status.HTTP_400_BAD_REQUEST)
return Response({
'field_errors': field_errors,
}, status=status.HTTP_400_BAD_REQUEST)
if course_key and not has_team_api_access(request.user, course_key):
return Response(status=status.HTTP_403_FORBIDDEN)
topic_id = request.data.get('topic_id')
if topic_id not in course_module.teams_configuration.teamsets_by_id or (
not has_specific_teamset_access(request.user, course_module, topic_id)
):
return Response(status=status.HTTP_404_NOT_FOUND)
# The user has to have access to this teamset at this point, so we can return 403
# and not leak the existance of a private teamset
if not can_user_create_team_in_topic(request.user, course_key, topic_id):
return Response(
build_api_error(ugettext_noop("You can't create a team in an instructor managed topic.")),
status=status.HTTP_403_FORBIDDEN
)
# Course and global staff, as well as discussion "privileged" users, will not automatically
# be added to a team when they create it. They are allowed to create multiple teams.
is_team_administrator = (has_access(request.user, 'staff', course_key)
or has_discussion_privileges(request.user, course_key))
if not is_team_administrator and (
CourseTeamMembership.user_in_team_for_course(request.user, course_key, topic_id=topic_id)
):
error_message = build_api_error(
ugettext_noop('You are already in a team in this teamset.'),
course_id=course_id,
teamset_id=topic_id,
)
return Response(error_message, status=status.HTTP_400_BAD_REQUEST)
data = request.data.copy()
data['course_id'] = six.text_type(course_key)
@@ -603,7 +627,7 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
emit_team_event('edx.team.created', course_key, {
'team_id': team.team_id
})
if not team_administrator:
if not is_team_administrator:
# Add the creating user to the team.
team.add_user(request.user)
emit_team_event(