diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py index c7fcd125aa..d119a8e21d 100644 --- a/common/djangoapps/enrollment/tests/test_views.py +++ b/common/djangoapps/enrollment/tests/test_views.py @@ -2,6 +2,7 @@ Tests for user enrollment. """ import json +import itertools import unittest import datetime @@ -91,11 +92,11 @@ class EnrollmentTestMixin(object): return response - def assert_enrollment_activation(self, expected_activation, expected_mode=CourseMode.VERIFIED): + def assert_enrollment_activation(self, expected_activation, expected_mode): """Change an enrollment's activation and verify its activation and mode are as expected.""" self.assert_enrollment_status( as_server=True, - mode=None, + mode=expected_mode, is_active=expected_activation, expected_status=status.HTTP_200_OK ) @@ -603,6 +604,58 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase): self.assertTrue(is_active) self.assertEqual(course_mode, CourseMode.HONOR) + @ddt.data(*itertools.product( + (CourseMode.HONOR, CourseMode.VERIFIED), + (CourseMode.HONOR, CourseMode.VERIFIED), + (True, False), + (True, False), + )) + @ddt.unpack + def test_change_mode_from_server(self, old_mode, new_mode, old_is_active, new_is_active): + """ + Server-to-server calls should be allowed to change the mode of any + enrollment, as long as the enrollment is not being deactivated during + the same call (this is assumed to be an error on the client's side). + """ + for mode in [CourseMode.HONOR, CourseMode.VERIFIED]: + CourseModeFactory.create( + course_id=self.course.id, + mode_slug=mode, + mode_display_name=mode, + ) + + # Set up the initial enrollment + self.assert_enrollment_status(as_server=True, mode=old_mode, is_active=old_is_active) + course_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) + self.assertEqual(is_active, old_is_active) + self.assertEqual(course_mode, old_mode) + + expected_status = status.HTTP_400_BAD_REQUEST if ( + old_mode != new_mode and + old_is_active != new_is_active and + not new_is_active + ) else status.HTTP_200_OK + + # simulate the server-server api call under test + response = self.assert_enrollment_status( + as_server=True, + mode=new_mode, + is_active=new_is_active, + expected_status=expected_status, + ) + + course_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) + if expected_status == status.HTTP_400_BAD_REQUEST: + # nothing should have changed + self.assertEqual(is_active, old_is_active) + self.assertEqual(course_mode, old_mode) + # error message should contain specific text. Otto checks for this text in the message. + self.assertRegexpMatches(json.loads(response.content)['message'], 'Enrollment mode mismatch') + else: + # call should have succeeded + self.assertEqual(is_active, new_is_active) + self.assertEqual(course_mode, new_mode) + def test_change_mode_invalid_user(self): """ Attempts to change an enrollment for a non-existent user should result in an HTTP 404 for non-server users, diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index a5c610c5d5..fca223264f 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -3,6 +3,8 @@ The Enrollment API Views should be simple, lean HTTP endpoints for API access. T consist primarily of authentication, request validation, and serialization. """ +import logging + from ipware.ip import get_ip from django.core.exceptions import ObjectDoesNotExist from django.utils.decorators import method_decorator @@ -31,6 +33,9 @@ from enrollment.errors import ( from student.models import User +log = logging.getLogger(__name__) + + class EnrollmentCrossDomainSessionAuth(SessionAuthenticationAllowInactiveUser, SessionAuthenticationCrossDomainCsrf): """Session authentication that allows inactive users and cross-domain requests. """ pass @@ -421,7 +426,18 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): ) enrollment = api.get_enrollment(username, unicode(course_id)) - if has_api_key_permissions and enrollment and enrollment['mode'] != mode: + mode_changed = enrollment and mode is not None and enrollment['mode'] != mode + active_changed = enrollment and is_active is not None and enrollment['is_active'] != is_active + if has_api_key_permissions and (mode_changed or active_changed): + if mode_changed and active_changed and not is_active: + # if the requester wanted to deactivate but specified the wrong mode, fail + # the request (on the assumption that the requester had outdated information + # about the currently active enrollment). + msg = u"Enrollment mode mismatch: active mode={}, requested mode={}. Won't deactivate.".format( + enrollment["mode"], mode + ) + log.warning(msg) + return Response(status=status.HTTP_400_BAD_REQUEST, data={"message": msg}) response = api.update_enrollment(username, unicode(course_id), mode=mode, is_active=is_active) else: # Will reactivate inactive enrollments.