From 4d992565ac8428315f4355b5cca78a9225bd6bf4 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 17 Mar 2017 11:38:13 +1030 Subject: [PATCH] Allows course staff to access any enrolled learner's grades via the Grades API Also adds JwtAuthentication to the list of allowed authentication methods, so the Grades REST API can be easily accessed from Enterprise management commands. --- lms/djangoapps/grades/api/tests/test_views.py | 51 +++++++++- lms/djangoapps/grades/api/views.py | 96 +++++++++++++------ 2 files changed, 113 insertions(+), 34 deletions(-) diff --git a/lms/djangoapps/grades/api/tests/test_views.py b/lms/djangoapps/grades/api/tests/test_views.py index 5d17441f2f..ffa4a4238f 100644 --- a/lms/djangoapps/grades/api/tests/test_views.py +++ b/lms/djangoapps/grades/api/tests/test_views.py @@ -9,6 +9,7 @@ from opaque_keys import InvalidKeyError from pytz import UTC from rest_framework import status from rest_framework.test import APITestCase +from urllib import urlencode from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from edx_oauth2_provider.tests.factories import AccessTokenFactory, ClientFactory @@ -104,6 +105,8 @@ class CurrentGradeViewTest(SharedModuleStoreTestCase, APITestCase): cls.student = UserFactory(username='dummy', password=cls.password) cls.other_student = UserFactory(username='foo', password=cls.password) cls.other_user = UserFactory(username='bar', password=cls.password) + cls.staff = StaffFactory(course_key=cls.course.id, password=cls.password) + cls.global_staff = GlobalStaffFactory.create() date = datetime(2013, 1, 22, tzinfo=UTC) for user in (cls.student, cls.other_student, ): CourseEnrollmentFactory( @@ -128,7 +131,10 @@ class CurrentGradeViewTest(SharedModuleStoreTestCase, APITestCase): 'course_id': self.course_key, } ) - return "{0}?username={1}".format(base_url, username) + query_string = '' + if username: + query_string = '?' + urlencode(dict(username=username)) + return base_url + query_string def test_anonymous(self): """ @@ -151,12 +157,17 @@ class CurrentGradeViewTest(SharedModuleStoreTestCase, APITestCase): resp = self.client.get(self.get_url(self.student.username)) self.assertEqual(resp.status_code, status.HTTP_200_OK) + # and again, with the username defaulting to the current user + with check_mongo_calls(3): + resp = self.client.get(self.get_url(None)) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + def test_nonexistent_user(self): """ Test that a request for a nonexistent username returns an error. """ resp = self.client.get(self.get_url('IDoNotExist')) - self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) self.assertIn('error_code', resp.data) # pylint: disable=no-member self.assertEqual(resp.data['error_code'], 'user_mismatch') # pylint: disable=no-member @@ -167,7 +178,7 @@ class CurrentGradeViewTest(SharedModuleStoreTestCase, APITestCase): self.client.logout() self.client.login(username=self.other_student.username, password=self.password) resp = self.client.get(self.get_url(self.student.username)) - self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) self.assertIn('error_code', resp.data) # pylint: disable=no-member self.assertEqual(resp.data['error_code'], 'user_mismatch') # pylint: disable=no-member @@ -224,6 +235,40 @@ class CurrentGradeViewTest(SharedModuleStoreTestCase, APITestCase): 'user_or_course_does_not_exist' ) + @ddt.data( + 'staff', 'global_staff' + ) + def test_staff_can_see_student(self, staff_user): + """ + Ensure that staff members can see her student's grades. + """ + self.client.logout() + self.client.login(username=getattr(self, staff_user).username, password=self.password) + resp = self.client.get(self.get_url(self.student.username)) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + expected_data = [{ + 'username': self.student.username, + 'letter_grade': None, + 'percent': 0.0, + 'course_key': str(self.course_key), + 'passed': False + }] + self.assertEqual(resp.data, expected_data) # pylint: disable=no-member + + @ddt.data( + 'staff', 'global_staff' + ) + def test_staff_requests_nonexistent_user(self, staff_user): + """ + Test that a staff request for a nonexistent username returns an error. + """ + self.client.logout() + self.client.login(username=getattr(self, staff_user).username, password=self.password) + resp = self.client.get(self.get_url('IDoNotExist')) + self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) + self.assertIn('error_code', resp.data) # pylint: disable=no-member + self.assertEqual(resp.data['error_code'], 'user_does_not_exist') # pylint: disable=no-member + def test_no_grade(self): """ Test the grade for a user who has not answered any test. diff --git a/lms/djangoapps/grades/api/views.py b/lms/djangoapps/grades/api/views.py index 738d2fa83a..5590f051c5 100644 --- a/lms/djangoapps/grades/api/views.py +++ b/lms/djangoapps/grades/api/views.py @@ -1,36 +1,33 @@ """ API v0 views. """ import logging +from django.contrib.auth import get_user_model from django.http import Http404 from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from rest_framework import status -from rest_framework.authentication import SessionAuthentication from rest_framework.exceptions import AuthenticationFailed from rest_framework.generics import GenericAPIView, ListAPIView -from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response +from courseware.access import has_access from lms.djangoapps.ccx.utils import prep_course_for_grading from lms.djangoapps.courseware import courses from lms.djangoapps.grades.api.serializers import GradingPolicySerializer from lms.djangoapps.grades.new.course_grade import CourseGradeFactory from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser -from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin +from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes +from student.roles import CourseStaffRole log = logging.getLogger(__name__) +USER_MODEL = get_user_model() +@view_auth_classes() class GradeViewMixin(DeveloperErrorViewMixin): """ Mixin class for Grades related views. """ - authentication_classes = ( - OAuth2AuthenticationAllowInactiveUser, - SessionAuthentication, - ) - permission_classes = (IsAuthenticated,) - def _get_course(self, course_key_string, user, access_action): """ Returns the course for the given course_key_string after @@ -60,6 +57,48 @@ class GradeViewMixin(DeveloperErrorViewMixin): error_code='user_or_course_does_not_exist' ) + def _get_effective_user(self, request, course): + """ + Returns the user object corresponding to the request's 'username' parameter, + or the current request.user if no 'username' was provided. + + Verifies that the request.user has access to the requested users's grades. + Returns a 403 error response if access is denied, or a 404 error response if the user does not exist. + """ + + # Use the request user's if none provided. + if 'username' in request.GET: + username = request.GET.get('username') + else: + username = request.user.username + + if request.user.username == username: + # Any user may request her own grades + return request.user + + # Only a user with staff access may request grades for a user other than herself. + if not has_access(request.user, CourseStaffRole.ROLE, course): + log.info( + 'User %s tried to access the grade for user %s.', + request.user.username, + username + ) + return self.make_error_response( + status_code=status.HTTP_403_FORBIDDEN, + developer_message='The user requested does not match the logged in user.', + error_code='user_mismatch' + ) + + try: + return USER_MODEL.objects.get(username=username) + + except USER_MODEL.DoesNotExist: + return self.make_error_response( + status_code=status.HTTP_404_NOT_FOUND, + developer_message='The user matching the requested username does not exist.', + error_code='user_does_not_exist' + ) + def perform_authentication(self, request): """ Ensures that the user is authenticated (e.g. not an AnonymousUser), unless DEBUG mode is enabled. @@ -73,8 +112,10 @@ class UserGradeView(GradeViewMixin, GenericAPIView): """ **Use Case** - * Get the current course grades for users in a course. - Currently, getting the grade for only an individual user is supported. + * Get the current course grades for a user in a course. + + The currently logged-in user may request her own grades, or a user with staff access to the course may request + any enrolled user's grades. **Example Request** @@ -82,10 +123,11 @@ class UserGradeView(GradeViewMixin, GenericAPIView): **GET Parameters** - A GET request must include the following parameters. + A GET request may include the following parameters. - * course_id: A string representation of a Course ID. - * username: A string representation of a user's username. + * course_id: (required) A string representation of a Course ID. + * username: (optional) A string representation of a user's username. + Defaults to the currently logged-in user's username. **GET Response Values** @@ -128,30 +170,22 @@ class UserGradeView(GradeViewMixin, GenericAPIView): Return: A JSON serialized representation of the requesting user's current grade status. """ - username = request.GET.get('username') - - # only the student can access her own grade status info - if request.user.username != username: - log.info( - 'User %s tried to access the grade for user %s.', - request.user.username, - username - ) - return self.make_error_response( - status_code=status.HTTP_404_NOT_FOUND, - developer_message='The user requested does not match the logged in user.', - error_code='user_mismatch' - ) course = self._get_course(course_id, request.user, 'load') if isinstance(course, Response): + # Returns a 404 if course_id is invalid, or request.user is not enrolled in the course return course - prep_course_for_grading(course, request) - course_grade = CourseGradeFactory().create(request.user, course) + grade_user = self._get_effective_user(request, course) + if isinstance(grade_user, Response): + # Returns a 403 if the request.user can't access grades for the requested user, + # or a 404 if the requested user does not exist. + return grade_user + prep_course_for_grading(course, request) + course_grade = CourseGradeFactory().create(grade_user, course) return Response([{ - 'username': username, + 'username': grade_user.username, 'course_key': course_id, 'passed': course_grade.passed, 'percent': course_grade.percent,