diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 9c94c6306fd962001c7bb6a5730c4800bb81595c..77bcd5abd1fd170a4131b023a813624ad62fd550 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -145,7 +145,7 @@ def anonymous_id_for_user(user, course_id, save=True): # This part is for ability to get xblock instance in xblock_noauth handlers, where user is unauthenticated. assert user - if user.is_anonymous(): + if user.is_anonymous: return None cached_id = getattr(user, '_anonymous_id', {}).get(course_id) @@ -245,6 +245,17 @@ def get_all_retired_emails_by_email(email): return user_util.get_all_retired_emails(email, settings.RETIRED_USER_SALTS, settings.RETIRED_EMAIL_FMT) +def get_potentially_retired_user_by_username(username): + """ + Attempt to return a User object based on the username, or if it + does not exist, then any hashed username salted with the historical + salts. + """ + locally_hashed_usernames = list(get_all_retired_usernames_by_username(username)) + locally_hashed_usernames.append(username) + return User.objects.get(username__in=locally_hashed_usernames) + + def get_potentially_retired_user_by_username_and_hash(username, hashed_username): """ To assist in the retirement process this method will: @@ -259,13 +270,8 @@ def get_potentially_retired_user_by_username_and_hash(username, hashed_username) if hashed_username not in locally_hashed_usernames: raise Exception('Mismatched hashed_username, bad salt?') - try: - return User.objects.get(username=username) - except User.DoesNotExist: - # The 2nd DoesNotExist will bubble up from here if necessary, - # an assumption is being made here that our hashed username format - # is something that a user cannot create for themselves. - return User.objects.get(username__in=locally_hashed_usernames) + locally_hashed_usernames.append(username) + return User.objects.get(username__in=locally_hashed_usernames) class UserStanding(models.Model): @@ -1220,7 +1226,7 @@ class CourseEnrollment(models.Model): """ assert user - if user.is_anonymous(): + if user.is_anonymous: return None try: return cls.objects.get( @@ -1637,7 +1643,7 @@ class CourseEnrollment(models.Model): """ assert user - if user.is_anonymous(): + if user.is_anonymous: return None cache_key = cls.enrollment_status_hash_cache_key(user) @@ -1960,7 +1966,7 @@ class CourseEnrollment(models.Model): """ assert user - if user.is_anonymous(): + if user.is_anonymous: return CourseEnrollmentState(None, None) enrollment_state = cls._get_enrollment_in_request_cache(user, course_key) if not enrollment_state: diff --git a/lms/djangoapps/email_marketing/signals.py b/lms/djangoapps/email_marketing/signals.py index 3d8720e199788c2c1a1c1c5d854b3d1affe8bac2..d8b5e297a7a1e3fceaa9fefbcf857c673bcc086e 100644 --- a/lms/djangoapps/email_marketing/signals.py +++ b/lms/djangoapps/email_marketing/signals.py @@ -139,7 +139,7 @@ def email_marketing_register_user(sender, user, registration, return # ignore anonymous users - if user.is_anonymous(): + if user.is_anonymous: return # perform update asynchronously @@ -165,7 +165,7 @@ def email_marketing_user_field_changed(sender, user=None, table=None, setting=No """ # ignore anonymous users - if user.is_anonymous(): + if user.is_anonymous: return # ignore anything but User, Profile or UserPreference tables diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index ceb69d02575a64ef79f508f3579ba057634fcf5d..005cf70c6164c9e37d7c42d7ed13460c491e3dd4 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -14,7 +14,7 @@ from six import text_type from lms.djangoapps.badges.utils import badges_enabled from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_api import errors -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.models import RetirementState, UserRetirementStatus, UserPreference from openedx.core.djangoapps.user_api.serializers import ReadOnlyFieldsSerializerMixin from student.models import UserProfile, LanguageProficiency, SocialLink @@ -332,6 +332,48 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea return instance +class RetirementUserProfileSerializer(serializers.ModelSerializer): + """ + Serialize a small subset of UserProfile data for use in RetirementStatus APIs + """ + class Meta(object): + model = UserProfile + fields = ('id', 'name') + + +class RetirementUserSerializer(serializers.ModelSerializer): + """ + Serialize a small subset of User data for use in RetirementStatus APIs + """ + profile = RetirementUserProfileSerializer(read_only=True) + + class Meta(object): + model = User + fields = ('id', 'username', 'email', 'profile') + + +class RetirementStateSerializer(serializers.ModelSerializer): + """ + Serialize a small subset of RetirementState data for use in RetirementStatus APIs + """ + class Meta(object): + model = RetirementState + fields = ('id', 'state_name', 'state_execution_order') + + +class UserRetirementStatusSerializer(serializers.ModelSerializer): + """ + Perform serialization for the RetirementStatus model + """ + user = RetirementUserSerializer(read_only=True) + current_state = RetirementStateSerializer(read_only=True) + last_state = RetirementStateSerializer(read_only=True) + + class Meta(object): + model = UserRetirementStatus + exclude = ['responses', ] + + def get_extended_profile(user_profile): """Returns the extended user profile fields stored in user_profile.meta""" diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_models.py b/openedx/core/djangoapps/user_api/accounts/tests/test_models.py new file mode 100644 index 0000000000000000000000000000000000000000..44eb229545c6ad9357281f8d4e431763867f2831 --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_models.py @@ -0,0 +1,89 @@ +""" +Model specific tests for user_api +""" +import pytest + +from openedx.core.djangoapps.user_api.models import RetirementState, RetirementStateError, UserRetirementStatus +from student.models import get_retired_email_by_email, get_retired_username_by_username +from student.tests.factories import UserFactory + + +# Tell pytest it's ok to use the database +pytestmark = pytest.mark.django_db + + +@pytest.fixture +def setup_retirement_states(): + """ + Pytest fixture to create some basic states for testing. Duplicates functionality of the + Django test runner in test_views.py unfortunately, but they're not compatible. + """ + default_states = [ + ('PENDING', 1, False, True), + ('LOCKING_ACCOUNT', 20, False, False), + ('LOCKING_COMPLETE', 30, False, False), + ('RETIRING_LMS', 40, False, False), + ('LMS_COMPLETE', 50, False, False), + ('ERRORED', 60, True, True), + ('ABORTED', 70, True, True), + ('COMPLETE', 80, True, True), + ] + + for name, ex, dead, req in default_states: + RetirementState.objects.create( + state_name=name, + state_execution_order=ex, + is_dead_end_state=dead, + required=req + ) + + +def _assert_retirementstatus_is_user(retirement, user): + """ + Helper function to compare a newly created UserRetirementStatus object to expected values for + the given user. + """ + pending = RetirementState.objects.all().order_by('state_execution_order')[0] + retired_username = get_retired_username_by_username(user.username) + retired_email = get_retired_email_by_email(user.email) + + assert retirement.user == user + assert retirement.original_username == user.username + assert retirement.original_email == user.email + assert retirement.original_name == user.profile.name + assert retirement.retired_username == retired_username + assert retirement.retired_email == retired_email + assert retirement.current_state == pending + assert retirement.last_state == pending + assert pending.state_name in retirement.responses + + +def test_retirement_create_success(setup_retirement_states): # pylint: disable=unused-argument, redefined-outer-name + """ + Basic test to make sure default creation succeeds + """ + user = UserFactory() + retirement = UserRetirementStatus.create_retirement(user) + _assert_retirementstatus_is_user(retirement, user) + + +def test_retirement_create_no_default_state(): + """ + Confirm that if no states have been loaded we fail with a RetirementStateError + """ + user = UserFactory() + + with pytest.raises(RetirementStateError): + UserRetirementStatus.create_retirement(user) + + +def test_retirement_create_already_retired(setup_retirement_states): # pylint: disable=unused-argument, redefined-outer-name + """ + Confirm the correct error bubbles up if the user already has a retirement row + """ + user = UserFactory() + retirement = UserRetirementStatus.create_retirement(user) + _assert_retirementstatus_is_user(retirement, user) + + with pytest.raises(RetirementStateError): + UserRetirementStatus.create_retirement(user) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index c7aba4a673a992c76c0581c86fbab7f516e0ad16..e9557793d00d251f67f51860f0919626fe742ae5 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -10,6 +10,7 @@ from copy import deepcopy import ddt import pytest +import pytz from django.conf import settings from django.contrib.auth.models import User from django.core.urlresolvers import reverse @@ -25,7 +26,7 @@ from social_django.models import UserSocialAuth from openedx.core.djangoapps.user_api.accounts import ACCOUNT_VISIBILITY_PREF_KEY from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_MAILINGS -from openedx.core.djangoapps.user_api.models import UserPreference, UserOrgTag +from openedx.core.djangoapps.user_api.models import RetirementState, UserRetirementStatus, UserPreference, UserOrgTag from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms from openedx.core.lib.token_utils import JwtBuilder @@ -940,6 +941,10 @@ class TestAccountRetireMailings(TestCase): self.url = reverse('accounts_retire_mailings', kwargs={'username': self.test_user.username}) + def build_post(self, user): + retired_username = get_retired_username_by_username(user.username) + return {'retired_username': retired_username} + def build_jwt_headers(self, user): """ Helper function for creating headers for the JWT authentication. @@ -950,10 +955,6 @@ class TestAccountRetireMailings(TestCase): } return headers - def build_post(self, user): - retired_username = get_retired_username_by_username(user.username) - return {'retired_username': retired_username} - def assert_status_and_tag_count(self, headers, expected_status=status.HTTP_204_NO_CONTENT, expected_tag_count=2, expected_tag_value="False", expected_content=None): """ @@ -1088,3 +1089,488 @@ class TestDeactivateLogout(TestCase): headers = self.build_jwt_headers(self.test_superuser) response = self.client.post(self.url, self.build_post(""), **headers) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + +class RetirementTestCase(TestCase): + """ + Test case with a helper methods for retirement + """ + + def setup_states(self): + """ + Create basic states that mimic our current understanding of the retirement process + """ + default_states = [ + ('PENDING', 1, False, True), + ('LOCKING_ACCOUNT', 20, False, False), + ('LOCKING_COMPLETE', 30, False, False), + ('RETIRING_CREDENTIALS', 40, False, False), + ('CREDENTIALS_COMPLETE', 50, False, False), + ('RETIRING_ECOM', 60, False, False), + ('ECOM_COMPLETE', 70, False, False), + ('RETIRING_FORUMS', 80, False, False), + ('FORUMS_COMPLETE', 90, False, False), + ('RETIRING_EMAIL_LISTS', 100, False, False), + ('EMAIL_LISTS_COMPLETE', 110, False, False), + ('RETIRING_ENROLLMENTS', 120, False, False), + ('ENROLLMENTS_COMPLETE', 130, False, False), + ('RETIRING_NOTES', 140, False, False), + ('NOTES_COMPLETE', 150, False, False), + ('NOTIFYING_PARTNERS', 160, False, False), + ('PARTNERS_NOTIFIED', 170, False, False), + ('RETIRING_LMS', 180, False, False), + ('LMS_COMPLETE', 190, False, False), + ('ERRORED', 200, True, True), + ('ABORTED', 210, True, True), + ('COMPLETE', 220, True, True), + ] + + for name, ex, dead, req in default_states: + RetirementState.objects.create( + state_name=name, + state_execution_order=ex, + is_dead_end_state=dead, + required=req + ) + + def build_jwt_headers(self, user): + """ + Helper function for creating headers for the JWT authentication. + """ + token = JwtBuilder(user).build_token([]) + headers = { + 'HTTP_AUTHORIZATION': 'JWT ' + token + } + return headers + + def _create_retirement(self, state, create_datetime=None): + """ + Helper method to create a RetirementStatus with useful defaults + """ + if create_datetime is None: + create_datetime = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=8) + + user = UserFactory() + return UserRetirementStatus.objects.create( + user=user, + original_username=user.username, + original_email=user.email, + original_name=user.profile.name, + retired_username=get_retired_username_by_username(user.username), + retired_email=get_retired_email_by_email(user.email), + current_state=state, + last_state=state, + responses="", + created=create_datetime, + modified=create_datetime + ) + + def _retirement_to_dict(self, retirement, all_fields=False): + """ + Return a dict format of this model to a consistent format for serialization, removing the long text field + `responses` for performance reasons. + """ + retirement_dict = { + u'id': retirement.id, + u'user': { + u'id': retirement.user.id, + u'username': retirement.user.username, + u'email': retirement.user.email, + u'profile': { + u'id': retirement.user.profile.id, + u'name': retirement.user.profile.name + }, + }, + u'original_username': retirement.original_username, + u'original_email': retirement.original_email, + u'original_name': retirement.original_name, + u'retired_username': retirement.retired_username, + u'retired_email': retirement.retired_email, + u'current_state': { + u'id': retirement.current_state.id, + u'state_name': retirement.current_state.state_name, + u'state_execution_order': retirement.current_state.state_execution_order, + }, + u'last_state': { + u'id': retirement.last_state.id, + u'state_name': retirement.last_state.state_name, + u'state_execution_order': retirement.last_state.state_execution_order, + }, + u'created': retirement.created, + u'modified': retirement.modified + } + + if all_fields: + retirement_dict['responses'] = retirement.responses + + return retirement_dict + + def _create_users_all_states(self): + return [self._create_retirement(state) for state in RetirementState.objects.all()] + + def _get_non_dead_end_states(self): + return [state for state in RetirementState.objects.filter(is_dead_end_state=False)] + + def _get_dead_end_states(self): + return [state for state in RetirementState.objects.filter(is_dead_end_state=True)] + + +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') +class TestAccountRetirementList(RetirementTestCase): + """ + Tests the account retirement endpoint. + """ + + def setUp(self): + super(TestAccountRetirementList, self).setUp() + self.test_superuser = SuperuserFactory() + self.headers = self.build_jwt_headers(self.test_superuser) + self.url = reverse('accounts_retirement_queue') + self.maxDiff = None + self.setup_states() + + def assert_status_and_user_list( + self, + expected_data, + expected_status=status.HTTP_200_OK, + states_to_request=None, + cool_off_days=7 + ): + """ + Helper function for making a request to the retire subscriptions endpoint, asserting the status, and + optionally asserting data returned. + """ + if states_to_request is None: + # These are just a couple of random states that should be used in any implementation + states_to_request = ['PENDING', 'LOCKING_ACCOUNT'] + else: + # Can pass in RetirementState objects or strings here + try: + states_to_request = [s.state_name for s in states_to_request] + except AttributeError: + states_to_request = states_to_request + + data = {'cool_off_days': cool_off_days, 'states': ','.join(states_to_request)} + response = self.client.get(self.url, data, **self.headers) + self.assertEqual(response.status_code, expected_status) + response_data = response.json() + + if expected_data: + # These datetimes won't match up due to serialization, but they're inherited fields tested elsewhere + for data in (response_data, expected_data): + for retirement in data: + del retirement['created'] + del retirement['modified'] + + self.assertItemsEqual(response_data, expected_data) + + def test_empty(self): + """ + Verify that an empty array is returned if no users are awaiting retirement + """ + self.assert_status_and_user_list([]) + + def test_users_exist_none_in_correct_status(self): + """ + Verify that users in dead end states are not returned + """ + for state in self._get_dead_end_states(): + self._create_retirement(state) + self.assert_status_and_user_list([], states_to_request=self._get_non_dead_end_states()) + + def test_users_exist(self): + """ + Verify users in different states are returned with correct data or filtered out + """ + self.maxDiff = None + retirement_values = [] + states_to_request = [] + + dead_end_states = self._get_dead_end_states() + + for retirement in self._create_users_all_states(): + if retirement.current_state not in dead_end_states: + states_to_request.append(retirement.current_state) + retirement_values.append(self._retirement_to_dict(retirement)) + + self.assert_status_and_user_list(retirement_values, states_to_request=self._get_non_dead_end_states()) + + def test_date_filter(self): + """ + Verifies the functionality of the `cool_off_days` parameter by creating 1 retirement per day for + 10 days. Then requests different 1-10 `cool_off_days` to confirm the correct retirements are returned. + """ + retirements = [] + days_back_to_test = 10 + + # Create a retirement per day for the last 10 days, from oldest date to newest. We want these all created + # before we start checking, thus the two loops. + # retirements = [2018-04-10..., 2018-04-09..., 2018-04-08...] + pending_state = RetirementState.objects.get(state_name='PENDING') + for days_back in range(1, days_back_to_test, -1): + create_datetime = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=days_back) + retirements.append(self._create_retirement(state=pending_state, create_datetime=create_datetime)) + + # Confirm we get the correct number and data back for each day we add to cool off days + # For each day we add to `cool_off_days` we expect to get one fewer retirement. + for cool_off_days in range(1, days_back_to_test): + # Start with 9 days back + req_days_back = days_back_to_test - cool_off_days + + retirement_dicts = [self._retirement_to_dict(ret) for ret in retirements[:cool_off_days]] + + self.assert_status_and_user_list( + retirement_dicts, + cool_off_days=req_days_back + ) + + def test_bad_cool_off_days(self): + """ + Check some bad inputs to make sure we get back the expected status + """ + self.assert_status_and_user_list(None, expected_status=status.HTTP_400_BAD_REQUEST, cool_off_days=-1) + self.assert_status_and_user_list(None, expected_status=status.HTTP_400_BAD_REQUEST, cool_off_days='ABCDERTP') + + def test_bad_states(self): + """ + Check some bad inputs to make sure we get back the expected status + """ + self.assert_status_and_user_list( + None, + expected_status=status.HTTP_400_BAD_REQUEST, + states_to_request=['TUNA', 'TACO']) + self.assert_status_and_user_list(None, expected_status=status.HTTP_400_BAD_REQUEST, states_to_request=[]) + + def test_missing_params(self): + """ + All params are required, make sure that is enforced + """ + response = self.client.get(self.url, **self.headers) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + response = self.client.get(self.url, {}, **self.headers) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + response = self.client.get(self.url, {'cool_off_days': 7}, **self.headers) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + RetirementState.objects.get(state_name='PENDING') + response = self.client.get(self.url, {'states': ['PENDING']}, **self.headers) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') +class TestAccountRetirementRetrieve(RetirementTestCase): + """ + Tests the account retirement retrieval endpoint. + """ + def setUp(self): + super(TestAccountRetirementRetrieve, self).setUp() + self.test_user = UserFactory() + self.test_superuser = SuperuserFactory() + self.url = reverse('accounts_retirement_retrieve', kwargs={'username': self.test_user.username}) + self.headers = self.build_jwt_headers(self.test_superuser) + self.maxDiff = None + self.setup_states() + + def assert_status_and_user_data(self, expected_data, expected_status=status.HTTP_200_OK, username_to_find=None): + """ + Helper function for making a request to the retire subscriptions endpoint, asserting the status, + and optionally asserting the expected data. + """ + if username_to_find is not None: + self.url = reverse('accounts_retirement_retrieve', kwargs={'username': username_to_find}) + + response = self.client.get(self.url, **self.headers) + self.assertEqual(response.status_code, expected_status) + + if expected_data is not None: + response_data = response.json() + + # These won't match up due to serialization, but they're inherited fields tested elsewhere + for data in (expected_data, response_data): + del data['created'] + del data['modified'] + + self.assertDictEqual(response_data, expected_data) + return response_data + + def test_no_retirement(self): + """ + Confirm we get a 404 if a retirement for the user can be found + """ + self.assert_status_and_user_data(None, status.HTTP_404_NOT_FOUND) + + def test_retirements_all_states(self): + """ + Create a bunch of retirements and confirm we get back the correct data for each + """ + retirements = [] + + for state in RetirementState.objects.all(): + retirements.append(self._create_retirement(state)) + + for retirement in retirements: + values = self._retirement_to_dict(retirement) + self.assert_status_and_user_data(values, username_to_find=values['user']['username']) + + def test_retrieve_by_old_username(self): + """ + Simulate retrieving a retirement by the old username, after the name has been changed to the hashed one + """ + pending_state = RetirementState.objects.get(state_name='PENDING') + retirement = self._create_retirement(pending_state) + original_username = retirement.user.username + + hashed_username = get_retired_username_by_username(original_username) + + retirement.user.username = hashed_username + retirement.user.save() + + values = self._retirement_to_dict(retirement) + self.assert_status_and_user_data(values, username_to_find=original_username) + + +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') +class TestAccountRetirementUpdate(RetirementTestCase): + """ + Tests the account retirement endpoint. + """ + def setUp(self): + super(TestAccountRetirementUpdate, self).setUp() + self.setup_states() + self.pending_state = RetirementState.objects.get(state_name='PENDING') + self.locking_state = RetirementState.objects.get(state_name='LOCKING_ACCOUNT') + + self.retirement = self._create_retirement(self.pending_state) + self.test_user = self.retirement.user + self.test_superuser = SuperuserFactory() + self.headers = self.build_jwt_headers(self.test_superuser) + self.headers['content_type'] = "application/merge-patch+json" + self.url = reverse('accounts_retirement_update') + + def update_and_assert_status(self, data, expected_status=status.HTTP_204_NO_CONTENT): + """ + Helper function for making a request to the retire subscriptions endpoint, and asserting the status. + """ + if 'username' not in data: + data['username'] = self.test_user.username + + response = self.client.patch(self.url, json.dumps(data), **self.headers) + self.assertEqual(response.status_code, expected_status) + + def test_single_update(self): + """ + Basic test to confirm changing state works and saves the given response + """ + data = {'new_state': 'LOCKING_ACCOUNT', 'response': 'this should succeed'} + self.update_and_assert_status(data) + + # Refresh the retirment object and confirm the messages and state are correct + retirement = UserRetirementStatus.objects.get(id=self.retirement.id) + self.assertEqual(retirement.current_state, RetirementState.objects.get(state_name='LOCKING_ACCOUNT')) + self.assertEqual(retirement.last_state, RetirementState.objects.get(state_name='PENDING')) + self.assertIn('this should succeed', retirement.responses) + + def test_move_through_process(self): + """ + Simulate moving a retirement through the process and confirm they end up in the + correct state, with all relevant response messages logged. + """ + fake_retire_process = [ + {'new_state': 'LOCKING_ACCOUNT', 'response': 'accountlockstart'}, + {'new_state': 'LOCKING_COMPLETE', 'response': 'accountlockcomplete'}, + {'new_state': 'RETIRING_CREDENTIALS', 'response': 'retiringcredentials'}, + {'new_state': 'CREDENTIALS_COMPLETE', 'response': 'credentialsretired'}, + {'new_state': 'COMPLETE', 'response': 'accountretirementcomplete'}, + ] + + for update_data in fake_retire_process: + self.update_and_assert_status(update_data) + + # Refresh the retirment object and confirm the messages and state are correct + retirement = UserRetirementStatus.objects.get(id=self.retirement.id) + self.assertEqual(retirement.current_state, RetirementState.objects.get(state_name='COMPLETE')) + self.assertEqual(retirement.last_state, RetirementState.objects.get(state_name='CREDENTIALS_COMPLETE')) + self.assertIn('accountlockstart', retirement.responses) + self.assertIn('accountlockcomplete', retirement.responses) + self.assertIn('retiringcredentials', retirement.responses) + self.assertIn('credentialsretired', retirement.responses) + self.assertIn('accountretirementcomplete', retirement.responses) + + def test_unknown_state(self): + """ + Test that trying to set to an unknown state fails with a 400 + """ + data = {'new_state': 'BOGUS_STATE', 'response': 'this should fail'} + self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) + + def test_bad_vars(self): + """ + Test various ways of sending the wrong variables to make sure they all fail correctly + """ + # No `new_state` + data = {'response': 'this should fail'} + self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) + + # No `response` + data = {'new_state': 'COMPLETE'} + self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) + + # Unknown `new_state` + data = {'new_state': 'BOGUS_STATE', 'response': 'this should fail'} + self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) + + # No `new_state` or `response` + data = {} + self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) + + # Unexpected param `should_not_exist` + data = {'should_not_exist': 'bad', 'new_state': 'COMPLETE', 'response': 'this should fail'} + self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) + + def test_no_retirement(self): + """ + Confirm that trying to operate on a non-existent retirement for an existing user 404s + """ + # Delete the only retirement, created in setUp + UserRetirementStatus.objects.all().delete() + data = {'new_state': 'LOCKING_ACCOUNT', 'response': 'this should fail'} + self.update_and_assert_status(data, status.HTTP_404_NOT_FOUND) + + def test_no_user(self): + """ + Confirm that trying to operate on a non-existent user 404s + """ + data = {'new_state': 'LOCKING_ACCOUNT', 'response': 'this should fail', 'username': 'does not exist'} + self.update_and_assert_status(data, status.HTTP_404_NOT_FOUND) + + def test_move_from_dead_end(self): + """ + Confirm that trying to move from a dead end state to any other state fails + """ + retirement = UserRetirementStatus.objects.get(id=self.retirement.id) + retirement.current_state = RetirementState.objects.filter(is_dead_end_state=True)[0] + retirement.save() + + data = {'new_state': 'LOCKING_ACCOUNT', 'response': 'this should fail'} + self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) + + def test_move_backward(self): + """ + Confirm that trying to move to an earlier step in the process fails + """ + retirement = UserRetirementStatus.objects.get(id=self.retirement.id) + retirement.current_state = RetirementState.objects.get(state_name='COMPLETE') + retirement.save() + + data = {'new_state': 'PENDING', 'response': 'this should fail'} + self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) + + def test_move_same(self): + """ + Confirm that trying to move to the same step in the process fails + """ + # Should already be in 'PENDING' + data = {'new_state': 'PENDING', 'response': 'this should fail'} + self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 3dc396a74b27e4b38c22adf01e5e0af4a97249b0..04a96b2a3cc4266e24adac4370147b49acc25b4f 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -4,6 +4,7 @@ An API for retrieving user account information. For additional information and historical context, see: https://openedx.atlassian.net/wiki/display/TNL/User+API """ +import datetime from django.contrib.auth import get_user_model from django.db import transaction @@ -15,6 +16,7 @@ from rest_framework.views import APIView from rest_framework.viewsets import ViewSet from six import text_type from social_django.models import UserSocialAuth +import pytz from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in from openedx.core.lib.api.authentication import ( @@ -22,13 +24,19 @@ from openedx.core.lib.api.authentication import ( OAuth2AuthenticationAllowInactiveUser, ) from openedx.core.lib.api.parsers import MergePatchParser -from student.models import User, get_potentially_retired_user_by_username_and_hash, get_retired_email_by_email +from student.models import ( + User, + get_retired_email_by_email, + get_potentially_retired_user_by_username_and_hash, + get_potentially_retired_user_by_username +) from .api import get_account_settings, update_account_settings from .permissions import CanDeactivateUser, CanRetireUser +from .serializers import UserRetirementStatusSerializer from .signals import USER_RETIRE_MAILINGS from ..errors import UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError -from ..models import UserOrgTag +from ..models import UserOrgTag, RetirementState, RetirementStateError, UserRetirementStatus class AccountViewSet(ViewSet): @@ -374,3 +382,97 @@ def _set_unusable_password(user): """ user.set_unusable_password() user.save() + + +class AccountRetirementView(ViewSet): + """ + Provides API endpoints for managing the user retirement process. + """ + authentication_classes = (JwtAuthentication,) + permission_classes = (permissions.IsAuthenticated, CanRetireUser, ) + parser_classes = (MergePatchParser, ) + serializer_class = UserRetirementStatusSerializer + + def retirement_queue(self, request): + """ + GET /api/user/v1/accounts/accounts_to_retire/ + {'cool_off_days': 7, 'states': ['PENDING', 'COMPLETE']} + + Returns the list of RetirementStatus users in the given states that were + created in the retirement queue at least `cool_off_days` ago. + """ + try: + cool_off_days = int(request.GET['cool_off_days']) + states = request.GET['states'].split(',') + + if cool_off_days < 0: + raise RetirementStateError('Invalid argument for cool_off_days, must be greater than 0.') + + state_objs = RetirementState.objects.filter(state_name__in=states) + if state_objs.count() != len(states): + found = [s.state_name for s in state_objs] + raise RetirementStateError('Unknown state. Requested: {} Found: {}'.format(states, found)) + + earliest_datetime = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=cool_off_days) + + retirements = UserRetirementStatus.objects.select_related( + 'user', 'current_state', 'last_state' + ).filter( + current_state__in=state_objs, created__lt=earliest_datetime + ).order_by( + 'id' + ) + serializer = UserRetirementStatusSerializer(retirements, many=True) + return Response(serializer.data) + # This should only occur on the int() converstion of cool_off_days at this point + except ValueError: + return Response('Invalid cool_off_days, should be integer.', status=status.HTTP_400_BAD_REQUEST) + except KeyError as exc: + return Response('Missing required parameter: {}'.format(text_type(exc)), status=status.HTTP_400_BAD_REQUEST) + except RetirementStateError as exc: + return Response(text_type(exc), status=status.HTTP_400_BAD_REQUEST) + + def retrieve(self, request, username): # pylint: disable=unused-argument + """ + GET /api/user/v1/accounts/{username}/retirement_status/ + Returns the RetirementStatus of a given user, or 404 if that row + doesn't exist. + """ + try: + user = get_potentially_retired_user_by_username(username) + retirement = UserRetirementStatus.objects.select_related( + 'user', 'current_state', 'last_state' + ).get(user=user) + serializer = UserRetirementStatusSerializer(instance=retirement) + return Response(serializer.data) + except (UserRetirementStatus.DoesNotExist, User.DoesNotExist): + return Response(status=status.HTTP_404_NOT_FOUND) + + def partial_update(self, request): + """ + PATCH /api/user/v1/accounts/update_retirement_status/ + + { + 'username': 'user_to_retire', + 'new_state': 'LOCKING_COMPLETE', + 'response': 'User account locked and logged out.' + } + + Updates the RetirementStatus row for the given user to the new + status, and append any messages to the message log. + + Note that this implementation is the "merge patch" implementation proposed in + https://tools.ietf.org/html/rfc7396. The content_type must be "application/merge-patch+json" or + else an error response with status code 415 will be returned. + """ + try: + username = request.data['username'] + retirement = UserRetirementStatus.objects.get(user__username=username) + retirement.update_state(request.data) + return Response(status=status.HTTP_204_NO_CONTENT) + except UserRetirementStatus.DoesNotExist: + return Response(status=status.HTTP_404_NOT_FOUND) + except RetirementStateError as exc: + return Response(text_type(exc), status=status.HTTP_400_BAD_REQUEST) + except Exception as exc: # pylint: disable=broad-except + return Response(text_type(exc), status=status.HTTP_500_INTERNAL_SERVER_ERROR) diff --git a/openedx/core/djangoapps/user_api/admin.py b/openedx/core/djangoapps/user_api/admin.py new file mode 100644 index 0000000000000000000000000000000000000000..0ea9aac4d4355b64c8c1007c49f29832c9795237 --- /dev/null +++ b/openedx/core/djangoapps/user_api/admin.py @@ -0,0 +1,33 @@ +""" +Django admin configuration pages for the user_api app +""" +from django.contrib import admin + +from .models import RetirementState, UserRetirementStatus + + +@admin.register(RetirementState) +class RetirementStateAdmin(admin.ModelAdmin): + """ + Admin interface for the RetirementState model. + """ + list_display = ('state_name', 'state_execution_order', 'is_dead_end_state', 'required',) + list_filter = ('is_dead_end_state', 'required',) + search_fields = ('state_name',) + + class Meta(object): + model = RetirementState + + +@admin.register(UserRetirementStatus) +class UserRetirementStatusAdmin(admin.ModelAdmin): + """ + Admin interface for the UserRetirementStatusAdmin model. + """ + list_display = ('user', 'original_username', 'current_state', 'modified') + list_filter = ('current_state',) + raw_id_fields = ('user',) + search_fields = ('original_username', 'retired_username', 'original_email', 'retired_email', 'original_name') + + class Meta(object): + model = UserRetirementStatus diff --git a/openedx/core/djangoapps/user_api/migrations/0002_retirementstate_userretirementstatus.py b/openedx/core/djangoapps/user_api/migrations/0002_retirementstate_userretirementstatus.py new file mode 100644 index 0000000000000000000000000000000000000000..516fc960f60986ebd575ed01b2545786abf5c8ae --- /dev/null +++ b/openedx/core/djangoapps/user_api/migrations/0002_retirementstate_userretirementstatus.py @@ -0,0 +1,54 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.12 on 2018-04-19 17:55 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('user_api', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='RetirementState', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('state_name', models.CharField(max_length=30, unique=True)), + ('state_execution_order', models.SmallIntegerField(unique=True)), + ('is_dead_end_state', models.BooleanField(db_index=True, default=False)), + ('required', models.BooleanField(default=False)), + ], + options={ + 'ordering': ('state_execution_order',), + }, + ), + migrations.CreateModel( + name='UserRetirementStatus', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('original_username', models.CharField(db_index=True, max_length=150)), + ('original_email', models.EmailField(db_index=True, max_length=254)), + ('original_name', models.CharField(blank=True, db_index=True, max_length=255)), + ('retired_username', models.CharField(db_index=True, max_length=150)), + ('retired_email', models.EmailField(db_index=True, max_length=254)), + ('responses', models.TextField()), + ('current_state', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='current_state', to='user_api.RetirementState')), + ('last_state', models.ForeignKey(blank=True, on_delete=django.db.models.deletion.CASCADE, related_name='last_state', to='user_api.RetirementState')), + ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'User Retirement Status', + 'verbose_name_plural': 'User Retirement Statuses', + }, + ), + ] diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index 77f48460caac564ff12307993a3091485b850168..5b824139bbb4c09b797c75b7ed26ef25bc83c5e0 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -4,7 +4,7 @@ Django ORM model specifications for the User API application from django.contrib.auth.models import User from django.core.validators import RegexValidator from django.db import models -from django.db.models.signals import post_delete, post_save, pre_save +from django.db.models.signals import post_delete, post_save, pre_delete, pre_save from django.dispatch import receiver from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import CourseKeyField @@ -15,9 +15,22 @@ from opaque_keys.edx.django.models import CourseKeyField # but currently the rest of the system assumes that "student" defines # certain models. For now we will leave the models in "student" and # create an alias in "user_api". + +# pylint: disable=unused-import +from student.models import ( + PendingEmailChange, + Registration, + UserProfile, + get_retired_email_by_email, + get_retired_username_by_username +) from util.model_utils import emit_setting_changed_event, get_changed_fields_dict +class RetirementStateError(Exception): + pass + + class UserPreference(models.Model): """A user's preference, stored as generic text to be processed by client""" KEY_REGEX = r"[-_a-zA-Z0-9]+" @@ -109,7 +122,8 @@ class UserCourseTag(models.Model): class UserOrgTag(TimeStampedModel): - """ Per-Organization user tags. + """ + Per-Organization user tags. Allows settings to be configured at an organization level. @@ -121,3 +135,138 @@ class UserOrgTag(TimeStampedModel): class Meta(object): unique_together = ("user", "org", "key") + + +class RetirementState(models.Model): + """ + Stores the list and ordering of the steps of retirement, this should almost never change + as updating it can break the retirement process of users already in the queue. + """ + state_name = models.CharField(max_length=30, unique=True) + state_execution_order = models.SmallIntegerField(unique=True) + is_dead_end_state = models.BooleanField(default=False, db_index=True) + required = models.BooleanField(default=False) + + def __unicode__(self): + return u'{} (step {})'.format(self.state_name, self.state_execution_order) + + class Meta(object): + ordering = ('state_execution_order',) + + @classmethod + def get_dead_end_states(cls): + return cls.objects.filter(is_dead_end_state=True) + + @classmethod + def get_dead_end_state_names_list(cls): + return cls.objects.filter(is_dead_end_state=True).values_list('state_name', flat=True) + + @classmethod + def get_state_names_list(cls): + return cls.objects.all().values_list('state_name', flat=True) + + +@receiver(pre_delete, sender=RetirementState) +def retirementstate_pre_delete_callback(_, **kwargs): + """ + Event changes to user preferences. + """ + state = kwargs["instance"] + if state.required: + raise Exception('Required RetirementStates cannot be deleted.') + + +class UserRetirementStatus(TimeStampedModel): + """ + Tracks the progress of a user's retirement request + """ + user = models.OneToOneField(User) + original_username = models.CharField(max_length=150, db_index=True) + original_email = models.EmailField(db_index=True) + original_name = models.CharField(max_length=255, blank=True, db_index=True) + retired_username = models.CharField(max_length=150, db_index=True) + retired_email = models.EmailField(db_index=True) + current_state = models.ForeignKey(RetirementState, related_name='current_state') + last_state = models.ForeignKey(RetirementState, blank=True, related_name='last_state') + responses = models.TextField() + + class Meta(object): + verbose_name = 'User Retirement Status' + verbose_name_plural = 'User Retirement Statuses' + + def _validate_state_update(self, new_state): + """ + Confirm that the state move that's trying to be made is allowed + """ + dead_end_states = list(RetirementState.get_dead_end_state_names_list()) + states = list(RetirementState.get_state_names_list()) + if self.current_state in dead_end_states: + raise RetirementStateError('RetirementStatus: Unable to move user from {}'.format(self.current_state)) + + try: + new_state_index = states.index(new_state) + if new_state_index <= states.index(self.current_state.state_name): + raise ValueError() + except ValueError: + err = '{} does not exist or is an eariler state than current state {}'.format(new_state, self.current_state) + raise RetirementStateError(err) + + def _validate_update_data(self, data): + """ + Confirm that the data passed in is properly formatted + """ + required_keys = ('username', 'new_state', 'response') + + for required_key in required_keys: + if required_key not in data: + raise RetirementStateError('RetirementStatus: Required key {} missing from update'.format(required_key)) + + for key in data: + if key not in required_keys: + raise RetirementStateError('RetirementStatus: Unknown key {} in update'.format(key)) + + @classmethod + def create_retirement(cls, user): + """ + Creates a UserRetirementStatus for the given user, in the correct initial state. Will + fail if the user already has a UserRetirementStatus row or if states are not yet populated. + """ + try: + pending = RetirementState.objects.all().order_by('state_execution_order')[0] + except IndexError: + raise RetirementStateError('Default state does not exist! Populate retirement states to retire users.') + + if cls.objects.filter(user=user).exists(): + raise RetirementStateError('User {} already has a retirement row!'.format(user)) + + retired_username = get_retired_username_by_username(user.username) + retired_email = get_retired_email_by_email(user.email) + + return cls.objects.create( + user=user, + original_username=user.username, + original_email=user.email, + original_name=user.profile.name, + retired_username=retired_username, + retired_email=retired_email, + current_state=pending, + last_state=pending, + responses='Created in state {} by create_retirement'.format(pending) + ) + + def update_state(self, update): + """ + Perform the necessary checks for a state change and update the state and response if passed + or throw a RetirementStateError with a useful error message + """ + self._validate_update_data(update) + self._validate_state_update(update['new_state']) + + old_state = self.current_state + self.current_state = RetirementState.objects.get(state_name=update['new_state']) + self.last_state = old_state + self.responses += "\n Moved from {} to {}:\n{}\n".format(old_state, self.current_state, update['response']) + self.save() + + def __unicode__(self): + return u'User: {} State: {} Last Updated: {}'.format(self.user.id, self.current_state, self.modified) diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index f94dd12c6b628ddbea4a1a5050e4866886cf7589..2c9bb31a6f953c3c52b483ff627788a0ce766ee5 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -6,7 +6,13 @@ from django.conf import settings from django.conf.urls import url from ..profile_images.views import ProfileImageView -from .accounts.views import AccountDeactivationView, AccountRetireMailingsView, AccountViewSet, DeactivateLogoutView +from .accounts.views import ( + AccountDeactivationView, + AccountRetireMailingsView, + AccountRetirementView, + AccountViewSet, + DeactivateLogoutView +) from .preferences.views import PreferencesDetailView, PreferencesView from .verification_api.views import PhotoVerificationStatusView from .validation.views import RegistrationValidationView @@ -24,6 +30,19 @@ ACCOUNT_DETAIL = AccountViewSet.as_view({ 'patch': 'partial_update', }) +RETIREMENT_QUEUE = AccountRetirementView.as_view({ + 'get': 'retirement_queue' +}) + +RETIREMENT_RETRIEVE = AccountRetirementView.as_view({ + 'get': 'retrieve' +}) + +RETIREMENT_UPDATE = AccountRetirementView.as_view({ + 'patch': 'partial_update', +}) + + urlpatterns = [ url( r'^v1/me$', @@ -65,6 +84,21 @@ urlpatterns = [ PhotoVerificationStatusView.as_view(), name='verification_status' ), + url( + r'^v1/accounts/{}/retirement_status/$'.format(settings.USERNAME_PATTERN), + RETIREMENT_RETRIEVE, + name='accounts_retirement_retrieve' + ), + url( + r'^v1/accounts/retirement_queue/$', + RETIREMENT_QUEUE, + name='accounts_retirement_queue' + ), + url( + r'^v1/accounts/update_retirement_status/$', + RETIREMENT_UPDATE, + name='accounts_retirement_update' + ), url( r'^v1/validation/registration$', RegistrationValidationView.as_view(),