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,