Merge pull request #18154 from edx/jeskew/add_model_for_retired_users

Refuse password reset for retired users.
This commit is contained in:
J Eskew
2018-05-14 13:02:53 -04:00
committed by GitHub
6 changed files with 175 additions and 13 deletions

View File

@@ -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}

View File

@@ -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,

View File

@@ -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)

View File

@@ -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)

View File

@@ -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

View File

@@ -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()