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

EDUCATOR-5001: Don't return private teamsets unless user has access to them (#23612)

Browse Source 
* hide 'hidden' teamsets from topic listing endpoint
This commit is contained in:
Jansen Kantor
2020-04-09 13:30:05 -04:00
committed by GitHub
parent 534547c24f
commit 933cb490f2
2 changed files with 55 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1491,19 +1491,19 @@ class TestListTopicsAPI(TeamAPITestCase):
"""Test cases for the topic listing endpoint."""
@ddt.data(
(None, 401),
('student_inactive', 401),
('student_unenrolled', 403),
('student_enrolled', 200),
('staff', 200),
('course_staff', 200),
('community_ta', 200),
(None, 401, None),
('student_inactive', 401, None),
('student_unenrolled', 403, None),
('student_enrolled', 200, 4),
('staff', 200, 6),
('course_staff', 200, 6),
('community_ta', 200, 4),
)
@ddt.unpack
def test_access(self, user, status):
def test_access(self, user, status, expected_topics_count):
topics = self.get_topics_list(status, {'course_id': str(self.test_course_1.id)}, user=user)
if status == 200:
self.assertEqual(topics['count'], self.topics_count)
self.assertEqual(topics['count'], expected_topics_count)
@ddt.data('A+BOGUS+COURSE', 'A/BOGUS/COURSE')
def test_invalid_course_key(self, course_id):
@@ -1513,14 +1513,11 @@ class TestListTopicsAPI(TeamAPITestCase):
self.get_topics_list(400)
@ddt.data(
(None, 200, ['Coal Power', 'Nuclear Power', 'private_topic_1_name', 'private_topic_2_name',
u'Sólar power', 'Wind Power'], 'name'),
('name', 200, ['Coal Power', 'Nuclear Power', 'private_topic_1_name', 'private_topic_2_name',
u'Sólar power', 'Wind Power'], 'name'),
(None, 200, ['Coal Power', 'Nuclear Power', u'Sólar power', 'Wind Power'], 'name'),
('name', 200, ['Coal Power', 'Nuclear Power', u'Sólar power', 'Wind Power'], 'name'),
# Note that "Nuclear Power" will have 2 teams. "Coal Power" "Wind Power" and "Solar Power"
# all have 1 team. The secondary sort is alphabetical by name.
('team_count', 200, ['Nuclear Power', 'Coal Power', u'Sólar power', 'Wind Power',
'private_topic_1_name', 'private_topic_2_name'], 'team_count'),
('team_count', 200, ['Nuclear Power', 'Coal Power', u'Sólar power', 'Wind Power'], 'team_count'),
('no_such_field', 400, [], None),
)
@ddt.unpack
@@ -1620,10 +1617,10 @@ class TestListTopicsAPI(TeamAPITestCase):
@ddt.unpack
@ddt.data(
('student_enrolled', 2),
('student_on_team_1_private_set_1', 2),
('student_on_team_2_private_set_1', 2),
('student_masters', 2),
('student_enrolled', 0),
('student_on_team_1_private_set_1', 1),
('student_on_team_2_private_set_1', 1),
('student_masters', 0),
('staff', 2)
)
def test_teamset_type(self, requesting_user, expected_private_teamsets):
@@ -1632,9 +1629,6 @@ class TestListTopicsAPI(TeamAPITestCase):
Staff should be able to see both teamsets, and anyone enrolled in a private teamset should see that and
only that teamset
TODO: student_enrolled and student_masters should see no private teamsets.
sot1ps1 and sot2ps3 should only see their teamset
"""
topics = self.get_topics_list(
data={'course_id': str(self.test_course_1.id)},
@@ -1645,6 +1639,23 @@ class TestListTopicsAPI(TeamAPITestCase):
]
self.assertEqual(len(private_teamsets_returned), expected_private_teamsets)
@ddt.unpack
@ddt.data(
('student_on_team_1_private_set_1', 2),
('student_on_team_2_private_set_1', 2),
('staff', 2)
)
def test_private_teamset_team_count(self, requesting_user, expected_team_count):
"""
TODO: the two students should probably not see that there's another team that they don't see
"""
topics = self.get_topics_list(
data={'course_id': str(self.test_course_1.id)},
user=requesting_user
)
private_teamset_1 = [topic for topic in topics['results'] if topic['name'] == 'private_topic_1_name'][0]
self.assertEqual(private_teamset_1['team_count'], expected_team_count)
@ddt.ddt
class TestDetailTopicAPI(TeamAPITestCase):

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

@@ -930,6 +930,8 @@ class TopicListView(GenericAPIView):
# in the case of "team_count".
organization_protection_status = user_organization_protection_status(request.user, course_id)
topics = get_alphabetical_topics(course_module)
topics = self._filter_hidden_private_teamsets(topics, course_module)
if ordering == 'team_count':
add_team_count(topics, course_id, organization_protection_status)
topics.sort(key=lambda t: t['team_count'], reverse=True)
@@ -956,6 +958,26 @@ class TopicListView(GenericAPIView):
return response
def _filter_hidden_private_teamsets(self, teamsets, course_module):
"""
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(self.request.user, course_module.id):
return teamsets
private_teamset_ids = [teamset.teamset_id for teamset in course_module.teamsets if teamset.is_private_managed]
teamset_ids_user_has_access_to = set(
CourseTeam.objects.filter(
course_id=course_module.id,
topic_id__in=private_teamset_ids,
membership__user=self.request.user
).values_list('topic_id', flat=True)
)
return [
teamset for teamset in teamsets
if teamset['type'] != TeamsetType.private_managed.value or teamset['id'] in teamset_ids_user_has_access_to
]
def get_alphabetical_topics(course_module):
"""Return a list of team topics sorted alphabetically.