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

Merge pull request #18405 from edx/jeskew/PLAT_2185_idempotent_endpoints

Browse Source 
Make retirement endpoints idempotent.
This commit is contained in:
J Eskew
2018-06-21 16:11:06 -04:00
committed by GitHub
parent fe82954835 a09a81b138
commit 64ce2a0d8a
4 changed files with 66 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
common/djangoapps/enrollment/tests/test_views.py
View File

@@ -31,12 +31,20 @@ from enrollment.views import EnrollmentUserThrottle
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.embargo.models import Country, CountryAccessRule, RestrictedCourse
from openedx.core.djangoapps.embargo.test_utils import restrict_course
from openedx.core.djangoapps.user_api.models import UserOrgTag
from openedx.core.djangoapps.user_api.models import (
RetirementState,
UserRetirementStatus,
UserOrgTag
)
from openedx.core.lib.django_test_client_utils import get_absolute_url
from openedx.core.lib.token_utils import JwtBuilder
from openedx.features.enterprise_support.tests import FAKE_ENTERPRISE_CUSTOMER
from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseServiceMockMixin
from student.models import CourseEnrollment
from student.models import (
CourseEnrollment,
get_retired_username_by_username,
get_retired_email_by_email,
)
from student.roles import CourseStaffRole
from student.tests.factories import AdminFactory, UserFactory, SuperuserFactory
from util.models import RateLimitConfiguration
@@ -1288,6 +1296,20 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase):
for course in self.courses:
self.assert_enrollment_status(course_id=str(course.id), username=self.USERNAME, is_active=True)
def _create_test_retirement(self, user=None):
"""
Helper method to create a RetirementStatus with useful defaults
"""
pending_state = RetirementState.objects.create(
state_name='PENDING',
state_execution_order=1,
is_dead_end_state=False,
required=False
)
if user is None:
user = UserFactory()
return UserRetirementStatus.create_retirement(user)
def build_jwt_headers(self, user):
"""
Helper function for creating headers for the JWT authentication.
@@ -1299,6 +1321,7 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase):
def test_deactivate_enrollments(self):
self._assert_active()
self._create_test_retirement(self.user)
response = self._submit_unenroll(self.superuser, self.user.username)
self.assertEqual(response.status_code, status.HTTP_200_OK)
data = json.loads(response.content)
@@ -1306,6 +1329,11 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase):
self.assertEqual(set(data), self.orgs)
self._assert_inactive()
def test_deactivate_enrollments_no_retirement_status(self):
self._assert_active()
response = self._submit_unenroll(self.superuser, self.user.username)
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
def test_deactivate_enrollments_unauthorized(self):
self._assert_active()
response = self._submit_unenroll(self.user, self.user.username)
@@ -1322,26 +1350,25 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase):
def test_deactivate_enrollments_empty_username(self):
self._assert_active()
self._create_test_retirement(self.user)
response = self._submit_unenroll(self.superuser, "")
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
data = json.loads(response.content)
self.assertEqual(data, u'The user "" does not exist.')
self._assert_active()
def test_deactivate_enrollments_invalid_username(self):
self._assert_active()
self._create_test_retirement(self.user)
response = self._submit_unenroll(self.superuser, "a made up username")
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
data = json.loads(response.content)
self.assertEqual(data, u'The user "a made up username" does not exist.')
self._assert_active()
def test_deactivate_enrollments_called_twice(self):
self._assert_active()
self._create_test_retirement(self.user)
response = self._submit_unenroll(self.superuser, self.user.username)
self.assertEqual(response.status_code, status.HTTP_200_OK)
response = self._submit_unenroll(self.superuser, self.user.username)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
self.assertEqual(response.content, "")
self._assert_inactive()

36
common/djangoapps/enrollment/views.py
View File

@@ -19,6 +19,7 @@ from openedx.core.djangoapps.cors_csrf.authentication import SessionAuthenticati
from openedx.core.djangoapps.cors_csrf.decorators import ensure_csrf_cookie_cross_domain
from openedx.core.djangoapps.embargo import api as embargo_api
from openedx.core.djangoapps.user_api.accounts.permissions import CanRetireUser
from openedx.core.djangoapps.user_api.models import UserRetirementStatus
from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in
from openedx.core.lib.api.authentication import (
OAuth2AuthenticationAllowInactiveUser,
@@ -316,21 +317,23 @@ class UnenrollmentView(APIView):
"username": "username12345"
}
**POST Parameters**
**POST Parameters**
A POST request must include the following parameter.
A POST request must include the following parameter.
* username: The username of the user being unenrolled.
This will never match the username from the request,
since the request is issued as a privileged service user.
* username: The username of the user being unenrolled.
This will never match the username from the request,
since the request is issued as a privileged service user.
**POST Response Values**
If the user does not exist, or the user is already unenrolled
from all courses, the request returns an HTTP 404 "Does Not Exist"
response.
If the user has not requested retirement and does not have a retirement
request status, the request returns an HTTP 404 "Does Not Exist" response.
If an unexpected error occurs, the request returns an HTTP 500 response.
If the user is already unenrolled from all courses, the request returns
an HTTP 204 "No Content" response.
If an unexpected error occurs, the request returns an HTTP 500 response.
If the request is successful, an HTTP 200 "OK" response is
returned along with a list of all courses from which the user was unenrolled.
@@ -342,23 +345,20 @@ class UnenrollmentView(APIView):
"""
Unenrolls the specified user from all courses.
"""
username = None
user_model = get_user_model()
try:
# Get the username from the request and check that it exists
# Get the username from the request.
username = request.data['username']
user_model.objects.get(username=username)
# Ensure that a retirement request status row exists for this username.
UserRetirementStatus.get_retirement_for_retirement_action(username)
enrollments = api.get_enrollments(username)
active_enrollments = [enrollment for enrollment in enrollments if enrollment['is_active']]
if len(active_enrollments) < 1:
return Response(status=status.HTTP_200_OK)
return Response(status=status.HTTP_204_NO_CONTENT)
return Response(api.unenroll_user_from_all_courses(username))
except KeyError:
return Response(u'Username not specified.', status=status.HTTP_404_NOT_FOUND)
except user_model.DoesNotExist:
return Response(u'The user "{}" does not exist.'.format(username), status=status.HTTP_404_NOT_FOUND)
except UserRetirementStatus.DoesNotExist:
return Response(u'No retirement request status for username.', status=status.HTTP_404_NOT_FOUND)
except Exception as exc: # pylint: disable=broad-except
return Response(text_type(exc), status=status.HTTP_500_INTERNAL_SERVER_ERROR)

24
openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py
View File

@@ -1177,14 +1177,6 @@ class TestAccountRetirementPost(RetirementTestCase):
}
self.assertEqual(expected_user_profile_pii, USER_PROFILE_PII)
def test_retire_user_where_user_does_not_exist(self):
path = 'openedx.core.djangoapps.user_api.accounts.views.is_username_retired'
with mock.patch(path, return_value=False) as mock_retired_username:
data = {'username': 'not_a_user'}
response = self.post_and_assert_status(data, status.HTTP_404_NOT_FOUND)
self.assertFalse(response.content)
mock_retired_username.assert_called_once_with('not_a_user')
def test_retire_user_server_error_is_raised(self):
path = 'openedx.core.djangoapps.user_api.models.UserRetirementStatus.get_retirement_for_retirement_action'
with mock.patch(path, side_effect=Exception('Unexpected Exception')) as mock_get_retirement:
@@ -1197,9 +1189,9 @@ class TestAccountRetirementPost(RetirementTestCase):
path = 'openedx.core.djangoapps.user_api.accounts.views.is_username_retired'
with mock.patch(path, return_value=True) as mock_is_username_retired:
data = {'username': self.test_user.username}
response = self.post_and_assert_status(data, status.HTTP_404_NOT_FOUND)
response = self.post_and_assert_status(data, status.HTTP_204_NO_CONTENT)
self.assertFalse(response.content)
mock_is_username_retired.assert_called_once_with(self.original_username)
mock_is_username_retired.assert_not_called()
def test_retire_user_where_username_not_provided(self):
response = self.post_and_assert_status({}, status.HTTP_404_NOT_FOUND)
@@ -1251,6 +1243,11 @@ class TestAccountRetirementPost(RetirementTestCase):
self.assertFalse(CourseEnrollmentAllowed.objects.filter(email=self.original_email).exists())
self.assertFalse(UnregisteredLearnerCohortAssignments.objects.filter(email=self.original_email).exists())
def test_retire_user_twice_idempotent(self):
data = {'username': self.original_username}
self.post_and_assert_status(data)
self.post_and_assert_status(data)
def test_deletes_pii_from_user_profile(self):
for model_field, value_to_assign in USER_PROFILE_PII.iteritems():
if value_to_assign == '':
@@ -1476,3 +1473,10 @@ class TestLMSAccountRetirementPost(RetirementTestCase, ModuleStoreTestCase):
self.assertEqual(retired_api_access_request.company_name, '')
self.assertEqual(retired_api_access_request.reason, '')
self.assertEqual(SurveyAnswer.objects.get(user=self.test_user).field_value, '')
def test_retire_user_twice_idempotent(self):
# check that a second call to the retire_misc endpoint will work
UserRetirementStatus.get_retirement_for_retirement_action(self.test_user.username)
data = {'username': self.original_username}
self.post_and_assert_status(data)
self.post_and_assert_status(data)

7
openedx/core/djangoapps/user_api/accounts/views.py
View File

@@ -721,8 +721,6 @@ class LMSAccountRetirementView(ViewSet):
"""
username = request.data['username']
if is_username_retired(username):
return Response(status=status.HTTP_404_NOT_FOUND)
try:
retirement = UserRetirementStatus.get_retirement_for_retirement_action(username)
@@ -770,8 +768,6 @@ class AccountRetirementView(ViewSet):
any other PII associated with this user.
"""
username = request.data['username']
if is_username_retired(username):
return Response(status=status.HTTP_404_NOT_FOUND)
try:
retirement_status = UserRetirementStatus.get_retirement_for_retirement_action(username)
@@ -801,9 +797,6 @@ class AccountRetirementView(ViewSet):
CourseEnrollmentAllowed.delete_by_user_value(original_email, field='email')
UnregisteredLearnerCohortAssignments.delete_by_user_value(original_email, field='email')
# TODO: Password Reset links - https://openedx.atlassian.net/browse/PLAT-2104
# TODO: Delete OAuth2 records - https://openedx.atlassian.net/browse/EDUCATOR-2703
user.first_name = ''
user.last_name = ''
user.is_active = False