diff --git a/common/djangoapps/student/tests/test_reset_password.py b/common/djangoapps/student/tests/test_reset_password.py index 6e45685a09..0a6217c892 100644 --- a/common/djangoapps/student/tests/test_reset_password.py +++ b/common/djangoapps/student/tests/test_reset_password.py @@ -22,6 +22,7 @@ from provider.oauth2 import models as dop_models from openedx.core.djangoapps.oauth_dispatch.tests import factories as dot_factories from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from openedx.core.djangoapps.user_api.models import UserRetirementRequest from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from student.tests.factories import UserFactory @@ -38,7 +39,8 @@ from .test_configuration_overrides import fake_get_value ) @ddt.ddt class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): - """ Tests that clicking reset password sends email, and doesn't activate the user + """ + Tests that clicking reset password sends email, and doesn't activate the user """ request_factory = RequestFactory() @@ -59,7 +61,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): @patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) def test_user_bad_password_reset(self): - """Tests password reset behavior for user with password marked UNUSABLE_PASSWORD_PREFIX""" + """ + Tests password reset behavior for user with password marked UNUSABLE_PASSWORD_PREFIX + """ bad_pwd_req = self.request_factory.post('/password_reset/', {'email': self.user_bad_passwd.email}) bad_pwd_resp = password_reset(bad_pwd_req) @@ -74,7 +78,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): @patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) def test_nonexist_email_password_reset(self): - """Now test the exception cases with of reset_password called with invalid email.""" + """ + Now test the exception cases with of reset_password called with invalid email. + """ bad_email_req = self.request_factory.post('/password_reset/', {'email': self.user.email + "makeItFail"}) bad_email_resp = password_reset(bad_email_req) @@ -91,7 +97,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): @patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) def test_password_reset_ratelimited(self): - """ Try (and fail) resetting password 30 times in a row on an non-existant email address """ + """ + Try (and fail) resetting password 30 times in a row on an non-existant email address + """ cache.clear() for i in xrange(30): @@ -113,7 +121,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): @patch('django.core.mail.send_mail') @patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) def test_reset_password_email(self, send_email): - """Tests contents of reset password email, and that user is not active""" + """ + Tests contents of reset password email, and that user is not active + """ good_req = self.request_factory.post('/password_reset/', {'email': self.user.email}) good_req.user = self.user @@ -236,7 +246,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): ) @ddt.unpack def test_reset_password_bad_token(self, uidb36, token): - """Tests bad token and uidb36 in password reset""" + """ + Tests bad token and uidb36 in password reset + """ if uidb36 is None: uidb36 = self.uidb36 if token is None: @@ -253,7 +265,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): self.assertFalse(self.user.is_active) def test_reset_password_good_token(self): - """Tests good token and uidb36 in password reset""" + """ + Tests good token and uidb36 in password reset + """ url = reverse( "password_reset_confirm", kwargs={"uidb36": self.uidb36, "token": self.token} @@ -264,7 +278,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): self.assertTrue(self.user.is_active) def test_password_reset_fail(self): - """Tests that if we provide mismatched passwords, user is not marked as active.""" + """ + Tests that if we provide mismatched passwords, user is not marked as active. + """ self.assertFalse(self.user.is_active) url = reverse( @@ -282,6 +298,27 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): self.assertEqual(resp.status_code, 200) self.assertFalse(User.objects.get(pk=self.user.pk).is_active) + def test_password_reset_retired_user_fail(self): + """ + Tests that if a retired user attempts to reset their password, it fails. + """ + self.assertFalse(self.user.is_active) + + # Retire the user. + UserRetirementRequest.create_retirement_request(self.user) + + url = reverse( + 'password_reset_confirm', + kwargs={'uidb36': self.uidb36, 'token': self.token} + ) + reset_req = self.request_factory.get(url) + resp = password_reset_confirm_wrapper(reset_req, self.uidb36, self.token) + + # Verify the response status code is: 200 with password reset fail and also verify that + # the user is not marked as active. + self.assertEqual(resp.status_code, 200) + self.assertFalse(User.objects.get(pk=self.user.pk).is_active) + def test_password_reset_prevent_auth_user_writes(self): with waffle().override(PREVENT_AUTH_USER_WRITES, True): url = reverse( @@ -307,7 +344,8 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): } ) def test_password_reset_with_invalid_length(self, password_dict): - """Tests that if we provide password characters less then PASSWORD_MIN_LENGTH, + """ + Tests that if we provide password characters less then PASSWORD_MIN_LENGTH, or more than PASSWORD_MAX_LENGTH, password reset will fail with error message. """ @@ -326,7 +364,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): @patch('student.views.management.password_reset_confirm') @patch("openedx.core.djangoapps.site_configuration.helpers.get_value", fake_get_value) def test_reset_password_good_token_configuration_override(self, reset_confirm): - """Tests password reset confirmation page for site configuration override.""" + """ + Tests password reset confirmation page for site configuration override. + """ url = reverse( "password_reset_confirm", kwargs={"uidb36": self.uidb36, "token": self.token} diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 7b41f79aaf..41e4a44b3e 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -66,6 +66,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_ from openedx.core.djangoapps.theming import helpers as theming_helpers from openedx.core.djangoapps.user_api import accounts as accounts_settings from openedx.core.djangoapps.user_api.accounts.utils import generate_password +from openedx.core.djangoapps.user_api.models import UserRetirementRequest from openedx.core.djangoapps.user_api.preferences import api as preferences_api from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle from openedx.core.djangolib.markup import HTML, Text @@ -1150,6 +1151,19 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None): request, uidb64=uidb64, token=token, extra_context=platform_name ) + if UserRetirementRequest.has_user_requested_retirement(user): + # Refuse to reset the password of any user that has requested retirement. + context = { + 'validlink': True, + 'form': None, + 'title': _('Password reset unsuccessful'), + 'err_msg': _('Error in resetting your password.'), + } + context.update(platform_name) + return TemplateResponse( + request, 'registration/password_reset_confirm.html', context + ) + if waffle().is_enabled(PREVENT_AUTH_USER_WRITES): context = { 'validlink': False, diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index cdae548483..54edf2df70 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -645,6 +645,7 @@ class OrderItem(TimeStampedModel): """ class Meta(object): app_label = "shoppingcart" + base_manager_name = 'objects' objects = InheritanceManager() order = models.ForeignKey(Order, db_index=True) @@ -1091,6 +1092,7 @@ class InvoiceItem(TimeStampedModel): """ class Meta(object): app_label = "shoppingcart" + base_manager_name = 'objects' objects = InheritanceManager() invoice = models.ForeignKey(Invoice, db_index=True) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_models.py b/openedx/core/djangoapps/user_api/accounts/tests/test_models.py index 44eb229545..1f59f86c31 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_models.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_models.py @@ -3,7 +3,12 @@ Model specific tests for user_api """ import pytest -from openedx.core.djangoapps.user_api.models import RetirementState, RetirementStateError, UserRetirementStatus +from openedx.core.djangoapps.user_api.models import ( + RetirementState, + RetirementStateError, + UserRetirementRequest, + UserRetirementStatus +) from student.models import get_retired_email_by_email, get_retired_username_by_username from student.tests.factories import UserFactory @@ -87,3 +92,47 @@ def test_retirement_create_already_retired(setup_retirement_states): # pylint: with pytest.raises(RetirementStateError): UserRetirementStatus.create_retirement(user) + + +def test_retirement_request_create_success(): + """ + Ensure that retirement request record creation succeeds. + """ + user = UserFactory() + UserRetirementRequest.create_retirement_request(user) + assert UserRetirementRequest.has_user_requested_retirement(user) + + +def test_retirement_request_created_upon_status(setup_retirement_states): # pylint: disable=unused-argument, redefined-outer-name + """ + Ensure that retirement request record is created upon retirement status creation. + """ + user = UserFactory() + UserRetirementStatus.create_retirement(user) + assert UserRetirementRequest.has_user_requested_retirement(user) + + +def test_retirement_request_deleted_upon_pending_status_delete(setup_retirement_states): # pylint: disable=unused-argument, redefined-outer-name + """ + Ensure that retirement request record is deleted upon deletion of a PENDING retirement status. + """ + user = UserFactory() + retirement_status = UserRetirementStatus.create_retirement(user) + assert UserRetirementRequest.has_user_requested_retirement(user) + pending = RetirementState.objects.all().order_by('state_execution_order')[0] + assert retirement_status.current_state == pending + retirement_status.delete() + assert not UserRetirementRequest.has_user_requested_retirement(user) + + +def test_retirement_request_preserved_upon_non_pending_status_delete(setup_retirement_states): # pylint: disable=unused-argument, redefined-outer-name + """ + Ensure that retirement request record is not deleted upon deletion of a non-PENDING retirement status. + """ + user = UserFactory() + retirement_status = UserRetirementStatus.create_retirement(user) + assert UserRetirementRequest.has_user_requested_retirement(user) + non_pending = RetirementState.objects.all().order_by('state_execution_order')[1] + retirement_status.current_state = non_pending + retirement_status.delete() + assert UserRetirementRequest.has_user_requested_retirement(user) diff --git a/openedx/core/djangoapps/user_api/admin.py b/openedx/core/djangoapps/user_api/admin.py index 0ea9aac4d4..62f3aae3b0 100644 --- a/openedx/core/djangoapps/user_api/admin.py +++ b/openedx/core/djangoapps/user_api/admin.py @@ -3,7 +3,7 @@ Django admin configuration pages for the user_api app """ from django.contrib import admin -from .models import RetirementState, UserRetirementStatus +from .models import RetirementState, UserRetirementStatus, UserRetirementRequest @admin.register(RetirementState) @@ -31,3 +31,15 @@ class UserRetirementStatusAdmin(admin.ModelAdmin): class Meta(object): model = UserRetirementStatus + + +@admin.register(UserRetirementRequest) +class UserRetirementRequestAdmin(admin.ModelAdmin): + """ + Admin interface for the UserRetirementRequestAdmin model. + """ + list_display = ('user', 'created') + raw_id_fields = ('user',) + + class Meta(object): + model = UserRetirementRequest diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index d73e92458a..8fce760f89 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -167,6 +167,38 @@ class RetirementState(models.Model): return cls.objects.all().values_list('state_name', flat=True) +class UserRetirementRequest(TimeStampedModel): + """ + Records and perists every user retirement request. + Users that have requested to cancel their retirement before retirement begins can be removed. + All other retired users persist in this table forever. + """ + user = models.OneToOneField(User) + + class Meta(object): + verbose_name = 'User Retirement Request' + verbose_name_plural = 'User Retirement Requests' + + @classmethod + def create_retirement_request(cls, user): + """ + Creates a UserRetirementRequest for the specified user. + """ + if cls.has_user_requested_retirement(user): + raise RetirementStateError('User {} already has a retirement request row!'.format(user)) + return cls.objects.create(user=user) + + @classmethod + def has_user_requested_retirement(cls, user): + """ + Checks to see if a UserRetirementRequest has been created for the specified user. + """ + return cls.objects.filter(user=user).exists() + + def __unicode__(self): + return u'User: {} Requested: {}'.format(self.user.id, self.created) + + class UserRetirementStatus(TimeStampedModel): """ Tracks the progress of a user's retirement request @@ -228,11 +260,13 @@ class UserRetirementStatus(TimeStampedModel): 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)) + raise RetirementStateError('User {} already has a retirement status row!'.format(user)) retired_username = get_retired_username_by_username(user.username) retired_email = get_retired_email_by_email(user.email) + UserRetirementRequest.create_retirement_request(user) + return cls.objects.create( user=user, original_username=user.username, @@ -282,3 +316,14 @@ class UserRetirementStatus(TimeStampedModel): def __unicode__(self): return u'User: {} State: {} Last Updated: {}'.format(self.user.id, self.current_state, self.modified) + + +@receiver(models.signals.post_delete, sender=UserRetirementStatus) +def remove_pending_retirement_request(sender, instance, **kwargs): # pylint: disable=unused-argument + """ + Whenever a UserRetirementStatus record is deleted, remove the user's UserRetirementRequest record + IFF the UserRetirementStatus record was still PENDING. + """ + pending_state = RetirementState.objects.filter(state_name='PENDING')[0] + if pending_state and instance.current_state == pending_state: + UserRetirementRequest.objects.filter(user=instance.user).delete()